From: Maxim Levitsky <mlevitsk@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: "Fam Zheng" <fam@euphon.net>,
"Laurent Vivier" <lvivier@redhat.com>,
"Thomas Huth" <thuth@redhat.com>,
"Daniel P. Berrangé" <berrange@redhat.com>,
"Eduardo Habkost" <ehabkost@redhat.com>,
qemu-block@nongnu.org, "Michael S. Tsirkin" <mst@redhat.com>,
"Jason Wang" <jasowang@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>,
"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
"John Snow" <jsnow@redhat.com>,
"Stefan Berger" <stefanb@linux.ibm.com>
Subject: Re: [PATCH v7 03/13] qtest: switch users back to qtest_qmp_receive
Date: Tue, 06 Oct 2020 16:17:27 +0300 [thread overview]
Message-ID: <4e549e434dbf3fc44456e61cdbead79ce392fdfb.camel@redhat.com> (raw)
In-Reply-To: <0d0727c2-5770-6a9a-6ace-f8b49b83bd78@redhat.com>
On Tue, 2020-10-06 at 14:56 +0200, Paolo Bonzini wrote:
> 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!
I was afraid that a partial conversion will break things, since I had changed
the qtest_vqmp_fds, qtest_vqmp and qtest_qmp, so there are quite a lot of
places where this is called implicitly. Now looking again at this,
it is probably possible to separate this chinage. If I need to send another
version I'll do this.
Best regards,
Maxim Levitsky
>
> 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");
> >
next prev parent reply other threads:[~2020-10-06 13:19 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-06 12:38 [PATCH v7 00/13] Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread Maxim Levitsky
2020-10-06 12:38 ` [PATCH v7 01/13] qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict Maxim Levitsky
2020-10-12 11:13 ` Thomas Huth
2020-10-06 12:38 ` [PATCH v7 02/13] qtest: Reintroduce qtest_qmp_receive Maxim Levitsky
2020-10-12 11:14 ` Thomas Huth
2020-10-12 13:47 ` Paolo Bonzini
2020-10-12 13:49 ` Thomas Huth
2020-10-12 16:56 ` Paolo Bonzini
2020-10-06 12:38 ` [PATCH v7 03/13] qtest: switch users back to qtest_qmp_receive Maxim Levitsky
2020-10-06 12:56 ` Paolo Bonzini
2020-10-06 13:17 ` Maxim Levitsky [this message]
2020-10-06 12:38 ` [PATCH v7 04/13] qdev: add "check if address free" callback for buses Maxim Levitsky
2020-10-06 12:38 ` [PATCH v7 05/13] scsi: switch to bus->check_address Maxim Levitsky
2020-10-06 12:38 ` [PATCH v7 06/13] scsi/scsi_bus: switch search direction in scsi_device_find Maxim Levitsky
2020-10-06 12:38 ` [PATCH v7 07/13] device_core: use drain_call_rcu in in qmp_device_add Maxim Levitsky
2020-10-06 12:38 ` [PATCH v7 08/13] device-core: use RCU for list of children of a bus Maxim Levitsky
2020-10-06 12:39 ` [PATCH v7 09/13] device-core: use atomic_set on .realized property Maxim Levitsky
2020-10-06 12:39 ` [PATCH v7 10/13] scsi/scsi-bus: scsi_device_find: don't return unrealized devices Maxim Levitsky
2020-10-06 12:39 ` [PATCH v7 11/13] scsi/scsi_bus: Add scsi_device_get Maxim Levitsky
2020-10-06 12:39 ` [PATCH v7 12/13] virtio-scsi: use scsi_device_get Maxim Levitsky
2020-10-06 12:39 ` [PATCH v7 13/13] scsi/scsi_bus: fix races in REPORT LUNS Maxim Levitsky
2020-10-06 13:01 ` [PATCH v7 00/13] Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread no-reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4e549e434dbf3fc44456e61cdbead79ce392fdfb.camel@redhat.com \
--to=mlevitsk@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=ehabkost@redhat.com \
--cc=f4bug@amsat.org \
--cc=fam@euphon.net \
--cc=jasowang@redhat.com \
--cc=jsnow@redhat.com \
--cc=lvivier@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanb@linux.ibm.com \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.