Re: [Qemu-devel] [PATCH v3 07/12] qtest: Add a new helper qmp_cmd() and friends

[Eric Blake]

On 07/28/2017 01:45 PM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> When interpolating a QMP command from the testsuite, we have
>> a very common pattern that the command to be executed is a
>> string constant, while the arguments may need to be crafted
>> from qobject_from_jsonf() or even by hand.  Make it easier to
>> craft the arguments without having to save all the interpolation
>> to the final qmp() call, by adding new helper functions; the
>> function takes QObject instead of QDict in order to reduce the
>> number of conversions between the two.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>> ---
>>  tests/libqtest.h | 31 +++++++++++++++++++++++++++++++
>>  tests/libqtest.c | 25 +++++++++++++++++++++++++
>>  2 files changed, 56 insertions(+)
>>
>> diff --git a/tests/libqtest.h b/tests/libqtest.h
>> index 82e89b4d4f..26930fed4d 100644
>> --- a/tests/libqtest.h
>> +++ b/tests/libqtest.h
>> @@ -68,6 +68,17 @@ void qtest_qmp_discard_response(QTestState *s, const char 
>> *fmt, ...);
>>  QDict *qtest_qmp(QTestState *s, const char *fmt, ...);
>>
>>  /**
>> + * qtest_qmp_cmd:
>> + * @s: #QTestState instance to operate on.
>> + * @cmd: Command name to send
>> + * @args: Arguments to transfer to the command, or NULL.
> 
> Do we really need "or NULL"?

It allows me to do:

qmp_cmd("cont", NULL)

instead of

qmp("{'execute':'cont'}")

> 
>> + *
>> + * Sends a QMP message to QEMU and returns the response. Calling this will
>> + * reduce the reference count of @args.
> 
> Suggest "decrement".  Same for the wrappers below.

Okay.


>> +QDict *qtest_qmp_cmd(QTestState *s, const char *cmd, QObject *args)
>> +{
>> +    QDict *dict = qdict_new();
>> +
>> +    qdict_put_str(dict, "execute", cmd);
>> +    if (args) {
>> +        qdict_put_obj(dict, "arguments", args);
>> +    }
>> +    return qtest_qmp(s, "%p", QOBJECT(dict));
> 
> This function should be a one-liner:
> 
>        return qtest_qmp(s, "{'execute': %s, 'arguments': %p }", cmd, args);
> 
> A bit more complicated if we need to support null @args.

Well, I did use NULL args.  But it looks like this is leftovers from my
original attempt to ban qobject_from_jsonf() altogether (which we have
now swung in the opposite direction of "let's keep it and make it safer
with compiler checks"), so you are indeed right that not building a
manual QDict would be a bit nicer.

{
    if (!args) {
        args = qdict_new();
    }
    return qtest_qmp(s, "{'execute': %s, 'arguments': %p }", cmd, args);
}

would work, except that it slightly changes what is passed over the
wire, albeit with no semantic change (use of 'arguments':{} is optional)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature