Re: [Qemu-devel] [PATCH 2/6] change vectored block I/O API to plain iovecs

[Christoph Hellwig <hch@lst.de>]

On Sun, Mar 15, 2009 at 02:42:05PM +0200, Avi Kivity wrote:
> Christoph Hellwig wrote:
> >QEMUIOVector is a useful helper for the dma-helper.c internals but for a 
> >generic
> >block API it's more of hindrance.  Some top-level consumers like virtio-blk
> >already have the plain iovec and segment number at hand and can pass it 
> >down
> >directly, and for those that just have a single element and need to fake up
> >a vector the plain iovec also is a lot easier.
> >  
> 
> virtio gets its iovecs through a hacky (and incorrect, try >=4G) 
> method.  IMO virtio should be fixed to use the dma api, at which point 
> it will start to use QEMUIOVector anyway,

I would not call it hacky an incorrect.  And the current dma API
certainly won't work due to the layering between the generic virtio
layer and virtio-block.

> Internally yes, but why should bdrv_* not use QEMUIOVector?  That API 
> isn't very interested in posix.

Because it makes life a lot easier.  We already pass the length in
sector units anyway.  While QEMUIOVector could replace instead of
currently duplicate it that would mean another translation between
byte and sector units at the block level.  And then comes the issue
of feeding in iovecs - there is the case of iovecs coming from other
layers like virtio-blk and the more important one of just creating
one-entry static iovecs in many places.  These would mean another
dynamic allocation and lots of API churn.

Keep the QEMUIOVector as a nice abstraction for the memory-managment
issues in dma-helper.c but I think as an API for passing data (which
doesn't care about how the iovec array is allocated) they aren't
very helpful.

> 
> > static BlockDriverAIOCB *bdrv_aio_rw_vector(BlockDriverState *bs,
> >                                             int64_t sector_num,
> >-                                            QEMUIOVector *iov,
> >+                                            struct iovec *iov,
> >+					    int nr_iov,
> >                                             int nb_sectors,
> >                                             BlockDriverCompletionFunc *cb,
> >                                             void *opaque,
> >  
> 
> Note qemu never uses tabs in source code.

Sorry, tried to get rid of most of them, but it's a real pain in the
ass writing code that way ;-)  Guess I need to write me a little
obsfucate.awk script before submitting patches..