Re: [Qemu-block] [Qemu-devel] [PATCH v6 3/4] qmp: add monitor command to add/remove a child

[Kevin Wolf]

Am 13.11.2015 um 11:25 hat Wen Congyang geschrieben:
> On 11/10/2015 09:40 AM, Wen Congyang wrote:
> > On 11/10/2015 12:04 AM, Kevin Wolf wrote:
> >> Am 16.10.2015 um 10:57 hat Wen Congyang geschrieben:
> >>> +##
> >>> +# @ChangeOperation:
> >>> +#
> >>> +# An enumeration of block device change operation.
> >>> +#
> >>> +# @add: Add a new block driver state to a existed block driver state.
> >>> +#
> >>> +# @delete: Delete a block driver state's child.
> >>> +#
> >>> +# Since: 2.5
> >>> +##
> >>> +{ 'enum': 'ChangeOperation',
> >>> +  'data': [ 'add', 'delete' ] }
> >>
> >> What's the advantage of this enum compared to separate QMP commands? The
> >> way it is specified here, ChangeOperation is already implicit by whether
> >> or not child and node are given.
> >>
> >>> +##
> >>> +# @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.
> >>> +#
> >>> +# @operation: the chanage operation. It can be add, delete.
> >>> +#
> >>> +# @parent: the id or node name of which node will be changed.
> >>> +#
> >>> +# @child: the child node-name which will be deleted.
> >>
> >> #optional
> >>
> >> Must be present for operation = delete, must not be present otherwise.
> >>
> >>> +# @node: the new node-name which will be added.
> >>
> >> #optional
> >>
> >> Must be present for operation = add, must not be present otherwise.
> >>
> >>> +#
> >>> +# Note: this command is experimental, and not a stable API.
> >>> +#
> >>> +# Since: 2.5
> >>> +##
> >>> +{ 'command': 'x-blockdev-change',
> >>> +  'data' : { 'operation': 'ChangeOperation',
> >>> +             'parent': 'str',
> >>> +             '*child': 'str',
> >>> +             '*node': 'str' } }
> >>
> >> Let me suggest this alternative:
> >>
> >> { 'command': 'x-blockdev-change',
> >>   'data' : { 'parent': 'str',
> >>              'child': 'str',
> >>              '*node': 'str' } }
> >>
> >> child doesn't describe a node name then, but a child name (adds a
> >> dependency on my patches which add a name to BdrvChild, though).
> > 
> > Where is the patch? I don't find it.

The current developement branch version is here:

http://repo.or.cz/qemu/kevin.git/commitdiff/b8f3aba84160564576a5a068398f20eca13768af

I hope to get the series merged early in the 2.6 cycle.

> >> Depending on whether node is given and whether the child already exists,
> >> this may add, remove or replace a child.
> > 
> > If the user wants to insert a filter driver between parent and child, we
> > also needs three parameters: parent, child, node. So it is why I add the
> > parameter operation.

The child node is uniquely identified with parent node and child name,
so my version can't describe less than something including the child
node name.

The reverse isn't true, though: In theory, the same node could be
attached twice to the same parent in different roles. Knowing the node
name doesn't uniquely identify the child name then.

Kevin