On 08/09/2017 10:57 AM, Markus Armbruster wrote: > I don't like the qmp_args name. It's not about arguments, it's about > sending a command with arguments. > > I'm afraid I'm getting pretty thorougly confused by the evolving > interface. So I stop and think how it should look like when done.
At a bare minimum, I like your idea that we want:
FOO_send() # sends a message, does not wait for a reply
FOO_receive() # reads one JSON object, returns QObject counterpart
FOO() # combo of FOO_send() and FOO_receive()
Then FOO_discard() is a convenient shorthand for QDECREF(FOO_receive()).
Which FOO do we need? Right now, we have 'qmp' for anything using the
global_qtest, 'qtest_qmp' for anything using an explicit QTestState (but
we're arguing that is overkill), and 'qmp_fd' for anything using a raw
fd (test-qga.c - and where I added 'qga' in 11/22, although that
addition was local rather than in libqtest.h).
I also just had an insight: it is possible to write a macro
qmp_send(...) that will expand to qmp_send_cmd(name) if there is only
one argument, but to qmp_send_cmdf(name, fmt, ...) if there is more than
one argument (and later expose a public qmp_send_cmdv(name, fmt,
va_list) as pair if it is needed, although right now it is not).
Perhaps we can even make the one-arg form expand to qmp_send_cmdf(name,
" "), if that's enough to silence gcc's -Wformat-zero-length, and if our
underlying routines are smart enough to recognize a single-space
argument as not being worthy of calling qobject_from_jsonf() (since
qobject_from_jsonf() aborts rather than returning NULL when presented
just whitespace).
In fact, having a macro qmp_send(cmd, ...) expand to
qmp_send_internal(cmd, " " __VA_ARGS__) might even do the trick without
preprocessor magic of checking whether __VA_ARGS__ contains a comma
(although it has the subtle limitation that if present, a format
argument MUST be a string literal because we now rely on string
concatenation with " " - although given gcc's -Wformat-nonliteral, we
are probably okay with that limitation).
So, with a nice macro, the bulk of the testsuite would just write in
this style:
qmp_send("foo");
qmp_send("foo", "{literal_args...}");
qmp_send("foo", "{'name':%s}", value);
for send without waiting, or:
obj = qmp("foo");
obj = qmp(foo", "{literal_args...}");
obj = qmp("foo", "{'name':%s}", value);
where the result matters. And the names we choose for internal
implementation are no longer quite as important (although still helpful
to get right).
>
> We need send and receive. Convenience functions to do both, and to
> receive and throw away are nice to have.
Do we want both a convenience function to throw away a single read, AND
a function to send a command + throw away a read? Prior to this series,
we have qmp_discard_response() which is send + discard, but nothing that
does plain discard; although plain discard is a better building block
(precisely because then we don't have to duplicate all the different
send forms) - the convenience of send+receive in one call should be
limited to the most common case.
Also, the qmp_eventwait() is a nice convenience function for wait in a
loop until the received object matches a given name; whether we also
need the convenience of qmp_eventwait_ref() is a bit more debatable
(perhaps we could just as easily have qmp_event_wait() always return an
object, and require the caller to QDECREF() it, for one less function to
maintain).
>
> We need qmp_raw(). We haven't found a need for a raw receive function.
Yes, qmp_raw() (or maybe it should be named qmp_send_raw(), depending on
whether the receive is coupled with it?) is our backdoor for sending
JSON that does not match the normal {'execute':'name','arguments':{}}
paradigm (and pretty much qmp-test.c, or maybe also test-qga.c, would be
the only clients).
[Side node - why can't we pick a consistent naming of our tests? The
name 'FOO-test' is a bit nicer than 'test-FOO' when it comes to writing
.gitignore entries. Should we do a bulk cleanup rename to get all the
tests into one consistent naming style?]
>
> Convenience functions to build commands and send are nice to have. They
> come in pairs, without arguments, to avoid -Wno-format-zero-length
> (aside: that's one of the sillier warnings).
It's possible to alter CFLAGS to shut gcc up on that one while still
getting the rest of -Wformat, if we don't think it adds value to qemu.
My idea above is that you can use a macro to hide the worst of the
paired nature, so that the bulk of the testsuite can supply or omit an
arguments format string as desired.
>
> We used to have almost everything with and without QTestState, but we're
> refusing to continue that self-flagellation.
And for v5, I think I'll reorder my series to get rid of QTestState
sooner, rather than later.
>
> For every function taking ..., we want one taking va_list.
Under the hood, probably. Whether the va_list form has to be exported
is a different question (we might be able to get by with just the ... form).
>
> Makes sense?
>
> Can we derive a sensible set of function names from this?
>
I gave it a shot.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
