From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47217) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YwiRl-0001c8-K8 for qemu-devel@nongnu.org; Sun, 24 May 2015 22:49:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YwiRg-00079N-HS for qemu-devel@nongnu.org; Sun, 24 May 2015 22:48:57 -0400 Date: Mon, 25 May 2015 10:48:31 +0800 From: Fam Zheng Message-ID: <20150525024831.GD7135@ad.nay.redhat.com> References: <1432190583-10518-1-git-send-email-famz@redhat.com> <1432190583-10518-13-git-send-email-famz@redhat.com> <5560B4DA.8000908@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5560B4DA.8000908@redhat.com> Subject: Re: [Qemu-devel] [PATCH v6 12/13] block: Block "device IO" during bdrv_drain and bdrv_drain_all List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Kevin Wolf , qemu-block@nongnu.org, jcody@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, Stefan Hajnoczi , amit.shah@redhat.com, pbonzini@redhat.com On Sat, 05/23 19:11, Max Reitz wrote: > On 21.05.2015 08:43, Fam Zheng wrote: > >We don't want new requests from guest, so block the operation around the > >nested poll. > > > >It also avoids looping forever when iothread is submitting a lot of requests. > > > >Signed-off-by: Fam Zheng > >--- > > block/io.c | 22 ++++++++++++++++++++-- > > 1 file changed, 20 insertions(+), 2 deletions(-) > > Hm, I don't know about this. When I see someone calling > bdrv_drain()/bdrv_drain_all(), I'm expecting that every request has been > drained afterwards. This patch implies that this is not necessarily the > case, because apparently in some configurations the guest can still submit > I/O even while bdrv_drain() is running, In dataplane, aio_poll in bdrv_drain_all will poll the ioeventfd, which could call the handlers of virtio queues. That's how guest I/O sneaks in. > but this means that even after this > patch, the same can happen if I/O is submitted after bdrv_op_unblock() and > before anything the caller of bdrv_drain() wants to do while the BDS is > still drained. So this looks to me more like the caller must ensure that the > BDS won't receive new requests, and do so before bdrv_drain() is called. Yes, callers of bdrv_drain*() should use a blocker like you reasoned. Other patches in this series looked at qmp_transaction, but there are more, which may still be wrong until they're fixed. This patch, however, fixes one of the potential issues of those callers: > >It also avoids looping forever when iothread is submitting a lot of > >requests. Fam