diff mbox

[v10,3/3] qmp: add monitor command to add/remove a child

Message ID 1455615450-15138-4-git-send-email-xiecl.fnst@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Changlong Xie Feb. 16, 2016, 9:37 a.m. UTC
From: Wen Congyang <wency@cn.fujitsu.com>

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>
Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
---
 blockdev.c           | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/block-core.json | 32 +++++++++++++++++++++++++++++++
 qmp-commands.hx      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 136 insertions(+)

Comments

Max Reitz March 5, 2016, 6:33 p.m. UTC | #1
On 16.02.2016 10:37, Changlong Xie wrote:
> From: Wen Congyang <wency@cn.fujitsu.com>
> 
> 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>
> Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
> ---
>  blockdev.c           | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/block-core.json | 32 +++++++++++++++++++++++++++++++
>  qmp-commands.hx      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 136 insertions(+)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 1f73478..ca040b0 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3983,6 +3983,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;
> +}

As I said for patch 1, making this function return a BdrvChild would be
trivial...

> +
> +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 parameters child and node are in conflict");
> +        } else {
> +            error_setg(errp, "Either child or node must be specified");
> +        }
> +        return;
> +    }
> +
> +    if (has_child) {
> +        child_bs = bdrv_find_child(parent_bs, child);
> +        if (!child_bs) {
> +            error_setg(errp, "Node '%s' does not have child '%s'",
> +                       parent, child);
> +            return;
> +        }
> +        bdrv_del_child(parent_bs, child_bs, errp);

...and then we could pass the BdrvChild here.

(It's your choice.)

> +    }
> +
> +    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 33012b8..92eb7fe 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2482,3 +2482,35 @@
>  ##
>  { '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 graph node. Currently only the
> +# Quorum driver implements this feature to add or remove its child. This
> +# is useful to fix a broken quorum child.
> +#
> +# If @node is specified, it will be inserted under @parent. @child
> +# may not be specified in this case. If both @parent and @child are
> +# specified but @node is not, @child will be detached from @parent.
> +#
> +# @parent: the id or name of the parent node.
> +#
> +# @child: #optional the name of a child under the given parent node.
> +#
> +# @node: #optional the name of the node that will be added.
> +#
> +# Note: this command is experimental, and its API is not stable. It
> +# does not support all kinds of operations, all kinds of children, nor
> +# all block drivers.
> +#
> +# Warning: The data in a new quorum child MUST be consistent with that of
> +# the rest of the array.
> +#
> +# 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 020e5ee..1c9a06f 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4364,6 +4364,56 @@ 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 graph node. Currently only the
> +Quorum driver implements this feature to add or remove its child. This
> +is useful to fix a broken quorum child.

The explanation of its behavior is missing here (the second paragraph in
the qapi/block-core.json comment).

Max

> +
> +Arguments:
> +- "parent": the id or name of the parent node (json-string)
> +- "child": the name of a child under the given parent node (json-string, optional)
> +- "node": the name of the node that 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.
> +
> +Warning: The data in a new quorum child MUST be consistent with that of
> +the rest of the array.
> +
> +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.1" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>          .name       = "query-named-block-nodes",
>          .args_type  = "",
>          .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
>
Changlong Xie March 7, 2016, 9:15 a.m. UTC | #2
On 03/06/2016 02:33 AM, Max Reitz wrote:
> On 16.02.2016 10:37, Changlong Xie wrote:
>> From: Wen Congyang <wency@cn.fujitsu.com>
>>
>> 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>
>> Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com>
>> ---
>>   blockdev.c           | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qapi/block-core.json | 32 +++++++++++++++++++++++++++++++
>>   qmp-commands.hx      | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 136 insertions(+)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 1f73478..ca040b0 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -3983,6 +3983,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;
>> +}
>
> As I said for patch 1, making this function return a BdrvChild would be
> trivial...
>

Hi Max

okay.

>> +
>> +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 parameters child and node are in conflict");
>> +        } else {
>> +            error_setg(errp, "Either child or node must be specified");
>> +        }
>> +        return;
>> +    }
>> +
>> +    if (has_child) {
>> +        child_bs = bdrv_find_child(parent_bs, child);
>> +        if (!child_bs) {
>> +            error_setg(errp, "Node '%s' does not have child '%s'",
>> +                       parent, child);
>> +            return;
>> +        }
>> +        bdrv_del_child(parent_bs, child_bs, errp);
>
> ...and then we could pass the BdrvChild here.

ditto

>
> (It's your choice.)
>
>> +    }
>> +
>> +    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 33012b8..92eb7fe 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2482,3 +2482,35 @@
>>   ##
>>   { '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 graph node. Currently only the
>> +# Quorum driver implements this feature to add or remove its child. This
>> +# is useful to fix a broken quorum child.
>> +#
>> +# If @node is specified, it will be inserted under @parent. @child
>> +# may not be specified in this case. If both @parent and @child are
>> +# specified but @node is not, @child will be detached from @parent.
>> +#
>> +# @parent: the id or name of the parent node.
>> +#
>> +# @child: #optional the name of a child under the given parent node.
>> +#
>> +# @node: #optional the name of the node that will be added.
>> +#
>> +# Note: this command is experimental, and its API is not stable. It
>> +# does not support all kinds of operations, all kinds of children, nor
>> +# all block drivers.
>> +#
>> +# Warning: The data in a new quorum child MUST be consistent with that of
>> +# the rest of the array.
>> +#
>> +# 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 020e5ee..1c9a06f 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -4364,6 +4364,56 @@ 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 graph node. Currently only the
>> +Quorum driver implements this feature to add or remove its child. This
>> +is useful to fix a broken quorum child.
>
> The explanation of its behavior is missing here (the second paragraph in
> the qapi/block-core.json comment).
>

Ditto.

Thanks
	-Xie

> Max
>
>> +
>> +Arguments:
>> +- "parent": the id or name of the parent node (json-string)
>> +- "child": the name of a child under the given parent node (json-string, optional)
>> +- "node": the name of the node that 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.
>> +
>> +Warning: The data in a new quorum child MUST be consistent with that of
>> +the rest of the array.
>> +
>> +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.1" } }
>> +<- { "return": {} }
>> +
>> +EQMP
>> +
>> +    {
>>           .name       = "query-named-block-nodes",
>>           .args_type  = "",
>>           .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,
>>
>
>
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 1f73478..ca040b0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3983,6 +3983,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 parameters child and node are in conflict");
+        } else {
+            error_setg(errp, "Either child or node must be specified");
+        }
+        return;
+    }
+
+    if (has_child) {
+        child_bs = bdrv_find_child(parent_bs, child);
+        if (!child_bs) {
+            error_setg(errp, "Node '%s' does not 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 33012b8..92eb7fe 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2482,3 +2482,35 @@ 
 ##
 { '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 graph node. Currently only the
+# Quorum driver implements this feature to add or remove its child. This
+# is useful to fix a broken quorum child.
+#
+# If @node is specified, it will be inserted under @parent. @child
+# may not be specified in this case. If both @parent and @child are
+# specified but @node is not, @child will be detached from @parent.
+#
+# @parent: the id or name of the parent node.
+#
+# @child: #optional the name of a child under the given parent node.
+#
+# @node: #optional the name of the node that will be added.
+#
+# Note: this command is experimental, and its API is not stable. It
+# does not support all kinds of operations, all kinds of children, nor
+# all block drivers.
+#
+# Warning: The data in a new quorum child MUST be consistent with that of
+# the rest of the array.
+#
+# 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 020e5ee..1c9a06f 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4364,6 +4364,56 @@  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 graph node. Currently only the
+Quorum driver implements this feature to add or remove its child. This
+is useful to fix a broken quorum child.
+
+Arguments:
+- "parent": the id or name of the parent node (json-string)
+- "child": the name of a child under the given parent node (json-string, optional)
+- "node": the name of the node that 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.
+
+Warning: The data in a new quorum child MUST be consistent with that of
+the rest of the array.
+
+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.1" } }
+<- { "return": {} }
+
+EQMP
+
+    {
         .name       = "query-named-block-nodes",
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_query_named_block_nodes,