From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NiTQb-00064i-E8 for qemu-devel@nongnu.org; Fri, 19 Feb 2010 09:01:57 -0500 Received: from [199.232.76.173] (port=43772 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NiTQa-00064X-2S for qemu-devel@nongnu.org; Fri, 19 Feb 2010 09:01:56 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NiTQW-0004Z1-Jd for qemu-devel@nongnu.org; Fri, 19 Feb 2010 09:01:55 -0500 Received: from mail-ew0-f209.google.com ([209.85.219.209]:46155) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NiTQW-0004Yv-3b for qemu-devel@nongnu.org; Fri, 19 Feb 2010 09:01:52 -0500 Received: by ewy1 with SMTP id 1so114822ewy.16 for ; Fri, 19 Feb 2010 06:01:51 -0800 (PST) Message-ID: <4B7E99C9.3040802@codemonkey.ws> Date: Fri, 19 Feb 2010 08:01:45 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] QMP: Spec: Private Extensions support References: <20100218182458.07c3be6c@redhat.com> <4B7DB6FC.7040900@codemonkey.ws> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Luiz Capitulino On 02/19/2010 07:04 AM, Markus Armbruster wrote: > Anthony Liguori writes: > > >> We need a bit more than just this. Here's my suggestion: >> > I think this is much more restrictive than necessary. Unnecessarily > restrictive rules are more likely to be ignored, and we don't want that. > Details below. > I disagree. The point of QMP is to allow third party tools to manage qemu in a robust way. Compatibility is a key part of this. Downstream QMP extension is a bad thing. It means management tools have to have a lot of special logic to deal with different downstreams. I believe we need to strongly discourage downstreams from making protocol changes while leaving them enough wiggle room to make the changes when they have no other choice. >> 6. Downstream modification of QMP >> ----------------------------------------------------------- >> >> We strongly recommend that downstream consumers of QEMU do _not_ >> modify the behaviour of any QMP command or introduce new QMP commands. >> This is important to allow management tools to support both upstream and >> downstream versions of QMP without special logic. >> >> However, we recognize that it is sometimes impossible for downstreams to >> avoid modifying QMP. If this happens, the following rules should be >> observed >> > Bad line break. > > >> to attempt to preserve long term compatibility and interoperability. >> > Better move your items 5-6) here, and make them talk about "names", not > just commands. Items 1-4) make much more sense with the name space > issue settled. > Good suggestion. >> 1) Only introduce new commands. If you want to change an existing command, >> leave the old command in place and introduce a new one with the new >> behaviour. >> > Extending an existing command in a compatible way should be fine. An > extension is "compatible" if > > 1. it adds only optional arguments, and > > 2. it behaves differently only when new optional arguments are supplied. > From an implementation perspective, it's very easy to modify an existing command, call it a new name, then add a new command with the old name that passes only the compatible parameters. So I have a hard time believing this is really a burden for downstreams. Think about it from an end-to-end perspective. If I'm writing a client library, and I expose a function qmp_commit(session, disk, force=false), and the QMP protocol introduces a vendor extension to qmp_commit(), I now need to change the qmp_commit() interface in my library to support that new parameter. Except since this is a vendor parameter, now every caller has to have knowledge about dealing with different vendors. On the other handle, a qmp_rhel_commit() command happily lives in a separate namespace. In fact, in a well written client library, it could easily be a plugin. The normal library consumer never has to think about vendor functions unless they explicitly need a function that only lives in a vendor namespace. >> 2) Only introduce new asynchronous messages. Do not change an >> existing message. >> > Why outlaw adding new data members? What would that break? > It's the same argument as above. If you add new data members, you expose vendor extensions to the core protocol which has to propagate all the way down to the end user. >> 3) Only introduce new error types and only use those error types in >> new commands. >> New commands can use existing error types, but you should never add a >> new error type >> to an existing command. >> > A client that can't cope with unknown error types will only ever work on > a single upstream version. Such clients will go the way of the Dodo > real quick. > > With that established: why is adding new error types bad? > Same as above. The core API should not behave any differently in a downstream. Clients who only use the core API should not have to adapt their behaviour for downstreams. With respect to ignoring error types, I disagree. There's certainly going to be a class (perhaps the majority) of clients that treat errors roughly the same. However, there is going to be a class of clients that very clearly handle errors in special ways depending on the error type. >> 4) Do not introduce any new capabilities. Capabilities directly >> affect a client's ability to >> parse the protocol correctly and new capabilities can not be supported >> in an upstream >> compatible way. Please work capabilities through upstream first. >> > This is factually incorrect. Clients that don't recognize new > capabilities just ignore them, and continue to work as before. > I think you misinterpreted my statement. Capabilities absolute affect the client's ability to parse the protocol if you enable them. If you support downstream specific capabilities, then knowledge of downstream needs to be very deep within any client library. That's an unnecessary level of complexity IMHO. Capabilities will be rare. I doubt a downstream would ever reasonably need to add a capability that isn't upstream. N.B. backporting upstream capabilities/commands/error messages is not what we're talking about here. All of those things are perfectly fine without using a vendor namespace. We're specifically talking about adding new features that are not present upstream. I think it's completely reasonable to suggest that capabilities should be done upstream first. Regards, Anthony Liguori