Re: [Qemu-devel] [PATCH v6 12/13] block: Block "device IO" during bdrv_drain and bdrv_drain_all

[Fam Zheng <famz@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 <famz@redhat.com>
> >---
> >  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