1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 0/4] qtest: fix "device_del" out-of-order events
  4. View patch

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

Peter Xu posted 4 patches 8 years, 4 months ago
[Qemu-devel] [PATCH 1/4] libqtest: add qmp_device_del()
Posted by Peter Xu 8 years, 4 months ago 
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 <peterx@redhat.com>
---
 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"));
+
+    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);
+}
+
 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
  *
-- 
2.7.4
Re: [Qemu-devel] [PATCH 1/4] libqtest: add qmp_device_del()
Posted by Markus Armbruster 8 years, 4 months ago 
Peter Xu <peterx@redhat.com> 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 <peterx@redhat.com>
> ---
>  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
>   *
Re: [Qemu-devel] [PATCH 1/4] libqtest: add qmp_device_del()
Posted by Eric Blake 8 years, 4 months ago 
On 10/02/2017 12:25 PM, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> 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 <peterx@redhat.com>
>> ---
>>  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)

>> +
>> +    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?

In fact, if I'm reading correctly, this is an early version, which was
retitled into commit acd80015 adding qtest_qmp_device_del().

At any rate, when I post my respin of my libqtest cleanups, I have the
qmp("") slated to be fixed.

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.