Re: [PATCH v2] scsi: bsg: fix errno when scsi_bsg_register_queue fails

From: Jackie Liu <liu.yun@linux.dev>
To: Guenter Roeck <linux@roeck-us.net>
Cc: martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
	hch@lst.de, axboe@kernel.dk
Subject: Re: [PATCH v2] scsi: bsg: fix errno when scsi_bsg_register_queue fails
Date: Thu, 23 Dec 2021 09:07:08 +0800	[thread overview]
Message-ID: <6671fc20-e543-71c5-c505-395e6ee7e744@linux.dev> (raw)
In-Reply-To: <20211222165435.GA283263@roeck-us.net>

Hi Guenter.

Before commit ead09dd3aed5c ("scsi: bsg: Simplify device registration", 
the errno need return to the caller, I restore the old logic, and print
this errno.

--
Jackie Liu

在 2021/12/23 上午12:54, Guenter Roeck 写道:
> On Fri, Oct 22, 2021 at 09:02:01AM +0800, Jackie Liu wrote:
>> From: Jackie Liu <liuyun01@kylinos.cn>
>>
>> When the value of error is printed, it will always be 0. Here, we should be
>> print the correct error code when scsi_bsg_register_queue fails.
>>
> 
> The comment above the changed code says:
> 
> "
> We're treating error on bsg register as non-fatal, so pretend nothing went wrong.
> "
> 
> With this patch in place, "error" is returned to the caller, and the code
> no longer pretends that nothing is wrong. Also, the message is a dev_info
> message, not dev_err, suggesting that ignoring the error was indeed on
> purpose. Assuming the comment is correct, this patch is plain wrong;
> the message should have printed PTR_ERR(sdev->bsg_dev) instead and not set
> the 'error' variable.
> 
> Guenter
> 
>> Fixes: ead09dd3aed5 ("scsi: bsg: Simplify device registration")
>> Cc: Jens Axboe <axboe@kernel.dk>
>> Cc: Christoph Hellwig <hch@lst.de>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>> ---
>>   v1->v2:
>>   resend to linux-scsi mail list.
>>
>>   drivers/scsi/scsi_sysfs.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
>> index 86793259e541..d8789f6cda62 100644
>> --- a/drivers/scsi/scsi_sysfs.c
>> +++ b/drivers/scsi/scsi_sysfs.c
>> @@ -1379,6 +1379,7 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
>>   			 * We're treating error on bsg register as non-fatal, so
>>   			 * pretend nothing went wrong.
>>   			 */
>> +			error = PTR_ERR(sdev->bsg_dev);
>>   			sdev_printk(KERN_INFO, sdev,
>>   				    "Failed to register bsg queue, errno=%d\n",
>>   				    error);
>> -- 
>> 2.25.1
>>