[PATCH] scsi_lib: replace ambiguous "Unhandled error code" messages.

[PATCH] scsi_lib: replace ambiguous "Unhandled error code" messages.

[Maurizio Lombardi]

During IO with fabric faults, one generally sees several "Unhandled error
code" messages in the syslog as shown below:

sd 4:0:6:2: [sdbw] Unhandled error code
sd 4:0:6:2: [sdbw] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK
sd 4:0:6:2: [sdbw] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00
end_request: I/O error, dev sdbw, sector 0

This comes from scsi_io_completion (in scsi_lib.c) while handling error
codes other than DID_RESET or not deferred sense keys i.e. this is
actually handled by the SCSI mid layer. But what gets displayed here is
"Unhandled error code" which is quite misleading as it indicates
something that is not addressed by the mid layer.

This patch replaces "Unhandled error code" with "Extended error description
not available".

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/scsi/scsi_lib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9db097a..f2a1e13 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -955,12 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 			action = ACTION_FAIL;
 			break;
 		default:
-			description = "Unhandled sense code";
+			description = "Extended sense code description not available";
 			action = ACTION_FAIL;
 			break;
 		}
 	} else {
-		description = "Unhandled error code";
+		description = "Extended error code description not available";
 		action = ACTION_FAIL;
 	}
 
-- 
Maurizio Lombardi

Re: [PATCH] scsi_lib: replace ambiguous "Unhandled error code" messages.

[Christoph Hellwig]

On Mon, May 26, 2014 at 12:13:24PM +0200, Maurizio Lombardi wrote:
> During IO with fabric faults, one generally sees several "Unhandled error
> code" messages in the syslog as shown below:
> 
> sd 4:0:6:2: [sdbw] Unhandled error code
> sd 4:0:6:2: [sdbw] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK
> sd 4:0:6:2: [sdbw] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00
> end_request: I/O error, dev sdbw, sector 0
> 
> This comes from scsi_io_completion (in scsi_lib.c) while handling error
> codes other than DID_RESET or not deferred sense keys i.e. this is
> actually handled by the SCSI mid layer. But what gets displayed here is
> "Unhandled error code" which is quite misleading as it indicates
> something that is not addressed by the mid layer.
> 
> This patch replaces "Unhandled error code" with "Extended error description
> not available".

How about simple not setting description at all for this case?

Re: [PATCH] scsi_lib: replace ambiguous "Unhandled error code" messages.

[Maurizio Lombardi]

Hi,

On Mon, May 26, 2014 at 09:25:06AM -0700, Christoph Hellwig wrote:
> On Mon, May 26, 2014 at 12:13:24PM +0200, Maurizio Lombardi wrote:
> > During IO with fabric faults, one generally sees several "Unhandled error
> > code" messages in the syslog as shown below:
> > 
> > sd 4:0:6:2: [sdbw] Unhandled error code
> > sd 4:0:6:2: [sdbw] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK
> > sd 4:0:6:2: [sdbw] CDB: Read(10): 28 00 00 00 00 00 00 00 08 00
> > end_request: I/O error, dev sdbw, sector 0
> > 
> > This comes from scsi_io_completion (in scsi_lib.c) while handling error
> > codes other than DID_RESET or not deferred sense keys i.e. this is
> > actually handled by the SCSI mid layer. But what gets displayed here is
> > "Unhandled error code" which is quite misleading as it indicates
> > something that is not addressed by the mid layer.
> > 
> > This patch replaces "Unhandled error code" with "Extended error description
> > not available".
> 
> How about simple not setting description at all for this case?
> 

It has already been proposed before but James didn't like the idea.

http://markmail.org/message/dumujpz4gfp3s4fp#query:+page:1+mid:dumujpz4gfp3s4fp+state:results

Regards,
Maurizio Lombardi

Re: [PATCH] scsi_lib: replace ambiguous "Unhandled error code" messages.

[Christoph Hellwig]

On Mon, May 26, 2014 at 09:27:15PM +0200, Maurizio Lombardi wrote:
> > How about simple not setting description at all for this case?
> > 
> 
> It has already been proposed before but James didn't like the idea.
> 
> http://markmail.org/message/dumujpz4gfp3s4fp#query:+page:1+mid:dumujpz4gfp3s4fp+state:results

James never replied to Robs question.  I can't think of any value add
that "Unhandled XYZ" adds when we already get a usesul message from the
same error.

James, care to comment what'd you like to see printed here?

Re: [PATCH] scsi_lib: replace ambiguous "Unhandled error code" messages.

[James Bottomley]

On Wed, 2014-05-28 at 04:00 -0700, Christoph Hellwig wrote:
> On Mon, May 26, 2014 at 09:27:15PM +0200, Maurizio Lombardi wrote:
> > > How about simple not setting description at all for this case?
> > > 
> > 
> > It has already been proposed before but James didn't like the idea.
> > 
> > http://markmail.org/message/dumujpz4gfp3s4fp#query:+page:1+mid:dumujpz4gfp3s4fp+state:results
> 
> James never replied to Robs question.  I can't think of any value add
> that "Unhandled XYZ" adds when we already get a usesul message from the
> same error.
> 
> James, care to comment what'd you like to see printed here?

I'm happy with eliminating "Unhanndled Error Code" because that is
misleading ... we should only get there if we have a DID_X return and
they're all fatal errors which will be printed.

I'm less happy removing "Unhandled Sense Code".  The danger is that we
get some harmless sense code we should have handled and instead error
the command.  Since for this error we know the next print will be the
sense code, what about changing it to "Failing Command with sense code:"

James

Re: [PATCH] scsi_lib: replace ambiguous "Unhandled error code" messages.

[Maurizio Lombardi]

On Wed, May 28, 2014 at 06:27:52PM +0400, James Bottomley wrote:
> 
> I'm happy with eliminating "Unhanndled Error Code" because that is
> misleading ... we should only get there if we have a DID_X return and
> they're all fatal errors which will be printed.
> 
> I'm less happy removing "Unhandled Sense Code".  The danger is that we
> get some harmless sense code we should have handled and instead error
> the command.  Since for this error we know the next print will be the
> sense code, what about changing it to "Failing Command with sense code:"

It's ok with me,

I'm going to resend the patch in a few minutes.

Thanks,
Maurizio Lombardi