On 05/07/2015 02:18 PM, Bart Van Assche wrote: > On 05/04/15 14:42, Hannes Reinecke wrote: >> +static unsigned alua_stpg(struct scsi_device *sdev, struct >> alua_dh_data *h) >> +{ >> + int retval, err = SCSI_DH_RETRY; >> + unsigned char sense[SCSI_SENSE_BUFFERSIZE]; >> + struct scsi_sense_hdr sense_hdr; >> + >> + if (!(h->tpgs & TPGS_MODE_EXPLICIT)) { >> + /* Only implicit ALUA supported, retry */ >> + return SCSI_DH_RETRY; >> + } >> + switch (h->state) { >> + case TPGS_STATE_OPTIMIZED: >> + return SCSI_DH_OK; >> + case TPGS_STATE_NONOPTIMIZED: >> + if ((h->flags & ALUA_OPTIMIZE_STPG) && >> + (!h->pref) && >> + (h->tpgs & TPGS_MODE_IMPLICIT)) >> + return SCSI_DH_OK; >> + break; >> + case TPGS_STATE_STANDBY: >> + case TPGS_STATE_UNAVAILABLE: >> + break; >> + case TPGS_STATE_OFFLINE: >> + return SCSI_DH_IO; >> + break; >> + case TPGS_STATE_TRANSITIONING: >> + break; >> + default: >> + sdev_printk(KERN_INFO, sdev, >> + "%s: stpg failed, unhandled TPGS state %d", >> + ALUA_DH_NAME, pg->state); >> + return SCSI_DH_NOSYS; >> + break; >> + } >> + /* Set state to transitioning */ >> + h->state = TPGS_STATE_TRANSITIONING; >> + retval = submit_stpg(sdev, h->group_id, sense); >> + >> + if (retval) { >> + if (!(driver_byte(retval) & DRIVER_SENSE) || >> + !scsi_normalize_sense(sense, SCSI_SENSE_BUFFERSIZE, >> + &sense_hdr)) { >> + sdev_printk(KERN_INFO, sdev, >> + "%s: stpg failed, result %d", >> + ALUA_DH_NAME, retval); >> + /* Retry RTPG */ >> + return err; >> + } >> + err = alua_check_sense(h->sdev, &sense_hdr); >> + sdev_printk(KERN_INFO, h->sdev, "%s: stpg failed\n", >> + ALUA_DH_NAME); >> + scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); >> + err = SCSI_DH_RETRY; >> + } >> + return err; >> +} > > The value returned by alua_check_sense() is assigned to the variable > 'err' but that value is not used. Otherwise in this function the > variable 'err' always has the value SCSI_DH_RETRY. Does that mean > that that variable can be removed ? Hmm. Yes, we could. > Additionally, why is 'retval' > only printed if normalizing the sense data succeeds and not if > normalizing the sense data fails ? Has it been considered to print > the ASC and ASCQ values upon STPG failure ? Having these values > available can be a big help during debugging. > We do print the ASC / ASCQ values; that's what scsi_print_sense_hdr does. And the above line works on the assumption that iff DRIVER_SENSE is set in the retval we'll have a sense code, and this sense code will give more information than any other (internally generated) error status. So 'retval' isn't printed in that case, only if no sense code is provided or if we fail to decode it. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@xxxxxxx +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html