[Qemu-devel] [PATCH] block: Make op blocker recursive

[Benoît Canet]

As the code will start to operate on arbitratry nodes we need the op blocker
to recursively block or unblock whole BDS subtrees.

Also add a function to reset all blocker from a BDS.

This patch also take care of changing blocker user so they are not broken.

Signed-off-by: Benoit Canet <address@hidden>
---
 block.c                   | 87 +++++++++++++++++++++++++++++++++++++++++++++++
 block/blkverify.c         | 10 ++++++
 block/commit.c            |  1 +
 block/mirror.c            |  1 +
 block/quorum.c            | 13 +++++++
 include/block/block.h     | 12 +++++++
 include/block/block_int.h |  5 +++
 7 files changed, 129 insertions(+)

diff --git a/block.c b/block.c
index 1fdfa66..fe78f42 100644
--- a/block.c
+++ b/block.c
@@ -5414,6 +5414,8 @@ void bdrv_op_block(BlockDriverState *bs, BlockOpType op, 
Error *reason)
     blocker = g_malloc0(sizeof(BdrvOpBlocker));
     blocker->reason = reason;
     QLIST_INSERT_HEAD(&bs->op_blockers[op], blocker, list);
+
+    bdrv_recurse_op_block(bs, op, ACTION_BLOCK, reason);
 }
 
 void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason)
@@ -5426,6 +5428,20 @@ void bdrv_op_unblock(BlockDriverState *bs, BlockOpType 
op, Error *reason)
             g_free(blocker);
         }
     }
+
+    bdrv_recurse_op_block(bs, op, ACTION_UNBLOCK, reason);
+}
+
+/* This remove unconditionally all blockers of type op of the subtree */
+void bdrv_op_reset(BlockDriverState *bs, BlockOpType op)
+{
+    BdrvOpBlocker *blocker, *next;
+    QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, next) {
+        QLIST_REMOVE(blocker, list);
+        g_free(blocker);
+    }
+
+    bdrv_recurse_op_block(bs, op, ACTION_RESET, NULL);
 }
 
 void bdrv_op_block_all(BlockDriverState *bs, Error *reason)
@@ -5444,6 +5460,15 @@ void bdrv_op_unblock_all(BlockDriverState *bs, Error 
*reason)
     }
 }
 
+/* This remove unconditionally all blockers */
+void bdrv_op_reset_all(BlockDriverState *bs)
+{
+    int i;
+    for (i = 0; i < BLOCK_OP_TYPE_MAX; i++) {
+        bdrv_op_reset(bs, i);
+    }
+}
+
 bool bdrv_op_blocker_is_empty(BlockDriverState *bs)
 {
     int i;
@@ -5456,6 +5481,68 @@ bool bdrv_op_blocker_is_empty(BlockDriverState *bs)
     return true;
 }
 
+/* Used to prevent recursion loop. A case exists in block commit mirror usage 
*/
+static BlockDriverState *recurse_op_bs = NULL;
+/* take note of the recursion depth to allow assigning recurse_op_bs once */
+static uint64_t recurse_op_depth = 0;
+
+/* set or unset an op blocker to a BDS whether set is true or false */
+void bdrv_op_block_action(BlockDriverState *bs, BlockOpType op,
+                          BlockerAction action, Error *reason)
+{
+    if (!bs) {
+        return;
+    }
+
+    recurse_op_depth++;
+    switch (action) {
+    case ACTION_BLOCK:
+        bdrv_op_block(bs, op, reason);
+        break;
+    case ACTION_UNBLOCK:
+        bdrv_op_unblock(bs, op, reason);
+        break;
+    case ACTION_RESET:
+        bdrv_op_reset(bs, op);
+        break;
+    default:
+        assert(false);
+    }
+    recurse_op_depth--;
+}
+
+/* Recursively set or unset an op block to a BDS tree whether set is true or
+ * false
+ */
+void bdrv_recurse_op_block(BlockDriverState *bs, BlockOpType op,
+                           BlockerAction action, Error *reason)
+{
+    /* If recursion is detected simply return */
+    if (recurse_op_bs == bs) {
+        return;
+    }
+
+    /* if we are starting recursion remeber the bs for later comparison */
+    if (!recurse_op_depth) {
+        recurse_op_bs = bs;
+    }
+
+    /* try to set or unset on bs->file and bs->backing_hd first */
+    bdrv_op_block_action(bs->file, op, action, reason);
+    bdrv_op_block_action(bs->backing_hd, op, action, reason);
+
+    /* if the BDS is a filter with multiple childs ask the driver to recurse */
+    if (bs->drv && bs->drv->bdrv_recurse_op_block) {
+         recurse_op_depth++;
+         bs->drv->bdrv_recurse_op_block(bs, op, action, reason);
+         recurse_op_depth--;
+    }
+
+    if (!recurse_op_depth) {
+        recurse_op_bs = NULL;
+    }
+}
+
 void bdrv_iostatus_enable(BlockDriverState *bs)
 {
     bs->iostatus_enabled = true;
diff --git a/block/blkverify.c b/block/blkverify.c
index 621b785..dd0859a 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -304,6 +304,14 @@ static bool 
blkverify_recurse_is_first_non_filter(BlockDriverState *bs,
     return bdrv_recurse_is_first_non_filter(s->test_file, candidate);
 }
 
+static void blkverify_recurse_op_block(BlockDriverState *bs, BlockOpType op,
+                                       BlockerAction action, Error *reason)
+{
+    BDRVBlkverifyState *s = bs->opaque;
+    bdrv_op_block_action(bs->file, op , action, reason);
+    bdrv_op_block_action(s->test_file, op , action, reason);
+}
+
 /* Propagate AioContext changes to ->test_file */
 static void blkverify_detach_aio_context(BlockDriverState *bs)
 {
@@ -339,6 +347,8 @@ static BlockDriver bdrv_blkverify = {
 
     .is_filter                        = true,
     .bdrv_recurse_is_first_non_filter = blkverify_recurse_is_first_non_filter,
+
+    .bdrv_recurse_op_block            = blkverify_recurse_op_block,
 };
 
 static void bdrv_blkverify_init(void)
diff --git a/block/commit.c b/block/commit.c
index 5c09f44..d1ca7bd 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -141,6 +141,7 @@ wait:
 
     if (!block_job_is_cancelled(&s->common) && sector_num == end) {
         /* success */
+        bdrv_op_reset_all(active);
         ret = bdrv_drop_intermediate(active, top, base);
     }
 
diff --git a/block/mirror.c b/block/mirror.c
index 94c8661..a99417d 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -502,6 +502,7 @@ immediate_exit:
             bdrv_unref(p);
         }
     }
+    bdrv_op_reset_all(s->target);
     bdrv_unref(s->target);
     block_job_completed(&s->common, ret);
 }
diff --git a/block/quorum.c b/block/quorum.c
index 426077a..7ec135a 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -683,6 +683,17 @@ static bool 
quorum_recurse_is_first_non_filter(BlockDriverState *bs,
     return false;
 }
 
+static void quorum_recurse_op_block(BlockDriverState *bs, BlockOpType op,
+                                    BlockerAction action, Error *reason)
+{
+    BDRVQuorumState *s = bs->opaque;
+    int i;
+
+    for (i = 0; i < s->num_children; i++) {
+        bdrv_op_block_action(s->bs[i], op , action, reason);
+    }
+}
+
 static int quorum_valid_threshold(int threshold, int num_children, Error 
**errp)
 {
 
@@ -891,6 +902,8 @@ static BlockDriver bdrv_quorum = {
 
     .is_filter                          = true,
     .bdrv_recurse_is_first_non_filter   = quorum_recurse_is_first_non_filter,
+
+    .bdrv_recurse_op_block              = quorum_recurse_op_block,
 };
 
 static void bdrv_quorum_init(void)
diff --git a/include/block/block.h b/include/block/block.h
index f15b99b..13ebc94 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -182,6 +182,12 @@ typedef enum BlockOpType {
     BLOCK_OP_TYPE_MAX,
 } BlockOpType;
 
+typedef enum BlockerAction {
+    ACTION_BLOCK,
+    ACTION_UNBLOCK,
+    ACTION_RESET,
+} BlockerAction;
+
 void bdrv_iostatus_enable(BlockDriverState *bs);
 void bdrv_iostatus_reset(BlockDriverState *bs);
 void bdrv_iostatus_disable(BlockDriverState *bs);
@@ -476,9 +482,15 @@ void bdrv_unref(BlockDriverState *bs);
 bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
 void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
 void bdrv_op_unblock(BlockDriverState *bs, BlockOpType op, Error *reason);
+void bdrv_op_reset(BlockDriverState *bs, BlockOpType op);
 void bdrv_op_block_all(BlockDriverState *bs, Error *reason);
 void bdrv_op_unblock_all(BlockDriverState *bs, Error *reason);
+void bdrv_op_reset_all(BlockDriverState *bs);
 bool bdrv_op_blocker_is_empty(BlockDriverState *bs);
+void bdrv_op_block_action(BlockDriverState *bs, BlockOpType op,
+                          BlockerAction action, Error *reason);
+void bdrv_recurse_op_block(BlockDriverState *bs, BlockOpType op,
+                           BlockerAction action, Error *reason);
 
 enum BlockAcctType {
     BDRV_ACCT_READ,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7aa2213..de4a75e 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -86,6 +86,11 @@ struct BlockDriver {
      */
     bool (*bdrv_recurse_is_first_non_filter)(BlockDriverState *bs,
                                              BlockDriverState *candidate);
+    /* In order to be able to recursively block operation on BDS trees filter
+     * like quorum can implement this callback
+     */
+    void (*bdrv_recurse_op_block)(BlockDriverState *bs, BlockOpType op,
+                                  BlockerAction action, Error *reason);
 
     int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
     int (*bdrv_probe_device)(const char *filename);
-- 
1.9.1