[PATCH v6 04/25] QmpSession: add a return callback

[Marc-André Lureau]

Introduce a return_cb to allow delaying finishing the dispatch
and sending the response asynchronously. For now, this is just
modifying qmp_dispatch() to call the callback synchronously.

Signed-off-by: Marc-André Lureau <address@hidden>
---
 include/qapi/qmp/dispatch.h | 10 ++++--
 monitor/qmp.c               | 47 ++++++++++---------------
 qapi/qmp-dispatch.c         | 22 +++++++++---
 qga/main.c                  | 34 +++++++++++-------
 tests/test-qmp-cmds.c       | 69 ++++++++++++++++++-------------------
 5 files changed, 98 insertions(+), 84 deletions(-)

diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
index 3b53cfd788..d1ce631a93 100644
--- a/include/qapi/qmp/dispatch.h
+++ b/include/qapi/qmp/dispatch.h
@@ -38,16 +38,20 @@ typedef struct QmpCommand
 typedef QTAILQ_HEAD(QmpCommandList, QmpCommand) QmpCommandList;
 
 typedef struct QmpSession QmpSession;
+typedef void (QmpDispatchReturn) (QmpSession *session, QDict *rsp);
 
 struct QmpSession {
     const QmpCommandList *cmds;
+    QmpDispatchReturn *return_cb;
 };
 
 void qmp_register_command(QmpCommandList *cmds, const char *name,
                           QmpCommandFunc *fn, QmpCommandOptions options);
 const QmpCommand *qmp_find_command(const QmpCommandList *cmds,
                                    const char *name);
-void qmp_session_init(QmpSession *session, const QmpCommandList *cmds);
+void qmp_session_init(QmpSession *session,
+                      const QmpCommandList *cmds,
+                      QmpDispatchReturn *return_cb);
 void qmp_session_destroy(QmpSession *session);
 void qmp_disable_command(QmpCommandList *cmds, const char *name);
 void qmp_enable_command(QmpCommandList *cmds, const char *name);
@@ -56,8 +60,8 @@ bool qmp_command_is_enabled(const QmpCommand *cmd);
 const char *qmp_command_name(const QmpCommand *cmd);
 bool qmp_has_success_response(const QmpCommand *cmd);
 QDict *qmp_error_response(Error *err);
-QDict *qmp_dispatch(QmpSession *session, QObject *request,
-                    bool allow_oob);
+void qmp_dispatch(QmpSession *session, QObject *request,
+                  bool allow_oob);
 bool qmp_is_oob(const QDict *dict);
 
 typedef void (*qmp_cmd_callback_fn)(const QmpCommand *cmd, void *opaque);
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 24ccdeb4a4..b215cb70f3 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -96,45 +96,35 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
     qobject_unref(json);
 }
 
-/*
- * Emit QMP response @rsp to @mon.
- * Null @rsp can only happen for commands with QCO_NO_SUCCESS_RESP.
- * Nothing is emitted then.
- */
-static void monitor_qmp_respond(MonitorQMP *mon, QDict *rsp)
+static void dispatch_return_cb(QmpSession *session, QDict *rsp)
 {
-    if (rsp) {
-        qmp_send_response(mon, rsp);
+    MonitorQMP *mon = container_of(session, MonitorQMP, session);
+
+    if (mon->session.cmds == &qmp_cap_negotiation_commands) {
+        QDict *error = qdict_get_qdict(rsp, "error");
+        if (error
+            && !g_strcmp0(qdict_get_try_str(error, "class"),
+                          QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) {
+            /* Provide a more useful error message */
+            qdict_del(error, "desc");
+            qdict_put_str(error, "desc", "Expecting capabilities negotiation"
+                          " with 'qmp_capabilities'");
+        }
     }
+
+    qmp_send_response(mon, rsp);
 }
 
 static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req)
 {
     Monitor *old_mon;
-    QDict *rsp;
-    QDict *error;
 
     old_mon = cur_mon;
     cur_mon = &mon->common;
 
-    rsp = qmp_dispatch(&mon->session, req, qmp_oob_enabled(mon));
+    qmp_dispatch(&mon->session, req, qmp_oob_enabled(mon));
 
     cur_mon = old_mon;
-
-    if (mon->session.cmds == &qmp_cap_negotiation_commands) {
-        error = qdict_get_qdict(rsp, "error");
-        if (error
-            && !g_strcmp0(qdict_get_try_str(error, "class"),
-                    QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) {
-            /* Provide a more useful error message */
-            qdict_del(error, "desc");
-            qdict_put_str(error, "desc", "Expecting capabilities negotiation"
-                          " with 'qmp_capabilities'");
-        }
-    }
-
-    monitor_qmp_respond(mon, rsp);
-    qobject_unref(rsp);
 }
 
 /*
@@ -211,7 +201,7 @@ void monitor_qmp_bh_dispatcher(void *data)
         assert(req_obj->err);
         rsp = qmp_error_response(req_obj->err);
         req_obj->err = NULL;
-        monitor_qmp_respond(mon, rsp);
+        qmp_send_response(req_obj->mon, rsp);
         qobject_unref(rsp);
     }
 
@@ -318,7 +308,8 @@ static void monitor_qmp_event(void *opaque, int event)
 
     switch (event) {
     case CHR_EVENT_OPENED:
-        qmp_session_init(&mon->session, &qmp_cap_negotiation_commands);
+        qmp_session_init(&mon->session,
+                         &qmp_cap_negotiation_commands, dispatch_return_cb);
         monitor_qmp_caps_reset(mon);
         data = qmp_greeting(mon);
         qmp_send_response(mon, data);
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 0e6b003dd8..1314f4929f 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -164,17 +164,28 @@ bool qmp_is_oob(const QDict *dict)
         && !qdict_haskey(dict, "execute");
 }
 
-void qmp_session_init(QmpSession *session, const QmpCommandList *cmds)
+void qmp_session_init(QmpSession *session,
+                      const QmpCommandList *cmds,
+                      QmpDispatchReturn *return_cb)
 {
+    assert(return_cb);
+    assert(!session->return_cb);
+
     session->cmds = cmds;
+    session->return_cb = return_cb;
 }
 
 void qmp_session_destroy(QmpSession *session)
 {
+    if (!session->return_cb) {
+        return;
+    }
+
     session->cmds = NULL;
+    session->return_cb = NULL;
 }
 
-QDict *qmp_dispatch(QmpSession *session, QObject *request, bool allow_oob)
+void qmp_dispatch(QmpSession *session, QObject *request, bool allow_oob)
 {
     Error *err = NULL;
     QDict *dict = qobject_to(QDict, request);
@@ -189,12 +200,13 @@ QDict *qmp_dispatch(QmpSession *session, QObject 
*request, bool allow_oob)
         qdict_put_obj(rsp, "return", ret);
     } else {
         /* Can only happen for commands with QCO_NO_SUCCESS_RESP */
-        rsp = NULL;
+        return;
     }
 
-    if (rsp && id) {
+    if (id) {
         qdict_put_obj(rsp, "id", qobject_ref(id));
     }
 
-    return rsp;
+    session->return_cb(session, rsp);
+    qobject_unref(rsp);
 }
diff --git a/qga/main.c b/qga/main.c
index 61190db5f3..c291d06491 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -558,29 +558,37 @@ static int send_response(GAState *s, const QDict *rsp)
     return 0;
 }
 
+static void dispatch_return_cb(QmpSession *session, QDict *rsp)
+{
+    GAState *s = container_of(session, GAState, session);
+    int ret = send_response(s, rsp);
+    if (ret < 0) {
+        g_warning("error sending response: %s", strerror(-ret));
+    }
+}
+
 /* handle requests/control events coming in over the channel */
 static void process_event(void *opaque, QObject *obj, Error *err)
 {
     GAState *s = opaque;
-    QDict *rsp;
     int ret;
 
     g_debug("process_event: called");
     assert(!obj != !err);
-    if (err) {
-        rsp = qmp_error_response(err);
-        goto end;
-    }
 
-    g_debug("processing command");
-    rsp = qmp_dispatch(&s->session, obj, false);
+    if (err) {
+        QDict *rsp = qmp_error_response(err);
 
-end:
-    ret = send_response(s, rsp);
-    if (ret < 0) {
-        g_warning("error sending error response: %s", strerror(-ret));
+        ret = send_response(s, rsp);
+        if (ret < 0) {
+            g_warning("error sending error response: %s", strerror(-ret));
+        }
+        qobject_unref(rsp);
+    } else {
+        g_debug("processing command");
+        qmp_dispatch(&s->session, obj, false);
     }
-    qobject_unref(rsp);
+
     qobject_unref(obj);
 }
 
@@ -1339,7 +1347,7 @@ static GAState *initialize_agent(GAConfig *config, int 
socket_activation)
     ga_command_state_init(s, s->command_state);
     ga_command_state_init_all(s->command_state);
     json_message_parser_init(&s->parser, process_event, s, NULL);
-    qmp_session_init(&s->session, &ga_commands);
+    qmp_session_init(&s->session, &ga_commands, dispatch_return_cb);
 
 #ifndef _WIN32
     if (!register_signal_handlers()) {
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 962c2cb2e1..cca15e79ba 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -143,22 +143,23 @@ __org_qemu_x_Union1 
*qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
     return ret;
 }
 
+static void dispatch_cmd_return(QmpSession *session, QDict *resp)
+{
+    assert(resp != NULL);
+    assert(!qdict_haskey(resp, "error"));
+}
 
 /* test commands with no input and no return value */
 static void test_dispatch_cmd(void)
 {
     QmpSession session = { 0, };
     QDict *req = qdict_new();
-    QDict *resp;
 
-    qmp_session_init(&session, &qmp_commands);
+    qmp_session_init(&session, &qmp_commands, dispatch_cmd_return);
     qdict_put_str(req, "execute", "user_def_cmd");
 
-    resp = qmp_dispatch(&session, QOBJECT(req), false);
-    assert(resp != NULL);
-    assert(!qdict_haskey(resp, "error"));
+    qmp_dispatch(&session, QOBJECT(req), false);
 
-    qobject_unref(resp);
     qobject_unref(req);
     qmp_session_destroy(&session);
 }
@@ -167,82 +168,80 @@ static void test_dispatch_cmd_oob(void)
 {
     QmpSession session = { 0, };
     QDict *req = qdict_new();
-    QDict *resp;
 
-    qmp_session_init(&session, &qmp_commands);
+    qmp_session_init(&session, &qmp_commands, dispatch_cmd_return);
     qdict_put_str(req, "exec-oob", "test-flags-command");
 
-    resp = qmp_dispatch(&session, QOBJECT(req), true);
-    assert(resp != NULL);
-    assert(!qdict_haskey(resp, "error"));
+    qmp_dispatch(&session, QOBJECT(req), true);
 
-    qobject_unref(resp);
     qobject_unref(req);
     qmp_session_destroy(&session);
 }
 
+static void dispatch_cmd_failure_return(QmpSession *session, QDict *resp)
+{
+    assert(resp != NULL);
+    assert(qdict_haskey(resp, "error"));
+}
+
 /* test commands that return an error due to invalid parameters */
 static void test_dispatch_cmd_failure(void)
 {
     QmpSession session = { 0, };
     QDict *req = qdict_new();
     QDict *args = qdict_new();
-    QDict *resp;
 
-    qmp_session_init(&session, &qmp_commands);
+    qmp_session_init(&session, &qmp_commands, dispatch_cmd_failure_return);
     qdict_put_str(req, "execute", "user_def_cmd2");
 
-    resp = qmp_dispatch(&session, QOBJECT(req), false);
-    assert(resp != NULL);
-    assert(qdict_haskey(resp, "error"));
+    qmp_dispatch(&session, QOBJECT(req), false);
 
-    qobject_unref(resp);
     qobject_unref(req);
 
     /* check that with extra arguments it throws an error */
     req = qdict_new();
     qdict_put_int(args, "a", 66);
     qdict_put(req, "arguments", args);
-
     qdict_put_str(req, "execute", "user_def_cmd");
 
-    resp = qmp_dispatch(&session, QOBJECT(req), false);
-    assert(resp != NULL);
-    assert(qdict_haskey(resp, "error"));
+    qmp_dispatch(&session, QOBJECT(req), false);
 
-    qobject_unref(resp);
     qobject_unref(req);
     qmp_session_destroy(&session);
 }
 
+static QObject *dispatch_ret;
+
 static void test_dispatch_cmd_success_response(void)
 {
     QmpSession session = { 0, };
     QDict *req = qdict_new();
-    QDict *resp;
 
-    qmp_session_init(&session, &qmp_commands);
+    qmp_session_init(&session, &qmp_commands, (QmpDispatchReturn *)abort);
     qdict_put_str(req, "execute", "cmd-success-response");
-    resp = qmp_dispatch(&session, QOBJECT(req), false);
-    g_assert_null(resp);
+
+    qmp_dispatch(&session, QOBJECT(req), false);
+
     qobject_unref(req);
     qmp_session_destroy(&session);
 }
 
+static void dispatch_return(QmpSession *session, QDict *resp)
+{
+    assert(resp && !qdict_haskey(resp, "error"));
+    dispatch_ret = qdict_get(resp, "return");
+    qobject_ref(dispatch_ret);
+}
 
 static QObject *test_qmp_dispatch(QDict *req)
 {
     QmpSession session = { 0, };
-    QDict *resp;
     QObject *ret;
 
-    qmp_session_init(&session, &qmp_commands);
-    resp = qmp_dispatch(&session, QOBJECT(req), false);
-    assert(resp && !qdict_haskey(resp, "error"));
-    ret = qdict_get(resp, "return");
-    assert(ret);
-    qobject_ref(ret);
-    qobject_unref(resp);
+    qmp_session_init(&session, &qmp_commands, dispatch_return);
+    qmp_dispatch(&session, QOBJECT(req), false);
+    ret = dispatch_ret;
+    dispatch_ret = NULL;
     qmp_session_destroy(&session);
     return ret;
 }
-- 
2.24.0