[PATCHv2 0/5] scsi: More detailed I/O errors

[PATCHv2 0/5] scsi: More detailed I/O errors

[Hannes Reinecke]

As discussed at LSF the SCSI stack already knows about several
error conditions, it's just not well documented.
Also there are some more conditions upper layers might be
interested in.

This patchset improves the documentation of the existing
I/O error codes and adds three more error codes, ENOSPC
for thin provisioning failure, ENODATA for medium error,
and ENXIO for an invalid or removed device.

Changes since the initial version:
- Correctly document all return codes in
  __scsi_error_from_host_byte()
- Set hostbyte status in scsi_check_sense()
- Return ENXIO on invalid or removed device

Hannes Reinecke (5):
  scsi: Document enhanced error codes
  scsi: Set hostbyte status in scsi_check_sense()
  scsi: return ENOSPC on thin provisioning failure
  scsi: Return ENODATA on medium error
  scsi: Return ENXIO on invalid device

 block/blk-core.c          |  9 +++++++++
 drivers/scsi/scsi_error.c | 28 +++++++++++++---------------
 drivers/scsi/scsi_lib.c   | 29 ++++++++++++++++++++++++++++-
 include/scsi/scsi.h       |  3 ++-
 4 files changed, 52 insertions(+), 17 deletions(-)

-- 
1.7.12.4

[PATCH 1/5] scsi: Document enhanced error codes

[Hannes Reinecke]

Document the various error codes returned on I/O failure.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_lib.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 86d5220..d3fc50f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -700,6 +700,18 @@ void scsi_release_buffers(struct scsi_cmnd *cmd)
 }
 EXPORT_SYMBOL(scsi_release_buffers);
 
+/**
+ * __scsi_error_from_host_byte - translate SCSI error code into errno
+ * @cmd:	SCSI command (unused)
+ * @result:	scsi error code
+ *
+ * Translate SCSI error code into standard UNIX errno.
+ * Return values:
+ * -ENOLINK	temporary transport failure
+ * -EREMOTEIO	permanent target failure, do not retry
+ * -EBADE	permanent nexus failure, retry on other path
+ * -EIO		unspecified I/O error
+ */
 static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
 {
 	int error = 0;
-- 
1.7.12.4

[PATCH 2/5] scsi: Set hostbyte status in scsi_check_sense()

[Hannes Reinecke]

We should be modifying the host_byte status in scsi_check_sense()
directly; this saves us to introduce a special return code for
each and every condition.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_error.c | 21 +++++++--------------
 include/scsi/scsi.h       |  1 -
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 84369f2..e5cb162 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -248,7 +248,7 @@ static inline void scsi_eh_prt_fail_stats(struct Scsi_Host *shost,
  * @scmd:	Cmd to have sense checked.
  *
  * Return value:
- *	SUCCESS or FAILED or NEEDS_RETRY or TARGET_ERROR
+ *	SUCCESS or FAILED or NEEDS_RETRY or ADD_TO_MLQUEUE
  *
  * Notes:
  *	When a deferred error is detected the current command has
@@ -379,13 +379,15 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 	case MISCOMPARE:
 	case BLANK_CHECK:
 	case DATA_PROTECT:
-		return TARGET_ERROR;
+		set_host_byte(scmd, DID_TARGET_FAILURE);
+		return SUCCESS;
 
 	case MEDIUM_ERROR:
 		if (sshdr.asc == 0x11 || /* UNRECOVERED READ ERR */
 		    sshdr.asc == 0x13 || /* AMNF DATA FIELD */
 		    sshdr.asc == 0x14) { /* RECORD NOT FOUND */
-			return TARGET_ERROR;
+			set_host_byte(scmd, DID_TARGET_FAILURE);
+			return SUCCESS;
 		}
 		return NEEDS_RETRY;
 
@@ -393,14 +395,14 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 		if (scmd->device->retry_hwerror)
 			return ADD_TO_MLQUEUE;
 		else
-			return TARGET_ERROR;
+			set_host_byte(scmd, DID_TARGET_FAILURE);
 
 	case ILLEGAL_REQUEST:
 		if (sshdr.asc == 0x20 || /* Invalid command operation code */
 		    sshdr.asc == 0x21 || /* Logical block address out of range */
 		    sshdr.asc == 0x24 || /* Invalid field in cdb */
 		    sshdr.asc == 0x26) { /* Parameter value invalid */
-			return TARGET_ERROR;
+			set_host_byte(scmd, DID_TARGET_FAILURE);
 		}
 		return SUCCESS;
 
@@ -863,7 +865,6 @@ retry:
 		case SUCCESS:
 		case NEEDS_RETRY:
 		case FAILED:
-		case TARGET_ERROR:
 			break;
 		case ADD_TO_MLQUEUE:
 			rtn = NEEDS_RETRY;
@@ -1690,14 +1691,6 @@ int scsi_decide_disposition(struct scsi_cmnd *scmd)
 		rtn = scsi_check_sense(scmd);
 		if (rtn == NEEDS_RETRY)
 			goto maybe_retry;
-		else if (rtn == TARGET_ERROR) {
-			/*
-			 * Need to modify host byte to signal a
-			 * permanent target failure
-			 */
-			set_host_byte(scmd, DID_TARGET_FAILURE);
-			rtn = SUCCESS;
-		}
 		/* if rtn == FAILED, we have no sense information;
 		 * returning FAILED will wake the error handler thread
 		 * to collect the sense and redo the decide
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 66216c1..07d4bd916 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -481,7 +481,6 @@ static inline int scsi_is_wlun(unsigned int lun)
 #define TIMEOUT_ERROR   0x2007
 #define SCSI_RETURN_NOT_HANDLED   0x2008
 #define FAST_IO_FAIL	0x2009
-#define TARGET_ERROR    0x200A
 
 /*
  * Midlevel queue return values.
-- 
1.7.12.4

[PATCH 3/5] scsi: return ENOSPC on thin provisioning failure

[Hannes Reinecke]

When the thin provisioning hard threshold is reached we
should return ENOSPC to inform upper layers about this fact.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-core.c          | 3 +++
 drivers/scsi/scsi_error.c | 7 ++++++-
 drivers/scsi/scsi_lib.c   | 5 +++++
 include/scsi/scsi.h       | 1 +
 4 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index d5745b5..170607c 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2315,6 +2315,9 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
 		case -EBADE:
 			error_type = "critical nexus";
 			break;
+		case -ENOSPC:
+			error_type = "critical space allocation";
+			break;
 		case -EIO:
 		default:
 			error_type = "I/O";
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index e5cb162..aeec0e7 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -374,11 +374,16 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 		return SUCCESS;
 
 		/* these are not supported */
+	case DATA_PROTECT:
+		if (sshdr.asc == 0x27 && sshdr.ascq == 0x07) {
+			/* Thin provisioning hard threshold reached */
+			set_host_byte(scmd, DID_ALLOC_FAILURE);
+			return SUCCESS;
+		}
 	case COPY_ABORTED:
 	case VOLUME_OVERFLOW:
 	case MISCOMPARE:
 	case BLANK_CHECK:
-	case DATA_PROTECT:
 		set_host_byte(scmd, DID_TARGET_FAILURE);
 		return SUCCESS;
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d3fc50f..8a2f92d 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -710,6 +710,7 @@ EXPORT_SYMBOL(scsi_release_buffers);
  * -ENOLINK	temporary transport failure
  * -EREMOTEIO	permanent target failure, do not retry
  * -EBADE	permanent nexus failure, retry on other path
+ * -ENOSPC	No write space available
  * -EIO		unspecified I/O error
  */
 static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
@@ -728,6 +729,10 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
 		set_host_byte(cmd, DID_OK);
 		error = -EBADE;
 		break;
+	case DID_ALLOC_FAILURE:
+		set_host_byte(cmd, DID_OK);
+		error = -ENOSPC;
+		break;
 	default:
 		error = -EIO;
 		break;
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 07d4bd916..7d5b8e2 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -452,6 +452,7 @@ static inline int scsi_is_wlun(unsigned int lun)
 				 * other paths */
 #define DID_NEXUS_FAILURE 0x11  /* Permanent nexus failure, retry on other
 				 * paths might yield different results */
+#define DID_ALLOC_FAILURE 0x12  /* Space allocation on the device failed */
 #define DRIVER_OK       0x00	/* Driver status                           */
 
 /*
-- 
1.7.12.4

[PATCH 4/5] scsi: Return ENODATA on medium error

[Hannes Reinecke]

When a medium error is detected the SCSI stack should return
ENODATA to the upper layers.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-core.c          | 3 +++
 drivers/scsi/scsi_error.c | 2 +-
 drivers/scsi/scsi_lib.c   | 5 +++++
 include/scsi/scsi.h       | 1 +
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 170607c..1093ca1 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2318,6 +2318,9 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
 		case -ENOSPC:
 			error_type = "critical space allocation";
 			break;
+		case -ENODATA:
+			error_type = "critical medium";
+			break;
 		case -EIO:
 		default:
 			error_type = "I/O";
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index aeec0e7..7f794d4 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -391,7 +391,7 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 		if (sshdr.asc == 0x11 || /* UNRECOVERED READ ERR */
 		    sshdr.asc == 0x13 || /* AMNF DATA FIELD */
 		    sshdr.asc == 0x14) { /* RECORD NOT FOUND */
-			set_host_byte(scmd, DID_TARGET_FAILURE);
+			set_host_byte(scmd, DID_MEDIUM_ERROR);
 			return SUCCESS;
 		}
 		return NEEDS_RETRY;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8a2f92d..70ca1f6 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -711,6 +711,7 @@ EXPORT_SYMBOL(scsi_release_buffers);
  * -EREMOTEIO	permanent target failure, do not retry
  * -EBADE	permanent nexus failure, retry on other path
  * -ENOSPC	No write space available
+ * -ENODATA	Medium error
  * -EIO		unspecified I/O error
  */
 static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
@@ -733,6 +734,10 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
 		set_host_byte(cmd, DID_OK);
 		error = -ENOSPC;
 		break;
+	case DID_MEDIUM_ERROR:
+		set_host_byte(cmd, DID_OK);
+		error = -ENODATA;
+		break;
 	default:
 		error = -EIO;
 		break;
diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 7d5b8e2..95ce204 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -453,6 +453,7 @@ static inline int scsi_is_wlun(unsigned int lun)
 #define DID_NEXUS_FAILURE 0x11  /* Permanent nexus failure, retry on other
 				 * paths might yield different results */
 #define DID_ALLOC_FAILURE 0x12  /* Space allocation on the device failed */
+#define DID_MEDIUM_ERROR  0x13  /* Medium error */
 #define DRIVER_OK       0x00	/* Driver status                           */
 
 /*
-- 
1.7.12.4

[PATCH 5/5] scsi: Return ENXIO on invalid device

[Hannes Reinecke]

The scsi stack already uses DID_NO_CONNECT internally to signal
an invalid or removed device. This should be mapped to ENXIO,
as this then allows us to use a correct error code in
scsi_prep_return().

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 block/blk-core.c        | 3 +++
 drivers/scsi/scsi_lib.c | 7 ++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 1093ca1..875b9da 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2321,6 +2321,9 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
 		case -ENODATA:
 			error_type = "critical medium";
 			break;
+		case -ENXIO:
+			error_type = "invalid device";
+			break;
 		case -EIO:
 		default:
 			error_type = "I/O";
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 70ca1f6..86d9980 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -712,6 +712,7 @@ EXPORT_SYMBOL(scsi_release_buffers);
  * -EBADE	permanent nexus failure, retry on other path
  * -ENOSPC	No write space available
  * -ENODATA	Medium error
+ * -ENXIO	Invalid or removed device
  * -EIO		unspecified I/O error
  */
 static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
@@ -738,6 +739,10 @@ static int __scsi_error_from_host_byte(struct scsi_cmnd *cmd, int result)
 		set_host_byte(cmd, DID_OK);
 		error = -ENODATA;
 		break;
+	case DID_NO_CONNECT:
+		set_host_byte(cmd, DID_OK);
+		error = -ENXIO;
+		break;
 	default:
 		error = -EIO;
 		break;
@@ -1286,7 +1291,7 @@ int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 
 	switch (ret) {
 	case BLKPREP_KILL:
-		req->errors = DID_NO_CONNECT << 16;
+		req->errors = -ENXIO;
 		/* release the command and kill it */
 		if (req->special) {
 			struct scsi_cmnd *cmd = req->special;
-- 
1.7.12.4

Re: [PATCH 4/5] scsi: Return ENODATA on medium error

[Jun'ichi Nomura]

On 07/01/13 17:12, Hannes Reinecke wrote:
> When a medium error is detected the SCSI stack should return
> ENODATA to the upper layers.

Hi Hannes,

since you change the error code from -EREMOTEIO to -ENODATA/-ENOSPC,
upper layers that checks -EREMOTEIO have to be updated as well.

Something like below for dm-multipath.
It seems btrfs checking -EREMOTEIO, too.

-- 
Jun'ichi Nomura, NEC Corporation

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index bdf26f5..15bf881 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1261,6 +1261,21 @@ static void activate_path(struct work_struct *work)
 				pg_init_done, pgpath);
 }
 
+static int maybe_path_failure(int error)
+{
+	switch(error) {
+	case -EOPNOTSUPP:
+	case -EREMOTEIO:
+	case -EILSEQ:
+	case -ENOSPC:
+	case -ENODATA:
+		return 0;
+	}
+
+	/* Anything else could be a path failure */
+	return 1;
+}
+
 /*
  * end_io handling
  */
@@ -1284,7 +1299,7 @@ static int do_end_io(struct multipath *m, struct request *clone,
 	if (!error && !clone->errors)
 		return 0;	/* I/O complete */
 
-	if (error == -EOPNOTSUPP || error == -EREMOTEIO || error == -EILSEQ)
+	if (!maybe_path_failure(error))
 		return error;
 
 	if (mpio->pgpath)

Re: [PATCH 4/5] scsi: Return ENODATA on medium error

[Hannes Reinecke]

On 07/01/2013 11:12 AM, Jun'ichi Nomura wrote:
> On 07/01/13 17:12, Hannes Reinecke wrote:
>> When a medium error is detected the SCSI stack should return
>> ENODATA to the upper layers.
> 
> Hi Hannes,
> 
> since you change the error code from -EREMOTEIO to -ENODATA/-ENOSPC,
> upper layers that checks -EREMOTEIO have to be updated as well.
> 
> Something like below for dm-multipath.
> It seems btrfs checking -EREMOTEIO, too.
> 
Good point.

Will be updating the patchset.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html