From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:40942)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1X2GLG-0001Yl-JH
	for qemu-devel@nongnu.org; Wed, 02 Jul 2014 04:56:44 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1X2GLA-0003Tu-D3
	for qemu-devel@nongnu.org; Wed, 02 Jul 2014 04:56:38 -0400
Received: from mx1.redhat.com ([209.132.183.28]:37364)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1X2GLA-0003Tq-3L
	for qemu-devel@nongnu.org; Wed, 02 Jul 2014 04:56:32 -0400
Message-ID: <53B3C932.7000700@redhat.com>
Date: Wed, 02 Jul 2014 10:56:18 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1404201119-6646-1-git-send-email-ming.lei@canonical.com>	<1404201119-6646-2-git-send-email-ming.lei@canonical.com>	<20140701111843.GE4587@noname.str.redhat.com>	<CACVXFVMfU_aYhzrGRy1UVeOJVYQ1iAQivs-M-wynNvAb-6zRsg@mail.gmail.com>	<CACVXFVMuZfC0g+FPBC2GwueSUQc2RF3NGHuFig3uZhGX4peJdQ@mail.gmail.com>	<20140701152130.GK4587@noname.str.redhat.com>	<53B2E82D.8020000@redhat.com>	<CACVXFVMem3Hyux=jvkto6icBLw2raz-=nhbS2VOma4ktbK=0kQ@mail.gmail.com>	<53B3C059.3060601@redhat.com>
	<CACVXFVOm0Ekd1-8bQ2n5gPWLy0qFJb878a8sMvJQrRr1Pmtk4w@mail.gmail.com>
In-Reply-To: <CACVXFVOm0Ekd1-8bQ2n5gPWLy0qFJb878a8sMvJQrRr1Pmtk4w@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2 1/3] block: block: introduce
	bdrv_io_plug() and bdrv_io_unplug()
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Ming Lei <ming.lei@canonical.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Fam Zheng <famz@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, qemu-devel <qemu-devel@nongnu.org>, Stefan Hajnoczi <stefanha@redhat.com>

Il 02/07/2014 10:39, Ming Lei ha scritto:
> Then start to read payload in original path, but no plug/unplug any
> more. Also another request may follows, and another plug&unplug
> comes too, which makes thing more complicated, so I suggest to
> enable plug&unplug only for raw driver now.

That's just a performance issue (and actually one that wasn't in 2.0 
because qcow2 on dataplane wasn't supported there).  In many cases the 
cache hit of the qcow2 metadata cache can be very high, and avoiding 
plug/unplug would prevent an easy performance bonus.

I don't especially like plug/unplug as an API (I think it's better to 
extend aio_multiwrite to include other kind of requests), but:

- either we have qualms on the correctness of it, and then we should 
live with the regressions

- or if the patches are not messy and reverting them is easy, we should 
go for it.  This is what we did for dataplane in the first place, and we 
can keep doing it in the 2.1 dataplane code.

> > If you could make it an assertion, that would also be great.  (BTW, it's
> > probably best to add a nesting count to plug/unplug).
>
> I'd rather to add it in future if we really need that.

I think this is a matter of correctness as Kevin's review pointed out.

Paolo