Re: [Qemu-devel] [Qemu-block] [PATCH v5 3/4] qmp: add monitor command to add/remove a child

[Dr. David Alan Gilbert]

* Max Reitz (address@hidden) wrote:
> On 08.10.2015 08:15, Markus Armbruster wrote:
> > Max Reitz <address@hidden> 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.
Adding the experimental stuff makes it easier for people to try and
get some feedback on.
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
does remove their right to moan if it changes.

Dave

> 
> > The x- prefix enables work spanning multiple releases.  Until the
> > feature is complete, we have a hard time seeing the whole picture, and
> > therefore the risk of interface mistakes is higher than normal.  Once
> > it's complete, we drop the x-.
> 
> I'm arguing the feature is complete as far as what it's supposed to do goes.
> 
> >>                                                                   Would
> >>     we actually remember to revisit this function once in a while and
> >>     consider making it stable?
> > 
> > Has that been a problem in the past?
> 
> I don't know, because I never witnessed an external experimental
> interface, but I haven't been closely involved with qemu for too long.
> 
> > If the feature is of any use, there's always been mounting pressure to
> > finish the job and drop the x-.  If it's of no use, not dropping the x-
> > would do no harm.  The opposite, actually.
> 
> This is of use, however.
> 
> I do see your point, but I'm still arguing that I don't see why we need
> an external interface then. But COLO needs an external interface (which
> management tools should be able to use!) so there's that.
> 
> >> (2) While marking things experimental *should* keep people from using it
> >>     in their tools, nobody can guarantee that it *does* keep them from
> >>     doing so. So we may find ourselves in the situation of having to
> >>     keep a compatibility interface for an experimental feature...
> > 
> > You can't force people not to do stupid things, but you *can* improve
> > their chances at avoiding them.  Clearly marking experimental stuff
> > improves chances.
> 
> OK, right.
> 
> >> For the second point, you should also consider how useful this feature
> >> is to management tools. Just being able to remove and attach children
> >> from a quorum node seems very useful on its own. I don't see why we
> >> should wait for having support for other block drivers; also, for most
> >> block drivers there is no meaningful way of adding or removing children
> >> as nicely as that is possible for quorum.
> > 
> > Okay, this is an argument I might be able to buy.
> > 
> >> E.g. you may have a block filter in the future where you want to
> >> exchange its child BDS. This exchange should be an atomic operation, so
> >> we cannot use this interface there anyway. For quorum, such an exchange
> >> does not need to be atomic, since you can just add the new child first
> >> and remove the old one afterwards.
> >>
> >> So maybe in the future we get some block driver other than quorum for
> >> which adding and removing children (as opposed to atomically exchanging
> >> them) makes sense, but for now I can only see quorum. Therefore, that
> >> this works for quorum only is in my opinion not a reason to make it
> >> experimental. I think we actually want to keep it that way.
> > 
> > Are you telling us the existing interface is insufficiently general?
> > That the general interface neeeds to support atomic replacement?
> 
> The general interface for all block drivers and situations (as described
> by Kevin), yes. The interface for adding/removing children from quorum
> in regards to what COLO needs, no.
> 
> > If yes, why isn't the general interface is what we should do for quorum?
> > Delete is atomic replacement by nothing, add is atomic replacement of
> > nothing.
> 
> Yes, but I personally don't have a problem with macro functions. I don't
> see the harm in implementing blockdev-child-{add,del} now the way they
> are and later replacing them by calls to the general function.
> 
> But why should we do that?
> 
> Some people really want COLO in better sooner than later. Telling them
> to wait for until the block layer is as nice as we want it to be is not
> really an option I'd be willing to accept. I don't see why we should
> delay the COLO work just so that we don't have these two (macro) functions.
> 
> The alternative would be to keep it experimental and then tell every
> COLO user to use the general function once it's available. But since
> COLO users are probably mostly management tools, that seems much more
> difficult to me than to just keep these two macro functions as legacy in
> qemu.
> 
> >> So the question would then be: What ways can you imagine to change this
> >> interface, which would necessitate an incompatible change, therefore
> >> calling for an experimental interface?
> > 
> > Yes, that's an important question.
> > 
> > Another important question is whether this is the interface we want.
> 
> I can see why this is an important question to you, but to me it is not
> so much. As I argued above, I'm not opposed to adding interface that are
> actually not what we want, but that are what users want, and that are
> easy to implement with the interface that we want. It's what I call a
> “macro function”.
> 
> > A secondary question is whether the incompleteness of the implementation
> > demands an x- to warn users.
> 
> We don't want to shoehorn generality into these two functions, but add a
> new one anyway. We might want to add optional parameters later on (e.g.
> changing the threshold), but that would be a compatible change.
> 
> >> (My point is that with such an experimental interface, management tools
> >> cannot use it, even though it'd be a very nice functionality to have)
> > 
> > How much work is it to finish the job?  Unless it's a substantial chunk,
> > debating x- is much ado about nothing: just finish the job already :)
> 
> We have proven in the past to need a significant amount of time for even
> for non-substantial chunks. Often, non-substantial chunks turn out to be
> substantial when actually tackling them.
> 
> It basically comes down to whether the COLO authors are willing to wait
> for us to do the job. And frankly, if I were them, I wouldn't be.
> 
> Max
> 


--
Dr. David Alan Gilbert / address@hidden / Manchester, UK