[Top][All Lists]

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

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

From: Michael Roth
Subject: [Qemu-devel] Re: [RFC][PATCH v1 05/12] qapi: fix handling for null-return async callbacks
Date: Mon, 28 Mar 2011 12:59:36 -0500
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv: Gecko/20110303 Thunderbird/3.1.9

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

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

I think this is another topic, but also one we should hash out a bit better.

Currently the plan is that the C API not expose asynchronicity, underneath the covers the library will issue the request, then do a blocking read for the response. So the API call will block till completion, and no other command's will be serviced through the same underlying session until it is completed or cancelled.

For the JSON-based clients, the behavior is different. You have an optional tag you can pass in for an async command, and after issuing one, you can immediately begin issuing other async or non-async commands. As a result, the responses you receive will not necessarily be in FIFO order.

The upside to this is you can implement async commands on the client side without using separate threads, and can exploit some level of concurrency being able to do work within a session while a slower host->guest command completes. The downsides are that:

1) There is some inconsistency between this and the C API semantics.
2) The "optional" tags are basically required tags, at least for async commands, unless the client side does something to force synchronicity.

One option would be to disable the QMP session's read handler once a JSON object is received, and not re-enable it until we send the response. This would enforce FIFO-ordering. It might also add reduce the potential for a client being able to blow up our TX buffers by issuing lots of requests and not handling the responses in a timely enough manner (have seen this just from piping responses to stdout).


reply via email to

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