> -----Original Message----- > From: linux-scsi-owner@xxxxxxxxxxxxxxx [mailto:linux-scsi- > owner@xxxxxxxxxxxxxxx] On Behalf Of Christoph Hellwig > Sent: Wednesday, 25 June, 2014 6:07 AM > To: Maurizio Lombardi > Cc: James.Bottomley@xxxxxxxxxxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx; > hch@xxxxxxxxxxxxx > 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_SENSE 0x00 #define RECOVERED_ERROR 0x01 #define MEDIUM_ERROR 0x03 #define HARDWARE_ERROR 0x04 #define DATA_PROTECT 0x07 #define BLANK_CHECK 0x08 #define COPY_ABORTED 0x0a #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 Elliott HP Server Storage -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
