[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 08/12] qtests: convert tests to use qmp_cmd
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 08/12] qtests: convert tests to use qmp_cmd |
Date: |
Fri, 28 Jul 2017 14:08:12 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/28/2017 01:53 PM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> Now that we have the qmp_cmd() helper, we can further simplify
>> some of the tests by using it.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>> ---
>> }
>> - resp = qmp("{'execute': 'qom-list-types',"
>> - " 'arguments': %p }", args);
>> + resp = qmp_cmd("qom-list-types", QOBJECT(args));
>
> This one's a clear win.
Well, it'd be even NICER if I could pass QDict instead of QObject around.
>> +++ b/tests/ide-test.c
>> @@ -651,7 +651,7 @@ static void test_retry_flush(const char *machine)
>> qmp_eventwait("STOP");
>>
>> /* Complete the command */
>> - qmp_discard_response("{'execute':'cont' }");
>> + qmp_cmd_discard_response("cont", NULL);
>
> This one isn't.
Fair enough.
>> @@ -86,7 +87,7 @@ void set_context(QOSState *s)
>>
>> static QDict *qmp_execute(const char *command)
>> {
>> - return qmp("{ 'execute': %s }", command);
>> + return qmp_cmd(command, NULL);
>
> Marginal.
>
>> }
>>
>> void migrate(QOSState *from, QOSState *to, const char *uri)
>> @@ -106,7 +107,7 @@ void migrate(QOSState *from, QOSState *to, const char
>> *uri)
>> QDECREF(rsp);
>>
>> /* Issue the migrate command. */
>> - rsp = qmp("{ 'execute': 'migrate', 'arguments': { 'uri': %s } }", uri);
>> + rsp = qmp_cmd("migrate", qobject_from_jsonf("{ 'uri': %s }", uri));
>
> Not a simplification. The opposite, I'm afraid.
>
> I could become one if qmp_cmd() worked like this:
>
> rsp = qmp_cmd("migrate", "{ 'uri': %s }", uri));
Oh, nice. But it makes qmp_cmd become varargs, at which point...
>
> Even better if qmp_cmd("cont", "") just works. Hmm, unles gcc whines
> about the empty format string.
yeah, we'd have to figure out a way to shut up gcc when no arguments are
wanted. Here's a compromise that does the best for all three categories
you pointed out above:
qmp_cmd("cont");
qmp_cmd_args("migrate", "{ 'uri': %s }", uri);
qmp_cmd_dict("qom-list-types", args);
Sounds like I have a plan of attack! Also, since I'm somewhat churning
on the stuff you did in a previous patch, should we reorder any of this
series (add the helper first, then a single fix the callers that benefit
from the helpers; instead of fixing callers twice in three patches)?
>
>> g_assert(qdict_haskey(rsp, "return"));
>> QDECREF(rsp);
>>
> [More of the same snipped...]
>
And, as I said in the cover letter, there's probably a lot more we could
touch in the testsuite if we like the new pattern.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v3 07/12] qtest: Add a new helper qmp_cmd() and friends, (continued)
[Qemu-devel] [PATCH v3 08/12] qtests: convert tests to use qmp_cmd, Eric Blake, 2017/07/25
[Qemu-devel] [PATCH v3 10/12] tests: Clean up wait for event, Eric Blake, 2017/07/25
[Qemu-devel] [PATCH v3 11/12] tests/libqtest: Clean up how we read the QMP greeting, Eric Blake, 2017/07/25
[Qemu-devel] [PATCH v3 12/12] tests/libqtest: Enable compile-time format string checking, Eric Blake, 2017/07/25