Re: [Qemu-devel] [RFC 1/1] pci-dma-api-v2

[Avi Kivity <avi@redhat.com>]

Anthony Liguori wrote:

>>>
>>> What should be fixed?  Are these emulated functions wrong?
>>>
>>> There's a lack of symmetry here.  We should have a bdrv_readv and 
>>> bdrv_aio_readv.  bdrv_read and bdrv_aio_read should disappear.  We 
>>> can maintain wrappers that create a compatible interface for older 
>>> code but just adding a new API is wrong.
>>>
>>
>> It was understood a real aio readv/writev was being worked on, so the 
>> emulation could be a temporary step.
>
> Yes, but that's orthogonal to what I'm saying here.  I'm saying that 
> instead of adding an optional ->aio_readv() member, we should 
> eliminate the ->aio_read() member and replace it with ->aio_readv().  
> There are only three or four places in the code that implement aio so 
> it's not a big change.  It avoids introducing a new API too.

The api replacement would make a lot more sense when there's an 
implementation backing it up.  I have no concrete objection to this 
though, maybe Andrea can whip up a follow on patch to kill aio_read.

> I also think we should have complimentary synchronous vector functions 
> although I'd like to see the synchronous API disappear completely.

I don't see a user for the synchronous ops.  I think the sync ops are 
used only by the implementation of block formats, not by disk controller 
emulation, and these only use them to access metadata.  Haven't checked 
this though.

>>>
>>> struct iovec does not exist on Windows.  I also don't think you've 
>>> got the abstraction right.  Reading and writing may require actions 
>>> to happen.  I don't think you can just introduce something as simple 
>>> as an iovec here.  I think we need something more active.
>>>
>>
>> Can you elaborate?  Actions happen in the completion callback.  This 
>> is just an straightforward extension of the block API, I don't think 
>> a dma api should materially change the block api.
>
> If we're not going to try and fold in the IOMMU/PCI BUS API at this 
> pass, then this is less important.  But to implement a proper PCI bus 
> API, I think there has to be function pointers associated with each 
> element of the scatter/gather list that control how data is copied in 
> and out of each element.

Certainly not with each element, that would increase overhead for the 
common zero copy case.  The way to do transformation is before/after 
bouncing, in the slow path.

The way I see it is:
- transformations that only manipulate addresses, maintaining alignment 
requirements, can be done at the sg->iovec transform phase without any 
need for further manipulation
- if there are alignment problems, we bounce, but again no need for 
further manipulation
- transformations that manipulate data are bounced and manipulated on 
the bounce buffer

It can all be done within the dma api, no need to involve the block 
layer with callbacks.  That's not its job.


-- 
error compiling committee.c: too many arguments to function