[stgt] [PATCH] SBC READ6/10/12/16: Add handling of residual overflow/underflow.

[ronnie sahlberg]

You are absolutely right.
That works too and makes the patch a lot simpler/better.

I will resend the patch with your suggestion.

Thanks!

On Sun, Mar 24, 2013 at 5:33 AM, Alexander Nezhinsky
<nezhinsky at gmail.com> wrote:
> sorry for resending, sent HTML for the first time...
>
> On Sat, Mar 23, 2013 at 11:16 PM, Ronnie Sahlberg
> <ronniesahlberg at gmail.com> wrote:
>>
>> This patch adds support to return overflow/underflow residuals
>> for the cases where the transport layer (iSCSI, ...) specifies
>> an EDTL that is not matching the SCSI transdfer length.
>>
>> Additionally, in case of residual underflow it will also clamp the size of
>> returned DATA-IN to the amount of data that was actually read and not the
>> EDTL that was requested.
>>
>
>>
>> diff --git a/usr/sbc.c b/usr/sbc.c
>>
>> index 48dab04..26cceb1 100644
>> --- a/usr/sbc.c
>> +++ b/usr/sbc.c
>> @@ -226,6 +226,8 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
>>         int ret;
>>         uint64_t lba;
>>         uint32_t tl;
>> +       long residual = 0;
>> +       long length;
>>         unsigned char key = ILLEGAL_REQUEST;
>>         uint16_t asc = ASC_LUN_NOT_SUPPORTED;
>>         struct scsi_lu *lu = cmd->dev;
>> @@ -335,12 +337,36 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
>>         cmd->offset = lba << cmd->dev->blk_shift;
>>         cmd->tl     = tl  << cmd->dev->blk_shift;
>>
>> +       /* Handle residuals */
>> +       switch (cmd->scb[0]) {
>> +       case READ_6:
>> +       case READ_10:
>> +       case READ_12:
>> +       case READ_16:
>> +               length = scsi_get_in_length(cmd);
>> +               if (length != cmd->tl) {
>> +                       residual = length - cmd->tl;
>> +
>> +                       if (length > cmd->tl) {
>> +                               length = cmd->tl;
>> +                               scsi_set_in_transfer_len(cmd, cmd->tl);
>> +                               scsi_set_in_length(cmd, cmd->tl);
>> +                       }
>> +               }
>> +               break;
>> +       }
>> +
>>         ret = cmd->dev->bst->bs_cmd_submit(cmd);
>>         if (ret) {
>>                 key = HARDWARE_ERROR;
>>                 asc = ASC_INTERNAL_TGT_FAILURE;
>> -       } else
>> -               return SAM_STAT_GOOD;
>> +               goto sense;
>> +       }
>> +
>> +       if (residual)
>> +               scsi_set_in_resid(cmd, residual);
>> +
>> +       return SAM_STAT_GOOD;
>>
>>  sense:
>>         cmd->offset = 0;
>> --
>> 1.7.3.1
>>
>
> I think that scsi_set_in_resid_by_actual() already does all the work,
> doesn't it?
> If i don't miss any details the following patch is enough:
>
> diff --git a/usr/sbc.c b/usr/sbc.c
> index 48dab04..f3248a1 100644
> --- a/usr/sbc.c
> +++ b/usr/sbc.c
> @@ -230,6 +230,17 @@ static int sbc_rw(int host_no, struct scsi_cmd *cmd)
>
>         uint16_t asc = ASC_LUN_NOT_SUPPORTED;
>         struct scsi_lu *lu = cmd->dev;
>
> +       switch (cmd->scb[0]) {
> +       case READ_6:
> +       case READ_10:
> +       case READ_12:
> +       case READ_16:
> +               if (cmd->tl != scsi_get_in_length(cmd))
> +                       scsi_set_in_resid_by_actual(cmd, cmd->tl);
> +       default:
> +               break;
> +       }
> +
>         ret = device_reserved(cmd);
>         if (ret)
>                 return SAM_STAT_RESERVATION_CONFLICT;
--
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