Re: [Qemu-devel] [PATCH v3 08/12] qtests: convert tests to use qmp_cmd

[Eric Blake]

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