Re: [PATCH V2] scsi_lib: removes ambiguous Unhandled error code messages.

2014-07-09 Thread Hannes Reinecke 

On 06/25/2014 01:06 PM, Christoph Hellwig wrote:

Can I get another review for this one?

On Fri, Jun 06, 2014 at 10:10:35AM +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 removes Unhandled error code and replaces Unhandled sense code
with Failing command with sense code:.


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

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 9db097a..b3c25cd 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -955,14 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
int good_bytes)
action = ACTION_FAIL;
break;
default:
-   description = Unhandled sense code;
+   description = Failing command with sense code:;
action = ACTION_FAIL;
break;
}
-   } else {
-   description = Unhandled error code;
+   } else
action = ACTION_FAIL;
-   }

if (action != ACTION_FAIL 
time_before(cmd-jiffies_at_alloc + wait_for, jiffies)) {
--
Maurizio Lombardi


Good to see it finally went in.
We're carrying the same patch in our tree for ages as the original 
attempt to push it upstream failed.


Reviewed-by: Hannes Reinecke h...@suse.de

Cheers,

Hannes
--
Dr. Hannes Reinecke   zSeries  Storage
h...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] scsi_lib: removes ambiguous Unhandled error code messages.

2014-07-08 Thread Maurizio Lombardi 


On 07/03/2014 10:41 AM, Christoph Hellwig wrote:
 On Wed, Jun 25, 2014 at 08:38:47PM +, Elliott, Robert (Server Storage) 
 wrote:
 Since the ACTION_FAIL case always prints the sense key
 and additional sense code:
 
 perhaps the description string should be removed altogether?
 
 
 I'm tempted to agree and just remove the description.  Do you want to
 send a patch for this?

So I'll get rid of the description string completely...
I'm going to send a new patch later today.

Regards,
Maurizio Lombardi
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] scsi_lib: removes ambiguous Unhandled error code messages.

2014-07-03 Thread Christoph Hellwig 
On Wed, Jun 25, 2014 at 08:38:47PM +, Elliott, Robert (Server Storage) 
wrote:
 Since the ACTION_FAIL case always prints the sense key
 and additional sense code:

 perhaps the description string should be removed altogether?

 For the Unhandled error code (for which you are proposing
 removing the string) and the timeout case, the scsi_print_result 
 call already prints hostbyte and driverbyte, which explain what 
 happened in more detail:

I'm tempted to agree and just remove the description.  Do you want to
send a patch for this?
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] scsi_lib: removes ambiguous Unhandled error code messages.

2014-06-25 Thread Christoph Hellwig 
Can I get another review for this one?

On Fri, Jun 06, 2014 at 10:10:35AM +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 removes Unhandled error code and replaces Unhandled sense code
 with Failing command with sense code:.
 
 
 Signed-off-by: Maurizio Lombardi mlomb...@redhat.com
 ---
  drivers/scsi/scsi_lib.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
 index 9db097a..b3c25cd 100644
 --- a/drivers/scsi/scsi_lib.c
 +++ b/drivers/scsi/scsi_lib.c
 @@ -955,14 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
 int good_bytes)
   action = ACTION_FAIL;
   break;
   default:
 - description = Unhandled sense code;
 + description = Failing command with sense code:;
   action = ACTION_FAIL;
   break;
   }
 - } else {
 - description = Unhandled error code;
 + } else
   action = ACTION_FAIL;
 - }
  
   if (action != ACTION_FAIL 
   time_before(cmd-jiffies_at_alloc + wait_for, jiffies)) {
 -- 
 Maurizio Lombardi
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2] scsi_lib: removes ambiguous Unhandled error code messages.

2014-06-25 Thread Mike Christie 
On 06/06/2014 03:10 AM, 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 removes Unhandled error code and replaces Unhandled sense code
 with Failing command with sense code:.
 
 
 Signed-off-by: Maurizio Lombardi mlomb...@redhat.com
 ---
  drivers/scsi/scsi_lib.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
 index 9db097a..b3c25cd 100644
 --- a/drivers/scsi/scsi_lib.c
 +++ b/drivers/scsi/scsi_lib.c
 @@ -955,14 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned 
 int good_bytes)
   action = ACTION_FAIL;
   break;
   default:
 - description = Unhandled sense code;
 + description = Failing command with sense code:;
   action = ACTION_FAIL;
   break;
   }
 - } else {
 - description = Unhandled error code;
 + } else
   action = ACTION_FAIL;
 - }
  
   if (action != ACTION_FAIL 
   time_before(cmd-jiffies_at_alloc + wait_for, jiffies)) {
 

Looks ok to me.

Reviewed-by: Mike Christie micha...@cs.wisc.edu
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V2] scsi_lib: removes ambiguous Unhandled error code messages.

2014-06-25 Thread Elliott, Robert (Server Storage) 


 -Original Message-
 From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
 ow...@vger.kernel.org] On Behalf Of Christoph Hellwig
 Sent: Wednesday, 25 June, 2014 6:07 AM
 To: Maurizio Lombardi
 Cc: james.bottom...@hansenpartnership.com; linux-scsi@vger.kernel.org;
 h...@infradead.org
 Subject: Re: [PATCH V2] scsi_lib: removes ambiguous Unhandled error code
 messages.
 
 Can I get another review for this one?
 
 On Fri, Jun 06, 2014 at 10:10:35AM +0200, Maurizio Lombardi wrote:
...
  diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
...
  @@ -955,14 +955,12 @@ void scsi_io_completion(struct scsi_cmnd *cmd,
 unsigned int good_bytes)
  action = ACTION_FAIL;
  break;
  default:
  -   description = Unhandled sense code;
  +   description = Failing command with sense code:;

The default case handles sense keys other than:
#define NOT_READY   0x02
#define ILLEGAL_REQUEST 0x05
#define UNIT_ATTENTION  0x06
#define ABORTED_COMMAND 0x0b
#define VOLUME_OVERFLOW 0x0d

which means these #defines (and any other values)
#define NO_SENSE0x00
#define RECOVERED_ERROR 0x01
#define MEDIUM_ERROR0x03
#define HARDWARE_ERROR  0x04
#define DATA_PROTECT0x07
#define BLANK_CHECK 0x08
#define COPY_ABORTED0x0a
#define MISCOMPARE  0x0e

The other description strings that result in ACTION_FAIL are based
on the sense key and sometimes the additional sense code:
description = Media Changed;
description = Host Data Integrity Failure;
description = Discard failure;
description = Write same failure;
description = Invalid command failure;
description = Target Data Integrity Failure;
description = Device not ready;

Also, there is this one, which is not based on the sense key:
description = Command timed out;

Since the ACTION_FAIL case always prints the sense key
and additional sense code:
if (!(req-cmd_flags  REQ_QUIET)) {
if (description)
scmd_printk(KERN_INFO, cmd,  % \n,
description);
scsi_print_result(cmd);
if (driver_byte(result)  DRIVER_SENSE)
scsi_print_sense(, cmd);
scsi_print_command(cmd);
}

perhaps the description string should be removed altogether?

In this example:
sd 1:0:0:0: [sdb] Unhandled sense code
sd 1:0:0:0: [sdb]
Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
sd 1:0:0:0: [sdb]
Sense Key : Medium Error [current]
sd 1:0:0:0: [sdb]
Add. Sense: Unrecovered read error
sd 1:0:0:0: [sdb] CDB:
Read(10): 28 00 00 18 f8 a8 00 00 08 00
end_request: critical medium error, dev sdb, sector 1636520

Sense Key : Medium Error is much more informative than
Unhandled sense code or Failing command with sense code.
All the other descriptions represent failing commands,
so using different wording is a bit confusing.

For the Unhandled error code (for which you are proposing
removing the string) and the timeout case, the scsi_print_result 
call already prints hostbyte and driverbyte, which explain what 
happened in more detail:

sd 1:0:0:0: [sda] Unhandled error code  

sd 1:0:0:0: [sda]   

Result: hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK  

sd 1:0:0:0: [sda] CDB:  

Test Unit Ready:

end_request: I/O error, dev sda, sector 481013632   


---
Rob ElliottHP Server Storage




--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html