From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1LppjW-0001Nl-33
	for qemu-devel@nongnu.org; Fri, 03 Apr 2009 16:11:22 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1LppjQ-0001E6-Vj
	for qemu-devel@nongnu.org; Fri, 03 Apr 2009 16:11:21 -0400
Received: from [199.232.76.173] (port=38004 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1LppjQ-0001Dp-FK
	for qemu-devel@nongnu.org; Fri, 03 Apr 2009 16:11:16 -0400
Received: from mx2.redhat.com ([66.187.237.31]:56267)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <kraxel@redhat.com>) id 1LppjP-0000W3-Mo
	for qemu-devel@nongnu.org; Fri, 03 Apr 2009 16:11:15 -0400
Message-ID: <49D66D5C.5090800@redhat.com>
Date: Fri, 03 Apr 2009 22:11:08 +0200
From: Gerd Hoffmann <kraxel@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 05/10] xen: add block device backend driver.
References: <1238621982-18333-1-git-send-email-kraxel@redhat.com>
	<1238621982-18333-6-git-send-email-kraxel@redhat.com>
	<20090402170209.GA10089@lst.de>
In-Reply-To: <20090402170209.GA10089@lst.de>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Christoph Hellwig <hch@lst.de>
Cc: xen-devel@lists.xensource.com, qemu-devel@nongnu.org

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