return all status bytes for sg io.

PATCH In review Mike Christie 3 participants 3 messages Jun 2, 2004

Subsystems (2)

SCSI Block layer

lore

Share & tools

lore

Mike Christie

author

2004-06-02 12:25 UTC #0 lore raw

PATCH 1/1 return all status bytes for sg io.

Hi Jens,

For block/scsi_ioctl.c only the status_byte is returned. However, when sg io is performed through a sg device the host, msg and driver bytes are passed back. I was not sure if this is a bug on the block layers part (becuase as you see in the patch, after the msg, host and driver bytes are zerod out they are tested), or is the bug in the app and it should be modifed to access sg devices instead of block devs. The attached patch was built against 2.6.7-rc2.

Thanks,

Mike

drivers/block/scsi_ioctl.c +5 -5

diff -arup linux-2.6.7-rc2/drivers/block/scsi_ioctl.c linux-2.6.7-rc2-work/drivers/block/scsi_ioctl.c
@@ -185,11 +185,11 @@ static int sg_io(request_queue_t *q, str
 	blk_execute_rq(q, bd_disk, rq);
 
 	/* write to all output members */
-	hdr->status = rq->errors;	
-	hdr->masked_status = (hdr->status >> 1) & 0x1f;
-	hdr->msg_status = 0;
-	hdr->host_status = 0;
-	hdr->driver_status = 0;
+	hdr->status = 0xff & rq->errors;
+	hdr->masked_status = status_byte(rq->errors);
+	hdr->msg_status = msg_byte(rq->errors);
+	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)
 		hdr->info |= SG_INFO_CHECK;

drivers/scsi/scsi_lib.c +1 -2

diff -arup linux-2.6.7-rc2/drivers/scsi/scsi_lib.c linux-2.6.7-rc2-work/drivers/scsi/scsi_lib.c
@@ -708,8 +708,7 @@ void scsi_io_completion(struct scsi_cmnd
 	}
 
 	if (blk_pc_request(req)) { /* SG_IO ioctl from block level */
-		req->errors = (driver_byte(result) & DRIVER_SENSE) ?
-			      (CHECK_CONDITION << 1) : (result & 0xff);
+		req->errors = result; 
 		if (result) {
 			clear_errors = 0;
 			if (cmd->sense_buffer[0] & 0x70) {

Jens Axboe

2004-06-02 19:32 UTC #1 lore raw

Quoted 21 lines · Mike Christie

Hi Jens,
For block/scsi_ioctl.c only the status_byte is returned. However, when sg io is performed through a sg device the host, msg and driver bytes are passed back. I was not sure if this is a bug on the block layers part (becuase as you see in the patch, after the msg, host and driver bytes are zerod out they are tested), or is the bug in the app and it should be modifed to access sg devices instead of block devs. The attached patch was built against 2.6.7-rc2.
Thanks,
Mike
diff -arup linux-2.6.7-rc2/drivers/block/scsi_ioctl.c linux-2.6.7-rc2-work/drivers/block/scsi_ioctl.c
drivers/block/scsi_ioctl.c +0 -0
@@ -185,11 +185,11 @@ static int sg_io(request_queue_t *q, str
 	blk_execute_rq(q, bd_disk, rq);
 
 	/* write to all output members */

drivers/block/scsi_ioctl.c static int sg_io(request_queue_t *q, str · line 192

Quoted 10 lines

drivers/block/scsi_ioctl.c +5 -5

-	hdr->status = rq->errors;	
-	hdr->masked_status = (hdr->status >> 1) & 0x1f;
-	hdr->msg_status = 0;
-	hdr->host_status = 0;
-	hdr->driver_status = 0;
+	hdr->status = 0xff & rq->errors;
+	hdr->masked_status = status_byte(rq->errors);
+	hdr->msg_status = msg_byte(rq->errors);
+	hdr->host_status = host_byte(rq->errors);
+	hdr->driver_status = driver_byte(rq->errors);

For SCSI, this looks good. I need to look at ide-cd to be sure it's ok, did you verify this?

Signature · 1 line

Jens Axboe

Mike Christie

2004-06-02 23:42 UTC #2 lore raw

Quoted 17 lines · Jens Axboe

Quoted 15 lines · Mike Christie
Hi Jens,
For block/scsi_ioctl.c only the status_byte is returned. However, when sg io is performed through a sg device the host, msg and driver bytes are passed back. I was not sure if this is a bug on the block layers part (becuase as you see in the patch, after the msg, host and driver bytes are zerod out they are tested), or is the bug in the app and it should be modifed to access sg devices instead of block devs. The attached patch was built against 2.6.7-rc2.
Thanks,
Mike
diff -arup linux-2.6.7-rc2/drivers/block/scsi_ioctl.c linux-2.6.7-rc2-work/drivers/block/scsi_ioctl.c
drivers/block/scsi_ioctl.c +0 -0
@@ -185,11 +185,11 @@ static int sg_io(request_queue_t *q, str
 	blk_execute_rq(q, bd_disk, rq);
 

Quoted 16 lines

drivers/block/scsi_ioctl.c static int sg_io(request_queue_t *q, str · line 192
Quoted 11 lines
drivers/block/scsi_ioctl.c +5 -5
 	/* write to all output members */
-	hdr->status = rq->errors;	
-	hdr->masked_status = (hdr->status >> 1) & 0x1f;
-	hdr->msg_status = 0;
-	hdr->host_status = 0;
-	hdr->driver_status = 0;
+	hdr->status = 0xff & rq->errors;
+	hdr->masked_status = status_byte(rq->errors);
+	hdr->msg_status = msg_byte(rq->errors);
+	hdr->host_status = host_byte(rq->errors);
+	hdr->driver_status = driver_byte(rq->errors);
For SCSI, this looks good. I need to look at ide-cd to be sure it's ok, did you verify this?

For ide-cd I mostly executed sg commands while doing things like pulling
cables and or running on bad CD's to get SCSI stats like Busy and Check
Condidtion to verify that it worked like before my patch. As far as verifing the code it looked like ide-cd was just using the scsi status bits like ERROR_MAX mapped to SAMS_STAT_BUSY and for sense SAM_STAT_CHECK_CONDITION was set. I would occasionaly get Unknown status (0xb) with and without my patch when unplugging and replugging in the drive mulitple times. I was thinking this was due to the sense value ABORTED_COMMAND getting set to rq->errors similar to how ide-cd.c's ide_cdrom_prep_pc() sets rq->errors to ILLEGAL_REQUEST. Are those type of things bugs, or a result of not having a request_sense structure available and now other apps rely on this behavior?

Signature · 2 lines

Mike Christie
[email protected]