Re: [PATCH] PoC: Rust binding for QAPI (qemu-ga only, for now)

[Paolo Bonzini]

On 30/09/20 09:51, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Sep 30, 2020 at 11:34 AM Markus Armbruster <armbru@redhat.com
> <mailto:armbru@redhat.com>> wrote:
> 
>     Marc-André Lureau <marcandre.lureau@gmail.com
>     <mailto:marcandre.lureau@gmail.com>> writes:
> 
>     > Hi
>     >
>     > On Tue, Sep 29, 2020 at 3:01 PM Paolo Bonzini <pbonzini@redhat.com
>     <mailto:pbonzini@redhat.com>> wrote:
>     [...]
>     >> Marc-André, we are totally in agreement about that!  The problem
>     is that
>     >> you have already decided what the solution looks like, and that's
>     what
>     >> I'm not sure about because your solution also implies completely
>     >> revisiting the schema.
>     >>
>     >
>     > Did I? Which schema change are you (or I) implying? Versioning the
>     > interface? It's necessary at the client level, unless everything is
>     > dynamic, after introspection, which makes automated static bindings
>     > impractical.
> 
>     I disagree with "necessary".
> 
>     A client can use a specific version of QMP, and still talk to a server
>     with a different version, because we designed that capability into QMP.
> 
>  "A client can use a specific version of QMP" == versioning on the
> client side

No: "a client can use code generated for a specific version of QMP" does
not imply versioning on the client side.  It does imply that, before
using *certain* features, the client must verify that they are
implemented in the server.

>     1. Opaque dictionaries are far from the only way to do keyword arguments
>     in a language that lacks them.
> 
> Oh one can always be creative. The point is trying to stay idiomatic in
> the target language.

And builders in Rust, or keyword arguments in Python, are perfectly
idiomatic.

> Or a badly designed QMP command.
> Or a badly designed QMP command ?

Great.  Do you have ideas around how to design good QMP commands?  It
doesn't have to be patches for code, just documentation.

>     > varlink does non-optional members and versioning the same way I propose
>     > here, for what I could tell. Although they use JSON, and have similar
>     > transport "benefits", this basic rule allow them to have very decent
>     > automated binding in various languages, without resorting to
>     unorthodox
>     > solutions, ex:
>     >
>     https://github.com/varlink/rust/blob/master/examples/example/src/main.rs
> 
>     Paolo pointed out successful protocols that make tradeoffs similar to
>     QMP to support the idea that these tradeoffs can make sense and are
>     workable.
> 
>     Pointing out other, dissimilar protocols is not a convincing
>     counter-argument :)
> 
> It's relevant. Did you study varlink a bit? It's so close to QMP, you
> will find it hard to point out real dissimilarities.

I played a bit with varlink, and it seems to me that varlink does
actually support QMP-/OpenAPI-like extensibility.  Missing nullable
fields are treated as null, and on the client side it generally does not
give an error if it receives unknown fields (just like QMP). [1]  Also
just like QMP there are limits to the server's liberality, for example
sending extra fields to the server is an error.

In fact I could have skipped the experiments and read the Varlink
documentation: :)

  Varlink interfaces do not have a version number, they only have a
  feature set described in detail by the interface definition, which is
  part of the wire protocol.

  If your interface has users, you should not break the interface, only
  extend it, never remove or incompatibly change things which might be
  already in use.

  [...]

  Varlink does not use positional parameters or fixed-size objects in
  its interface definition or on the wire, all parameters are identified
  by their name and can be extended later.

  Extending existing structures should be done via optional fields
  (nullable type, maybe). The result of the methods, when passed
  parameters without the optional fields should be the same as in older
  versions. Method parameters can be extended the same way. The expected
  behavior for omitted fields/parameters should be documented. Removing
  fields, types, errors or methods are not backward compatible and
  should be avoided.

which is pretty much what QMP does.  So even though some users of
varlink might have chosen to rein themselves in on the extensibility
allowed by Varlink, that's a self-imposed limitation.  It may be due to
the desire to interoperate with varlink language bindings that use
positional parameters, but that's again a limitation of the bindings and
not the protocol.

So I don't see varlink as a reason to change the existing practice for
QMP, more as a second example of someone else doing the same as QMP.

Again, I'm not saying that DBus's choice with respect to versioning and
backwards/forwards-compatibility is _wrong_.  I do not like Protobuf's
numbered fields for that matter either, but it's not wrong either.  I am
saying that the choices in QMP are one of many different tradeoffs, and
that there's no point in saying that they get in the way of writing
language bindings or even just in the way of writing good/idiomatic
language bindings, because they don't.

Paolo




[1] Try this:

$ git clone git://github.com/bonzini/python
$ git checkout nullable-ping
$ python3 -m varlink.tests.test_orgexamplemore --varlink="unix:/tmp/test" &

And then:

$ varlink info unix:/tmp/test org.example.more
...
method Ping(ping: ?string) -> (pong: ?string)
...
$ python -m varlink.cli call unix:/tmp/test/org.example.more.Ping '{}'
{
  "pang": "xxx",
  "pong": null
}

You can see that the argument was implicitly treated as null, and there
were no complaints about the extra returned field.

I didn't try returning extra fields with the Rust bindings, but I did
try optional arguments and they work fine.  A "recent" client with
optional argument can talk to an "old" server with non-optional
argument, and it will only fail if the client actually takes advantage
of the newer interface.  As long as the argument is there, everything
works just fine.