Re: [Qemu-block] [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 <address@hidden>
>> Signed-off-by: zhanghailiang <address@hidden>
>> Signed-off-by: Gonglei <address@hidden>
>> ---
> 
>> +++ 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.