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

Hannes Reinecke
2015-05-11T13:19:07Z

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