Re: [Qemu-devel] [PATCH 1/4] libqtest: add qmp_device_del()

[Markus Armbruster]

Peter Xu <address@hidden> writes:

> Device deletion is tricky since we'll get both a response and an event,
> while the order of arrival may vary.  Provide a helper to handle this
> complexity.
>
> Signed-off-by: Peter Xu <address@hidden>
> ---
>  tests/libqtest.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/libqtest.h |  8 ++++++++
>  2 files changed, 56 insertions(+)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index b9a1f18..a34d8c4 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -925,6 +925,54 @@ QDict *qmp(const char *fmt, ...)
>      return response;
>  }
>  
> +void qmp_device_del(const char *id)
> +{
> +    QDict *response1, *response2, *event = NULL;
> +    char *cmd;
> +
> +    /*
> +     * device deletion will get one response and one event. E.g.:
> +     *
> +     * {'execute': 'device_del','arguments': { 'id': 'scsi-hd'}}
> +     *
> +     * will get this one:
> +     *
> +     * {"timestamp": {"seconds": 1505289667, "microseconds": 569862},
> +     *  "event": "DEVICE_DELETED", "data": {"device": "scsi-hd",
> +     *  "path": "/machine/peripheral/scsi-hd"}}
> +     *
> +     * and this one:
> +     *
> +     * {"return": {}}
> +     *
> +     * But the order of arrival may vary.  Detect both.
> +     */
> +
> +    cmd = g_strdup_printf("{'execute': 'device_del',"
> +                          " 'arguments': {"
> +                          "   'id': '%s'"
> +                          "}}", id);
> +    response1 = qmp(cmd);
> +    g_free(cmd);
> +    g_assert(response1);
> +    g_assert(!qdict_haskey(response1, "error"));
> +
> +    response2 = qmp("");
> +    g_assert(response2);
> +    g_assert(!qdict_haskey(response2, "error"));

qmp_receive() would be cleaner than qmp("").

> +
> +    if (qdict_haskey(response1, "event")) {
> +        event = response1;
> +    } else if (qdict_haskey(response2, "event")) {
> +        event = response2;
> +    }
> +    g_assert(event);
> +    g_assert(!strcmp(qdict_get_str(event, "event"), "DEVICE_DELETED"));
> +
> +    QDECREF(response1);
> +    QDECREF(response2);
> +}
> +

Uh, this looks similar to existing qtest_qmp_device_del().  Do we need both?

>  void qmp_async(const char *fmt, ...)
>  {
>      va_list ap;
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 3ae5709..0d48e4b 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -920,6 +920,14 @@ QDict *qmp_fdv(int fd, const char *fmt, va_list ap);
>  QDict *qmp_fd(int fd, const char *fmt, ...);
>  
>  /**
> + * qmp_device_del:
> + * @id: The device ID to be deleted
> + *
> + * Delete the device with ID @id from QMP interface.
> + */
> +void qmp_device_del(const char *id);
> +
> +/**
>   * qtest_cb_for_every_machine:
>   * @cb: Pointer to the callback function
>   *