[stgt] [PATCH] Handle ILI bit via sg backing store

[Mark Harvey]

On Thu, Apr 14, 2011 at 9:01 PM, Mark Harvey <markh794 at gmail.com> wrote:
> On 14/04/11 18:54, FUJITA Tomonori wrote:
>>
>> Sorry for the delay,
>>
>> On Fri, 8 Apr 2011 21:43:12 -0700
>> Mark Harvey<mark_harvey at symantec.com>  wrote:
>>
>>> Apologies for the attachment.. Outlook just does its own thing no matter
>>> what I want.
>>>
>>> Basically same patch as before, but replaced Illegal with Invalid.
>>
>> Should be 'incorrect length indicator'?
>>
> I'll double-check the SSC3 spec.

I stand corrected - again :)

I'll update the patch to reflect 'Incorrect' wording.

>>>
>>> Cheers
>>> Mark
>>>
>>>
>>> $ git diff
>>> diff --git a/usr/bs_sg.c b/usr/bs_sg.c
>>> index 69cce9b..a1be14b 100644
>>> --- a/usr/bs_sg.c
>>> +++ b/usr/bs_sg.c
>>> @@ -247,6 +247,7 @@ static void bs_sg_cmd_complete(int fd, int events,
>>> void *dat
>>>         struct sg_io_hdr io_hdr;
>>>         struct scsi_cmd *cmd;
>>>         int err;
>>> +       unsigned resid;
>>>
>>>         memset(&io_hdr, 0, sizeof(io_hdr));
>>>         io_hdr.interface_id = 'S';
>>> @@ -261,9 +262,15 @@ static void bs_sg_cmd_complete(int fd, int events,
>>> void *da
>>>                 scsi_set_out_resid(cmd, io_hdr.resid);
>>>                 scsi_set_in_resid(cmd, io_hdr.resid);
>>>         } else {
>>> +               /* NO SENSE | ILI (Invalid Length Indicator) set */
>>> +               if (io_hdr.sbp[2] == 0x20)
>>> +                       resid = io_hdr.dxfer_len - io_hdr.resid;
>>> +               else
>>> +                       resid = 0;
>>> +
>>>                 cmd->sense_len = io_hdr.sb_len_wr;
>>> -               scsi_set_out_resid_by_actual(cmd, 0);
>>> -               scsi_set_in_resid_by_actual(cmd, 0);
>>> +               scsi_set_out_resid_by_actual(cmd, resid);
>>> +               scsi_set_in_resid_by_actual(cmd, resid);
>>
>> This should be
>>
>> scsi_set_in_resid_by_actual(cmd, io_hdr.resid);
>> scsi_set_in_resid_by_actual(cmd, io_hdr.resid);
>>
>> ?
>>
>> I thought that sg sets a proper resid value to io_hdr.resid. No?

typedef struct sg_io_hdr
{
<cut>
int resid;                  /* [o] dxfer_len - actual_transferred */
}

i.e. resid is the amount of data NOT transferred.

>>
> My testing reported 'resid' as the residual that was not returned.
> i.e. a "dd if=/dev/nst0 of=/tmp/b1 bs=64k count=1" when reading a 1k block
> will have 63k in 'resid'.
>
> It's the 1k of valid data that needs to be returned to the application
> (NetBackup in this instance).
>
> I did have a though this morning which I did not have time to test today.
> i.e. If the tape block size is 64k and the 'dd if=/dev/nst0 of=/tmp/b1
> bs=32k count=1', does the above patch return 32k of data ?
> My feeling, is it doesn't. I hope to test and confirm/correct the patch
> sometime tomorrow.

Unfortunately, I'm struggling to get access to a physical tape library
connected to a Linux host.

Once I test on real hardware, I'll re-send in the patch to suit.

>
> Cheers
> Mark

Many thanks
Mark
--
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