[Qemu-devel] [Patch v8 0/3] qapi: child add/delete support

[Qemu-devel] [Patch v8 0/3] qapi: child add/delete support

[Wen Congyang]

If quorum's child is broken, we can use mirror job to replace it.
But sometimes, the user only need to remove the broken child, and
add it later when the problem is fixed.

It is based on the Kevin's child name related patch:
http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg04949.html

ChangLog:
v8:
1. Rebase to the newest codes
2. Address the comments from Eric Blake
v7:
1. Remove the qmp command x-blockdev-change's parameter operation according
   to Kevin's comments.
2. Remove the hmp command.
v6:
1. Use a single qmp command x-blockdev-change to replace x-blockdev-child-add
   and x-blockdev-child-delete
v5:
1. Address Eric Blake's comments
v4:
1. drop nbd driver's implementation. We can use human-monitor-command
   to do it.
2. Rename the command name.
v3:
1. Don't open BDS in bdrv_add_child(). Use the existing BDS which is
   created by the QMP command blockdev-add.
2. The driver NBD can support filename, path, host:port now.
v2:
1. Use bdrv_get_device_or_node_name() instead of new function
   bdrv_get_id_or_node_name()
2. Update the error message
3. Update the documents in block-core.json

Wen Congyang (3):
  Add new block driver interface to add/delete a BDS's child
  quorum: implement bdrv_add_child() and bdrv_del_child()
  qmp: add monitor command to add/remove a child

 block.c                   |  58 ++++++++++++++++++++--
 block/quorum.c            | 124 +++++++++++++++++++++++++++++++++++++++++++++-
 blockdev.c                |  54 ++++++++++++++++++++
 include/block/block.h     |   9 ++++
 include/block/block_int.h |   5 ++
 qapi/block-core.json      |  23 +++++++++
 qmp-commands.hx           |  47 ++++++++++++++++++
 7 files changed, 314 insertions(+), 6 deletions(-)

-- 
2.5.0

[Qemu-devel] [Patch v8 1/3] Add new block driver interface to add/delete a BDS's child

[Wen Congyang]

In some cases, we want to take a quorum child offline, and take
another child online.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block.c                   | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h     |  5 +++++
 include/block/block_int.h |  5 +++++
 3 files changed, 60 insertions(+)

diff --git a/block.c b/block.c
index 60ff84f..255a36e 100644
--- a/block.c
+++ b/block.c
@@ -4321,3 +4321,53 @@ void bdrv_refresh_filename(BlockDriverState *bs)
         QDECREF(json);
     }
 }
+
+/*
+ * Hot add/remove a BDS's child. So the user can take a child offline when
+ * it is broken and take a new child online
+ */
+void bdrv_add_child(BlockDriverState *parent_bs, BlockDriverState *child_bs,
+                    Error **errp)
+{
+
+    if (!parent_bs->drv || !parent_bs->drv->bdrv_add_child) {
+        error_setg(errp, "The node %s doesn't support adding a child",
+                   bdrv_get_device_or_node_name(parent_bs));
+        return;
+    }
+
+    if (!QLIST_EMPTY(&child_bs->parents)) {
+        error_setg(errp, "The node %s already has a parent",
+                   child_bs->node_name);
+        return;
+    }
+
+    parent_bs->drv->bdrv_add_child(parent_bs, child_bs, errp);
+}
+
+void bdrv_del_child(BlockDriverState *parent_bs, BlockDriverState *child_bs,
+                    Error **errp)
+{
+    BdrvChild *child;
+
+    if (!parent_bs->drv || !parent_bs->drv->bdrv_del_child) {
+        error_setg(errp, "The node %s doesn't support removing a child",
+                   bdrv_get_device_or_node_name(parent_bs));
+        return;
+    }
+
+    QLIST_FOREACH(child, &parent_bs->children, next) {
+        if (child->bs == child_bs) {
+            break;
+        }
+    }
+
+    if (!child) {
+        error_setg(errp, "The node %s is not a child of %s",
+                   bdrv_get_device_or_node_name(child_bs),
+                   bdrv_get_device_or_node_name(parent_bs));
+        return;
+    }
+
+    parent_bs->drv->bdrv_del_child(parent_bs, child_bs, errp);
+}
diff --git a/include/block/block.h b/include/block/block.h
index d9b380c..06d3369 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -639,4 +639,9 @@ void bdrv_drained_begin(BlockDriverState *bs);
  */
 void bdrv_drained_end(BlockDriverState *bs);
 
+void bdrv_add_child(BlockDriverState *parent, BlockDriverState *child,
+                    Error **errp);
+void bdrv_del_child(BlockDriverState *parent, BlockDriverState *child,
+                    Error **errp);
+
 #endif
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6d7bd3b..ea20d12 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -302,6 +302,11 @@ struct BlockDriver {
      */
     void (*bdrv_drain)(BlockDriverState *bs);
 
+    void (*bdrv_add_child)(BlockDriverState *parent, BlockDriverState *child,
+                           Error **errp);
+    void (*bdrv_del_child)(BlockDriverState *parent, BlockDriverState *child,
+                           Error **errp);
+
     QLIST_ENTRY(BlockDriver) list;
 };
 
-- 
2.5.0

[Qemu-devel] [Patch v8 2/3] quorum: implement bdrv_add_child() and bdrv_del_child()

[Wen Congyang]

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 block.c               |   8 ++--
 block/quorum.c        | 124 +++++++++++++++++++++++++++++++++++++++++++++++++-
 include/block/block.h |   4 ++
 3 files changed, 130 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 255a36e..bfc2be8 100644
--- a/block.c
+++ b/block.c
@@ -1196,10 +1196,10 @@ static int bdrv_fill_options(QDict **options, const char *filename,
     return 0;
 }
 
-static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
-                                    BlockDriverState *child_bs,
-                                    const char *child_name,
-                                    const BdrvChildRole *child_role)
+BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
+                             BlockDriverState *child_bs,
+                             const char *child_name,
+                             const BdrvChildRole *child_role)
 {
     BdrvChild *child = g_new(BdrvChild, 1);
     *child = (BdrvChild) {
diff --git a/block/quorum.c b/block/quorum.c
index 2810e37..b7df14b 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -23,6 +23,7 @@
 #include "qapi/qmp/qstring.h"
 #include "qapi-event.h"
 #include "crypto/hash.h"
+#include "qemu/bitmap.h"
 
 #define HASH_LENGTH 32
 
@@ -80,6 +81,8 @@ typedef struct BDRVQuorumState {
     bool rewrite_corrupted;/* true if the driver must rewrite-on-read corrupted
                             * block if Quorum is reached.
                             */
+    unsigned long *index_bitmap;
+    int bsize;
 
     QuorumReadPattern read_pattern;
 } BDRVQuorumState;
@@ -875,9 +878,9 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
         ret = -EINVAL;
         goto exit;
     }
-    if (s->num_children < 2) {
+    if (s->num_children < 1) {
         error_setg(&local_err,
-                   "Number of provided children must be greater than 1");
+                   "Number of provided children must be 1 or more");
         ret = -EINVAL;
         goto exit;
     }
@@ -926,6 +929,7 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
     /* allocate the children array */
     s->children = g_new0(BdrvChild *, s->num_children);
     opened = g_new0(bool, s->num_children);
+    s->index_bitmap = bitmap_new(s->num_children);
 
     for (i = 0; i < s->num_children; i++) {
         char indexstr[32];
@@ -941,6 +945,8 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
 
         opened[i] = true;
     }
+    bitmap_set(s->index_bitmap, 0, s->num_children);
+    s->bsize = s->num_children;
 
     g_free(opened);
     goto exit;
@@ -997,6 +1003,117 @@ static void quorum_attach_aio_context(BlockDriverState *bs,
     }
 }
 
+static int get_new_child_index(BDRVQuorumState *s)
+{
+    int index;
+
+    index = find_next_zero_bit(s->index_bitmap, s->bsize, 0);
+    if (index < s->bsize) {
+        return index;
+    }
+
+    if ((s->bsize % BITS_PER_LONG) == 0) {
+        s->index_bitmap = bitmap_zero_extend(s->index_bitmap, s->bsize,
+                                             s->bsize + 1);
+    }
+
+    return s->bsize++;
+}
+
+static void remove_child_index(BDRVQuorumState *s, int index)
+{
+    int last_index;
+    long new_len;
+
+    assert(index < s->bsize);
+
+    clear_bit(index, s->index_bitmap);
+    if (index < s->bsize - 1) {
+        /*
+         * The last bit is always set, and we don't clear
+         * the last bit.
+         */
+        return;
+    }
+
+    last_index = find_last_bit(s->index_bitmap, s->bsize);
+    if (BITS_TO_LONGS(last_index + 1) == BITS_TO_LONGS(s->bsize)) {
+        s->bsize = last_index + 1;
+        return;
+    }
+
+    new_len = BITS_TO_LONGS(last_index + 1) * sizeof(unsigned long);
+    s->index_bitmap = g_realloc(s->index_bitmap, new_len);
+    s->bsize = last_index + 1;
+}
+
+static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs,
+                             Error **errp)
+{
+    BDRVQuorumState *s = bs->opaque;
+    BdrvChild *child;
+    char indexstr[32];
+    int index = find_next_zero_bit(s->index_bitmap, s->bsize, 0);
+    int ret;
+
+    index = get_new_child_index(s);
+    ret = snprintf(indexstr, 32, "children.%d", index);
+    if (ret < 0 || ret >= 32) {
+        error_setg(errp, "cannot generate child name");
+        return;
+    }
+
+    bdrv_drain(bs);
+
+    assert(s->num_children <= INT_MAX / sizeof(BdrvChild *));
+    if (s->num_children == INT_MAX / sizeof(BdrvChild *)) {
+        error_setg(errp, "Too many children");
+        return;
+    }
+    s->children = g_renew(BdrvChild *, s->children, s->num_children + 1);
+
+    bdrv_ref(child_bs);
+    child = bdrv_attach_child(bs, child_bs, indexstr, &child_format);
+    s->children[s->num_children++] = child;
+    set_bit(index, s->index_bitmap);
+}
+
+static void quorum_del_child(BlockDriverState *bs, BlockDriverState *child_bs,
+                             Error **errp)
+{
+    BDRVQuorumState *s = bs->opaque;
+    BdrvChild *child;
+    int i, index;
+
+    for (i = 0; i < s->num_children; i++) {
+        if (s->children[i]->bs == child_bs) {
+            break;
+        }
+    }
+
+    /* we have checked it in bdrv_del_child() */
+    assert(i < s->num_children);
+    child = s->children[i];
+
+    if (s->num_children <= s->threshold) {
+        error_setg(errp,
+            "The number of children cannot be lower than the vote threshold %d",
+            s->threshold);
+        return;
+    }
+
+    /* child->name is "children.%d" */
+    index = atoi(child->name + 9);
+
+    bdrv_drain(bs);
+    /* We can safely remove this child now */
+    memmove(&s->children[i], &s->children[i + 1],
+            (s->num_children - i - 1) * sizeof(void *));
+    s->children = g_renew(BdrvChild *, s->children, --s->num_children);
+    remove_child_index(s, index);
+    bdrv_unref_child(bs, child);
+}
+
 static void quorum_refresh_filename(BlockDriverState *bs, QDict *options)
 {
     BDRVQuorumState *s = bs->opaque;
@@ -1052,6 +1169,9 @@ static BlockDriver bdrv_quorum = {
     .bdrv_detach_aio_context            = quorum_detach_aio_context,
     .bdrv_attach_aio_context            = quorum_attach_aio_context,
 
+    .bdrv_add_child                     = quorum_add_child,
+    .bdrv_del_child                     = quorum_del_child,
+
     .is_filter                          = true,
     .bdrv_recurse_is_first_non_filter   = quorum_recurse_is_first_non_filter,
 };
diff --git a/include/block/block.h b/include/block/block.h
index 06d3369..1d3b9c6 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -514,6 +514,10 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
 void bdrv_ref(BlockDriverState *bs);
 void bdrv_unref(BlockDriverState *bs);
 void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child);
+BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
+                             BlockDriverState *child_bs,
+                             const char *child_name,
+                             const BdrvChildRole *child_role);
 
 bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
 void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
-- 
2.5.0

[Qemu-devel] [Patch v8 3/3] qmp: add monitor command to add/remove a child

[Wen Congyang]

The new QMP command name is x-blockdev-change. It's just for adding/removing
quorum's child now, and doesn't support all kinds of children, all kinds of
operations, nor all block drivers. So it is experimental now.

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 blockdev.c           | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 23 ++++++++++++++++++++++
 qmp-commands.hx      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 2b076fb..7d8a2b4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3836,6 +3836,60 @@ out:
     aio_context_release(aio_context);
 }
 
+static BlockDriverState *bdrv_find_child(BlockDriverState *parent_bs,
+                                         const char *child_name)
+{
+    BdrvChild *child;
+
+    QLIST_FOREACH(child, &parent_bs->children, next) {
+        if (strcmp(child->name, child_name) == 0) {
+            return child->bs;
+        }
+    }
+
+    return NULL;
+}
+
+void qmp_x_blockdev_change(const char *parent, bool has_child,
+                           const char *child, bool has_node,
+                           const char *node, Error **errp)
+{
+    BlockDriverState *parent_bs, *child_bs = NULL, *new_bs = NULL;
+
+    parent_bs = bdrv_lookup_bs(parent, parent, errp);
+    if (!parent_bs) {
+        return;
+    }
+
+    if (has_child == has_node) {
+        if (has_child) {
+            error_setg(errp, "The paramter child and node is conflict");
+        } else {
+            error_setg(errp, "Either child or node should be specified");
+        }
+        return;
+    }
+
+    if (has_child) {
+        child_bs = bdrv_find_child(parent_bs, child);
+        if (!child_bs) {
+            error_setg(errp, "Node '%s' doesn't have child %s",
+                       parent, child);
+            return;
+        }
+        bdrv_del_child(parent_bs, child_bs, errp);
+    }
+
+    if (has_node) {
+        new_bs = bdrv_find_node(node);
+        if (!new_bs) {
+            error_setg(errp, "Node '%s' not found", node);
+            return;
+        }
+        bdrv_add_child(parent_bs, new_bs, errp);
+    }
+}
+
 BlockJobInfoList *qmp_query_block_jobs(Error **errp)
 {
     BlockJobInfoList *head = NULL, **p_next = &head;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a07b13f..feb8da2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2400,3 +2400,26 @@
 ##
 { 'command': 'block-set-write-threshold',
   'data': { 'node-name': 'str', 'write-threshold': 'uint64' } }
+
+##
+# @x-blockdev-change
+#
+# Dynamically reconfigure the block driver state graph. It can be used
+# to add, remove, insert or replace a block driver state. Currently only
+# the Quorum driver implements this feature to add or remove its child.
+# This is useful to fix a broken quorum child.
+#
+# @parent: the id or name of the node that will be changed.
+#
+# @child: #optional the name of the child that will be deleted.
+#
+# @node: #optional the name of the node will be added.
+#
+# Note: this command is experimental, and its API is not stable.
+#
+# Since: 2.6
+##
+{ 'command': 'x-blockdev-change',
+  'data' : { 'parent': 'str',
+             '*child': 'str',
+             '*node': 'str' } }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9d8b42f..9b49d51 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4285,6 +4285,53 @@ Example:
 EQMP
 
     {
+        .name       = "x-blockdev-change",
+        .args_type  = "parent:B,child:B?,node:B?",
+        .mhandler.cmd_new = qmp_marshal_x_blockdev_change,
+    },
+
+SQMP
+x-blockdev-change
+-----------------
+
+Dynamically reconfigure the block driver state graph. It can be used to
+add, remove, insert, or replace a block driver state. Currently only
+the Quorum driver implements this feature to add and remove its child.
+This is useful to fix a broken quorum child.
+
+Arguments:
+- "parent": the id or node name of which node will be changed (json-string)
+- "child": the child name which will be deleted (json-string, optional)
+- "node": the new node-name which will be added (json-string, optional)
+
+Note: this command is experimental, and not a stable API. It doesn't
+support all kinds of operations, all kinds of children, nor all block
+drivers.
+
+Example:
+
+Add a new node to a quorum
+-> { "execute": blockdev-add",
+    "arguments": { "options": { "driver": "raw",
+                                "node-name": "new_node",
+                                "id": "test_new_node",
+                                "file": { "driver": "file",
+                                          "filename": "test.raw" } } } }
+<- { "return": {} }
+-> { "execute": "x-blockdev-change",
+    "arguments": { "parent": "disk1",
+                   "node": "new_node" } }
+<- { "return": {} }
+
+Delete a quorum's node
+-> { "execute": "x-blockdev-change",
+    "arguments": { "parent": "disk1",
+                   "child": "children.2" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "query-named-block-nodes",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
-- 
2.5.0

Re: [Qemu-devel] [Patch v8 0/3] qapi: child add/delete support

[Wen Congyang]

Kevin: ping

On 11/27/2015 02:06 PM, Wen Congyang wrote:
> If quorum's child is broken, we can use mirror job to replace it.
> But sometimes, the user only need to remove the broken child, and
> add it later when the problem is fixed.
> 
> It is based on the Kevin's child name related patch:
> http://lists.nongnu.org/archive/html/qemu-devel/2015-11/msg04949.html
> 
> ChangLog:
> v8:
> 1. Rebase to the newest codes
> 2. Address the comments from Eric Blake
> v7:
> 1. Remove the qmp command x-blockdev-change's parameter operation according
>    to Kevin's comments.
> 2. Remove the hmp command.
> v6:
> 1. Use a single qmp command x-blockdev-change to replace x-blockdev-child-add
>    and x-blockdev-child-delete
> v5:
> 1. Address Eric Blake's comments
> v4:
> 1. drop nbd driver's implementation. We can use human-monitor-command
>    to do it.
> 2. Rename the command name.
> v3:
> 1. Don't open BDS in bdrv_add_child(). Use the existing BDS which is
>    created by the QMP command blockdev-add.
> 2. The driver NBD can support filename, path, host:port now.
> v2:
> 1. Use bdrv_get_device_or_node_name() instead of new function
>    bdrv_get_id_or_node_name()
> 2. Update the error message
> 3. Update the documents in block-core.json
> 
> Wen Congyang (3):
>   Add new block driver interface to add/delete a BDS's child
>   quorum: implement bdrv_add_child() and bdrv_del_child()
>   qmp: add monitor command to add/remove a child
> 
>  block.c                   |  58 ++++++++++++++++++++--
>  block/quorum.c            | 124 +++++++++++++++++++++++++++++++++++++++++++++-
>  blockdev.c                |  54 ++++++++++++++++++++
>  include/block/block.h     |   9 ++++
>  include/block/block_int.h |   5 ++
>  qapi/block-core.json      |  23 +++++++++
>  qmp-commands.hx           |  47 ++++++++++++++++++
>  7 files changed, 314 insertions(+), 6 deletions(-)
> 

Re: [Qemu-devel] [Patch v8 2/3] quorum: implement bdrv_add_child() and bdrv_del_child()

[Li Zhijian]

On 11/27/2015 02:06 PM, Wen Congyang wrote:
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>   block.c               |   8 ++--
>   block/quorum.c        | 124 +++++++++++++++++++++++++++++++++++++++++++++++++-
>   include/block/block.h |   4 ++
>   3 files changed, 130 insertions(+), 6 deletions(-)
>
> diff --git a/block.c b/block.c
> index 255a36e..bfc2be8 100644
> --- a/block.c
> +++ b/block.c
> @@ -1196,10 +1196,10 @@ static int bdrv_fill_options(QDict **options, const char *filename,
>       return 0;
>   }
>
> -static BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
> -                                    BlockDriverState *child_bs,
> -                                    const char *child_name,
> -                                    const BdrvChildRole *child_role)
> +BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
> +                             BlockDriverState *child_bs,
> +                             const char *child_name,
> +                             const BdrvChildRole *child_role)
>   {
>       BdrvChild *child = g_new(BdrvChild, 1);
>       *child = (BdrvChild) {
> diff --git a/block/quorum.c b/block/quorum.c
> index 2810e37..b7df14b 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -23,6 +23,7 @@
>   #include "qapi/qmp/qstring.h"
>   #include "qapi-event.h"
>   #include "crypto/hash.h"
> +#include "qemu/bitmap.h"
>
>   #define HASH_LENGTH 32
>
> @@ -80,6 +81,8 @@ typedef struct BDRVQuorumState {
>       bool rewrite_corrupted;/* true if the driver must rewrite-on-read corrupted
>                               * block if Quorum is reached.
>                               */
> +    unsigned long *index_bitmap;
> +    int bsize;
>
>       QuorumReadPattern read_pattern;
>   } BDRVQuorumState;
> @@ -875,9 +878,9 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
>           ret = -EINVAL;
>           goto exit;
>       }
> -    if (s->num_children < 2) {
> +    if (s->num_children < 1) {
>           error_setg(&local_err,
> -                   "Number of provided children must be greater than 1");
> +                   "Number of provided children must be 1 or more");
>           ret = -EINVAL;
>           goto exit;
>       }
> @@ -926,6 +929,7 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
>       /* allocate the children array */
>       s->children = g_new0(BdrvChild *, s->num_children);
>       opened = g_new0(bool, s->num_children);
> +    s->index_bitmap = bitmap_new(s->num_children);
>
>       for (i = 0; i < s->num_children; i++) {
>           char indexstr[32];
> @@ -941,6 +945,8 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
>
>           opened[i] = true;
>       }
> +    bitmap_set(s->index_bitmap, 0, s->num_children);
> +    s->bsize = s->num_children;
>
>       g_free(opened);
>       goto exit;
> @@ -997,6 +1003,117 @@ static void quorum_attach_aio_context(BlockDriverState *bs,
>       }
>   }
>
> +static int get_new_child_index(BDRVQuorumState *s)
> +{
> +    int index;
> +
> +    index = find_next_zero_bit(s->index_bitmap, s->bsize, 0);
> +    if (index < s->bsize) {
> +        return index;
> +    }
> +
> +    if ((s->bsize % BITS_PER_LONG) == 0) {
> +        s->index_bitmap = bitmap_zero_extend(s->index_bitmap, s->bsize,
> +                                             s->bsize + 1);
> +    }
> +
> +    return s->bsize++;
> +}
> +
> +static void remove_child_index(BDRVQuorumState *s, int index)
> +{
> +    int last_index;
> +    long new_len;
> +
> +    assert(index < s->bsize);
> +
> +    clear_bit(index, s->index_bitmap);
> +    if (index < s->bsize - 1) {
> +        /*
> +         * The last bit is always set, and we don't clear
> +         * the last bit.
> +         */
> +        return;
> +    }
> +
> +    last_index = find_last_bit(s->index_bitmap, s->bsize);
> +    if (BITS_TO_LONGS(last_index + 1) == BITS_TO_LONGS(s->bsize)) {
> +        s->bsize = last_index + 1;
> +        return;
> +    }
> +
> +    new_len = BITS_TO_LONGS(last_index + 1) * sizeof(unsigned long);
> +    s->index_bitmap = g_realloc(s->index_bitmap, new_len);
> +    s->bsize = last_index + 1;
> +}
> +
> +static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs,
> +                             Error **errp)
> +{
> +    BDRVQuorumState *s = bs->opaque;
> +    BdrvChild *child;
> +    char indexstr[32];
> +    int index = find_next_zero_bit(s->index_bitmap, s->bsize, 0);
> +    int ret;
> +
> +    index = get_new_child_index(s);

double assignment of 'index', remove the first one pls

Thanks
Li

> +    ret = snprintf(indexstr, 32, "children.%d", index);
> +    if (ret < 0 || ret >= 32) {
> +        error_setg(errp, "cannot generate child name");
> +        return;
> +    }
> +
> +    bdrv_drain(bs);
> +
> +    assert(s->num_children <= INT_MAX / sizeof(BdrvChild *));
> +    if (s->num_children == INT_MAX / sizeof(BdrvChild *)) {
> +        error_setg(errp, "Too many children");
> +        return;
> +    }
> +    s->children = g_renew(BdrvChild *, s->children, s->num_children + 1);
> +
> +    bdrv_ref(child_bs);
> +    child = bdrv_attach_child(bs, child_bs, indexstr, &child_format);
> +    s->children[s->num_children++] = child;
> +    set_bit(index, s->index_bitmap);
> +}
> +
> +static void quorum_del_child(BlockDriverState *bs, BlockDriverState *child_bs,
> +                             Error **errp)
> +{
> +    BDRVQuorumState *s = bs->opaque;
> +    BdrvChild *child;
> +    int i, index;
> +
> +    for (i = 0; i < s->num_children; i++) {
> +        if (s->children[i]->bs == child_bs) {
> +            break;
> +        }
> +    }
> +
> +    /* we have checked it in bdrv_del_child() */
> +    assert(i < s->num_children);
> +    child = s->children[i];
> +
> +    if (s->num_children <= s->threshold) {
> +        error_setg(errp,
> +            "The number of children cannot be lower than the vote threshold %d",
> +            s->threshold);
> +        return;
> +    }
> +
> +    /* child->name is "children.%d" */
> +    index = atoi(child->name + 9);
> +
> +    bdrv_drain(bs);
> +    /* We can safely remove this child now */
> +    memmove(&s->children[i], &s->children[i + 1],
> +            (s->num_children - i - 1) * sizeof(void *));
> +    s->children = g_renew(BdrvChild *, s->children, --s->num_children);
> +    remove_child_index(s, index);
> +    bdrv_unref_child(bs, child);
> +}
> +
>   static void quorum_refresh_filename(BlockDriverState *bs, QDict *options)
>   {
>       BDRVQuorumState *s = bs->opaque;
> @@ -1052,6 +1169,9 @@ static BlockDriver bdrv_quorum = {
>       .bdrv_detach_aio_context            = quorum_detach_aio_context,
>       .bdrv_attach_aio_context            = quorum_attach_aio_context,
>
> +    .bdrv_add_child                     = quorum_add_child,
> +    .bdrv_del_child                     = quorum_del_child,
> +
>       .is_filter                          = true,
>       .bdrv_recurse_is_first_non_filter   = quorum_recurse_is_first_non_filter,
>   };
> diff --git a/include/block/block.h b/include/block/block.h
> index 06d3369..1d3b9c6 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -514,6 +514,10 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs);
>   void bdrv_ref(BlockDriverState *bs);
>   void bdrv_unref(BlockDriverState *bs);
>   void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child);
> +BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs,
> +                             BlockDriverState *child_bs,
> +                             const char *child_name,
> +                             const BdrvChildRole *child_role);
>
>   bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp);
>   void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason);
>

Re: [Qemu-devel] [Patch v8 3/3] qmp: add monitor command to add/remove a child

[Li Zhijian]

On 11/27/2015 02:06 PM, Wen Congyang wrote:
> The new QMP command name is x-blockdev-change. It's just for adding/removing
> quorum's child now, and doesn't support all kinds of children, all kinds of
> operations, nor all block drivers. So it is experimental now.
>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>   blockdev.c           | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   qapi/block-core.json | 23 ++++++++++++++++++++++
>   qmp-commands.hx      | 47 +++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 124 insertions(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index 2b076fb..7d8a2b4 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3836,6 +3836,60 @@ out:
>       aio_context_release(aio_context);
>   }
>
> +static BlockDriverState *bdrv_find_child(BlockDriverState *parent_bs,
> +                                         const char *child_name)
> +{
> +    BdrvChild *child;
> +
> +    QLIST_FOREACH(child, &parent_bs->children, next) {
> +        if (strcmp(child->name, child_name) == 0) {
> +            return child->bs;
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +void qmp_x_blockdev_change(const char *parent, bool has_child,
> +                           const char *child, bool has_node,
> +                           const char *node, Error **errp)
> +{
> +    BlockDriverState *parent_bs, *child_bs = NULL, *new_bs = NULL;
> +
> +    parent_bs = bdrv_lookup_bs(parent, parent, errp);
> +    if (!parent_bs) {
> +        return;
> +    }
> +
> +    if (has_child == has_node) {
> +        if (has_child) {
> +            error_setg(errp, "The paramter child and node is conflict");
> +        } else {
> +            error_setg(errp, "Either child or node should be specified");
> +        }
> +        return;
> +    }
> +
> +    if (has_child) {
> +        child_bs = bdrv_find_child(parent_bs, child);
> +        if (!child_bs) {
> +            error_setg(errp, "Node '%s' doesn't have child %s",
> +                       parent, child);
> +            return;
> +        }
> +        bdrv_del_child(parent_bs, child_bs, errp);
> +    }
> +
> +    if (has_node) {
> +        new_bs = bdrv_find_node(node);
> +        if (!new_bs) {
> +            error_setg(errp, "Node '%s' not found", node);
> +            return;
> +        }
> +        bdrv_add_child(parent_bs, new_bs, errp);
> +    }
> +}
> +
>   BlockJobInfoList *qmp_query_block_jobs(Error **errp)
>   {
>       BlockJobInfoList *head = NULL, **p_next = &head;
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index a07b13f..feb8da2 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2400,3 +2400,26 @@
>   ##
>   { 'command': 'block-set-write-threshold',
>     'data': { 'node-name': 'str', 'write-threshold': 'uint64' } }
> +
> +##
> +# @x-blockdev-change
> +#
> +# Dynamically reconfigure the block driver state graph. It can be used
> +# to add, remove, insert or replace a block driver state. Currently only
> +# the Quorum driver implements this feature to add or remove its child.
> +# This is useful to fix a broken quorum child.
> +#
> +# @parent: the id or name of the node that will be changed.
> +#
> +# @child: #optional the name of the child that will be deleted.
> +#
> +# @node: #optional the name of the node will be added.
> +#
> +# Note: this command is experimental, and its API is not stable.
> +#
> +# Since: 2.6
> +##
> +{ 'command': 'x-blockdev-change',
> +  'data' : { 'parent': 'str',
> +             '*child': 'str',
> +             '*node': 'str' } }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 9d8b42f..9b49d51 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4285,6 +4285,53 @@ Example:
>   EQMP
>
>       {
> +        .name       = "x-blockdev-change",
> +        .args_type  = "parent:B,child:B?,node:B?",
> +        .mhandler.cmd_new = qmp_marshal_x_blockdev_change,
> +    },
> +
> +SQMP
> +x-blockdev-change
> +-----------------
> +
> +Dynamically reconfigure the block driver state graph. It can be used to
> +add, remove, insert, or replace a block driver state. Currently only
> +the Quorum driver implements this feature to add and remove its child.
> +This is useful to fix a broken quorum child.
> +
> +Arguments:
> +- "parent": the id or node name of which node will be changed (json-string)
> +- "child": the child name which will be deleted (json-string, optional)
> +- "node": the new node-name which will be added (json-string, optional)
> +
> +Note: this command is experimental, and not a stable API. It doesn't
> +support all kinds of operations, all kinds of children, nor all block
> +drivers.
> +
> +Example:
> +
> +Add a new node to a quorum
> +-> { "execute": blockdev-add",

miss a '"' before 'blockdev-add'

Thanks
Li


> +    "arguments": { "options": { "driver": "raw",
> +                                "node-name": "new_node",
> +                                "id": "test_new_node",
> +                                "file": { "driver": "file",
> +                                          "filename": "test.raw" } } } }
> +<- { "return": {} }
> +-> { "execute": "x-blockdev-change",
> +    "arguments": { "parent": "disk1",
> +                   "node": "new_node" } }
> +<- { "return": {} }
> +
> +Delete a quorum's node
> +-> { "execute": "x-blockdev-change",
> +    "arguments": { "parent": "disk1",
> +                   "child": "children.2" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>           .name       = "query-named-block-nodes",
>           .args_type  = "",
>           .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
>

-- 
Best regards.
Li Zhijian (8555)