Re: drivers/block/scsi_ioctl problem

[Brian King <brking@us.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 394 bytes --]

Brian King wrote:
>> It was added to be able to find out which opcode was rejected and thus
>> caused an application malfunction. It dumps the specific opcode only
>> once, is that such a huge problem?
> 
> 
> Ok. I didn't realize that it was only dumped once. That's not as bad. I 
> guess it might be better if it were only dumped if the op actually failed.

How about something like this...

[-- Attachment #2: blk_scsi_ioctl_unknown_cmd.patch --]
[-- Type: text/plain, Size: 2679 bytes --]


Currently if an SG_IO ioctl is issued to a block device with an
unknown opcode, an error is logged. This error is logged regardless
the device successfully processes the command or not. This results in
an error getting logged for each unknown opcode ever issued. This patch
changes this policy and only prints the unknown opcode if the command
fails.

Signed-off-by: Brian King <brking@us.ibm.com>
---

 linux-2.6.10-rc3-bk8-bjking1/drivers/block/scsi_ioctl.c |   19 +++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff -puN drivers/block/scsi_ioctl.c~blk_scsi_ioctl_unknown_cmd drivers/block/scsi_ioctl.c
--- linux-2.6.10-rc3-bk8/drivers/block/scsi_ioctl.c~blk_scsi_ioctl_unknown_cmd	2004-12-15 14:19:34.000000000 -0600
+++ linux-2.6.10-rc3-bk8-bjking1/drivers/block/scsi_ioctl.c	2004-12-15 14:55:04.000000000 -0600
@@ -111,7 +111,7 @@ static int sg_emulated_host(request_queu
 #define safe_for_read(cmd)	[cmd] = CMD_READ_SAFE
 #define safe_for_write(cmd)	[cmd] = CMD_WRITE_SAFE
 
-static int verify_command(struct file *file, unsigned char *cmd)
+static int __verify_command(struct file *file, unsigned char *cmd, int logging)
 {
 	static unsigned char cmd_type[256] = {
 
@@ -199,7 +199,7 @@ static int verify_command(struct file *f
 			return 0;
 	}
 
-	if (!(type & CMD_WARNED)) {
+	if (logging && !(type & CMD_WARNED)) {
 		cmd_type[cmd[0]] = CMD_WARNED;
 		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
 	}
@@ -212,6 +212,16 @@ static int verify_command(struct file *f
 	return -EPERM;
 }
 
+static int verify_command(struct file *file, unsigned char *cmd)
+{
+	return __verify_command(file, cmd, 0);
+}
+
+static void log_unknown_opcode(struct file *file, unsigned char *cmd)
+{
+	__verify_command(file, cmd, 1);
+}
+
 static int sg_io(struct file *file, request_queue_t *q,
 		struct gendisk *bd_disk, struct sg_io_hdr *hdr)
 {
@@ -307,8 +317,10 @@ static int sg_io(struct file *file, requ
 	hdr->host_status = host_byte(rq->errors);
 	hdr->driver_status = driver_byte(rq->errors);
 	hdr->info = 0;
-	if (hdr->masked_status || hdr->host_status || hdr->driver_status)
+	if (hdr->masked_status || hdr->host_status || hdr->driver_status) {
 		hdr->info |= SG_INFO_CHECK;
+		log_unknown_opcode(file, cmd);
+	}
 	hdr->resid = rq->data_len;
 	hdr->duration = ((jiffies - start_time) * 1000) / HZ;
 	hdr->sb_len_wr = 0;
@@ -415,6 +427,7 @@ static int sg_scsi_ioctl(struct file *fi
 	blk_execute_rq(q, bd_disk, rq);
 	err = rq->errors & 0xff;	/* only 8 bit SCSI status */
 	if (err) {
+		log_unknown_opcode(file, rq->cmd);
 		if (rq->sense_len && rq->sense) {
 			bytes = (OMAX_SB_LEN > rq->sense_len) ?
 				rq->sense_len : OMAX_SB_LEN;
_