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

[Mark Harvey]

On Sun, Nov 23, 2008 at 4:26 AM, FUJITA Tomonori
<fujita.tomonori at lab.ntt.co.jp> wrote:
> 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?

Just re-tested on real hardware:
# [2:0:4:2]    tape    QUANTUM  SuperDLT1        2323  /dev/st1  /dev/sg8
# sg_inq -v /dev/sg8
    inquiry cdb: 12 00 00 00 24 00
standard INQUIRY:
    inquiry cdb: 12 00 00 00 3a 00
  PQual=0  Device_type=1  RMB=1  version=0x02  [SCSI-2]
  [AERC=0]  [TrmTsk=0]  NormACA=0  HiSUP=1  Resp_data_format=2
  SCCS=0  ACC=0  TGPS=0  3PC=0  Protect=0  BQue=0
  EncServ=0  MultiP=0  [MChngr=0]  [ACKREQQ=0]  Addr16=0
  [RelAdr=0]  WBus16=0  Sync=0  Linked=0  [TranDis=0]  CmdQue=0
  Clocking=0x0  QAS=0  IUS=0
    length=58 (0x3a)   Peripheral device type: tape
 Vendor identification: QUANTUM
 Product identification: SuperDLT1
 Product revision level: 2323
    inquiry cdb: 12 01 00 00 fc 00
    inquiry: requested 252 bytes but  got 9 bytes
    inquiry cdb: 12 01 80 00 fc 00
    inquiry: requested 252 bytes but  got 16 bytes
 Unit serial number: PKB53H0142


An attempt to read 128k from a 256k block results with an error message:
# dd if=/dev/st1 bs=128k count=1 of=/tmp/block0
dd: reading `/dev/st1': Cannot allocate memory
0+0 records in
0+0 records out
0 bytes (0 B) copied, 0.01287 seconds, 0.0 kB/s

And /var/log/messages:
kernel: st1: Failed to read 262144 byte block with 131072 byte transfer.

Disclaimer...
I'm 99.99% positive, last time I tried this I received a different
message and did in fact read 128k of data. This may be an
implementation of dd used..
# rpm -qf `which dd`
coreutils-5.93-22.14
# uname -a
Linux dpg-markh 2.6.16.46-0.12-smp #1 SMP Thu May 17 14:00:09 UTC 2007
i686 i686 i386 GNU/Linux

>
>
>> >> 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?
This is the correct response...


Many thanks for having a look at this.

>
>
>> 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