Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}()

[Kevin Wolf <kwolf@redhat.com>]

Am 20.10.2016 um 10:25 hat Alberto Garcia geschrieben:
> On Wed 19 Oct 2016 07:11:20 PM CEST, Kevin Wolf wrote:
> >> bdrv_drain_all() doesn't allow the caller to do anything after all
> >> pending requests have been completed but before block jobs are
> >> resumed.
> >> 
> >> This patch splits bdrv_drain_all() into _begin() and _end() for that
> >> purpose. It also adds aio_{disable,enable}_external() calls to
> >> disable external clients in the meantime.
> >> 
> >> Signed-off-by: Alberto Garcia <berto@igalia.com>
> >
> > This looks okay as a first step, possibly enough for this series
> > (we'll have to review this carefully), but it leaves us with a rather
> > limited version of bdrv_drain_all_begin/end that excludes many useful
> > cases. One of them is that John wants to use it around QMP
> > transactions.
> >
> > Specifically, you can't add a new BDS or a new block job in a
> > drain_all section because then bdrv_drain_all_end() would try to
> > unpause the new thing even though it has never been paused. Depending
> > on what else we did with it, this will either corrupt the pause
> > counters or just directly fail an assertion.
> 
> The problem is: do you want to be able to create a new block job and let
> it run? Because then you can end up having the same problem that this
> patch is trying to prevent if the new job attempts to reopen a
> BlockDriverState.

No, as I wrote it would have to be automatically paused on creation if
it is created in a drained_all section. It would only actually start to
run after bdrv_drain_all_end().

Kevin