@samitouri / QOSamiQemu / commits / f0d9ccd46c

commit: Drain nodes across all of bdrv_commit()

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 <kwolf@redhat.com> Message-ID: <20260427170520.101242-2-kwolf@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Tested-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Kevin Wolf committed Apr 27, 2026 at 19:05 UTC f0d9ccd46cf8fc576ab7d514f10f766546cdbc14
1 file changed +8 -2
block/commit.c
+8 -2
@@ -518,6 +518,7 @@ int bdrv_commit(BlockDriverState *bs)
518 if (!drv)
519 return -ENOMEDIUM;
520
521 + bdrv_drain_all_begin();
522 bdrv_graph_rdlock_main_loop();
523
524 backing_file_bs = bdrv_cow_bs(bs);
@@ -549,6 +550,10 @@ int bdrv_commit(BlockDriverState *bs)
550 BLK_PERM_ALL);
551 backing = blk_new(ctx, BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL);
552
553 + /* We drained all nodes, but still make requests through BlockBackends */
554 + blk_set_disable_request_queuing(src, true);
555 + blk_set_disable_request_queuing(backing, true);
556 +
557 ret = blk_insert_bs(src, bs, &local_err);
558 if (ret < 0) {
559 error_report_err(local_err);
@@ -565,7 +570,7 @@ int bdrv_commit(BlockDriverState *bs)
570
571 bdrv_graph_rdunlock_main_loop();
572
568 - bdrv_graph_wrlock_drained();
573 + bdrv_graph_wrlock();
574 bdrv_set_backing_hd(commit_top_bs, backing_file_bs, &error_abort);
575 bdrv_set_backing_hd(bs, commit_top_bs, &error_abort);
576 bdrv_graph_wrunlock();
@@ -647,7 +652,7 @@ ro_cleanup:
652 blk_unref(backing);
653
654 bdrv_graph_rdunlock_main_loop();
650 - bdrv_graph_wrlock_drained();
655 + bdrv_graph_wrlock();
656 if (bdrv_cow_bs(bs) != backing_file_bs) {
657 bdrv_set_backing_hd(bs, backing_file_bs, &error_abort);
658 }
@@ -663,6 +668,7 @@ ro_cleanup:
668
669 out:
670 bdrv_graph_rdunlock_main_loop();
671 + bdrv_drain_all_end();
672
673 return ret;
674 }