On 4/26/21 5:41 AM, Bart Van Assche wrote: > On 4/23/21 4:39 AM, Hannes Reinecke wrote: >> Introduce scsi_status_is_check_condition() > > Wasn't that macro introduced by patch 07/39? > Sheesh. Left-over from patch separation. Will be fixing it up. >> For backwards compability move the DRIVER_SENSE definition > ^^^^^^^^^^^ > typo? > Yep. >> diff --git a/block/bsg.c b/block/bsg.c >> index bd10922d5cbb..a70bb25ab906 100644 >> --- a/block/bsg.c >> +++ 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; > > If another value is already present in the driver_byte(), will |= > DRIVER_SENSE corrupt that value? > Yes, and no. It would corrupt it if there _were_ another value. But the whole point of this patchset is that there _is_ no value in the driver_byte field. And we're just fudging in that one parameter (DRIVER_SENSE) which actually has a meaning and (occasionally) is evaluated in user-space. >> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c >> index 99d58786e0d5..e59e1f70f3a5 100644 >> --- a/block/scsi_ioctl.c >> +++ b/block/scsi_ioctl.c >> @@ -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: since "driver_status" is not a bitfield, "|=" seems > conceptually wrong to me. > That, on the other hand, is correct. Will be fixing it up. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@xxxxxxx +49 911 74053 688 SUSE Software Solutions Germany GmbH, 90409 Nürnberg GF: F. Imendörffer, HRB 36809 (AG Nürnberg)
