All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@gmail.com>
Cc: "P. Berrange, Daniel" <berrange@redhat.com>,
	Sergio Lopez Pascual <slp@redhat.com>,
	"Hajnoczi, Stefan" <stefanha@gmail.com>,
	Markus Armbruster <armbru@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH] PoC: Rust binding for QAPI (qemu-ga only, for now)
Date: Tue, 29 Sep 2020 13:00:58 +0200	[thread overview]
Message-ID: <fb20f8e4-6bc9-3518-a983-86fad1915e49@redhat.com> (raw)
In-Reply-To: <CAJ+F1CJgEe3++UHDfT3iOGyu+r1tM4A_9jRXoKC0P-k-Mhq29w@mail.gmail.com>

On 29/09/20 12:34, Marc-André Lureau wrote:
>     That would not be backwards compatible as you would have to set all
>     optional fields.  Every time the command grows a new optional argument,
>     all clients would have to specify it; if a command becomes optional,
>     you'd have to add Some() around it.  So I would say that...
> 
> 
> Not necessarily, with ..default::Default()

That's true, I always forget about .. (though you'd still have to add
Some() for now-optional fields).

>     > Less idiomatic, but it also works around the optional arguments and
>     > ordering issue.
> 
>     ...  the builder pattern is not a workaround: it's the best and most
>     common Rust idiom to achieve what QAPI expresses as optional fields.
>     Likewise for keyword-only arguments in Python.
> 
> Except QAPI makes all fields potentially optional (and unordered),
> that's not idiomatic.

Yes, for some APIs you can always add hand-written, more idiomatic
versions.  Or you could mark them as fixed-arguments in the schema and
let the code generator do that (but then you need to add a compatibility
check).  But that would be an explicit choice, not something required by
the transport.

> D-Bus is machine-level oriented, it's easy to bind to various languages,
> it can be pretty efficient too. It's not designed to be a good network
> RPC. QMP tries to be a bit of both, but is perhaps not good enough in
> either.

No, only tries to be a good network RPC; not a particularly efficient
one, but future-proof.  And it mostly succeeds at that---with one
notable exception: JSON parsers that mess up with numbers bigger than 2^53.

>     If you want to "reinvent" QMP, instead of focusing on D-Bus you should
>     take a look at alternative IDLs and protocols (D-Bus is one but there's
>     also Protobuf and Flexbuffers), see how QAPI declarations would map to
>     those protocols, see how you would deal with extensibility, and rank
>     them according to various criteria.  For example:
> 
>     * JSON "just works" but needs a custom code generator and imposes some
>     extra complexity on the clients for the simplest commands
> 
>     * D-Bus has a good ecosystem and would keep simple commands simpler but
>     has issues with upgrade paths and is uglier for complex commands
> 
>     * Protobufs probably would also just work and would have better code
>     generators, but would require some kind of lint to ensure
>     backwards-compatibility
> 
> Again, the issues we are discussing are not specific to binding QMP over
> D-Bus. Binding QMP to various languages has similar problems.

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.

I say there are many candidates (the ones I know are Protobuf and
Flexbuffers) for serialization and many candidates for transport (REST
and gRPC to begin with) in addition to the two {QMP,JSON} and
{DBus,DBus} tuples.  We should at least look at how they do code
generation before deciding that JSON is bad and DBus is good.

> I would rather make those problems solved at the server level, that
> doesn't require any change to QMP today, just a more careful
> consideration when making changes (and probably some tools to help
> enforce some stability).

Problem is, "more careful consideration when making changes" is not a
small thing.  And other RPCs have evolved in a completely different
space (REST APIs for web services) that have chosen the same tradeoffs
as QMP, so why should we not learn from them?

Paolo



  reply	other threads:[~2020-09-29 11:05 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-10 17:48 [PATCH] PoC: Rust binding for QAPI (qemu-ga only, for now) marcandre.lureau
2020-09-11 10:24 ` Paolo Bonzini
2020-09-11 13:08   ` Paolo Bonzini
2020-09-11 14:00     ` Marc-André Lureau
2020-09-11 15:17       ` Paolo Bonzini
2020-09-29 17:55         ` Marc-André Lureau
2020-09-29 18:23           ` Paolo Bonzini
2020-09-30  9:15             ` Marc-André Lureau
2020-09-30 14:02               ` Paolo Bonzini
2020-09-11 10:46 ` Daniel P. Berrangé
2020-09-11 11:03   ` Marc-André Lureau
2020-09-11 11:28   ` Paolo Bonzini
2020-09-11 11:31     ` Daniel P. Berrangé
2020-09-11 14:19     ` John Snow
2020-09-11 14:17 ` Marc-André Lureau
2020-09-21  9:16 ` Markus Armbruster
2020-09-21  9:30   ` Paolo Bonzini
2020-09-22 14:59     ` Markus Armbruster
2020-09-21 10:04   ` Marc-André Lureau
2020-09-22 15:09     ` Markus Armbruster
2020-09-22 16:35       ` Marc-André Lureau
2020-09-22 17:08         ` Paolo Bonzini
2020-09-29  7:45           ` Marc-André Lureau
2020-09-29 10:14             ` Paolo Bonzini
2020-09-29 10:34               ` Marc-André Lureau
2020-09-29 11:00                 ` Paolo Bonzini [this message]
2020-09-29 11:34                   ` Marc-André Lureau
2020-09-30  7:34                     ` Markus Armbruster
2020-09-30  7:51                       ` Marc-André Lureau
2020-09-30 13:14                         ` Paolo Bonzini
2020-09-22 16:52       ` Daniel P. Berrangé
2020-09-23 11:51         ` Markus Armbruster
2020-09-21 10:11   ` Marc-André Lureau
2020-09-22 15:37     ` Markus Armbruster
2020-09-22 16:25       ` Marc-André Lureau
2020-09-24  7:31         ` Markus Armbruster

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=fb20f8e4-6bc9-3518-a983-86fad1915e49@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=marcandre.lureau@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=slp@redhat.com \
    --cc=stefanha@gmail.com \
    /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.