Re: [Qemu-devel] Re: [PATCH 09/16] scsi-disk: Allocate iovec dynamically

[Hannes Reinecke]

On 11/19/2010 12:43 PM, Kevin Wolf wrote:
> Am 18.11.2010 17:28, schrieb Hannes Reinecke:
>> On 11/18/2010 04:33 PM, Gerd Hoffmann wrote:
>>>   Hi,
>>>
>>>> +static size_t scsi_req_iov_len(SCSIDiskReq *r)
>>>> +{
>>>> +    size_t iov_len = 0;
>>>> +    int i;
>>>> +
>>>> +    for (i = 0; i<  r->iov_num; i++)
>>>> +        iov_len += r->iov[i].iov_len;
>>>> +
>>>> +    return iov_len;
>>>> +}
>>>
>>> You are aware that there is a QEMUIOVector type with helper functions
>>> which keeps track of both number of elements and total size?
>>>
>> Yes. But I'm passing passing in an entire iovec to the request.
>> However, the QEMUIOVector routines allow you to add only _one_
>> element at a time, which is pretty wasteful here.
> 
> Does the iov need to be changed afterwards, or why doesn't
> qemu_iovec_init_external work here?
> 
Oh, but I _do_ use qemu_iovec_init_external(); cf

hw/scsi_disk.c:scsi_read_data()

and yes, the iovec might be modified by the read/write operation;
see below.

>> And I'm counting the resulting length of the iovec, which might have
>> been changed by read/write operations. For which there is no generic
>> function either.
> 
> Can you explain which kind of read/write operations would change the
> iov? This is not completely clear to me.
> 
It is perfectly valid to send down an iovec larger than the command
would fill out; eg for a SCSI Inquiry you could easily request 255
bytes, but the command itself would only provide you with say 36 bytes.

Using SG_IO you would pass down the iovec (with the full size of
255), and you would be returned the same iovec. However, the
->iov_len parameter of the iovec elements would be modified
depending on the size of the actual data returned. Hence you need to
figure out the resulting size of the iovec by the above command.

> In general the same information that you're calculating here should be
> stored in qiov->size for a QEUMIOVector, but depending what changes you
> mean above it may not provide all operations you need.
> 
Ah. Seem to have missed it. Care to point it out to me?
But I don't think it'll work for the scsi-generic case, where we're
just using SG_IO.
But then, I tend to get lost in the callbacks-upon-callbacks
reference in the block layer, so there's a fair chance I haven't
seen it.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                   zSeries & Storage
address@hidden                        +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)