Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> Hi
>
> On Tue, Sep 29, 2020 at 3:01 PM Paolo Bonzini <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
You absolutely can create bindings for a specific version of QMP for the
client if you want. Just make sure the client as a whole obeys the
rules of the QMP game laid down in qmp-spec.txt and qapi-code-gen.txt.
>> 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.
>>
>
> Contrary to what you believe I am not focusing so much on DBus here :) It
> took about 200 loc to bind it, effortlessly (compared to sys<->rs
> conversion). All it does is to expose the same API we have in the generated
> C somehow (similar static types & functions - not all as a{sv} opaque
> dictionaries).
Two points.
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.
2. The API we generate for C is not exactly wonderful.
Behold this beauty:
void qmp_block_commit(bool has_job_id, const char *job_id, const char *device, bool has_base_node, const char *base_node, bool has_base, const char *base, bool has_top_node, const char *top_node, bool has_top, const char *top, bool has_backing_file, const char *backing_file, bool has_speed, int64_t speed, bool has_on_error, BlockdevOnError on_error, bool has_filter_node_name, const char *filter_node_name, bool has_auto_finalize, bool auto_finalize, bool has_auto_dismiss, bool auto_dismiss, Error **errp);
It's gotten so bad we added a second way to do the C API:
void qmp_drive_backup(DriveBackup *arg, Error **errp);
Turns out
DriveBackup arg = {
... initialize the optionals you need ...
}
qmp_drive_backup(&arg, &err);
is a lot easier on the eyes than passing 29 positional arguments.
So is writing the function arguments with indentation. Then I don't see much difference between a long list of arguments in a struct and that function. The main difference is that you make it easy to pass those arguments down. But often, you want to pass a subset, you don't want to pass the whole context as it may lead to bad design / bugs.
This could be viewed as a work-around for C's lack of positional
parameters.
Or a badly designed QMP command.
Even more fun:
void qmp_blockdev_add(BlockdevOptions *arg, Error **errp);
BlockdevOptions is a tagged union.
This could be viewed as a work-around for C's lack of function
overloading.
Or a badly designed QMP command ?
> It's easy for QEMU to generate a good static binding for C, because the
> version always matches. For a client, you wouldn't be able to write a
> similar idiomatic API in C, Rust, Python or Go, unfortunately.
I disagree. You won't be able to write good bindings using just
positional parameters. Not even if you add restrictions on how we can
evolve QMP. And no, I do not consider the C bindings we create for QEMU
itself "good". They're the best we could do, and good enough.
Sure they could be better, they are still quite idiomatic for C.
When you do bindings for another language, do bindings for that
language, not C bindings in that language.
Yes
Regardless of bindings, the client as a whole should obey the rules of
the QMP game laid down in qmp-spec.txt and qapi-code-gen.txt. If these
rules have become counter-productive, then it's time to replace QMP
wholesale.
Do not attempt to force a square peg into a round hole. If we must have
square pegs, design a square hole, and retire the round hole.
Hmm? I am trying to make the hole a bit more regular...
> Iow, I am not trying to sell DBus, I would like to make it easier to bind
> QMP in general. (although I do believe that DBus is a better protocol than
> QMP for local IPC, yes. And gRPC is probably better for remoting)
>
>> 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?
>>
>>
> I don't buy that generalization. A very recent protocol in this space, that
> aims to be a good low-level RPC on Linux (for containers, cloud etc) is
> varlink. (In many ways, we could compare it to QMP, but it lacks some
> important features, like events)
>
> 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.