On 4/26/21 5:20 PM, Christoph Hellwig wrote:
On Fri, Apr 23, 2021 at 01:39:13PM +0200, Hannes Reinecke wrote:
Introduce scsi_status_is_check_condition() and
That was added in the last patch, wasn't it?
Yeah, already fixed up.
+++ b/block/bsg.c
@@ -97,6 +97,8 @@ static int bsg_scsi_complete_rq(struct request *rq, struct sg_io_v4 *hdr)
hdr->device_status = sreq->result & 0xff;
hdr->transport_status = host_byte(sreq->result);
hdr->driver_status = driver_byte(sreq->result);
+ if (scsi_status_is_check_condition(sreq->result))
+ hdr->driver_status |= DRIVER_SENSE;
I think hdr->driver_status also needs to be cleared to 0 first. A little
comment on the history would be nice as well.
That will happen one of the later patches when we drop the
'driver_byte()' macro completely.
And I've added to explanation to the sg header file.
@@ -257,6 +257,8 @@ static int blk_complete_sghdr_rq(struct request *rq, struct sg_io_hdr *hdr,
hdr->msg_status = msg_byte(req->result);
hdr->host_status = host_byte(req->result);
hdr->driver_status = driver_byte(req->result);
+ if (hdr->status == SAM_STAT_CHECK_CONDITION)
+ hdr->driver_status |= DRIVER_SENSE;
Same here. Also why the open coded check in one please and the
SAM_STAT_CHECK_CONDITION comparism in another?
Maybe we need a little helper instead of duplicating the logic?
Check condition is cleaned up, and the clearing of the driver_byte value
will happen one of the later patches.
+ } else
+ hp->driver_status |= DRIVER_SENSE;
Shouldn't this be where the previous driver_byte call was? And
maybe also use a proper helper?
I'm not sure if we need a helper here; all other call sites dealing with
the SG header do it directly, and the driver_status setting will be
changed later on again.
@@ -131,6 +131,8 @@ struct compat_sg_io_hdr {
#define SG_INFO_DIRECT_IO 0x2 /* direct IO requested and performed */
#define SG_INFO_MIXED_IO 0x4 /* part direct, part indirect IO */
+/* Obsolete DRIVER_SENSE setting */
+#define DRIVER_SENSE 0x08
I think this needs a better documentation on what it means and why it
is obsolete.
Yes, will be doing so.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
