qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Yang Hongyang <yanghy@cn.fujitsu.com>,
	Alberto Garcia <berto@igalia.com>,
	zhanghailiang <zhang.zhanghailiang@huawei.com>,
	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>,
	qemu block <qemu-block@nongnu.org>
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v5 3/4] qmp: add monitor command to add/remove a child
Date: Fri, 9 Oct 2015 20:24:40 +0200	[thread overview]
Message-ID: <56180668.1090609@redhat.com> (raw)
In-Reply-To: <20151009164224.GA28665@work-vm>

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

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-.

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.

Max

>                                                                    but it
> does remove their right to moan if it changes.
> 
> Dave



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

  reply	other threads:[~2015-10-09 18:25 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 [this message]
2015-10-12  8:07             ` Dr. David Alan Gilbert
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=56180668.1090609@redhat.com \
    --to=mreitz@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=kwolf@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).