> -----Original Message-----
> From: Hannes Reinecke [mailto:h...@suse.de]
> Sent: Thursday, 06 November, 2014 2:31 AM
...
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
...
> diff --git a/drivers/scsi/scsi_logging.c
> b/drivers/scsi/scsi_logging.c
> index 065792a3..e7e7cab 100644
> --- a/drivers/scsi/scsi_logging.c
> +++ b/drivers/scsi/scsi_logging.c
> @@ -398,3 +398,47 @@ void scsi_print_sense(const struct scsi_cmnd
> *cmd)
> cmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
> }
> EXPORT_SYMBOL(scsi_print_sense);
> +
> +void scsi_print_result(struct scsi_cmnd *cmd, const char *msg, int
> disposition)
Since this function does not modify anything pointed to by cmd,
consider adding const to that argument.
> +{
> + char *logbuf;
> + size_t off, logbuf_len;
> + const char *mlret_string = scsi_mlreturn_string(disposition);
As mentioned in the last series, it might be good to
change the midlayer string from SUCCESS to COMPLETE,
since that is printed even for commands that fail with
errors.
(This patch series doesn't touch that function, so mentioning
the issue here at the only caller)
>
> + const char *hb_string = scsi_hostbyte_string(cmd->result);
> + const char *db_string = scsi_driverbyte_string(cmd->result);
> +
> + logbuf = scsi_log_reserve_buffer(&logbuf_len);
> + if (!logbuf)
> + return;
> +
> + off = sdev_format_header(logbuf, logbuf_len,
> + scmd_name(cmd), cmd->request->tag);
> +
> + if (msg)
> + off += scnprintf(logbuf + off, logbuf_len - off,
> + "%s: ", msg);
> + if (mlret_string)
> + off += scnprintf(logbuf + off, logbuf_len - off,
> + "%s ", mlret_string);
> + else
> + off += scnprintf(logbuf + off, logbuf_len - off,
> + "UNKNOWN(0x%02x) ", disposition);
> + off += scnprintf(logbuf + off, logbuf_len - off, "Result: ");
...
Consider printing "Result: " first. That would make
the messages more consistent since "Done:" is not always there.
Current:
[491784.462209] sd 1:0:0:0: [sdq] tag#0 Done: SUCCESS Result: hostbyte=DID_OK
driverbyte=DRIVER_OK
[491784.464962] sd 1:0:0:0: [sdq] tag#0 CDB: Write(10) 2a 00 39 8c fa 80 00 00
08 00
[ 259.667383] sd 2:0:0:0: [sdr] tag#334 FAILED Result: hostbyte=DID_OK
driverbyte=DRIVER_SENSE
[ 259.667387] sd 2:0:0:0: [sdr] tag#334 Sense Key : Hardware Error [current]
[ 259.667391] sd 2:0:0:0: [sdr] tag#334 Add. Sense: Logical unit failure
[ 259.667395] sd 2:0:0:0: [sdr] tag#334 CDB: Read(10) 28 00 1b 85 9d 30 00 00
08 00
[27907.647377] sd 1:0:0:0: [sdq] tag#768 Done: TIMEOUT_ERROR Result:
hostbyte=DID_OK driverbyte=DRIVER_OK
[27907.647380] sd 1:0:0:0: [sdq] tag#768 CDB: Write(10) 2a 00 39 8a f4 e0 00
00 08 00
[27907.647382] sd 1:0:0:0: [sdq] tag#768 scmd ffff880424761470 abort scheduled
Proposed:
[491784.462209] sd 1:0:0:0: [sdq] tag#0 Result: Done: COMPLETE hostbyte=DID_OK
driverbyte=DRIVER_OK
[491784.464962] sd 1:0:0:0: [sdq] tag#0 CDB: Write(10) 2a 00 39 8c fa 80 00 00
08 00
[ 259.667383] sd 2:0:0:0: [sdr] tag#334 Result: FAILED hostbyte=DID_OK
driverbyte=DRIVER_SENSE
[ 259.667387] sd 2:0:0:0: [sdr] tag#334 Sense Key : Hardware Error [current]
[ 259.667391] sd 2:0:0:0: [sdr] tag#334 Add. Sense: Logical unit failure
[ 259.667395] sd 2:0:0:0: [sdr] tag#334 CDB: Read(10) 28 00 1b 85 9d 30 00 00
08 00
[27907.647377] sd 1:0:0:0: [sdq] tag#768 Result: Done: TIMEOUT_ERROR
hostbyte=DID_OK driverbyte=DRIVER_OK
[27907.647380] sd 1:0:0:0: [sdq] tag#768 CDB: Write(10) 2a 00 39 8a f4 e0 00
00 08 00
[27907.647382] sd 1:0:0:0: [sdq] tag#768 scmd ffff880424761470 abort scheduled
Furthermore, consider dropping "Done" (the only msg argument
ever used) altogether. "Done" means scsi_log_completion is
printing this. No "Done" means scsi_io_completion's ACTION_FAIL
case is printing this.
The disposition (COMPLETE, FAILED, TIMEOUT_ERROR, etc.) seems
to convey that same information, in more detail.
Reviewed-by: Robert Elliott <elli...@hp.com>
---
Rob Elliott HP 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
