Eric Blake <ebl...@redhat.com> writes: > Upcoming patches will be adding new convenience methods for > constructing QMP commands. But making every variation of sending > support every variation of response handling becomes unwieldy; > it's easier to specify that discarding a JSON response is > unassociated with sending the command, where qmp_async() already > fits the bill for sending a command without tying up a reference > to the response. > > Doing this renders qtest_qmp[v]_discard_response() unused. > > Bonus: gets rid of a non-literal format string, which is a step > towards compile-time format string checking without triggering > -Wformat-nonliteral.
Where? (I'm feeling lazy today) > > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > tests/libqtest.h | 27 ++------------------------- > tests/libqtest.c | 30 ++++++------------------------ > tests/ahci-test.c | 20 ++++++++++---------- > tests/boot-order-test.c | 2 +- > tests/drive_del-test.c | 5 +++-- > tests/fdc-test.c | 11 ++++++----- > tests/ide-test.c | 5 ++--- > tests/postcopy-test.c | 3 ++- > tests/test-filter-mirror.c | 3 ++- > tests/test-filter-redirector.c | 6 ++++-- > tests/virtio-blk-test.c | 21 ++++++++++++--------- > 11 files changed, 50 insertions(+), 83 deletions(-) > > diff --git a/tests/libqtest.h b/tests/libqtest.h > index 917ec5cf92..6bae0223aa 100644 > --- a/tests/libqtest.h > +++ b/tests/libqtest.h > @@ -48,16 +48,6 @@ QTestState *qtest_init_without_qmp_handshake(const char > *extra_args); > void qtest_quit(QTestState *s); > > /** > - * qtest_qmp_discard_response: > - * @s: #QTestState instance to operate on. > - * @fmt...: QMP message to send to qemu; formats arguments through > - * json-lexer.c (only understands '%(PRI[ud]64|(l|ll)?[du]|[ipsf%])'). > - * > - * Sends a QMP message to QEMU and consumes the response. > - */ > -void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...); > - > -/** > * qtest_qmp: > * @s: #QTestState instance to operate on. > * @fmt...: QMP message to send to qemu; formats arguments through > @@ -78,17 +68,6 @@ QDict *qtest_qmp(QTestState *s, const char *fmt, ...); > void qtest_async_qmp(QTestState *s, const char *fmt, ...); > > /** > - * qtest_qmpv_discard_response: > - * @s: #QTestState instance to operate on. > - * @fmt: QMP message to send to QEMU; formats arguments through > - * json-lexer.c (only understands '%(PRI[ud]64|(l|ll)?[du]|[ipsf%])'). > - * @ap: QMP message arguments > - * > - * Sends a QMP message to QEMU and consumes the response. > - */ > -void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap); > - > -/** > * qtest_qmpv: > * @s: #QTestState instance to operate on. > * @fmt: QMP message to send to QEMU; formats arguments through > @@ -568,12 +547,10 @@ void qmp_async(const char *fmt, ...); > > /** > * qmp_discard_response: > - * @fmt...: QMP message to send to qemu; formats arguments through > - * json-lexer.c (only understands '%(PRI[ud]64|(l|ll)?[du]|[ipsf%])'). > * > - * Sends a QMP message to QEMU and consumes the response. > + * Read and discard a QMP response, typically after qmp_async(). > */ > -void qmp_discard_response(const char *fmt, ...); > +void qmp_discard_response(void); > > /** > * qmp_receive: > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 3071be2efb..f9781d67f5 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -235,7 +235,8 @@ QTestState *qtest_init(const char *extra_args) > /* Read the QMP greeting and then do the handshake */ > greeting = qtest_qmp_receive(s); > QDECREF(greeting); > - qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"); > + greeting = qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"); > + QDECREF(greeting); Here, you replace qtest_qmp_discard_response(ARGS...); effectively by QDECREF(qtest_qmp(ARGS...)); which is how qtest_qmp_discard_response() does its job before this patch. Okay. > > return s; > } > @@ -518,23 +519,6 @@ void qtest_async_qmp(QTestState *s, const char *fmt, ...) > va_end(ap); > } > > -void qtest_qmpv_discard_response(QTestState *s, const char *fmt, va_list ap) > -{ > - QDict *response = qtest_qmpv(s, fmt, ap); > - QDECREF(response); > -} > - > -void qtest_qmp_discard_response(QTestState *s, const char *fmt, ...) > -{ > - va_list ap; > - QDict *response; > - > - va_start(ap, fmt); > - response = qtest_qmpv(s, fmt, ap); > - va_end(ap); > - QDECREF(response); > -} > - > QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event) > { > QDict *response; > @@ -909,14 +893,12 @@ void qmp_async(const char *fmt, ...) > va_end(ap); > } > > -void qmp_discard_response(const char *fmt, ...) > +void qmp_discard_response(void) > { > - va_list ap; > - > - va_start(ap, fmt); > - qtest_qmpv_discard_response(global_qtest, fmt, ap); > - va_end(ap); > + QDict *response = qtest_qmp_receive(global_qtest); > + QDECREF(response); > } > + > char *hmp(const char *fmt, ...) > { > va_list ap; > diff --git a/tests/ahci-test.c b/tests/ahci-test.c > index 999121bb7c..9460843a9f 100644 > --- a/tests/ahci-test.c > +++ b/tests/ahci-test.c > @@ -1596,8 +1596,9 @@ static void test_atapi_tray(void) > rsp = qmp_receive(); > QDECREF(rsp); > > - qmp_discard_response("{'execute': 'x-blockdev-remove-medium', " > - "'arguments': {'device': 'drive0'}}"); > + qmp_async("{'execute': 'x-blockdev-remove-medium', " > + "'arguments': {'device': 'drive0'}}"); > + qmp_discard_response(); Here, you replace the same pattern (less the QTestState argument) by qmp_async(F, ...); qmp_discard_response(); Also okay. But why two ways to do the same things? Apropos QTestState argument: do we have a test with more than one state, or is having two versions of every function just "avoiding global state is virtuous" self-flagellation? > > /* Test the tray without a medium */ > ahci_atapi_load(ahci, port); [...]