From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53462) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkaIK-0000tk-3d for qemu-devel@nongnu.org; Fri, 09 Oct 2015 12:13:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZkaII-0003Mp-P6 for qemu-devel@nongnu.org; Fri, 09 Oct 2015 12:13:20 -0400 References: <1442907862-21376-1-git-send-email-wency@cn.fujitsu.com> <1442907862-21376-4-git-send-email-wency@cn.fujitsu.com> <56157599.9020608@redhat.com> <87vbahq41u.fsf@blackfin.pond.sub.org> From: Max Reitz Message-ID: <5617E78C.5080906@redhat.com> Date: Fri, 9 Oct 2015 18:13:00 +0200 MIME-Version: 1.0 In-Reply-To: <87vbahq41u.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bvE45g3502GP4Sckh7uNqkHlQ8GIaBMrq" Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v5 3/4] qmp: add monitor command to add/remove a child List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Kevin Wolf , Yang Hongyang , Alberto Garcia , zhanghailiang , Jiang Yunhong , Dong Eddie , qemu devel , "Dr. David Alan Gilbert" , Gonglei , Stefan Hajnoczi , qemu block This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --bvE45g3502GP4Sckh7uNqkHlQ8GIaBMrq Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 08.10.2015 08:15, Markus Armbruster wrote: > Max Reitz writes: >=20 >> On 22.09.2015 09:44, Wen Congyang wrote: >>> The new QMP command name is x-blockdev-child-add, and x-blockdev-chil= d-del. >>> It justs for adding/removing quorum's child now, and don't support al= l >>> 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.= >=20 > 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 blkdebu= g, >> 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 mak= e >> it stable. What would that point be? Who can guarantee that we >> wouldn't want to make any interface changes after that point? >=20 > 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? > 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 go= es. >> Woul= d >> we actually remember to revisit this function once in a while and >> consider making it stable? >=20 > 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... >=20 > 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 childre= n >> as nicely as that is possible for quorum. >=20 > Okay, this is an argument I might be able to buy. >=20 >> 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, s= o >> we cannot use this interface there anyway. For quorum, such an exchang= e >> 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 exchangin= g >> 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. >=20 > 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) function= s. 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 thi= s >> interface, which would necessitate an incompatible change, therefore >> calling for an experimental interface? >=20 > Yes, that's an important question. >=20 > 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 =E2=80=9Cmacro function=E2=80=9D. > A secondary question is whether the incompleteness of the implementatio= n > 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 tool= s >> cannot use it, even though it'd be a very nice functionality to have) >=20 > 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 --bvE45g3502GP4Sckh7uNqkHlQ8GIaBMrq Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWF+eMAAoJEDuxQgLoOKytplQH/j7z3+zuTAxwxRkevT/bpoqm TGcO0dXPFmvXJY7TNWtou0YvtGYHXibPAzhhlQoU7HPz9R+0NonPr5fg4avLJgH4 QvM2LtXWTO88svwUai1el/0JspZTobhXHeViUW8h+v/O8+jMI3SMCvS8Q+NlBn+b 3gSr+bvSLdFaYkN5hOOOSS+Vs3EhU2ndctKzNrWPWnUUsRM1qk0a1wqBC8EixNAs 2hTwpEBZ3ioxu/nyXqxNS+0/Tlukt2sAv2C6CnJo0QhqDOFWYro5Zv5F6WpAIEVN 6+gQqXXJ6YnIPmC0zLYM+l73H2KtrS624iIA78eGwZudbSLBgSW1BwtLzyWUREs= =656y -----END PGP SIGNATURE----- --bvE45g3502GP4Sckh7uNqkHlQ8GIaBMrq--