Re: [Qemu-devel] [Qemu-block] [PATCH RFC] block: add block-insert-node QMP command

[Manos Pitsidianakis <el13635@mail.ntua.gr>]

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

On Wed, Oct 04, 2017 at 02:49:27PM +0200, Max Reitz wrote:
>On 2017-08-15 09:45, Manos Pitsidianakis wrote:
>> block-insert-node and its pair command block-remove-node provide runtime
>> insertion and removal of filter nodes.
>>
>> block-insert-node takes a (parent, child) and (node, child) pair of
>> edges and unrefs the (parent, child) BdrvChild relationship and 
>> creates
>> a new (parent, node) child with the same BdrvChildRole.
>>
>> This is a different approach than x-blockdev-change which uses the driver
>> methods bdrv_add_child() and bdrv_del_child(),
>
>Why? :-)
>
>Can't we reuse x-blockdev-change? As far as I'm concerned, this was one
>of its roles, and at least I don't want to have both x-blockdev-change
>and these new commands.
>
>I think it would be a good idea just to change bdrv_add_child() and
>bdrv_del_child(): If the driver has a bdrv_{add,del}_child() callback,
>invoke that.  If it doesn't, then just attach the child.
>
>Of course, it may turn out that x-blockdev-change is lacking something
>(e.g. a way to specify as what kind of child a new node is to be
>attached).  Then we should either extend it so that it covers what it's
>lacking, or replace it completely by these new commands.  In the latter
>case, however, they would need to cover the existing use case which is
>reconfiguring the quorum driver.  (And that would mean it would have to
>invoke bdrv_add_child()/bdrv_del_child() when the driver has them.)
>
>Max
>

I think the two use cases are this:

a) Adding/removing a replication child to an existing quorum node
b) Insert a filter between two existing nodes.

These are not directly compatible semantically, but as you said 
x-blockdev-change can perform b) if bdrv_add_child()/bdrv_del_child() 
are not implemented. Wouldn't that be unnecessary overloading?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]