On 11/19/19 12:03 PM, Martin Wilck wrote: > On Tue, 2019-11-19 at 10:31 +0100, Hannes Reinecke wrote: >> On 11/19/19 10:02 AM, Martin Wilck wrote: >>> >>> While I generally like this change, the driver byte is part of the >>> sg_io user space API, and used in sg3_utils and multipath-tools (in >>> particular, DRIVER_SENSE), and likely in other user space tools as >>> well. Can we simply ditch it without adding some compatibility code >>> to >>> sg and bsg? >>> >> Why, but I did... >> sg and bsg should report to userland exactly the same values as >> before; >> or at least that was the plan. > > I am sorry, I overlooked that part in sg.c where you set DRIVER_SENSE. > It's the only DRIVER_xyz code that you set though. Is that sufficient? > > In bsg.c, I just see > > @@ -96,7 +96,7 @@ 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); > + hdr->driver_status = 0; > hdr->info = 0; > if (hdr->device_status || hdr->transport_status || hdr->driver_status) > hdr->info |= SG_INFO_CHECK; > hdr->driver_status = 0; > > To me that looks as if userspace might get a different result as > before. Similar in bsg_lib.c. I may be overlooking something again, but > not being familiar with the bsg driver, I thought I'd better ask. > For bsg the driver status was never set, so while in theory any of the DRIVER_XXX values could be returned, they were never set. So that change is valid. > (Nitpick: your change obsoletes the check for hdr->driver_status two > lines below). > Sure, can do. Cheers, Hannes -- Dr. Hannes Reinecke Teamlead Storage & Networking hare@xxxxxxx +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 247165 (AG München), GF: Felix Imendörffer
