[PATCH] Remove 'unhandled error code' messages

[PATCH] Remove 'unhandled error code' messages

[Hannes Reinecke]

scsi_io_completion() tries to take some action based on
the command result and sense code. It also displays
'unhandled error code' or 'unhandled sense code' in case
no special handling was found.
This serves as an additional source of confusion to
the unsuspecting user, as the message in fact means
'everything okay, no special casing required',
and not 'oh gosh, something has happened and the system
couldn't deal with it'.

Plus it doesn't add any value for debugging, as the
actual error and sense code is printed with the next
statement anyway.

So remove these lines.

Signed-off-by: Hannes Reinecke <hare@suse.de>

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f85cfa6..1975b3f 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -921,12 +921,10 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 			action = ACTION_FAIL;
 			break;
 		default:
-			description = "Unhandled sense code";
 			action = ACTION_FAIL;
 			break;
 		}
 	} else {
-		description = "Unhandled error code";
 		action = ACTION_FAIL;
 	}
 

Re: [PATCH] Remove 'unhandled error code' messages

[James Bottomley]

On Thu, 2011-11-10 at 15:12 +0100, Hannes Reinecke wrote:
> scsi_io_completion() tries to take some action based on
> the command result and sense code. It also displays
> 'unhandled error code' or 'unhandled sense code' in case
> no special handling was found.
> This serves as an additional source of confusion to
> the unsuspecting user, as the message in fact means
> 'everything okay, no special casing required',
> and not 'oh gosh, something has happened and the system
> couldn't deal with it'.

It means we're just about to fail the command with an error.  That's not
really an everything ok case.

Sometimes, you're right, this is the correct thing to do silently, but
often it's not.  So, same question to you as to Rob: What are the
circumstances you want silent failure for and can you special case them?

James

Re: [PATCH] Remove 'unhandled error code' messages

[Hannes Reinecke]

On 11/10/2011 03:55 PM, James Bottomley wrote:
> On Thu, 2011-11-10 at 15:12 +0100, Hannes Reinecke wrote:
>> scsi_io_completion() tries to take some action based on
>> the command result and sense code. It also displays
>> 'unhandled error code' or 'unhandled sense code' in case
>> no special handling was found.
>> This serves as an additional source of confusion to
>> the unsuspecting user, as the message in fact means
>> 'everything okay, no special casing required',
>> and not 'oh gosh, something has happened and the system
>> couldn't deal with it'.
>
> It means we're just about to fail the command with an error.  That's not
> really an everything ok case.
>
> Sometimes, you're right, this is the correct thing to do silently, but
> often it's not.  So, same question to you as to Rob: What are the
> circumstances you want silent failure for and can you special case them?
>
The whole point here is: It's not silent, even when the 
'description' is not set.

	switch (action) {
	case ACTION_FAIL:
		/* Give up and fail the remainder of the request */
		scsi_release_buffers(cmd);
		if (!(req->cmd_flags & REQ_QUIET)) {
			if (description)
				scmd_printk(KERN_INFO, cmd, "%s\n",
					    description);
			scsi_print_result(cmd);
			if (driver_byte(result) & DRIVER_SENSE)
				scsi_print_sense("", cmd);
			scsi_print_command(cmd);
		}
		if (blk_end_request_err(req, error))
			scsi_requeue_command(q, cmd);
		else
			scsi_next_command(cmd);
		break;

So the decoded result and sense code is printed always, regardless 
of the 'description'.
The 'description' just tells us if there are special
circumstances. And as we didn't do anything special
we should print anything here.

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-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Remove 'unhandled error code' messages

[James Bottomley]

On Thu, 2011-11-10 at 16:18 +0100, Hannes Reinecke wrote:
> On 11/10/2011 03:55 PM, James Bottomley wrote:
> > On Thu, 2011-11-10 at 15:12 +0100, Hannes Reinecke wrote:
> >> scsi_io_completion() tries to take some action based on
> >> the command result and sense code. It also displays
> >> 'unhandled error code' or 'unhandled sense code' in case
> >> no special handling was found.
> >> This serves as an additional source of confusion to
> >> the unsuspecting user, as the message in fact means
> >> 'everything okay, no special casing required',
> >> and not 'oh gosh, something has happened and the system
> >> couldn't deal with it'.
> >
> > It means we're just about to fail the command with an error.  That's not
> > really an everything ok case.
> >
> > Sometimes, you're right, this is the correct thing to do silently, but
> > often it's not.  So, same question to you as to Rob: What are the
> > circumstances you want silent failure for and can you special case them?
> >
> The whole point here is: It's not silent, even when the 
> 'description' is not set.

So all we do is a random dump of result, sense and command.

That doesn't look like an improvement to me.

We've had a set of discussions and proposals:

http://marc.info/?t=131854150100005
http://marc.info/?t=131806586600055

But nothing really leaps out as the solution.  What the system is saying
is that we encountered an error but it's not one of our handled cases,
so we're just going to fail the command and hope for the best.

James

Re: [PATCH] Remove 'unhandled error code' messages

[Rob Evers]

On 11/10/2011 10:44 AM, James Bottomley wrote:
> On Thu, 2011-11-10 at 16:18 +0100, Hannes Reinecke wrote:
>> On 11/10/2011 03:55 PM, James Bottomley wrote:
>>> On Thu, 2011-11-10 at 15:12 +0100, Hannes Reinecke wrote:
>>>> scsi_io_completion() tries to take some action based on
>>>> the command result and sense code. It also displays
>>>> 'unhandled error code' or 'unhandled sense code' in case
>>>> no special handling was found.
>>>> This serves as an additional source of confusion to
>>>> the unsuspecting user, as the message in fact means
>>>> 'everything okay, no special casing required',
>>>> and not 'oh gosh, something has happened and the system
>>>> couldn't deal with it'.
>>> It means we're just about to fail the command with an error.  That's not
>>> really an everything ok case.
>>>
>>> Sometimes, you're right, this is the correct thing to do silently, but
>>> often it's not.  So, same question to you as to Rob: What are the
>>> circumstances you want silent failure for and can you special case them?
>>>

This is what I have seen (recent rhel6):

Nov 10 03:42:06 <host-name> kernel: sd 2:0:0:1: [sdb] Unhandled error code
Nov 10 03:42:06 <host-name> kernel: sd 2:0:0:1: [sdb] Result: 
hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK
Nov 10 03:42:06 <host-name> kernel: sd 2:0:0:1: [sdb] CDB: Read(10): 28 
00 00 4e 91 e0 00 00 20 00
Nov 10 03:42:06 <host-name> kernel: end_request: I/O error, dev sdb, 
sector 5149152

Would the correct solution for this case to be to make description:

     ""

or a special case, something like:

     "Couldn't connect before timeout period"

as I posted here:
http://marc.info/?l=linux-scsi&m=131854148309464&w=2

In the post I made above, I also liked

+			description = "Default sense code handling";

for the sense case.

and potentially something like

+	description = "Default host byte handling";

for the host_byte case when a host_byte isn't handled as
an alternative catch-all.