| Message ID | 20170518131812.22956-3-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
On Thu, 2017-05-18 at 15:17 +0200, Christoph Hellwig wrote: > We will only have sense data if the command exectured and got a SCSI > result, so this is pointless. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/scsi/osd/osd_initiator.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c > index 8a1b94816419..14785177ce7b 100644 > --- a/drivers/scsi/osd/osd_initiator.c > +++ b/drivers/scsi/osd/osd_initiator.c > @@ -477,7 +477,7 @@ static void _set_error_resid(struct osd_request *or, struct request *req, > int error) > { > or->async_error = error; > - or->req_errors = scsi_req(req)->result ? : error; > + or->req_errors = scsi_req(req)->result; > or->sense_len = scsi_req(req)->sense_len; > if (or->sense_len) > memcpy(or->sense, scsi_req(req)->sense, or->sense_len); Hello Christoph, Are you sure that that code is not necessary? From osd_initiator.c: static void _put_request(struct request *rq) { /* * If osd_finalize_request() was called but the request was not * executed through the block layer, then we must release BIOs. * TODO: Keep error code in or->async_error. Need to audit all * code paths. */ if (unlikely(rq->bio)) blk_end_request(rq, -ENOMEM, blk_rq_bytes(rq)); else blk_put_request(rq); } Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Wed, May 24, 2017 at 04:04:40PM +0000, Bart Van Assche wrote: > Are you sure that that code is not necessary? From osd_initiator.c: > > static void _put_request(struct request *rq) > { > /* > * If osd_finalize_request() was called but the request was not > * executed through the block layer, then we must release BIOs. > * TODO: Keep error code in or->async_error. Need to audit all > * code paths. > */ > if (unlikely(rq->bio)) > blk_end_request(rq, -ENOMEM, blk_rq_bytes(rq)); > else > blk_put_request(rq); > } Which isn't using it at all. It has a ten year old comment to pass on some error, but even then ORing two different error types together would no be very helpful. > > Bart.---end quoted text--- -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c index 8a1b94816419..14785177ce7b 100644 --- a/drivers/scsi/osd/osd_initiator.c +++ b/drivers/scsi/osd/osd_initiator.c @@ -477,7 +477,7 @@ static void _set_error_resid(struct osd_request *or, struct request *req, int error) { or->async_error = error; - or->req_errors = scsi_req(req)->result ? : error; + or->req_errors = scsi_req(req)->result; or->sense_len = scsi_req(req)->sense_len; if (or->sense_len) memcpy(or->sense, scsi_req(req)->sense, or->sense_len);
We will only have sense data if the command exectured and got a SCSI result, so this is pointless. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/scsi/osd/osd_initiator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)