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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon, 28 Mar 2011 12:19:53 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 03/28/2011 12:06 PM, Luiz Capitulino wrote:
> > On Mon, 28 Mar 2011 12:01:16 -0500
> > Anthony Liguori<aliguori@us.ibm.com>  wrote:
> >
> >> 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?
> > No, it's not valid.
> 
> Do we have a reason for this?

We had to make a choice. We chose the current 'return' response. Iirc, one of
my first suggestions was "{ 'return': 'null' }" but you refused to have a 'null'
object, our parser doesn't even support it afaik.

But what's the problem with the current format?