[PATCH] return all status bytes for sg io.

[PATCH] return all status bytes for sg io.

[Mike Christie]

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

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

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

diff -arup linux-2.6.7-rc2/drivers/block/scsi_ioctl.c linux-2.6.7-rc2-work/drivers/block/scsi_ioctl.c
--- linux-2.6.7-rc2/drivers/block/scsi_ioctl.c	2004-05-29 23:26:48.000000000 -0700
+++ linux-2.6.7-rc2-work/drivers/block/scsi_ioctl.c	2004-06-02 04:32:27.000000000 -0700
@@ -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;
diff -arup linux-2.6.7-rc2/drivers/scsi/scsi_lib.c linux-2.6.7-rc2-work/drivers/scsi/scsi_lib.c
--- linux-2.6.7-rc2/drivers/scsi/scsi_lib.c	2004-05-29 23:26:10.000000000 -0700
+++ linux-2.6.7-rc2-work/drivers/scsi/scsi_lib.c	2004-06-02 02:51:16.000000000 -0700
@@ -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) {

Re: [PATCH] return all status bytes for sg io.

[Jens Axboe]

On Wed, Jun 02 2004, Mike Christie wrote:
> 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
> --- linux-2.6.7-rc2/drivers/block/scsi_ioctl.c	2004-05-29 23:26:48.000000000 -0700
> +++ linux-2.6.7-rc2-work/drivers/block/scsi_ioctl.c	2004-06-02 04:32:27.000000000 -0700
> @@ -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);

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

-- 
Jens Axboe

Re: [PATCH] return all status bytes for sg io.

[Mike Christie]

Jens Axboe wrote:
> On Wed, Jun 02 2004, Mike Christie wrote:
> 
>>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
>>--- linux-2.6.7-rc2/drivers/block/scsi_ioctl.c	2004-05-29 23:26:48.000000000 -0700
>>+++ linux-2.6.7-rc2-work/drivers/block/scsi_ioctl.c	2004-06-02 04:32:27.000000000 -0700
>>@@ -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);
> 
> 
> 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?


-- 
Mike Christie
mikenc@us.ibm.com