[PATCH 1/2] blockdev: qmp_transaction: harden transaction properties for bitmaps

[Andrey Zhadchenko]

Unlike other transaction commands, bitmap operations do not drain target
bds. If we have an IOThread, this may result in some inconsistencies, as
bitmap content may change during transaction command.
Add bdrv_drained_begin()/end() to bitmap operations.

Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
---
 blockdev.c | 46 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index e6eba61484..7a376fce90 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1932,10 +1932,19 @@ typedef struct BlockDirtyBitmapState {
     bool was_enabled;
 } BlockDirtyBitmapState;
 
+static void block_dirty_bitmap_state_clean(void *opaque)
+{
+    g_autofree BlockDirtyBitmapState *state = opaque;
+
+    if (state->bs) {
+        bdrv_drained_end(state->bs);
+    }
+}
+
 static void block_dirty_bitmap_add_abort(void *opaque);
 TransactionActionDrv block_dirty_bitmap_add_drv = {
     .abort = block_dirty_bitmap_add_abort,
-    .clean = g_free,
+    .clean = block_dirty_bitmap_state_clean,
 };
 
 static void block_dirty_bitmap_add_action(BlockDirtyBitmapAdd *action,
@@ -1946,6 +1955,12 @@ static void 
block_dirty_bitmap_add_action(BlockDirtyBitmapAdd *action,
 
     tran_add(tran, &block_dirty_bitmap_add_drv, state);
 
+    state->bs = bdrv_lookup_bs(action->node, action->node, errp);
+    if (!state->bs) {
+        return;
+    }
+    bdrv_drained_begin(state->bs);
+
     /* AIO context taken and released within qmp_block_dirty_bitmap_add */
     qmp_block_dirty_bitmap_add(action->node, action->name,
                                action->has_granularity, action->granularity,
@@ -1975,7 +1990,7 @@ static void block_dirty_bitmap_free_backup(void *opaque);
 TransactionActionDrv block_dirty_bitmap_clear_drv = {
     .abort = block_dirty_bitmap_restore,
     .commit = block_dirty_bitmap_free_backup,
-    .clean = g_free,
+    .clean = block_dirty_bitmap_state_clean,
 };
 
 static void block_dirty_bitmap_clear_action(BlockDirtyBitmap *action,
@@ -1993,6 +2008,8 @@ static void 
block_dirty_bitmap_clear_action(BlockDirtyBitmap *action,
         return;
     }
 
+    bdrv_drained_begin(state->bs);
+
     if (bdrv_dirty_bitmap_check(state->bitmap, BDRV_BITMAP_DEFAULT, errp)) {
         return;
     }
@@ -2019,7 +2036,7 @@ static void block_dirty_bitmap_free_backup(void *opaque)
 static void block_dirty_bitmap_enable_abort(void *opaque);
 TransactionActionDrv block_dirty_bitmap_enable_drv = {
     .abort = block_dirty_bitmap_enable_abort,
-    .clean = g_free,
+    .clean = block_dirty_bitmap_state_clean,
 };
 
 static void block_dirty_bitmap_enable_action(BlockDirtyBitmap *action,
@@ -2031,12 +2048,14 @@ static void 
block_dirty_bitmap_enable_action(BlockDirtyBitmap *action,
 
     state->bitmap = block_dirty_bitmap_lookup(action->node,
                                               action->name,
-                                              NULL,
+                                              &state->bs,
                                               errp);
     if (!state->bitmap) {
         return;
     }
 
+    bdrv_drained_begin(state->bs);
+
     if (bdrv_dirty_bitmap_check(state->bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
         return;
     }
@@ -2057,7 +2076,7 @@ static void block_dirty_bitmap_enable_abort(void *opaque)
 static void block_dirty_bitmap_disable_abort(void *opaque);
 TransactionActionDrv block_dirty_bitmap_disable_drv = {
     .abort = block_dirty_bitmap_disable_abort,
-    .clean = g_free,
+    .clean = block_dirty_bitmap_state_clean,
 };
 
 static void block_dirty_bitmap_disable_action(BlockDirtyBitmap *action,
@@ -2075,6 +2094,8 @@ static void 
block_dirty_bitmap_disable_action(BlockDirtyBitmap *action,
         return;
     }
 
+    bdrv_drained_begin(state->bs);
+
     if (bdrv_dirty_bitmap_check(state->bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
         return;
     }
@@ -2095,7 +2116,7 @@ static void block_dirty_bitmap_disable_abort(void *opaque)
 TransactionActionDrv block_dirty_bitmap_merge_drv = {
     .commit = block_dirty_bitmap_free_backup,
     .abort = block_dirty_bitmap_restore,
-    .clean = g_free,
+    .clean = block_dirty_bitmap_state_clean,
 };
 
 static void block_dirty_bitmap_merge_action(BlockDirtyBitmapMerge *action,
@@ -2105,6 +2126,12 @@ static void 
block_dirty_bitmap_merge_action(BlockDirtyBitmapMerge *action,
 
     tran_add(tran, &block_dirty_bitmap_merge_drv, state);
 
+    state->bs = bdrv_lookup_bs(action->node, action->node, errp);
+    if (!state->bs) {
+        return;
+    }
+    bdrv_drained_begin(state->bs);
+
     state->bitmap = block_dirty_bitmap_merge(action->node, action->target,
                                              action->bitmaps, &state->backup,
                                              errp);
@@ -2115,7 +2142,7 @@ static void block_dirty_bitmap_remove_abort(void *opaque);
 TransactionActionDrv block_dirty_bitmap_remove_drv = {
     .commit = block_dirty_bitmap_remove_commit,
     .abort = block_dirty_bitmap_remove_abort,
-    .clean = g_free,
+    .clean = block_dirty_bitmap_state_clean,
 };
 
 static void block_dirty_bitmap_remove_action(BlockDirtyBitmap *action,
@@ -2125,6 +2152,11 @@ static void 
block_dirty_bitmap_remove_action(BlockDirtyBitmap *action,
 
     tran_add(tran, &block_dirty_bitmap_remove_drv, state);
 
+    state->bs = bdrv_lookup_bs(action->node, action->node, errp);
+    if (!state->bs) {
+        return;
+    }
+    bdrv_drained_begin(state->bs);
 
     state->bitmap = block_dirty_bitmap_remove(action->node, action->name,
                                               false, &state->bs, errp);
-- 
2.39.2