On 20/12/2017 11:33, Kevin Wolf wrote: > This is the second part of my work to fix drain and hopefully to prevent > it from attracting bugs as much as it did in the past. There is > definitely at least a third part coming after this, see below. > > In this series, the following improvments are made: > * Fix several bugs and inconsistencies > * Create lots of unit tests for the drain functions > * Introduce bdrv_subtree_drained_begin/end() to drain a whole subtree > rather than only a single node > * Use this to make bdrv_reopen() safe (graph changes in callbacks > called during its internal bdrv_drain_all() frequently broke it) > > Planned for part three: Make graph modifications in callbacks safe by > avoiding BDRV_POLL_WHILE() calls while we're recursing through the > graph. We can recurse before BDRV_POLL_WHILE(), after it or while > evaluating its condition, but never call any callbacks while we're > iterating child or parent lists.
This is very nice, thanks for it! I only had two comments, one of them pretty minor. Paolo > Fam Zheng (1): > block: Remove unused bdrv_requests_pending > > Kevin Wolf (18): > block: Assert drain_all is only called from main AioContext > block: Make bdrv_drain() driver callbacks non-recursive > test-bdrv-drain: Test callback for bdrv_drain > test-bdrv-drain: Test bs->quiesce_counter > blockjob: Pause job on draining any job BDS > test-bdrv-drain: Test drain vs. block jobs > block: Don't block_job_pause_all() in bdrv_drain_all() > block: Nested drain_end must still call callbacks > test-bdrv-drain: Test nested drain sections > block: Don't notify parents in drain call chain > block: Add bdrv_subtree_drained_begin/end() > test-bdrv-drain: Tests for bdrv_subtree_drain > test-bdrv-drain: Test behaviour in coroutine context > test-bdrv-drain: Recursive draining with multiple parents > block: Allow graph changes in subtree drained section > test-bdrv-drain: Test graph changes in drained section > commit: Simplify reopen of base > block: Keep nodes drained between reopen_queue/multiple > > include/block/block.h | 20 +- > include/block/block_int.h | 3 +- > block.c | 74 +++++-- > block/commit.c | 8 +- > block/io.c | 128 ++++++++---- > block/replication.c | 6 + > blockjob.c | 22 +- > qemu-io-cmds.c | 3 + > tests/test-bdrv-drain.c | 520 > +++++++++++++++++++++++++++++++++++++++++++++- > 9 files changed, 696 insertions(+), 88 deletions(-) >
