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

[John Snow <jsnow@redhat.com>]

On 10/20/2016 04:25 AM, Alberto Garcia wrote:
> 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.
>
> Berto
>

The plan was to create jobs in a pre-started mode and only to engage 
them after the drained section. Do any jobs re-open a BDS prior to the 
creation of their coroutine?

--js