On Thu, Jun 01, 2023 at 11:26:46AM +0200, Thomas Huth wrote: > On 31/05/2023 15.23, Daniel P. Berrangé wrote: > > This function duplicates logic of qtest_qmp_assert_success_ref > > > > Signed-off-by: Daniel P. Berrangé <[email protected]> > > --- > > tests/qtest/migration-helpers.c | 22 ---------------------- > > tests/qtest/migration-helpers.h | 3 --- > > tests/qtest/migration-test.c | 29 +++++++++++++++-------------- > > 3 files changed, 15 insertions(+), 39 deletions(-) > > > > diff --git a/tests/qtest/migration-helpers.c > > b/tests/qtest/migration-helpers.c > > index f6f3c6680f..bddf3f8d4d 100644 > > --- a/tests/qtest/migration-helpers.c > > +++ b/tests/qtest/migration-helpers.c > > @@ -85,28 +85,6 @@ QDict *wait_command(QTestState *who, const char > > *command, ...) > > return ret; > > } > > -/* > > - * Execute the qmp command only > > - */ > > -QDict *qmp_command(QTestState *who, const char *command, ...) > > -{ > > - va_list ap; > > - QDict *resp, *ret; > > - > > - va_start(ap, command); > > - resp = qtest_vqmp(who, command, ap); > > - va_end(ap); > > - > > - g_assert(!qdict_haskey(resp, "error")); > > What about this g_assert(!qdict_haskey(resp, "error")) ? > qtest_qmp_assert_success_ref() does not have this assert... do we still need > it somewhere? If not, please add a comment to the patch description why it > can be ignored now.
The caller just wants the 'return' field data. If that is missing, qtest_qmp_assert_success_ref() will issue the diagnostic message printing the entire QMP resposne, which is way more debuggable than just asserting on 'error' without printing the error contents With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
