On Fri 21 Oct 2016 08:55:51 PM CEST, John Snow <js...@redhat.com> 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 <be...@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. >> > > 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? Yeah, block-commit for example (see commit_start()), and block-stream after this series. Berto