Re: [PATCH 02/42] scsi_ioctl: return error code when blk_rq_map_kern() fails

Hannes Reinecke
2021-04-22T06:29:45Z

On 4/21/21 11:01 PM, Bart Van Assche wrote:
> On 4/21/21 10:47 AM, Hannes Reinecke wrote:
>> The callers of sg_scsi_ioctl() already check for negative
>> return values, so we can drop the usage of DRIVER_ERROR
>> and return the error from blk_rq_map_kern() instead.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   block/scsi_ioctl.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
>> index 6599bac0a78c..99d58786e0d5 100644
>> --- a/block/scsi_ioctl.c
>> +++ b/block/scsi_ioctl.c
>> @@ -488,9 +488,10 @@ int sg_scsi_ioctl(struct request_queue *q, struct
>> gendisk *disk, fmode_t mode,
>>           break;
>>       }
>> -    if (bytes && blk_rq_map_kern(q, rq, buffer, bytes, GFP_NOIO)) {
>> -        err = DRIVER_ERROR << 24;
>> -        goto error;
>> +    if (bytes) {
>> +        err = blk_rq_map_kern(q, rq, buffer, bytes, GFP_NOIO);
>> +        if (err)
>> +            goto error;
>>       }
>>       blk_execute_rq(disk, rq, 0);
>
> Is this perhaps a backwards-incompatible user space ABI change since the
> sg_scsi_ioctl() return value is reported to user space?
>
Possibly; but we're dealing with semantics here.
All callers to sg_scsi_ioctl() _must_ be prepared to handle negative
values already, so userspace should not break.
We arguably _do_ return a different error here, but experience showed
that basically no-one checks for driver byte values (except for
DRIVER_SENSE, that is).
So I don't think it really matters; we do return an error, and for
userspace it doesn't matter as this command cannot be retried either way.

So while this arguably is a change visible to userspace, I doubt it
matters in the end.

Cheers,

Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer