qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 4/5] qmp: add monitor command to add/remove a


From: Kevin Wolf
Subject: Re: [Qemu-devel] [PATCH v3 4/5] qmp: add monitor command to add/remove a child
Date: Tue, 15 Sep 2015 11:20:42 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

Am 15.09.2015 um 10:56 hat Alberto Garcia geschrieben:
> On Mon 14 Sep 2015 05:37:00 PM CEST, Kevin Wolf <address@hidden> wrote:
> 
> >> +{ 'command': 'x-child-add',
> >> +  'data' : { 'parent': 'str', 'child': 'str' } }
> >
> > This is probably not future-proof and only made for the special case
> > of quorum. Specifically, one thing I'm missing is some way to specfiy
> > what kind of child the new node is when a node can take different
> > types of children (e.g. bs->file and bs->backing_hd).
> 
> Children here are specified by child roles, aren't they?

Possibly, but actually, currently I'm inclined to think that they
aren't. Child roles are still relatively new, though, so it's hard to
say if this is just because we didn't use them in that way so far, or
because they are really the wrong tool.

What I can say is that traditionally children are identified by option
names. Block drivers assign a ChildRole when processing the option, and
both are equivalent only if the same ChildRole is never used for two
different children. I believe that that's currently true, but I'm
doubtful whether it will remain this way (even looking at blkverify,
which has one &child_file and one &child_format, things start to look a
bit confusing).

Possibly the right and consistent way to change the set of children
would be through a QMP command exposing bdrv_reopen(), which would also
be used for changing other options at runtime.

My current pull request adds the qemu-io (and therefore HMP) way of
doing this, but finding a good QMP interface will still be a challenge.

> We could have a ChildRole enum with 'file', 'format' and 'backing' that
> would be passed to this command and use the same enumeration in
> bdrv_open_image() rather than a pointer to BdrvChildRole.

Yes, that would be an option if ChildRole turned out to be enough.

Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]