All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Alberto Garcia <berto@igalia.com>
Cc: qemu block <qemu-block@nongnu.org>,
	Markus Armbruster <armbru@redhat.com>,
	Jiang Yunhong <yunhong.jiang@intel.com>,
	Dong Eddie <eddie.dong@intel.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	qemu devel <qemu-devel@nongnu.org>,
	Gonglei <arei.gonglei@huawei.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Yang Hongyang <yanghy@cn.fujitsu.com>,
	zhanghailiang <zhang.zhanghailiang@huawei.com>
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	[thread overview]
Message-ID: <20150915092042.GB3986@noname.str.redhat.com> (raw)
In-Reply-To: <w51y4g8awmr.fsf@maestria.local.igalia.com>

Am 15.09.2015 um 10:56 hat Alberto Garcia geschrieben:
> On Mon 14 Sep 2015 05:37:00 PM CEST, Kevin Wolf <kwolf@redhat.com> 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	other threads:[~2015-09-15  9:20 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-10  9:55 [Qemu-devel] [PATCH v3 0/5] qapi: child add/delete support Wen Congyang
2015-09-10  9:55 ` [Qemu-devel] [PATCH v3 1/5] support nbd driver in blockdev-add Wen Congyang
2015-09-14 14:27   ` Markus Armbruster
2015-09-14 15:47     ` Eric Blake
2015-09-15  1:39       ` Wen Congyang
2015-09-15  7:37         ` Markus Armbruster
2015-09-15  8:01           ` Wen Congyang
2015-09-15 11:12             ` Markus Armbruster
2015-09-16  5:59               ` Wen Congyang
2015-09-16  8:21                 ` Markus Armbruster
2015-09-16  8:24                   ` Wen Congyang
2015-09-16 11:18                     ` Markus Armbruster
2015-09-16 14:53                       ` [Qemu-devel] [Qemu-block] " Eric Blake
2015-09-17  1:06                         ` Wen Congyang
2015-09-17  1:04                       ` [Qemu-devel] " Wen Congyang
2015-09-17  5:01                         ` Markus Armbruster
2015-09-15 13:03           ` Eric Blake
2015-09-15 13:26             ` Kevin Wolf
2015-09-15  2:20       ` Wen Congyang
2015-09-15  2:27         ` Wen Congyang
2015-09-15  3:46           ` Eric Blake
2015-09-15  3:58             ` Wen Congyang
2015-09-15 13:11               ` Eric Blake
2015-09-16  7:11                 ` Wen Congyang
2015-09-16 14:55                   ` Eric Blake
2015-09-10  9:55 ` [Qemu-devel] [PATCH v3 2/5] Add new block driver interface to add/delete a BDS's child Wen Congyang
2015-09-10  9:55 ` [Qemu-devel] [PATCH v3 3/5] quorum: implement bdrv_add_child() and bdrv_del_child() Wen Congyang
2015-09-10  9:55 ` [Qemu-devel] [PATCH v3 4/5] qmp: add monitor command to add/remove a child Wen Congyang
2015-09-10 10:04   ` Daniel P. Berrange
2015-09-10 10:34     ` Wen Congyang
2015-09-14 14:36   ` Markus Armbruster
2015-09-14 15:34     ` Kevin Wolf
2015-09-14 16:09       ` Markus Armbruster
2015-09-15  2:40     ` Wen Congyang
2015-09-15  7:49       ` Markus Armbruster
2015-09-15  7:57         ` Wen Congyang
2015-09-16  6:31         ` Wen Congyang
2015-09-16  8:29           ` Markus Armbruster
2015-09-14 15:37   ` Kevin Wolf
2015-09-15  2:33     ` Wen Congyang
2015-09-15  8:56     ` Alberto Garcia
2015-09-15  9:20       ` Kevin Wolf [this message]
2015-09-15  9:26         ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2015-09-10  9:55 ` [Qemu-devel] [PATCH v3 5/5] hmp: " Wen Congyang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150915092042.GB3986@noname.str.redhat.com \
    --to=kwolf@redhat.com \
    --cc=arei.gonglei@huawei.com \
    --cc=armbru@redhat.com \
    --cc=berto@igalia.com \
    --cc=dgilbert@redhat.com \
    --cc=eddie.dong@intel.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=yanghy@cn.fujitsu.com \
    --cc=yunhong.jiang@intel.com \
    --cc=zhang.zhanghailiang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.