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
