Re: [PATCH] scsi: core: set result when the command cannot be dispatched

[Bart Van Assche <bvanassche@acm.org>]

On Tue, 2019-04-09 at 14:53 -0700, Jaesoo Lee wrote:
> When SCSI blk-mq is enabled, there is a bug in handling errors in scsi_queue_rq.
> Specifically, the bug is not setting result field of scsi_request correctly when
> the dispatch of the command has been failed. Since the upper layer code
> including the sg_io ioctl expects to receive any error status from result field
> of scsi_request, the error is silently ignored and this could cause data
> corruptions for some applications. This commit also fixes another bug that the
> result field is not initialized when scsi_request is allocated.
> 
> Signed-off-by: Jaesoo Lee <jalee@purestorage.com>
> ---
>  block/scsi_ioctl.c      | 1 +
>  drivers/scsi/scsi_lib.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
> index 533f4ae..f2d7979 100644
> --- a/block/scsi_ioctl.c
> +++ b/block/scsi_ioctl.c
> @@ -723,6 +723,7 @@ void scsi_req_init(struct scsi_request *req)
>         req->cmd = req->__cmd;
>         req->cmd_len = BLK_MAX_CDB;
>         req->sense_len = 0;
> +       req->result = 0;
>  }
>  EXPORT_SYMBOL(scsi_req_init);

What makes you think that this assignment is necessary?

> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 2018967..af1488d 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1699,6 +1699,7 @@ static blk_status_t scsi_queue_rq(struct
> blk_mq_hw_ctx *hctx,
>                         ret = BLK_STS_DEV_RESOURCE;
>                 break;
>         default:
> +               scsi_req(req)->result = DID_NO_CONNECT << 16;
>                 /*
>                  * Make sure to release all allocated ressources when
>                  * we hit an error, as we will never see this command

What leads you to the conclusion that (ret != BLK_STS_OK &&
ret != BLK_STS_RESOUCE) means that there is a connectivity issue?

Thanks,

Bart.