[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Patch for-2.5 v2 5/6] qmp: add monitor command to add/
From: |
Wen Congyang |
Subject: |
Re: [Qemu-block] [Patch for-2.5 v2 5/6] qmp: add monitor command to add/remove a child |
Date: |
Tue, 1 Sep 2015 08:55:03 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 09/01/2015 03:04 AM, Eric Blake wrote:
> On 08/11/2015 01:51 AM, Wen Congyang wrote:
>> Signed-off-by: Wen Congyang <address@hidden>
>> Signed-off-by: zhanghailiang <address@hidden>
>> Signed-off-by: Gonglei <address@hidden>
>> ---
>> blockdev.c | 79
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> qapi/block-core.json | 40 ++++++++++++++++++++++++++
>> qmp-commands.hx | 67 ++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 186 insertions(+)
>>
>
>> +void qmp_child_add(const char *device, BlockdevOptionsChild *options,
>> + Error **errp)
>> +{
>> + QmpOutputVisitor *ov = qmp_output_visitor_new();
>> + QObject *obj;
>> + QDict *qdict;
>> + Error *local_err = NULL;
>> +
>> + if (options->child->has_id || options->child->has_discard ||
>> + options->child->has_cache || options->child->has_aio ||
>> + options->child->has_rerror || options->child->has_werror ||
>> + options->child->has_read_only || options->child->has_detect_zeroes)
>> {
>> + error_setg(errp, "id, discard, cache, aio, rerror, werror, readonly"
>> + " and detect_zeroes cann't be used for child-add");
>
> s/cann't/can't/
>
> If they can't be used, then why not write the qapi so that they can't
> even be provided? On the other hand, why can't they be used? Can't you
> specify some of these options separately for different quorum children
> when first creating a quorum, in which case you'd want to be able to do
> likewise when adding a new member to the quorum?
IIUC, it is just top BDS's option, and it is not be used by the hot-added
child. The hot-added child will use its parent flags.
>
>> +++ b/qapi/block-core.json
>> @@ -2122,3 +2122,43 @@
>> ##
>> { 'command': 'block-set-write-threshold',
>> 'data': { 'node-name': 'str', 'write-threshold': 'uint64' } }
>> +
>> +##
>> +# @BlockdevOptionsChild
>> +#
>> +# Driver specific block device options for child block device.
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'BlockdevOptionsChild',
>> + 'data': { 'child': 'BlockdevOptions' } }
>
> Do you need this struct? It causes extra nesting on the wire...
>
>> +
>> +##
>> +# @child-add
>> +#
>> +# Add a new child to the parent BDS. Currently only the Quorum driver
>> +# implements this feature. This is useful to fix a broken quorum child.
>> +#
>> +# @device: graph node name or id which the child will be added to.
>> +#
>> +# @options: the options to create child BDS.
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'command': 'child-add',
>> + 'data' : { 'device': 'str', 'options': 'BlockdevOptionsChild' } }
>
> ...Consider if you just did:
>
> { 'command': 'child-add',
> 'data': { 'device', 'str', 'child': 'BlockdevOptions' } }
OK, I will try it.
>
>>
>> +
>> +##
>> +# @child-del
>> +#
>> +# Remove a child from the parent BDS. Currently only the Quorum driver
>> +# implements this feature. This is useful to fix a broken quorum child.
>
> Might also be worth mentioning that you can't remove a child if it would
> bring the quorum below its threshold.
>
>> +++ b/qmp-commands.hx
>
>> +Add a child to a quorum node.
>> +
>> +This command is still a work in progress. It doesn't support all
>> +block drivers. Stay away from it unless you want it to help with
>> +its development.
>
> Maybe we should name it 'x-child-add' for now, so that we aren't baking
> ourselves into a corner.
Do you mean the command name should be x-child-add? It is OK.
>
>> +
>> +Arguments:
>> +
>> +- "device": the quorum's id or node name
>> +- "options": the new child options
>> +
>> +Example:
>> +
>> +-> { "execute": "child-add",
>> + "arguments": {
>> + "device": "disk1",
>> + "options" : {
>> + "child": {
>
> ...the simper command idea above would reduce one layer of {} nesting here.
>
Yes, I will try it.
Thanks
Wen Congyang
- Re: [Qemu-block] [Patch for-2.5 v2 2/6] support nbd driver in blockdev-add, (continued)
- [Qemu-block] [Patch for-2.5 v2 3/6] Add new block driver interface to add/delete a BDS's child, Wen Congyang, 2015/08/11
- [Qemu-block] [Patch for-2.5 v2 6/6] hmp: add monitor command to add/remove a child, Wen Congyang, 2015/08/11
- [Qemu-block] [Patch for-2.5 v2 5/6] qmp: add monitor command to add/remove a child, Wen Congyang, 2015/08/11
- [Qemu-block] [Patch for-2.5 v2 1/6] QAPI: move InetSocketAddress to qapi/common.json, Wen Congyang, 2015/08/11
- [Qemu-block] [Patch for-2.5 v2 4/6] quorum: implement bdrv_add_child() and bdrv_del_child(), Wen Congyang, 2015/08/11