From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Alberto Garcia <berto@igalia.com>,
zhanghailiang <zhang.zhanghailiang@huawei.com>,
qemu block <qemu-block@nongnu.org>,
Jiang Yunhong <yunhong.jiang@intel.com>,
Dong Eddie <eddie.dong@intel.com>,
Markus Armbruster <armbru@redhat.com>,
qemu devel <qemu-devel@nongnu.org>,
Gonglei <arei.gonglei@huawei.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
Yang Hongyang <yanghy@cn.fujitsu.com>
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v5 3/4] qmp: add monitor command to add/remove a child
Date: Mon, 12 Oct 2015 09:07:52 +0100 [thread overview]
Message-ID: <20151012080752.GA2687@work-vm> (raw)
In-Reply-To: <56180668.1090609@redhat.com>
* Max Reitz (mreitz@redhat.com) wrote:
> On 09.10.2015 18:42, Dr. David Alan Gilbert wrote:
> > * Max Reitz (mreitz@redhat.com) wrote:
> >> On 08.10.2015 08:15, Markus Armbruster wrote:
> >>> Max Reitz <mreitz@redhat.com> writes:
> >>>
> >>>> On 22.09.2015 09:44, Wen Congyang wrote:
> >>>>> The new QMP command name is x-blockdev-child-add, and x-blockdev-child-del.
> >>>>> It justs for adding/removing quorum's child now, and don't support all
> >>>>> kinds of children,
> >>>>
> >>>> It does support all kinds of children for quorum, doesn't it?
> >>>>
> >>>>> nor all block drivers. So it is experimental now.
> >>>>
> >>>> Well, that is not really a reason why we would have to make it
> >>>> experimental. For instance, blockdev-add (although some might argue it
> >>>> actually is experimental...) doesn't support all block drivers either.
> >>>
> >>> Yup, and not calling it x-blockdev-add until it's done was a mistake.
> >>> People tried using it, then found its current limitations the painful
> >>> way. Not nice.
> >>
> >> I knew I should have written s/some might/Markus does/. ;-)
> >>
> >>>> The reason I am hesitant of adding an experimental QMP interface that is
> >>>> actually visible to the user (compare x-image in blkverify and blkdebug,
> >>>> which are not documented and not to be used by the user) is twofold:
> >>>>
> >>>> (1) At some point we have to say "OK, this is good enough now" and make
> >>>> it stable. What would that point be? Who can guarantee that we
> >>>> wouldn't want to make any interface changes after that point?
> >>>
> >>> Nobody can, just like for any other interface. So?
> >>
> >> The main question is "what would that point be". As I can see you're
> >> arguing that that point would be "once people want to use it", but I'm
> >> arguing that people want to use it today or we wouldn't need this
> >> interface at all.
> >>
> >> I'm against adding external experimental interface because having
> >> external interface indicates that someone wants to use them, but making
> >> them experimental indicates that nobody should use them.
> >>
> >> This interface is added for the COLO series. The documentation added in
> >> patch 5 there explains usage of COLO with x-child-add. I don't think
> >> that should be there, because it's experimental. But why have an
> >> external interface if nobody should use it anyway?
> >
> > Because it lets people move forward; the COLO series is pretty huge, there
> > already seem to be side discussions spawning off about dynamic reconfiguration
> > of stuff, who knows how long those will take to pan out.
>
> Yes, and my point is that with these functions
> (blockdev-child-{add,del}) the result of that side discussion doesn't
> matter.
>
> > Adding the experimental stuff makes it easier for people to try and
> > get some feedback on.
>
> The thing is, I cannot imagine any feedback that would necessitate an
> incompatible change. “I want to change quorum's options while
> adding/removing children” can easily be accomplished with an additional
> optional parameter.
>
> But I do know that we want to keep things experimental exactly because
> there can be feedback which I cannot imagine right now.
>
> > If everyone turns out to love it then it only takes a trivial patch to promote
> > it; if people actually realise there is a better interface then it's
> > no problem to change it either - x- doesn't stop any one using it,
>
> But it should, shouldn't it? No management tool should be using an x-
> command, as far as I know. And these are functions which are clearly
> designed for management tools.
>
> If management tools are indeed free to use x- functions, then I'm
> completely fine with making these experimental for now. It's just that
> it looks to me like “Hey, look, we have these two new functions you can
> use!” and then, two versions later we remove them because we have a
> general reconfiguration option, and we'll say “It's your own fault for
> using experimental functions” if someone complains. That sounds
> hypocritical to me, but I'm probably being to “legal” here.
>
> (i.e. it's more like “Hey, look, two new cool functions! But don't use
> them.” which sounds like a contradiction to me, whereas it actually
> means “Feel free to use them but don't blame us”)
>
> tl;dr: May management tools use x- functions? And is it actually
> conceivable for them to do so? If so, my whole argument becomes moot, so
> let's make these functions x-.
My guess is the libvirt guys wont take the code to drive the x- methods;
but it still makes it easier if someone wants to try this stuff out, they
wont need to apply 2/3 sets of COLO code and then any management tools.
> Mainly I'd like to know about some example where we had an x- function
> in the past. Markus seemed to imply that was the case.
The RDMA code used to have x- for migration protocol and some of the
capabilities; we've recently added Jason Herne's cpu throttling with
similar x- flags (1626fee3bdbb295d5e8aff800f7621357bb376d6),
and input-send-event got moved into the x- world (df5b2adb7398d71016ee469f71e52075ed95e04e)
which is much worse than it starting out there.
Dave
>
> Max
>
> > but it
> > does remove their right to moan if it changes.
> >
> > Dave
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
next prev parent reply other threads:[~2015-10-12 8:08 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-22 7:44 [Qemu-devel] [PATCH v5 0/4] qapi: child add/delete support Wen Congyang
2015-09-22 7:44 ` [Qemu-devel] [PATCH v5 1/4] Add new block driver interface to add/delete a BDS's child Wen Congyang
2015-10-07 13:35 ` Alberto Garcia
2015-10-08 2:05 ` Wen Congyang
2015-10-07 18:33 ` [Qemu-devel] [Qemu-block] " Max Reitz
2015-10-08 2:06 ` Wen Congyang
2015-10-07 19:00 ` [Qemu-devel] " Dr. David Alan Gilbert
2015-10-08 2:03 ` Wen Congyang
2015-10-08 18:44 ` Dr. David Alan Gilbert
2015-09-22 7:44 ` [Qemu-devel] [PATCH v5 2/4] quorum: implement bdrv_add_child() and bdrv_del_child() Wen Congyang
2015-10-07 14:12 ` Alberto Garcia
2015-10-08 2:10 ` Wen Congyang
2015-10-07 18:51 ` [Qemu-devel] [Qemu-block] " Max Reitz
2015-10-08 8:12 ` Alberto Garcia
2015-10-09 15:51 ` Max Reitz
2015-10-12 11:56 ` Alberto Garcia
2015-09-22 7:44 ` [Qemu-devel] [PATCH v5 3/4] qmp: add monitor command to add/remove a child Wen Congyang
2015-10-07 14:33 ` Alberto Garcia
2015-10-07 19:42 ` [Qemu-devel] [Qemu-block] " Max Reitz
2015-10-08 6:15 ` Markus Armbruster
2015-10-08 8:29 ` Alberto Garcia
2015-10-08 10:03 ` Kevin Wolf
2015-10-08 10:13 ` Alberto Garcia
2015-10-09 16:14 ` Max Reitz
2015-10-08 11:02 ` [Qemu-devel] Dynamic reconfiguration (was: qmp: add monitor command to add/remove a child) Kevin Wolf
2015-10-08 11:10 ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2015-10-21 8:27 ` [Qemu-devel] [Qemu-block] Dynamic reconfiguration Markus Armbruster
2015-10-26 2:04 ` Wen Congyang
2015-10-26 7:24 ` Markus Armbruster
2015-10-26 7:25 ` Wen Congyang
2015-10-09 16:13 ` [Qemu-devel] [Qemu-block] [PATCH v5 3/4] qmp: add monitor command to add/remove a child Max Reitz
2015-10-09 16:42 ` Dr. David Alan Gilbert
2015-10-09 18:24 ` Max Reitz
2015-10-12 8:07 ` Dr. David Alan Gilbert [this message]
2015-10-12 8:18 ` Kevin Wolf
2015-10-12 7:58 ` Markus Armbruster
2015-10-12 7:56 ` Markus Armbruster
2015-10-12 16:27 ` Max Reitz
2015-09-22 7:44 ` [Qemu-devel] [PATCH v5 4/4] hmp: " Wen Congyang
2015-10-07 14:38 ` Alberto Garcia
2015-09-22 11:15 ` [Qemu-devel] [PATCH v5 0/4] qapi: child add/delete support Dr. David Alan Gilbert
2015-09-23 1:08 ` Wen Congyang
2015-09-23 9:21 ` Dr. David Alan Gilbert
2015-09-23 9:30 ` Wen Congyang
2015-10-07 6:40 ` 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=20151012080752.GA2687@work-vm \
--to=dgilbert@redhat.com \
--cc=arei.gonglei@huawei.com \
--cc=armbru@redhat.com \
--cc=berto@igalia.com \
--cc=eddie.dong@intel.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.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.