qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Markus Armbruster
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v3 08/12] qtests: convert tests to use qmp_cmd
Date: Mon, 31 Jul 2017 09:28:41 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)

Eric Blake <address@hidden> writes:

> 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)?

If easier review and cleaner history can justify the effort to rework.
Your call.

>>>      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.

One step at a time.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]