From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52689) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YxFjp-0002Xl-Rt for qemu-devel@nongnu.org; Tue, 26 May 2015 10:21:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YxFjo-0000QV-SA for qemu-devel@nongnu.org; Tue, 26 May 2015 10:21:49 -0400 Message-ID: <55648171.9030400@redhat.com> Date: Tue, 26 May 2015 16:21:37 +0200 From: Max Reitz MIME-Version: 1.0 References: <1432190583-10518-1-git-send-email-famz@redhat.com> <1432190583-10518-13-git-send-email-famz@redhat.com> <5560B4DA.8000908@redhat.com> <20150525024831.GD7135@ad.nay.redhat.com> In-Reply-To: <20150525024831.GD7135@ad.nay.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: Fam Zheng 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 25.05.2015 04:48, Fam Zheng wrote: > 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. Well, I can't say that I like fixes which are known (or at least suspected) to not be complete very much, but well... It's always better to fix something than nothing at all. The issue always is not forgetting about the potential problem. Reviewed-by: Max Reitz > 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