Re: [Qemu-devel] [PATCH 05/10] xen: add block device backend driver.

[Gerd Hoffmann <kraxel@redhat.com>]

Christoph Hellwig wrote:
> On Wed, Apr 01, 2009 at 11:39:37PM +0200, Gerd Hoffmann wrote:
>> +static void inline blkif_get_x86_32_req(blkif_request_t *dst, blkif_x86_32_request_t *src)
>> +{
> 
>> +static void inline blkif_get_x86_64_req(blkif_request_t *dst, blkif_x86_64_request_t *src)
>> +{
> 
> I think you'd be better of moving them to the .c file as normal static
> function and leave the inlining decisions to the compiler.

The .h file is a straight copy, thats why I left it as is.  I can
cut&paste it into the source file as well, I don't care that much.

>> +
>> +/*
>> + *  FIXME: the code is designed to handle multiple outstanding
>> + *         requests, which isn't used right now.  Plan is to
>> + *         switch over to the aio block functions once they got
>> + *         vector support.
>> + */
> 
> We already have bdrv_aio_readv/writev which currently linearize the
> buffer underneath.

Right, will adapt and kill the FIXME.

> Hopefully Anthony will have commited the patch to
> implement the real one while I'm writing this, too :)

Linus committed the preadv syscall patches too (abi is not final yet
though).

>> +    if (ioreq->presync)
>> +	bdrv_flush(blkdev->bs); /* FIXME: aio_flush() ??? */

> hdrv_flush doesn't actually empty the aio queues but only issues
> a fsync.  So we could still re-order requeuests around the barrier
> with this implementation.

Thats why there is a FIXME ...

>  I will soon submit a real block-layer level
> barrier implementation that just allows to flag a bdrv_aio_read/write
> request as barrier and deal with this under the hood.

Great.

cheers,
  Gerd