From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1JfPhk-0000WN-Hd
	for qemu-devel@nongnu.org; Fri, 28 Mar 2008 21:17:56 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1JfPhi-0000Vi-F0
	for qemu-devel@nongnu.org; Fri, 28 Mar 2008 21:17:55 -0400
Received: from [199.232.76.173] (helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1JfPhi-0000Vd-A4
	for qemu-devel@nongnu.org; Fri, 28 Mar 2008 21:17:54 -0400
Received: from mail2.shareable.org ([80.68.89.115])
	by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32)
	(Exim 4.60) (envelope-from <jamie@shareable.org>) id 1JfPhh-0006wI-UM
	for qemu-devel@nongnu.org; Fri, 28 Mar 2008 21:17:54 -0400
Date: Sat, 29 Mar 2008 01:17:52 +0000
From: Jamie Lokier <jamie@shareable.org>
Subject: Re: [kvm-devel] [Qemu-devel] [PATCH] QEMU: fsync AIO writes on flush
	request
Message-ID: <20080329011751.GB30219@shareable.org>
References: <20080328150517.GA18077@dmt>
	<200803281640.55185.paul@codesourcery.com>
	<20080328165941.GA19155@dmt>
	<200803281700.40420.paul@codesourcery.com>
	<20080328181311.GA19547@dmt>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20080328181311.GA19547@dmt>
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: qemu-devel@nongnu.org
Cc: kvm-devel <kvm-devel@lists.sourceforge.net>, Paul Brook <paul@codesourcery.com>

Marcelo Tosatti wrote:
>  static void raw_flush(BlockDriverState *bs)
>  {
>      BDRVRawState *s = bs->opaque;
> -    fsync(s->fd);
> +    raw_aio_flush(bs);
> +
> +    /* We rely on the fact that no other AIO will be submitted
> +     * in parallel, but this should be fixed by per-device
> +     * AIO queues when allowing multiple CPU's to process IO
> +     * in QEMU.
> +     */
> +    qemu_aio_flush();
>  }

It depends what raw_flush() is used for.

If you want to be sure this flushes AIO writes in-flight at the time
of the call, I still reckon you need an extra qemu_aio_flush() before
raw_aio_flush() - on at least some POSIX AIO implementations.  (I'd be
very interested to find out otherwise, if you know better).

But if, as Ian Jackson suggests, raw_flush() is _only_ used when the
guest driver issues a CACHE FLUSH command _and_ the guest driver
either cannot overlap operations, or cannot depend on overlapping
operations occuring in order, then you don't need it.

That'll depend on what kind of device is emulated.  Does the SCSI
emulation handle multiple in-flight commands with any guarantee on
order?  To be on the safe side, I'd include the extra qemu_aio_flush,
as I expect it's very unlikely to harm performance and might save
someone's data.

-- Jamie