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

[FUJITA Tomonori]

Very sorry for the late response.

On Fri, 31 Oct 2008 15:20:44 +1100
"Mark Harvey" <markh794 at gmail.com> wrote:

> On Mon, Oct 6, 2008 at 1:37 AM, FUJITA Tomonori
> <fujita.tomonori at lab.ntt.co.jp> wrote:
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori at lab.ntt.co.jp>
> > ---
> >  usr/bs_ssc.c |   43 ++++++++++++++++++++++++++++++++++++++-----
> >  1 files changed, 38 insertions(+), 5 deletions(-)
> >
> > diff --git a/usr/bs_ssc.c b/usr/bs_ssc.c
> > index cbbca83..134c670 100644
> > --- a/usr/bs_ssc.c
> > +++ b/usr/bs_ssc.c
> > @@ -318,13 +318,46 @@ static int space_blocks(struct scsi_cmd *cmd, int32_t count)
> >        return SAM_STAT_GOOD;
> >  }
> >
> > -/* Return error - util written */
> >  static int resp_var_read(struct scsi_cmd *cmd, uint8_t *buf, uint32_t length,
> > -                        int *transferfed)
> > +                        int *transferred)
> >  {
> > -       *transferfed = 0;
> > -       sense_data_build(cmd, ILLEGAL_REQUEST, ASC_INVALID_FIELD_IN_CDB);
> > -       return SAM_STAT_CHECK_CONDITION;
> > +       struct ssc_info *ssc = dtype_priv(cmd->dev);
> > +       int ret = 0, result = SAM_STAT_GOOD;
> > +
> > +       length = min(length, get_unaligned_be24(&cmd->scb[2]));
> > +       *transferred = 0;
> > +
> > +       if (length != ssc->c_blk->blk_sz) {
> > +               if (ssc->c_blk->blk_type == BLK_EOD)
> > +                       sense_data_build(cmd, 0x40 | BLANK_CHECK,
> > +                                        NO_ADDITIONAL_SENSE);
> > +               else
> > +                       sense_data_build(cmd, NO_SENSE, NO_ADDITIONAL_SENSE);
> > +
> > +               length = min(length, ssc->c_blk->blk_sz);
> > +               result = SAM_STAT_CHECK_CONDITION;
> > +               scsi_set_in_resid_by_actual(cmd, length);
> > +
> > +               if (!length)
> > +                       goto out;
> > +       }
> > +
> > +       ret = pread(cmd->dev->fd, buf, length,
> > +                   ssc->c_blk->curr + sizeof(struct blk_header));
> > +       if (ret != length) {
> > +               sense_data_build(cmd, MEDIUM_ERROR, ASC_READ_ERROR);
> > +               result = SAM_STAT_CHECK_CONDITION;
> > +               goto out;
> > +       }
> > +       *transferred = length;
> > +
> > +       ret = skip_next_header(cmd->dev);
> > +       if (ret) {
> > +               sense_data_build(cmd, MEDIUM_ERROR, ASC_READ_ERROR);
> > +               result = SAM_STAT_CHECK_CONDITION;
> > +       }
> > +out:
> > +       return result;
> >  }
> >
> >  static int resp_fixed_read(struct scsi_cmd *cmd, uint8_t *buf, uint32_t length,
> > --
> > 1.5.6.5
> >
> >
> 
> First off, many thanks for completing the variable-block support.
> 
> I'm finally in a position where I found some time to test all the
> improvements and am stuck with this one. I don't fully understand the
> return path.
> 
> I performed a basic test by 'dd' a single 256k block to the tape
> drive. I then tested what would happen if I tried to 'dd' a different
> block size from the tape.
> 
> Only a 'dd if=/dev/st0 bs=256k' would return any data.
> 
> Included below is the trace I captured. Hopefully it is enough to
> outline what I am seeing.
> BTW: Testing on real tape drives, typically return '0+xx records out'
> 
> 
> Any pointers as to where to start looking will be grateful.
> Basically, it looks like any status except SAM_STAT_GOOD does not
> return the payload.
> 
>    ============================
> Write a single 256k block to tape...
> # dd if=/dev/zero of=/dev/st0 bs=256k count=1
> 1+0 records in
> 1+0 records out
> 262144 bytes (262 kB) copied, 0.0434786 s, 6.0 MB/s
> 
> Confirm 'tape' contents contains a single 256k block..
> # ./dump_tape -f /d/01/tape2
> Media     : tape2
>  type     : Data
> Media serial number : tape2_1225355078, created Thu Oct 30 19:24:38 2008
> 
> Beginning of Tape(16): Capacity 500 MB, Blk No.: 0, prev 0, curr 0, next 1152
>  Uncompressed data(2): Blk No. 1, prev 0, curr 1152,  next 263344, sz 262144
>          Filemark(64): Blk No. 2, prev 1152, curr 263344,  next 263392, sz 0
>       End of Data(32): Blk No. 3, prev 263344, curr 263392,  next 263392, sz 0
> 
> 
> 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?


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