qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC/RFA PATCH] qapi: detect extra members inside struc


From: Michael Roth
Subject: Re: [Qemu-devel] [RFC/RFA PATCH] qapi: detect extra members inside structs
Date: Mon, 19 Mar 2012 18:45:59 -0500
User-agent: Mutt/1.5.21 (2010-09-15)

On Mon, Mar 19, 2012 at 05:38:54PM -0500, Anthony Liguori wrote:
> 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<address@hidden>  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.

Well, true, I guess it's worked out for long enough that we shouldn't assume
things would get that bad any time soon.

> 
> >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(<command>)"
> 
> The complexity of writing a client using get_schema() is close to staggering 
> :-/

I'm not sure, I mean, take libvirt, you need to marshall up the
arguments anyway and put them into a QMP payload, so in that case the
client developer is aware of the schema that they're coding against,
and also understand the QAPI schema specification, and if the schema
is nested then so is the client version of that "schema".

So, conceptually at least, it seems like it wouldn't be that big a jump
to maintain an internal representation of their schema to
programatically check against the specification they were coding
against, it's just the part where they then need to bake it into the
client implementation that's a bit heavy-handed.

Thinking about it more the, this does seem to be completely at odds with
any future prospects of a libqmp, so that's a pretty big trade-off...

> 
> Regards,
> 
> Anthony Liguori
> 
> >>
> >
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]