From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39905) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsY7f-0003cR-DT for qemu-devel@nongnu.org; Thu, 05 Jun 2014 09:54:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WsY7W-00047O-AK for qemu-devel@nongnu.org; Thu, 05 Jun 2014 09:54:27 -0400 Received: from mail-we0-x235.google.com ([2a00:1450:400c:c03::235]:62097) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsY7W-00045f-3T for qemu-devel@nongnu.org; Thu, 05 Jun 2014 09:54:18 -0400 Received: by mail-we0-f181.google.com with SMTP id w61so1126325wes.40 for ; Thu, 05 Jun 2014 06:54:13 -0700 (PDT) Date: Thu, 5 Jun 2014 15:54:10 +0200 From: Stefan Hajnoczi Message-ID: <20140605135410.GD8759@stefanha-thinkpad.redhat.com> References: <1401486037-25609-1-git-send-email-pl@kamp.de> <20140604151228.GL11073@stefanha-thinkpad.redhat.com> <538F3BE4.9020602@kamp.de> <20140605075355.GA27366@stefanha-thinkpad.redhat.com> <539025CD.40900@kamp.de> <20140605081304.GA4885@noname.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140605081304.GA4885@noname.redhat.com> Subject: Re: [Qemu-devel] [PATCHv3 RESEND] block: introduce BDRV_O_SEQUENTIAL List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: famz@redhat.com, Peter Lieven , qemu-devel@nongnu.org, Stefan Hajnoczi , shadowsor@gmail.com, pbonzini@redhat.com On Thu, Jun 05, 2014 at 10:13:04AM +0200, Kevin Wolf wrote: > Am 05.06.2014 um 10:09 hat Peter Lieven geschrieben: > > On 05.06.2014 09:53, Stefan Hajnoczi wrote: > > >On Wed, Jun 04, 2014 at 05:31:48PM +0200, Peter Lieven wrote: > > >>Am 04.06.2014 17:12, schrieb Stefan Hajnoczi: > > >>>On Fri, May 30, 2014 at 11:40:37PM +0200, Peter Lieven wrote: > > >>>>diff --git a/block/raw-posix.c b/block/raw-posix.c > > >>>>index 6586a0c..9768cc4 100644 > > >>>>--- a/block/raw-posix.c > > >>>>+++ b/block/raw-posix.c > > >>>>@@ -447,6 +447,13 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, > > >>>> } > > >>>> #endif > > >>>>+#ifdef POSIX_FADV_SEQUENTIAL > > >>>>+ if (bs->open_flags & BDRV_O_SEQUENTIAL && > > >>>>+ !(bs->open_flags & BDRV_O_NOCACHE)) { > > >>>>+ posix_fadvise(s->fd, 0, 0, POSIX_FADV_SEQUENTIAL); > > >>>>+ } > > >>>>+#endif > > >>>This is only true if the image format is raw. If the image format on > > >>>top of this raw-posix BDS is non-raw then the read pattern may not be > > >>>sequential. > > >>You are right, but will the other formats set BDRV_O_SEQUENTIAL? > > >If the user specifies qemu-img convert -N then it will be set for any > > >image format. > > > > Of course, but when e.g. qcow2 opens its underlying file, then BDRV_O_SEQUENTIAL > > is not passed on, or is it? > > It isn't qcow2 but block.c that opens bs->file, and unless you > explicitly filter out a flag, bs->file inherits it. (If it didn't do > that, your patch would have no effect for raw either.) Yes, exactly. When a raw image file is opened there are actually two BlockDriverStates: raw_bsd ("drive0") file: raw-posix (anonymous) Since your patch affected the buffer cache counter, we know that the flag was propagated down to raw-posix (by block.c as Kevin explained). The qcow2 case looks like this: qcow2 ("drive0") file: raw-posix (anonymous) > > >Maybe qemu-img convert can always set BDRV_O_SEQUENTIAL and the have the > > >raw_bsd.c format propagate it to bs->file while other formats do not. > > >Then the user doesn't have to specify a command-line option and we don't > > >set it for non-raw image formats. > > > > This would be an option. > > I agree, though it's not quite clear how raw_bsd would do that. Would > that involve a bdrv_reopen() for bs->file? One way is to add a BlockDriver bitmask field for options that get propagated to its children. Only raw_bsd will include BDRV_O_SEQUENTIAL. Stefan