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@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