| Message ID | 20180404181453.30737-1-dgilbert@interlog.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 2018-04-04 at 14:14 -0400, Douglas Gilbert wrote: > Patch e39a97353e53 titled "scsi: core: return BLK_STS_OK for DID_OK in > __scsi_error_from_host_byte()" attempted to make that function return > BLK_STS_OK when it was given host_byte(result)==DID_OK. While that seems > sensible, it failed to take into account that there may be errors present > in the driver_byte and the status byte. Add those checks and expand > description of function accordingly. Hello Doug, Please clarify why you posted a new patch instead of reviewing "[PATCH v2] Fix DID_OK handling in __scsi_error_from_host_byte()". Did you perhaps miss that patch? Thanks, Bart.
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index aaf485e36450..6a8bf312d82f 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -727,13 +727,18 @@ static bool scsi_end_request(struct request *req, blk_status_t error, * @cmd: SCSI command (unused) * @result: scsi error code * - * Translate SCSI error code into block errors. + * Translate SCSI error code into block errors. In the case of result==DID_OK + * also check driver and status bytes for errors. */ static blk_status_t __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result) { switch (host_byte(result)) { case DID_OK: + if (driver_byte(result) != DRIVER_OK) + return BLK_STS_IOERR; + else if (status_byte(result) && !scsi_status_is_good(result)) + return BLK_STS_IOERR; /* e.g. reservation conflict */ return BLK_STS_OK; case DID_TRANSPORT_FAILFAST: return BLK_STS_TRANSPORT;
Patch e39a97353e53 titled "scsi: core: return BLK_STS_OK for DID_OK in __scsi_error_from_host_byte()" attempted to make that function return BLK_STS_OK when it was given host_byte(result)==DID_OK. While that seems sensible, it failed to take into account that there may be errors present in the driver_byte and the status byte. Add those checks and expand description of function accordingly. Signed-off-by: Douglas Gilbert <dgilbert@interlog.com> --- The function would be better called blk_status_from_scsi_result(). Is the leading "__" needed? drivers/scsi/scsi_lib.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)