Re: [PATCH 08/17] scsi_dh_alua: Make stpg synchronous

Bart Van Assche
2015-05-07T12:18:06Z

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 ? 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.

Thanks,

Bart.