Re: [Qemu-devel] [PATCH v4 16/22] libqtest: Add qmp_cmd() helper

2017-08-09 Thread Eric Blake
On 08/09/2017 10:40 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> Now that we've asserted that all of our interpolated QMP commands
>> include 'execute', we can reduce some of the caller boilerplate
>> by providing a helpr function to wrap commands with no arguments
> 
> helper
> 
> I don't get the dependency on asserting "contains 'execute'".

As mentioned elsewhere, the assertions helped me make sure I converted
all qmp() callers, but I'm fine not having it (and therefore adjusting
this commit message) in the next spin.

>> +void qmp_cmd_async(const char *cmd)
>> +{
>> +qtest_qmp_send(global_qtest, "{'execute':%s}", cmd);
>> +}
>> +
> 
> Hmm.  A possibly saner naming scheme:
> 
> FOO_send(): send a command
> FOO_receive(): receive a reply
> FOO: both

Yes, I like it.  That means s/FOO_async/FOO_send/.  And to some extent,
I already did that - as the name qmp_cmd() was temporary until I could
get rid of all older qmp() semantics, and then end with s/qmp_cmd/qmp/
in 22/22.  And since I'm already touching pretty much every client, it's
no worse churn to do a sane rename in the process.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v4 16/22] libqtest: Add qmp_cmd() helper

2017-08-09 Thread Markus Armbruster
Eric Blake  writes:

> Now that we've asserted that all of our interpolated QMP commands
> include 'execute', we can reduce some of the caller boilerplate
> by providing a helpr function to wrap commands with no arguments

helper

I don't get the dependency on asserting "contains 'execute'".

> (later patches will cover commands with arguments).
>
> Adjust all callers that can use the new helpers; in the process,
> fixing a couple of places where we would have failed
> -Wformat-nonliteral.  Likewise, libqos.c no longer needs
> qmp_execute(), which in turn fixes the fact that it is better
> to interpolate JSON strings through qobject_from_json() than
> through sprintf().
>
> The current name is long, but temporary: later patches will
> remove all other uses of qmp(), and then make the mass rename
> of qmp_cmd() down to qmp().
>
> Signed-off-by: Eric Blake 
> ---
>  tests/libqtest.h   | 16 
>  tests/libqtest.c   | 13 -
>  tests/ahci-test.c  |  4 +---
>  tests/boot-order-test.c|  2 +-
>  tests/ide-test.c   |  2 +-
>  tests/libqos/ahci.c|  4 ++--
>  tests/libqos/libqos.c  | 16 ++--
>  tests/numa-test.c  |  2 +-
>  tests/postcopy-test.c  |  8 
>  tests/q35-test.c   |  2 +-
>  tests/qmp-test.c   |  8 
>  tests/qom-test.c   |  2 +-
>  tests/test-filter-mirror.c |  2 +-
>  tests/test-filter-redirector.c |  4 ++--
>  tests/test-x86-cpuid-compat.c  |  2 +-
>  tests/virtio-net-test.c| 13 ++---
>  tests/vmgenid-test.c   |  2 +-
>  tests/wdt_ib700-test.c |  2 +-
>  18 files changed, 58 insertions(+), 46 deletions(-)
>
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 684cfb3507..e0d87d035a 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -483,6 +483,22 @@ QDict *qmp_raw(const char *msg);
>  void qmp_async(const char *fmt, ...);
>
>  /**
> + * qmp_cmd:
> + * @cmd: QMP command, with no arguments.
> + *
> + * Sends a QMP message to QEMU and returns the response.
> + */
> +QDict *qmp_cmd(const char *cmd);
> +
> +/**
> + * qmp_cmd_async:
> + * @cmd: QMP command, with no arguments.
> + *
> + * Sends a QMP message to QEMU and leaves the response in the stream.
> + */
> +void qmp_cmd_async(const char *cmd);
> +
> +/**
>   * qmp_discard_response:
>   *
>   * Read and discard a QMP response, typically after qmp_async().
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 2df01682c0..3926a4d481 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -858,6 +858,17 @@ void qmp_async(const char *fmt, ...)
>  va_end(ap);
>  }
>
> +QDict *qmp_cmd(const char *cmd)
> +{
> +qmp_cmd_async(cmd);
> +return qtest_qmp_receive(global_qtest);
> +}
> +
> +void qmp_cmd_async(const char *cmd)
> +{
> +qtest_qmp_send(global_qtest, "{'execute':%s}", cmd);
> +}
> +

Hmm.  A possibly saner naming scheme:

FOO_send(): send a command
FOO_receive(): receive a reply
FOO: both

>  void qmp_discard_response(void)
>  {
>  QDict *response = qtest_qmp_receive(global_qtest);
[...]