|
From: | Anthony Liguori |
Subject: | [Qemu-devel] Re: [RFC][PATCH v1 05/12] qapi: fix handling for null-return async callbacks |
Date: | Mon, 28 Mar 2011 12:01:16 -0500 |
User-agent: | Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.14) Gecko/20110223 Lightning/1.0b2 Thunderbird/3.1.8 |
On 03/28/2011 11:47 AM, Luiz Capitulino wrote:
On Fri, 25 Mar 2011 16:22:16 -0500 Anthony Liguori<address@hidden> 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<address@hidden> --- 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);
[Prev in Thread] | Current Thread | [Next in Thread] |