Re: [Qemu-devel] [PATCH v2 15/23] tests: New helper qtest_qmp_receive_success()

[Eric Blake]

On 07/27/2018 10:13 AM, Markus Armbruster wrote:
> Commit b21373d0713 copied wait_command() from tests/migration-test.c
> to tests/tpm-util.c.  Replace both copies by new libqtest helper
> qtest_qmp_receive_success().  Also use it to simplify
> qtest_qmp_device_del().
>
> Bonus: gets rid of a non-literal format string.  A step towards
> compile-time format string checking without triggering
> -Wformat-nonliteral.

Where? [1]

> Cc: Thomas Huth <address@hidden>
> Cc: Juan Quintela <address@hidden>
> Cc: Dr. David Alan Gilbert <address@hidden>
> Cc: Stefan Berger <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>
> Reviewed-by: Juan Quintela <address@hidden>
> Reviewed-by: Stefan Berger <address@hidden>
> ---

> +++ b/tests/libqtest.c
> @@ -1029,6 +1029,35 @@ void qtest_cb_for_every_machine(void (*cb)(const char 
> *machine))
>       qobject_unref(response);
>   }
>   
> +QDict *qtest_qmp_receive_success(QTestState *s,
> +                                 void (*event_cb)(void *opaque,
> +                                                  const char *event,
> +                                                  QDict *data),
> +                                 void *opaque)
> +{

I like the new signature!

> +++ b/tests/migration-test.c

>   /*
>    * Events can get in the way of responses we are actually waiting for.
>    */
>   static QDict *wait_command(QTestState *who, const char *command)
>   {

[1] This is still taking a format non-literal command...

> -    const char *event_string;
> -    QDict *response, *ret;
> -
> -    response = qtest_qmp(who, command);
...
> +    qtest_qmp_send(who, command);

and is passing it on through.

> +    return qtest_qmp_receive_success(who, stop_cb, NULL);
>   }
>   

> +++ b/tests/tpm-util.c

> -/*
> - * Events can get in the way of responses we are actually waiting for.
> - */
> -static QDict *tpm_util_wait_command(QTestState *who, const char *command)
> -{

Maybe you were counting this instance,

>   void tpm_util_wait_for_migration_complete(QTestState *who)
>   {
>       while (true) {
> -        QDict *rsp, *rsp_return;
> +        QDict *rsp_return;
>           bool completed;
>           const char *status;
>   
> -        rsp = tpm_util_wait_command(who, "{ 'execute': 'query-migrate' }");
> -        rsp_return = qdict_get_qdict(rsp, "return");
> +        qtest_qmp_send(who, "{ 'execute': 'query-migrate' }");

where you did indeed drop a format non-literal?

But on the assumption that there's more cleanups later in the series, 
this is indeed incrementally better, so

Reviewed-by: Eric Blake <address@hidden>

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