[Top][All Lists]

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

Re: [Qemu-devel] your mail

From: Eric Blake
Subject: Re: [Qemu-devel] your mail
Date: Fri, 29 Jun 2018 10:40:41 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0

On 06/29/2018 04:57 AM, Peter Xu wrote:

    Permit me a digression.  "control": { "run-oob": true } is awfully
    verbose.  Back when we hashed out the OOB design, I proposed to use
    "exec-oob" instead of "execute" to run a command OOB.  I missed when
    that morphed into flag "run-oob" wrapped in object "control".  Was it
    in response to reviewer feedback?  Can you give me a pointer?

It's me who proposed this, not from any reviewer's feedback.  It just
happened that no one was against it.

Only because I didn't notice the difference, and was helping clear the QAPI backlog before the release. You've now had both the maintainer and the backup express a desire for the shorter form.

    The counterpart to "exec-oob" would be "return-oob" and "error-oob".

I do prefer the less verbose proposal too.

But frankly speaking I still prefer current way.  QMP is majorly for
clients (computer programs) rather than users to use it.  Comparing to
verbosity, I would care more about coherency for how we define the
interface.  Say, OOB execution is still one way to execute a command,
so naturally it should still be using the same way that we execute a
QMP command, we just need some extra fields to tell the server that
"this message is more urgent than the other ones".

Code-wise, it's actually simpler for libvirt to write:

if (oob) {
    virJSONValueObjectCreate(&cmd, "s:exec-oob", cmdname, ...);
} else {
    virJSONValueObjectCreate(&cmd, "s:execute", cmdname, ...);

that it is to write:

virJSONValueObjectCreate(&cmd, "s:execute", cmdname, ...);
if (oob) {
    virJSONValuePtr control;
    virJSONValueObjectCreate(&control, "b:run-oob", true, NULL);
    virJSONValueObjectAppend(&cmd, "control", control);

(plus error-checking that I omitted).

In short, adding extra fields is MORE work than just using the command name AS the additional bit of information.

If we are discussing HMP interfaces, I'll have the same preference
with both of you to consider more about whether it's user-friendly or
not.  But now we are talking about QMP, then I'll prefer "control".

If you don't want to write the patch, then probably Markus or I will.

In the future, it's also possible to add some more sub-fields into the
"control" field.  When that happens, do we want to introduce another
standalone wording for that?  I would assume the answer is no.

We may add a "control" field at that time, and may even offer convenience syntax to allow "exec-oob" to behave as if a "control" field were passed. But the addition of new control features is rare, so I'd rather deal with that when we have something to actually add, rather than making us suffer with unneeded verbosity for potentially years waiting for that next control field to materialize.

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

reply via email to

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