| Message ID | 20210122083918.901-1-martin.kepplinger@puri.sm (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | scsi_logging: print cdb into new line after opcode | expand |
On Fri, 2021-01-22 at 09:39 +0100, Martin Kepplinger wrote: > The current log message results in a line like the following where > the first byte is duplicated, giving a wrong impression: > > sd 0:0:0:0: [sda] tag#0 CDB: opcode=0x28 28 00 00 00 60 40 00 00 01 > 00 > > Print the cdb into a new line in any case, not only when cmd_len is > greater than 16. The above example error will then read: > > sd 0:0:0:0: [sda] tag#0 CDB: opcode=0x28 > 28 00 01 c0 09 00 00 00 08 00 > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> > --- > drivers/scsi/scsi_logging.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/scsi_logging.c > b/drivers/scsi/scsi_logging.c > index 8ea44c6595ef..0081d3936f83 100644 > --- a/drivers/scsi/scsi_logging.c > +++ b/drivers/scsi/scsi_logging.c > @@ -200,10 +200,11 @@ void scsi_print_command(struct scsi_cmnd *cmd) > if (off >= logbuf_len) > goto out_printk; > > + /* Print opcode in one line and use separate lines for CDB */ > + off += scnprintf(logbuf + off, logbuf_len - off, "\n"); > + > /* print out all bytes in cdb */ > if (cmd->cmd_len > 16) { > - /* Print opcode in one line and use separate lines for > CDB */ > - off += scnprintf(logbuf + off, logbuf_len - off, "\n"); > dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", > logbuf); > for (k = 0; k < cmd->cmd_len; k += 16) { > size_t linelen = min(cmd->cmd_len - k, 16); > @@ -224,7 +225,6 @@ void scsi_print_command(struct scsi_cmnd *cmd) > goto out; > } > if (!WARN_ON(off > logbuf_len - 49)) { > - off += scnprintf(logbuf + off, logbuf_len - off, " "); > hex_dump_to_buffer(cmd->cmnd, cmd->cmd_len, 16, 1, > logbuf + off, logbuf_len - off, > false); I'd rather we not change this. -Ewan
Ewan, >> sd 0:0:0:0: [sda] tag#0 CDB: opcode=0x28 28 00 00 00 60 40 00 00 01 >> 00 >> >> Print the cdb into a new line in any case, not only when cmd_len is >> greater than 16. The above example error will then read: >> >> sd 0:0:0:0: [sda] tag#0 CDB: opcode=0x28 >> 28 00 01 c0 09 00 00 00 08 00 > > I'd rather we not change this. I agree. While the current format is suboptimal, there are lots of things out there parsing these error messages.
On 23.01.21 04:09, Martin K. Petersen wrote: > > Ewan, > >>> sd 0:0:0:0: [sda] tag#0 CDB: opcode=0x28 28 00 00 00 60 40 00 00 01 >>> 00 >>> >>> Print the cdb into a new line in any case, not only when cmd_len is >>> greater than 16. The above example error will then read: >>> >>> sd 0:0:0:0: [sda] tag#0 CDB: opcode=0x28 >>> 28 00 01 c0 09 00 00 00 08 00 >> >> I'd rather we not change this. > > I agree. While the current format is suboptimal, there are lots of > things out there parsing these error messages. > hi Ewan, hi Martin, That's totally fine. I had this on my list since Douglas suggested to change this during a discussion back in july and I basically wanted to get opinions: https://lore.kernel.org/linux-scsi/31f1ec62-7047-a34b-fdcb-5ea2a2104292@interlog.com/ thanks, martin
diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c index 8ea44c6595ef..0081d3936f83 100644 --- a/drivers/scsi/scsi_logging.c +++ b/drivers/scsi/scsi_logging.c @@ -200,10 +200,11 @@ void scsi_print_command(struct scsi_cmnd *cmd) if (off >= logbuf_len) goto out_printk; + /* Print opcode in one line and use separate lines for CDB */ + off += scnprintf(logbuf + off, logbuf_len - off, "\n"); + /* print out all bytes in cdb */ if (cmd->cmd_len > 16) { - /* Print opcode in one line and use separate lines for CDB */ - off += scnprintf(logbuf + off, logbuf_len - off, "\n"); dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf); for (k = 0; k < cmd->cmd_len; k += 16) { size_t linelen = min(cmd->cmd_len - k, 16); @@ -224,7 +225,6 @@ void scsi_print_command(struct scsi_cmnd *cmd) goto out; } if (!WARN_ON(off > logbuf_len - 49)) { - off += scnprintf(logbuf + off, logbuf_len - off, " "); hex_dump_to_buffer(cmd->cmnd, cmd->cmd_len, 16, 1, logbuf + off, logbuf_len - off, false);
The current log message results in a line like the following where the first byte is duplicated, giving a wrong impression: sd 0:0:0:0: [sda] tag#0 CDB: opcode=0x28 28 00 00 00 60 40 00 00 01 00 Print the cdb into a new line in any case, not only when cmd_len is greater than 16. The above example error will then read: sd 0:0:0:0: [sda] tag#0 CDB: opcode=0x28 28 00 01 c0 09 00 00 00 08 00 Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm> --- drivers/scsi/scsi_logging.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)