[Qemu-devel] Re: [RFC][PATCH v1 05/12] qapi: fix handling for null-return async callbacks

[Anthony Liguori <aliguori@us.ibm.com>]

On 03/28/2011 11:47 AM, Luiz Capitulino wrote:
> On Fri, 25 Mar 2011 16:22:16 -0500
> Anthony Liguori<aliguori@us.ibm.com>  wrote:
>
>> On 03/25/2011 02:47 PM, Michael Roth wrote:
>>> Async commands like 'guest-ping' have NULL retvals. Handle these by
>>> inserting an empty dictionary in the response's "return" field.
>>>
>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>>> ---
>>>    qmp-core.c |    5 ++++-
>>>    1 files changed, 4 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/qmp-core.c b/qmp-core.c
>>> index e33f7a4..9f3d182 100644
>>> --- a/qmp-core.c
>>> +++ b/qmp-core.c
>>> @@ -922,9 +922,12 @@ void qmp_async_complete_command(QmpCommandState *cmd, QObject *retval, Error *er
>>>        rsp = qdict_new();
>>>        if (err) {
>>>            qdict_put_obj(rsp, "error", error_get_qobject(err));
>>> -    } else {
>>> +    } else if (retval) {
>>>            qobject_incref(retval);
>>>            qdict_put_obj(rsp, "return", retval);
>>> +    } else {
>>> +        /* add empty "return" dict, this is the standard for NULL returns */
>>> +        qdict_put_obj(rsp, "return", QOBJECT(qdict_new()));
>> Luiz, I know we decided to return empty dicts because it lets us extend
>> things better, but did we want to rule out the use of a 'null' return
>> value entirely?
> For asynchronous commands you mean? No we didn't.

No, nothing to do with asynchronous commands.  Just in general.

The question is, is it legal for a command to return 'null'.  It's 
certain valid JSON, but is it valid QMP?

Regards,

Anthony Liguori

> *iirc*, what happens today is that no command using this api is truly async,
> for two reasons. First, changing from sync to async can break clients (that
> happened to query-balloon). Second, although I can't remember the exact
> details, the api that exists in the tree today is limited.
>
> But for a new thing, like QAPI, having different semantics for async commands
> seems the right thing to be done (ie. delaying the response).



>> For a command like this, I can't imagine ever wanting to extend the
>> return value...
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>        }
>>>        if (cmd->tag) {
>>>            qdict_put_obj(rsp, "tag", cmd->tag);