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

[Qemu-devel] [Patch v7 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://repo.or.cz/qemu/kevin.git/commitdiff/b8f3aba84160564576a5a068398f20eca13768af

ChangLog:
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      |  25 ++++++++++
 qmp-commands.hx           |  47 ++++++++++++++++++
 7 files changed, 316 insertions(+), 6 deletions(-)

-- 
2.5.0

[Qemu-devel] [Patch v7 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 adabdfb..ba5806e 100644
--- a/block.c
+++ b/block.c
@@ -4075,3 +4075,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 73edb1a..f5a73e2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -637,4 +637,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 b30a8d1..ecd9ab3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -301,6 +301,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 v7 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 ba5806e..fc067ee 100644
--- a/block.c
+++ b/block.c
@@ -1085,10 +1085,10 @@ static int bdrv_fill_options(QDict **options, const char **pfilename,
     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 b9ba028..1938546 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)
 {
     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 f5a73e2..9aac696 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -512,6 +512,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 v7 3/3] qmp: add monitor command to add/remove a child

[Wen Congyang]

The new QMP command name is x-blockdev-change. It justs for adding/removing
quorum's child now, and don'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 | 25 ++++++++++++++++++++++++
 qmp-commands.hx      | 47 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index 313841b..7736d84 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3837,6 +3837,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..22fc2ee 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2400,3 +2400,28 @@
 ##
 { '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.
+#
+# @operation: the change operation. It can be add, delete.
+#
+# @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..c65d693 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
+------------
+
+Dynamic reconfigure the block driver state graph. It can be used to
+add, remove, insert, 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
+- "child": the child name which will be delete
+- "node": the new node-name which will be added
+
+Note: this command is experimental, and not a stable API. It doesn't
+support all kinds of operations, all kindes of children, nor all block
+drivers.
+
+Example:
+
+Add a new quorum's node
+-> { "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 v7 3/3] qmp: add monitor command to add/remove a child

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 4006 bytes --]

On 11/22/2015 11:23 PM, Wen Congyang wrote:
> The new QMP command name is x-blockdev-change. It justs for adding/removing

s/It justs/It's just/

> quorum's child now, and don't support all kinds of children, all kinds of

s/don't/doesn't/

> 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>
> ---

> +++ b/qapi/block-core.json
> @@ -2400,3 +2400,28 @@
>  ##
>  { '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.
> +#
> +# @operation: the change operation. It can be add, delete.

Documented but not present below.  Are you missing a parameter, or
should this line be deleted?

> +#
> +# @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' } }

> +++ 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
> +------------

Make the --- divider as long as the text it is underlining.

> +
> +Dynamic reconfigure the block driver state graph. It can be used to

s/Dynamic/Dynamically/

> +add, remove, insert, replace a block driver state. Currently only

s/replace/or replace/

Isn't 'add' and 'insert' the same thing?

> +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
> +- "child": the child name which will be delete

s/delete/deleted/; mention that it is optional (if nothing is going to
be delted)

> +- "node": the new node-name which will be added

mention that it is optional (if nothing is going to be added)

> +
> +Note: this command is experimental, and not a stable API. It doesn't
> +support all kinds of operations, all kindes of children, nor all block

s/kindes/kinds/

> +drivers.
> +
> +Example:
> +
> +Add a new quorum's node

s/quorum's node/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,
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Wen Congyang]

On 11/24/2015 12:30 AM, Eric Blake wrote:
> On 11/22/2015 11:23 PM, Wen Congyang wrote:
>> The new QMP command name is x-blockdev-change. It justs for adding/removing
> 
> s/It justs/It's just/
> 
>> quorum's child now, and don't support all kinds of children, all kinds of
> 
> s/don't/doesn't/
> 
>> 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>
>> ---
> 
>> +++ b/qapi/block-core.json
>> @@ -2400,3 +2400,28 @@
>>  ##
>>  { '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.
>> +#
>> +# @operation: the change operation. It can be add, delete.
> 
> Documented but not present below.  Are you missing a parameter, or
> should this line be deleted?

This line should be deleted.

> 
>> +#
>> +# @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' } }
> 
>> +++ 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
>> +------------
> 
> Make the --- divider as long as the text it is underlining.

OK

> 
>> +
>> +Dynamic reconfigure the block driver state graph. It can be used to
> 
> s/Dynamic/Dynamically/
> 
>> +add, remove, insert, replace a block driver state. Currently only
> 
> s/replace/or replace/
> 
> Isn't 'add' and 'insert' the same thing?

'insert' means that a filter driver is inserted betweean A and B(A is B's parent).

> 
>> +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
>> +- "child": the child name which will be delete
> 
> s/delete/deleted/; mention that it is optional (if nothing is going to
> be delted)
> 
>> +- "node": the new node-name which will be added
> 
> mention that it is optional (if nothing is going to be added)

OK

> 
>> +
>> +Note: this command is experimental, and not a stable API. It doesn't
>> +support all kinds of operations, all kindes of children, nor all block
> 
> s/kindes/kinds/
> 
>> +drivers.
>> +
>> +Example:
>> +
>> +Add a new quorum's node
> 
> s/quorum's node/node to a quorum/

All comments will be addressed.

Thanks
Wen Congyang

> 
>> +-> { "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,
>>
> 


-- 
This message has been scanned for viruses and
dangerous content by FCNIC, and is
believed to be clean.