Re: [PATCH] Make virtual tapes more closely emulate physical ones

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sorry for late reply..
Christmas/New Year etc. leaves little free time ;)

On Tue, Jan 15, 2013 at 4:53 AM, Jay Fenlason <fenlason@xxxxxxxxxx> wrote:
> On Mon, Jan 14, 2013 at 09:02:34PM +0900, FUJITA Tomonori wrote:
>> On Thu, 10 Jan 2013 11:36:44 -0500
>> Jay Fenlason <fenlason@xxxxxxxxxx> wrote:
>>
>> > On my physical drive (a Sony AIT-2 drive, if it matters) when
>> > userspace reads a file mark, it gets an EOF, and the tape is advanced
>> > past the file mark so that a subsequent read will return data from the
>> > next file on the tape.  With a virtual drive, the file mark is
>> > sticky--reads continue to return EOF until the tape is manually
>> > advanced with a FSF ioctl.  I wrote this quick patch to make the
>> > virtual tape behave like my physical one.
>> >
>> > I'm not sure about the sense_data_build hunk, but it matches the code
>> > for fixed blocksize, and did no harm in my testing.
>> >
>> >             -- JF
>> >
>> > --- tgt-1.0.32/usr/bs_ssc.c.filemark        2012-09-30 18:39:21.000000000 -0400
>> > +++ tgt-1.0.32/usr/bs_ssc.c 2013-01-09 15:32:43.000000000 -0500
>> > @@ -325,6 +325,9 @@
>> >             if (h->blk_type == BLK_EOD)
>> >                     sense_data_build(cmd, 0x40 | BLANK_CHECK,
>> >                                      NO_ADDITIONAL_SENSE);
>> > +           else if (h->blk_type == BLK_FILEMARK)
>> > +                   ssc_sense_data_build(cmd, NO_SENSE | SENSE_FILEMARK,
>> > +                                        ASC_MARK, info, sizeof(info));
>> >             else
>> >                     ssc_sense_data_build(cmd, NO_SENSE | 0x20,
>> >                                          NO_ADDITIONAL_SENSE,
>> > @@ -339,8 +342,13 @@
>> >
>> >             result = SAM_STAT_CHECK_CONDITION;
>> >
>> > -           if (!length)
>> > +           if (!length) {
>> > +                   if (h->blk_type == BLK_FILEMARK) {
>> > +                           ret = skip_next_header(cmd->dev);
>> > +                           /* FIXME: ??? */
>>
>> Thanks, the patch looks correct. What's 'FIXME' part?
>
> We're already reporting an error, and we just got another error (of
> the "should never happen" variety).  What should we do?  It might be
> that the only correct thing to do is silently ignore the error, in
> which case the FIXME can be removed.  Mut if we should do something,
> code to do it should go where the FIXME is.

As there should always be an EOD marker at the end, returning a
ASC_MEDIUM_FORMAT_CORRUPT would be the right thing to do.

Looks good otherwise.

Cheers
Mark

>
>                         -- JF
> --
> To unsubscribe from this list: send the line "unsubscribe stgt" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux SCSI]     [Linux RAID]     [Linux Clusters]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]

  Powered by Linux