From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44872) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S9lEE-0006Yu-3u for qemu-devel@nongnu.org; Mon, 19 Mar 2012 18:39:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S9lEB-0005ig-1a for qemu-devel@nongnu.org; Mon, 19 Mar 2012 18:39:01 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:55510) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S9lEA-0005iN-SP for qemu-devel@nongnu.org; Mon, 19 Mar 2012 18:38:58 -0400 Received: by yenr5 with SMTP id r5so6866343yen.4 for ; Mon, 19 Mar 2012 15:38:57 -0700 (PDT) Message-ID: <4F67B57E.5040802@codemonkey.ws> Date: Mon, 19 Mar 2012 17:38:54 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1332185368-18708-1-git-send-email-pbonzini@redhat.com> <4F678A37.1020101@codemonkey.ws> <4F678E38.7050902@redhat.com> <4F678F6A.3050804@codemonkey.ws> <4F679576.60900@redhat.com> <4F679762.6000201@codemonkey.ws> <20120319174314.74720a38@doriath.home> <20120319222907.GA2994@illuin> In-Reply-To: <20120319222907.GA2994@illuin> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC/RFA PATCH] qapi: detect extra members inside structs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: Paolo Bonzini , Eric Blake , qemu-devel@nongnu.org, Luiz Capitulino On 03/19/2012 05:29 PM, Michael Roth wrote: > On Mon, Mar 19, 2012 at 05:43:14PM -0300, Luiz Capitulino wrote: >> On Mon, 19 Mar 2012 15:30:26 -0500 >> Anthony Liguori wrote: >> >>> On 03/19/2012 03:22 PM, Eric Blake wrote: >>>> On 03/19/2012 01:56 PM, Anthony Liguori wrote: >>>>>> For old clients that could be fine. But what about old servers? :) >>>>> >>>>> Same applies to old server. If a new client tries to use a new field, >>>>> if the old server refuses it, then the new client breaks. >>>> >>>> I recently asked this question, and I was told that it is a feature that >>>> unknown fields attempted by a new client are rejected by an old server: >>>> https://lists.gnu.org/archive/html/qemu-devel/2012-03/msg00815.html >>> >>> Unfortunately that's not entirely correct. >>> >>> The QMP server has a mechanism to validate that the input parameters that are >>> passed conform to the spec in qmp-commands.hx, but this spec can only handle >>> simple types. Anywhere where we bypass this check (like we do for transaction), >>> the checking is up to the callee which doesn't check. >> >> Right. >> >>>>> There's no way in QMP to detect whether a server supports a new field. >>>>> This is why I proposed instituting a policy of never adding/removing >>>>> fields to structures and why I had advocating use a C version of libqapi >>>>> in terms of enforcing compatibility rules. >>>>> >>>>> I'm not sure if the "server ignores unknown" fields thing is even >>>>> reasonable to rely upon so maybe we should just draw a line in the sane >>>>> and make the change you're suggesting... >>>> >>>> For ideal back-compat, I think you want: >>>> >>>> On input to the server, we can add new fields, but such new fields must >>>> be optional (old clients that omit the fields get the default value, >>>> rather than a new server rejecting the command due to a missing field). >>>> The question arises when you have a new client talking to an old >>>> server; here, I think it's better to _always_ have the server reject >>>> things it doesn't recognize, so that clients can use this rejection as a >>>> feature probe, and then you _do_ have reliable ways of querying whether >>>> a feature was added, by whether the new argument associated with that >>>> feature is present. >>> >>> The problem is that this requires transactional semantics such that if the >>> command fails, there are no side effects. I don't think we're in a position to >>> guarantee that. >> >> IMHO failing with side effects is a bug. Although no command that does that comes >> to my mind right now. >> >>> I'd greatly prefer to simply not add new options to existing commands. It's >>> simple, maps well to how we do things in C, and is easy to enforce. >> >> We extensively discussed this some months ago. I honestly don't want to repeat >> that discussion. So, if there's consensus that adding new commands is better >> than extending existing ones, this is fine with me. >> >> Another solution would be to extend query-commands with arguments info, which >> is something that I think we will do soon or later... > > It's doable, we could even just give them QAPI schema, which is valid > json and serves as our QMP command documentation anyway. I think that's a > complete solution, and makes thing easy on the server/qemu-side. But for a > client it may be somewhat of a pain. Right, you would need to be able to fully interpret the schema which means recursing into each of the types to discover if a field was added anywhere. It's down right cruel to expect clients to do this IMHO. > New field -> new command is seems easier on the client-side in theory, but once > you go beyond a handful of variations and get to a point where several > include the desired option, you basically need to start encoding the > option "capability" in the command name, or using versioned commands, so > long term it may not scale well. It may also be difficult to ensure commands > with a certain name don't lose/gain fields downstream. And this is why we should take our time designing commands attempting to think through the future use cases. > So IMO, returning arguments actually seems easier for both clients and the > server, and is more resilient to downstream changes. It does require a more > formal specification of the protocol though. Basically: "an option/field > may not be supported in older/future versions of QEMU, so it is up to > the client to check in advance by referencing the QAPI schema for their > qemu version or programatically via get_schema()" The complexity of writing a client using get_schema() is close to staggering :-/ Regards, Anthony Liguori >> >