Re: [PATCH 04/17] scsi_dh_alua: Improve error handling

[Hannes Reinecke <hare@suse.de>]

On 05/07/2015 01:52 PM, Hannes Reinecke wrote:
> On 05/07/2015 01:48 PM, Bart Van Assche wrote:
>> On 05/04/15 14:42, Hannes Reinecke wrote:
>>> @@ -161,12 +164,12 @@ static unsigned submit_rtpg(struct
>>> scsi_device *sdev, struct alua_dh_data *h,
>>>       rq->sense_len = h->senselen = 0;
>>>
>>>       err = blk_execute_rq(rq->q, NULL, rq, 1);
>>> -    if (err == -EIO) {
>>> -        sdev_printk(KERN_INFO, sdev,
>>> -                "%s: rtpg failed with %x\n",
>>> -                ALUA_DH_NAME, rq->errors);
>>> +    if (err < 0) {
>>> +        if (!rq->errors)
>>> +            err = DID_ERROR << 16;
>>> +        else
>>> +            err = rq->errors;
>>>           h->senselen = rq->sense_len;
>>> -        err = SCSI_DH_IO;
>>>       }
>>>       blk_put_request(rq);
>>>   done:
>>
>> Running the grep query "->errors = " over the Linux kernel source
>> tree shows that sometimes a SCSI error code is written into that
>> field and sometimes a negative error code. Does this mean that the
>> test !rq->errors should be modified into !rq->errors &&
>> !IS_ERR_VALUE(rq->errors) ?
>>
> Hmm. I'm going to review this. 'rq->errors' should be used consistently.
> 
drivers/scsi/scsi_lib.c:scsi_execute() has this comment in the
header:

 * returns the req->errors value which is the scsi_cmnd result
 * field.

And later on 'req->errors' is used verbatim as the return value:

	if (resid)
		*resid = req->resid_len;
	ret = req->errors;
 out:
	blk_put_request(req);

	return ret;
}

As the above patch is modeled according to this, any issue here
would affect scsi_execute, too.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html