[stgt] [PATCH 12/15] ssc: add variable-length block read support

[FUJITA Tomonori]

On Thu, 13 Nov 2008 10:52:43 +1100
"Mark Harvey" <markh794 at gmail.com> wrote:

Seems that the attached patch fixes the 512-byte read case, but not
256-byte. I guess that I still misunderstand something...


> >> Now test we can read in a 256k block from the tape.
> >> # dd if=/dev/st0 of=/tmp/block0 bs=256k count=1
> >> 1+0 records in
> >> 1+0 records out
> >> 262144 bytes (262 kB) copied, 0.00424289 s, 61.8 MB/s
> >>
> >> Now test if we can read in a partial block (this fails returning no
> >> data - we should have read in the 128k)
> >> # dd if=/dev/st0 of=/tmp/block0 bs=128k count=1
> >> dd: reading `/dev/st0': Input/output error
> >> 0+0 records in
> >> 0+0 records out
> >> 0 bytes (0 B) copied, 0.00249643 s, 0.0 kB/s
> >
> > Sorry for my dumb question here:
> >
> > What should stgt return? The first 128 KB of the first block and no
> > sense?
> 
> The amount of data requested should be returned. In this example, 128k.
> 
> The sense returned should be :NO SENSE | VALID | ILI (Illegal Length
> Indicator) and the overrun values (size of data block - size of data
> requested) filled in sense_buffer[3] -> sense_buffer[7].

With the patch, stgt returns CHECK CONDITION with a sense (NO SENSE |
VALID | ILI ) and the information field sets to 128K.

I got (actually, stgt returns 128KB data though...):

nemo:/home/fujita# dd if=/dev/st0 of=/tmp/block0 bs=128k count=1
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.0030381 seconds, 0.0 kB/s

But the last line should be something like:

131072 bytes (131 kB) copied...

Right?


> >> Now test if we can read in a more then a block (this fails returning
> >> no data - we should have read in the 256k)
> >> # dd if=/dev/st0 of=/tmp/block0 bs=512k count=1
> >> dd: reading `/dev/st0': Input/output error
> >> 0+0 records in
> >> 0+0 records out
> >> 0 bytes (0 B) copied, 0.00705821 s, 0.0 kB/s
> >
> > stgt should return the whole first block without sense, right?
> >
> 
> The amount of data successfully read should be returned. In this example, 128k.
> 
> The sense returned should be :NO SENSE | VALID | ILI (Illegal Length
> Indicator) and the amount of data NOT successfully returned -> filled
> in sense_buffer[3] -> sense_buffer[7]

With the patch, stgt returns CHECK CONDITION with a sense (NO SENSE |
VALID | ILI ) and the information field sets to 256K.

nemo:/home/fujita# dd if=/dev/st0 of=/tmp/block0 bs=512k count=1
0+1 records in
0+1 records out
262144 bytes (262 kB) copied, 0.019265 seconds, 13.6 MB/s

This is the right response?


> On a side note (real world implementation):
> NetBackup as part of its logic to determine if the media has valid
> NetBackup data performs a :
> - Request 64k data
> - NetBackup expects to see only 1k returned with the sense | ILI set.
> Otherwise it treats the media as something it did not create.
> 
> 
> SSC3-r04a reads as follows (page 91): - note 'SILI == Supress Illegal
> Length Indicator'
> If the SILI bit is zero and an incorrect-length logical block is read,
> CHECK CONDITION status shall be returned. The ILI and VALID bits shall
> be set to one in the sense data and the additional sense code shall be
> set to NO ADDITIONAL SENSE INFORMATION
> 
> Upon termination, the logical position shall be after the
> incorrect-length logical block (end-of-partition side)
> 
> If the FIXED bit is one, the INFORMATION field shall be set to the
> requested transfer length minus the actual number of logical blocks
> read (not including the incorrect-length logical block).
> 
> If the FIXED bit is zero, the INFORMATION field shall be set to the
> requested transfer length minus the actual logical block length.
> 
> Logical units that do not support negative values shall set the
> INFORMATION field to zero if the overlength condition exists.

Thanks!

I still need to learn lots about ssc...


This patch is against the latest git head:

commit c467cd60be224f62f8f28cb4f892578ad9848064
Author: FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp>
Date:   Sun Nov 23 02:09:06 2008 +0900

    ssc: fix the information field in sense

=
diff --git a/usr/bs_ssc.c b/usr/bs_ssc.c
index 0b9fedb..4aff96c 100644
--- a/usr/bs_ssc.c
+++ b/usr/bs_ssc.c
@@ -317,15 +317,29 @@ static int resp_var_read(struct scsi_cmd *cmd, uint8_t *buf, uint32_t length,
 	*transferred = 0;
 
 	if (length != h->blk_sz) {
+		uint8_t info[4];
+
+		if (length > h->blk_sz)
+			put_unaligned_be32(length - h->blk_sz, info);
+		else
+			put_unaligned_be32(h->blk_sz - length, info);
+
 		if (h->blk_type == BLK_EOD)
 			sense_data_build(cmd, 0x40 | BLANK_CHECK,
 					 NO_ADDITIONAL_SENSE);
 		else
-			sense_data_build(cmd, NO_SENSE, NO_ADDITIONAL_SENSE);
+			ssc_sense_data_build(cmd, NO_SENSE | 0x20,
+					     NO_ADDITIONAL_SENSE,
+					     info, sizeof(info));
+
+		if (length > h->blk_sz)
+			scsi_set_in_resid_by_actual(cmd, length - h->blk_sz);
+		else
+			scsi_set_in_resid_by_actual(cmd, 0);
 
 		length = min(length, h->blk_sz);
+
 		result = SAM_STAT_CHECK_CONDITION;
-		scsi_set_in_resid_by_actual(cmd, length);
 
 		if (!length)
 			goto out;
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html