Re: [PATCH 4/5] tcm_vhost: fix iov array offset

[Zhi Yong Wu <zwu.kernel@xxxxxxxxx>]

On Wed, Apr 18, 2012 at 3:17 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> Il 18/04/2012 09:06, zwu.kernel@xxxxxxxxx ha scritto:
>> @@ -340,7 +340,7 @@ static void vhost_scsi_handle_vq(struct vhost_scsi *vs)
>>               }
>>
>>               exp_data_len = 0;
>> -             for (i = 2; i < out + in - 1; i++) {
>> +             for (i = 2; i < out + in; i++) {
>>                       exp_data_len += vq->iov[i].iov_len;
>>               }
>>
>
> This counts the iov[out] element as part of the data.  Should be
> something like this instead:
>
>  #warning FIXME: BIDI operation
>    if (in > 1) {
>        data_first = out + 1;
>        data_num = in - 1;
>    } else {
>        data_first = 1;
>        data_num = out - 1;
>    }
>
>    for (i = 0; i < data_num; i++) {
>        exp_data_len += vq->iov[data_first + i].iov_len;
>    }
I totally understand these lines of code, and it assume that one
request can not have both in>1 and out>1? That is, one request is
either read, or write, right?
>
>> -                     ret = vhost_scsi_map_iov_to_sgl(tv_cmd, &vq->iov[2],
>> -                                     out + in - 3, data_direction == DMA_TO_DEVICE);
>> +                     ret = vhost_scsi_map_iov_to_sgl(tv_cmd, &vq->iov[0],
>> +                                     out + in, data_direction == DMA_TO_DEVICE);
>
> This also looks strange... looks like this should also use something
> like data_first/data_num.
Yeah, i think that we should use.
>
> Paolo



-- 
Regards,

Zhi Yong Wu
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html