Re: [PATCHv2] scsi: Fix failed request error code

Bart Van Assche
2018-04-04T15:44:10Z

On Wed, 2018-04-04 at 09:27 +0200, Hannes Reinecke wrote:
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 393f9db8f41b..9389c41e2829 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -905,6 +905,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
> goto requeue;
>
> error = __scsi_error_from_host_byte(cmd, result);
> + /*
> + * If the hostbyte was DID_OK, but the sense code is valid
> + * we always should set BLK_STS_IOERR.
> + */
> + if (error == BLK_STS_OK && sense_valid)
> + error = BLK_STS_IOERR;
>
> if (host_byte(result) == DID_RESET) {
> /* Third party bus reset or reset for error recovery

__scsi_error_from_host_byte() has two callers. Why does this patch only
update one of these two callers?

Regarding commit e39a97353e53, the description of that commit is as follows:
"When converting __scsi_error_from_host_byte() to BLK_STS error codes the
case DID_OK was forgotten, resulting in it always returning an error."
However, the comment above that function reads as follows: "translate SCSI
error code into errno". If I have a look at the v4.12 SCSI core (before the
blkstatus_t conversion) then I see that __scsi_error_from_host_byte() never
returns 0. That means that the description of commit e39a97353e53 ("scsi:
core: return BLK_STS_OK for DID_OK in __scsi_error_from_host_byte()") is
wrong. Does that mean that commit e39a97353e53 should be reverted?

Thanks,

Bart.