The whole implementation of bdrv_commit() is only correct if no new writes come in while it's running: It has only a single loop checking the allocation status for each block and finally calls bdrv_make_empty() without checking if that throws away any new changes.
We already have to drain while taking the graph write lock. Just extend the drained section to all of bdrv_commit() to make sure that we don't get any inconsistencies. Signed-off-by: Kevin Wolf <[email protected]> Message-ID: <[email protected]> Reviewed-by: Denis V. Lunev <[email protected]> Tested-by: Denis V. Lunev <[email protected]> Signed-off-by: Kevin Wolf <[email protected]> --- block/commit.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/block/commit.c b/block/commit.c index 0d9e1a16d7a..c5e3ef03a21 100644 --- a/block/commit.c +++ b/block/commit.c @@ -518,6 +518,7 @@ int bdrv_commit(BlockDriverState *bs) if (!drv) return -ENOMEDIUM; + bdrv_drain_all_begin(); bdrv_graph_rdlock_main_loop(); backing_file_bs = bdrv_cow_bs(bs); @@ -549,6 +550,10 @@ int bdrv_commit(BlockDriverState *bs) BLK_PERM_ALL); backing = blk_new(ctx, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL); + /* We drained all nodes, but still make requests through BlockBackends */ + blk_set_disable_request_queuing(src, true); + blk_set_disable_request_queuing(backing, true); + ret = blk_insert_bs(src, bs, &local_err); if (ret < 0) { error_report_err(local_err); @@ -565,7 +570,7 @@ int bdrv_commit(BlockDriverState *bs) bdrv_graph_rdunlock_main_loop(); - bdrv_graph_wrlock_drained(); + bdrv_graph_wrlock(); bdrv_set_backing_hd(commit_top_bs, backing_file_bs, &error_abort); bdrv_set_backing_hd(bs, commit_top_bs, &error_abort); bdrv_graph_wrunlock(); @@ -647,7 +652,7 @@ ro_cleanup: blk_unref(backing); bdrv_graph_rdunlock_main_loop(); - bdrv_graph_wrlock_drained(); + bdrv_graph_wrlock(); if (bdrv_cow_bs(bs) != backing_file_bs) { bdrv_set_backing_hd(bs, backing_file_bs, &error_abort); } @@ -663,6 +668,7 @@ ro_cleanup: out: bdrv_graph_rdunlock_main_loop(); + bdrv_drain_all_end(); return ret; } -- 2.54.0
