qemu-block
[Top][All Lists]
Advanced

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

Re: [PATCH v7 03/13] qtest: switch users back to qtest_qmp_receive


From: Paolo Bonzini
Subject: Re: [PATCH v7 03/13] qtest: switch users back to qtest_qmp_receive
Date: Tue, 6 Oct 2020 14:56:32 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0

On 06/10/20 14:38, Maxim Levitsky wrote:
> The only remaining users of qtest_qmp_receive_dict are tests
> that fuzz the QMP protocol.
> 
> Tested with 'make check-qtest'.

Probably the qtest_qmp_receive_success conversion should have been in a
separate patch.  But it's a nice side effect that I hadn't anticipated!

Paolo

> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  tests/qtest/ahci-test.c         |  4 +-
>  tests/qtest/device-plug-test.c  |  2 +-
>  tests/qtest/drive_del-test.c    |  9 ++---
>  tests/qtest/libqos/libqtest.h   | 17 ---------
>  tests/qtest/libqtest.c          | 65 ++++-----------------------------
>  tests/qtest/migration-helpers.c | 25 ++++++++++---
>  tests/qtest/pvpanic-test.c      |  4 +-
>  tests/qtest/tpm-util.c          |  8 +++-
>  8 files changed, 41 insertions(+), 93 deletions(-)
> 
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index d42ebaeb4c..5e1954852e 100644
> --- a/tests/qtest/ahci-test.c
> +++ b/tests/qtest/ahci-test.c
> @@ -1590,7 +1590,7 @@ static void test_atapi_tray(void)
>      qtest_qmp_send(ahci->parent->qts, "{'execute': 'blockdev-open-tray', "
>                      "'arguments': {'id': 'cd0'}}");
>      atapi_wait_tray(ahci, true);
> -    rsp = qtest_qmp_receive_dict(ahci->parent->qts);
> +    rsp = qtest_qmp_receive(ahci->parent->qts);
>      qobject_unref(rsp);
>  
>      qmp_discard_response(ahci->parent->qts,
> @@ -1620,7 +1620,7 @@ static void test_atapi_tray(void)
>      qtest_qmp_send(ahci->parent->qts, "{'execute': 'blockdev-close-tray', "
>                     "'arguments': {'id': 'cd0'}}");
>      atapi_wait_tray(ahci, false);
> -    rsp = qtest_qmp_receive_dict(ahci->parent->qts);
> +    rsp = qtest_qmp_receive(ahci->parent->qts);
>      qobject_unref(rsp);
>  
>      /* Now, to convince ATAPI we understand the media has changed... */
> diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
> index a2247856be..9214892741 100644
> --- a/tests/qtest/device-plug-test.c
> +++ b/tests/qtest/device-plug-test.c
> @@ -23,7 +23,7 @@ static void device_del_start(QTestState *qtest, const char 
> *id)
>  
>  static void device_del_finish(QTestState *qtest)
>  {
> -    QDict *resp = qtest_qmp_receive_dict(qtest);
> +    QDict *resp = qtest_qmp_receive(qtest);
>  
>      g_assert(qdict_haskey(resp, "return"));
>      qobject_unref(resp);
> diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
> index ba0cd77445..9d20a1ed8b 100644
> --- a/tests/qtest/drive_del-test.c
> +++ b/tests/qtest/drive_del-test.c
> @@ -15,9 +15,6 @@
>  #include "libqos/virtio.h"
>  #include "qapi/qmp/qdict.h"
>  
> -/* TODO actually test the results and get rid of this */
> -#define qmp_discard_response(q, ...) qobject_unref(qtest_qmp(q, __VA_ARGS__))
> -
>  static void drive_add(QTestState *qts)
>  {
>      char *resp = qtest_hmp(qts, "drive_add 0 if=none,id=drive0");
> @@ -38,13 +35,13 @@ static void device_del(QTestState *qts)
>  {
>      QDict *response;
>  
> -    /* Complication: ignore DEVICE_DELETED event */
> -    qmp_discard_response(qts, "{'execute': 'device_del',"
> +    response = qtest_qmp(qts, "{'execute': 'device_del',"
>                           " 'arguments': { 'id': 'dev0' } }");
> -    response = qtest_qmp_receive_dict(qts);
>      g_assert(response);
>      g_assert(qdict_haskey(response, "return"));
>      qobject_unref(response);
> +
> +    qtest_qmp_eventwait(qts, "DEVICE_DELETED");
>  }
>  
>  static void test_drive_without_dev(void)
> diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
> index 19429a536d..a91e9e02e9 100644
> --- a/tests/qtest/libqos/libqtest.h
> +++ b/tests/qtest/libqos/libqtest.h
> @@ -240,23 +240,6 @@ QDict *qtest_qmp_eventwait_ref(QTestState *s, const char 
> *event);
>   */
>  QDict *qtest_qmp_event_ref(QTestState *s, const char *event);
>  
> -/**
> - * qtest_qmp_receive_success:
> - * @s: #QTestState instance to operate on
> - * @event_cb: Event callback
> - * @opaque: Argument for @event_cb
> - *
> - * Poll QMP messages until a command success response is received.
> - * If @event_cb, call it for each event received, passing @opaque,
> - * the event's name and data.
> - * Return the success response's "return" member.
> - */
> -QDict *qtest_qmp_receive_success(QTestState *s,
> -                                 void (*event_cb)(void *opaque,
> -                                                  const char *name,
> -                                                  QDict *data),
> -                                 void *opaque);
> -
>  /**
>   * qtest_hmp:
>   * @s: #QTestState instance to operate on.
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index d4c49a52ff..08929f5ff6 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -324,7 +324,7 @@ QTestState *qtest_init(const char *extra_args)
>      QDict *greeting;
>  
>      /* Read the QMP greeting and then do the handshake */
> -    greeting = qtest_qmp_receive_dict(s);
> +    greeting = qtest_qmp_receive(s);
>      qobject_unref(greeting);
>      qobject_unref(qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"));
>  
> @@ -700,7 +700,7 @@ QDict *qtest_vqmp_fds(QTestState *s, int *fds, size_t 
> fds_num,
>      qtest_qmp_vsend_fds(s, fds, fds_num, fmt, ap);
>  
>      /* Receive reply */
> -    return qtest_qmp_receive_dict(s);
> +    return qtest_qmp_receive(s);
>  }
>  
>  QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list ap)
> @@ -708,7 +708,7 @@ QDict *qtest_vqmp(QTestState *s, const char *fmt, va_list 
> ap)
>      qtest_qmp_vsend(s, fmt, ap);
>  
>      /* Receive reply */
> -    return qtest_qmp_receive_dict(s);
> +    return qtest_qmp_receive(s);
>  }
>  
>  QDict *qmp_fd(int fd, const char *fmt, ...)
> @@ -850,12 +850,6 @@ char *qtest_vhmp(QTestState *s, const char *fmt, va_list 
> ap)
>                       " 'arguments': {'command-line': %s}}",
>                       cmd);
>      ret = g_strdup(qdict_get_try_str(resp, "return"));
> -    while (ret == NULL && qdict_get_try_str(resp, "event")) {
> -        /* Ignore asynchronous QMP events */
> -        qobject_unref(resp);
> -        resp = qtest_qmp_receive_dict(s);
> -        ret = g_strdup(qdict_get_try_str(resp, "return"));
> -    }
>      g_assert(ret);
>      qobject_unref(resp);
>      g_free(cmd);
> @@ -1291,35 +1285,6 @@ 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)
> -{
> -    QDict *response, *ret, *data;
> -    const char *event;
> -
> -    for (;;) {
> -        response = qtest_qmp_receive_dict(s);
> -        g_assert(!qdict_haskey(response, "error"));
> -        ret = qdict_get_qdict(response, "return");
> -        if (ret) {
> -            break;
> -        }
> -        event = qdict_get_str(response, "event");
> -        data = qdict_get_qdict(response, "data");
> -        if (event_cb) {
> -            event_cb(opaque, event, data);
> -        }
> -        qobject_unref(response);
> -    }
> -
> -    qobject_ref(ret);
> -    qobject_unref(response);
> -    return ret;
> -}
> -
>  /*
>   * Generic hot-plugging test via the device_add QMP commands.
>   */
> @@ -1355,13 +1320,6 @@ void qtest_qmp_device_add(QTestState *qts, const char 
> *driver, const char *id,
>      qobject_unref(args);
>  }
>  
> -static void device_deleted_cb(void *opaque, const char *name, QDict *data)
> -{
> -    bool *got_event = opaque;
> -
> -    g_assert_cmpstr(name, ==, "DEVICE_DELETED");
> -    *got_event = true;
> -}
>  
>  /*
>   * Generic hot-unplugging test via the device_del QMP command.
> @@ -1378,24 +1336,17 @@ static void device_deleted_cb(void *opaque, const 
> char *name, QDict *data)
>   * and this one:
>   *
>   * {"return": {}}
> - *
> - * But the order of arrival may vary - so we've got to detect both.
>   */
>  void qtest_qmp_device_del(QTestState *qts, const char *id)
>  {
> -    bool got_event = false;
>      QDict *rsp;
>  
> -    qtest_qmp_send(qts, "{'execute': 'device_del', 'arguments': {'id': %s}}",
> -                   id);
> -    rsp = qtest_qmp_receive_success(qts, device_deleted_cb, &got_event);
> +    rsp = qtest_qmp(qts, "{'execute': 'device_del', 'arguments': {'id': 
> %s}}",
> +                    id);
> +
> +    g_assert(qdict_haskey(rsp, "return"));
>      qobject_unref(rsp);
> -    if (!got_event) {
> -        rsp = qtest_qmp_receive_dict(qts);
> -        g_assert_cmpstr(qdict_get_try_str(rsp, "event"),
> -                        ==, "DEVICE_DELETED");
> -        qobject_unref(rsp);
> -    }
> +    qtest_qmp_eventwait(qts, "DEVICE_DELETED");
>  }
>  
>  bool qmp_rsp_is_err(QDict *rsp)
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index 516093b39a..b799dbafb7 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -17,10 +17,12 @@
>  
>  bool got_stop;
>  
> -static void stop_cb(void *opaque, const char *name, QDict *data)
> +static void check_stop_event(QTestState *who)
>  {
> -    if (!strcmp(name, "STOP")) {
> +    QDict *event = qtest_qmp_event_ref(who, "STOP");
> +    if (event) {
>          got_stop = true;
> +        qobject_unref(event);
>      }
>  }
>  
> @@ -30,12 +32,19 @@ static void stop_cb(void *opaque, const char *name, QDict 
> *data)
>  QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...)
>  {
>      va_list ap;
> +    QDict *resp;
>  
>      va_start(ap, command);
>      qtest_qmp_vsend_fds(who, &fd, 1, command, ap);
>      va_end(ap);
>  
> -    return qtest_qmp_receive_success(who, stop_cb, NULL);
> +    resp = qtest_qmp_receive(who);
> +    check_stop_event(who);
> +
> +    g_assert(!qdict_haskey(resp, "error"));
> +    g_assert(qdict_haskey(resp, "return"));
> +
> +    return qdict_get_qdict(resp, "return");
>  }
>  
>  /*
> @@ -44,12 +53,18 @@ QDict *wait_command_fd(QTestState *who, int fd, const 
> char *command, ...)
>  QDict *wait_command(QTestState *who, const char *command, ...)
>  {
>      va_list ap;
> +    QDict *resp;
>  
>      va_start(ap, command);
> -    qtest_qmp_vsend(who, command, ap);
> +    resp = qtest_vqmp(who, command, ap);
>      va_end(ap);
>  
> -    return qtest_qmp_receive_success(who, stop_cb, NULL);
> +    check_stop_event(who);
> +
> +    g_assert(!qdict_haskey(resp, "error"));
> +    g_assert(qdict_haskey(resp, "return"));
> +
> +    return qdict_get_qdict(resp, "return");
>  }
>  
>  /*
> diff --git a/tests/qtest/pvpanic-test.c b/tests/qtest/pvpanic-test.c
> index b0b20ad340..0657de797f 100644
> --- a/tests/qtest/pvpanic-test.c
> +++ b/tests/qtest/pvpanic-test.c
> @@ -24,9 +24,7 @@ static void test_panic(void)
>  
>      qtest_outb(qts, 0x505, 0x1);
>  
> -    response = qtest_qmp_receive_dict(qts);
> -    g_assert(qdict_haskey(response, "event"));
> -    g_assert_cmpstr(qdict_get_str(response, "event"), ==, "GUEST_PANICKED");
> +    response = qtest_qmp_eventwait_ref(qts, "GUEST_PANICKED");
>      g_assert(qdict_haskey(response, "data"));
>      data = qdict_get_qdict(response, "data");
>      g_assert(qdict_haskey(data, "action"));
> diff --git a/tests/qtest/tpm-util.c b/tests/qtest/tpm-util.c
> index 3ed6c8548a..5a33a6ef0f 100644
> --- a/tests/qtest/tpm-util.c
> +++ b/tests/qtest/tpm-util.c
> @@ -237,12 +237,16 @@ void tpm_util_migrate(QTestState *who, const char *uri)
>  void tpm_util_wait_for_migration_complete(QTestState *who)
>  {
>      while (true) {
> +        QDict *rsp;
>          QDict *rsp_return;
>          bool completed;
>          const char *status;
>  
> -        qtest_qmp_send(who, "{ 'execute': 'query-migrate' }");
> -        rsp_return = qtest_qmp_receive_success(who, NULL, NULL);
> +        rsp = qtest_qmp(who, "{ 'execute': 'query-migrate' }");
> +        g_assert(qdict_haskey(rsp, "return"));
> +        rsp_return = qdict_get_qdict(rsp, "return");
> +
> +        g_assert(!qdict_haskey(rsp_return, "error"));
>          status = qdict_get_str(rsp_return, "status");
>          completed = strcmp(status, "completed") == 0;
>          g_assert_cmpstr(status, !=,  "failed");
> 




reply via email to

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