All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Eric Blake <eblake@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	lcapitulino@redhat.com, Michael Roth <mdroth@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [RFC/RFA PATCH] qapi: detect extra members inside structs
Date: Mon, 19 Mar 2012 15:30:26 -0500	[thread overview]
Message-ID: <4F679762.6000201@codemonkey.ws> (raw)
In-Reply-To: <4F679576.60900@redhat.com>

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.



>
>>
>> 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.

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.

Regards,

Anthony Liguori

>
> On output from the server, we can add new fields (such as more details
> about an error message), and old clients should ignore extra fields.
> Meanwhile, these fields should be documented as optional, so a new
> client can be prepared to deal with an older server that didn't send the
> field.
>
> So yes, it really does sound like you want different behavior depending
> on whether it is the client or the server that is originating the new
> fields.
>

  reply	other threads:[~2012-03-19 20:30 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-19 19:29 [Qemu-devel] [RFC/RFA PATCH] qapi: detect extra members inside structs Paolo Bonzini
2012-03-19 19:34 ` Anthony Liguori
2012-03-19 19:51   ` Paolo Bonzini
2012-03-19 19:56     ` Anthony Liguori
2012-03-19 20:22       ` Eric Blake
2012-03-19 20:30         ` Anthony Liguori [this message]
2012-03-19 20:43           ` Luiz Capitulino
2012-03-19 22:29             ` Michael Roth
2012-03-19 22:38               ` Anthony Liguori
2012-03-19 23:45                 ` Michael Roth
2012-03-20  0:49                   ` Anthony Liguori
2012-03-20 12:15                     ` Luiz Capitulino
2012-03-20 19:33                       ` Anthony Liguori
2012-03-20 17:31       ` Paolo Bonzini
2012-03-20  0:28 ` Michael Roth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F679762.6000201@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=eblake@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.