[Qemu-devel] [PATCH v5 19/20] monitor: teach HMP about asynchronous commands

[Marc-André Lureau]

Similar to how we handle both synchronous and asynchronous commands in
QMP, HMP gains a new async_cmd() that will allow the command to
complete asynchronously. For interactive reasons, and command
ordering, the HMP monitor is suspended until the asynchronous command
completes.

It is expected that HMP async commands will be implemented re-using
QMP async commands counterparts, so it reuses the QmpSession/QmpReturn
for context handling (instead of introducing HmpSession/HmpReturn and
having to convert from one to the other as we call QMP counterparts).

hmp_dispatch_return_cb() will handle printing the result to the
current monitor. It may have different ways to print the QmpReturn
result to the current monitor. Currently, only error reporting is
implemented.

QMP human-monitor-command is modified to deal with an async HMP
commands too. It creates a temporary session, and the return callback
will return asynchronously to the original QMP command and destroy the
temporary monitor when hmp->for_qmp_command is set.

Signed-off-by: Marc-André Lureau <address@hidden>
---
 include/monitor/monitor.h  |   2 +-
 monitor/hmp.c              | 110 +++++++++++++++++++++++++++++++++++--
 monitor/misc.c             |  40 --------------
 monitor/monitor-internal.h |   9 ++-
 monitor/qmp.c              |  14 +++--
 5 files changed, 123 insertions(+), 52 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 6a2907a366..5968b52fe2 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -44,6 +44,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd);
 void monitor_fdset_dup_fd_remove(int dup_fd);
 int64_t monitor_fdset_dup_fd_find(int dup_fd);
 
-Monitor *qmp_return_get_monitor(QmpReturn *qret);
+Monitor *qmp_return_get_monitor(QmpReturn *qret, bool hmp);
 
 #endif /* MONITOR_H */
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 5223661e82..b2b6dce8fb 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -26,11 +26,15 @@
 #include <dirent.h>
 #include "monitor-internal.h"
 #include "qapi/error.h"
+#include "qapi/qapi-commands-misc.h"
+#include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qnum.h"
+#include "qapi/qmp/qstring.h"
 #include "qemu/config-file.h"
 #include "qemu/ctype.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "qemu/option.h"
 #include "qemu/units.h"
@@ -38,6 +42,8 @@
 #include "sysemu/sysemu.h"
 #include "trace.h"
 
+static bool handle_hmp_command(MonitorHMP *mon, const char *cmdline);
+
 static void monitor_command_cb(void *opaque, const char *cmdline,
                                void *readline_opaque)
 {
@@ -1056,7 +1062,7 @@ fail:
     return NULL;
 }
 
-void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
+static bool handle_hmp_command(MonitorHMP *mon, const char *cmdline)
 {
     QDict *qdict;
     const HMPCommand *cmd;
@@ -1066,7 +1072,7 @@ void handle_hmp_command(MonitorHMP *mon, const char 
*cmdline)
 
     cmd = monitor_parse_command(mon, cmdline, &cmdline, hmp_cmds);
     if (!cmd) {
-        return;
+        return false;
     }
 
     qdict = monitor_parse_arguments(&mon->common, &cmdline, cmd);
@@ -1076,11 +1082,19 @@ void handle_hmp_command(MonitorHMP *mon, const char 
*cmdline)
         }
         monitor_printf(&mon->common, "Try \"help %.*s\" for more 
information\n",
                        (int)(cmdline - cmd_start), cmd_start);
-        return;
+        return false;
     }
 
-    cmd->cmd(&mon->common, qdict);
+    if (cmd->async) {
+        QmpReturn *qret = qmp_return_new(&mon->qmp_session, NULL);
+        monitor_suspend(&mon->common);
+        cmd->async_cmd(&mon->common, qdict, qret);
+    } else {
+        cmd->cmd(&mon->common, qdict);
+    }
     qobject_unref(qdict);
+
+    return cmd->async;
 }
 
 static void cmd_completion(MonitorHMP *mon, const char *name, const char *list)
@@ -1395,6 +1409,59 @@ static void monitor_readline_flush(void *opaque)
     monitor_flush(&mon->common);
 }
 
+static void free_hmp_monitor(void *opaque)
+{
+    MonitorHMP *hmp = opaque;
+
+    qmp_session_destroy(&hmp->qmp_session);
+    monitor_data_destroy(&hmp->common);
+    g_free(hmp);
+}
+
+static AioContext *monitor_get_aio_context(void)
+{
+    return iothread_get_aio_context(mon_iothread);
+}
+
+static void qmp_human_monitor_command_finish(MonitorHMP *hmp, QmpReturn *qret)
+{
+    char *output;
+
+    qemu_mutex_lock(&hmp->common.mon_lock);
+    if (qstring_get_length(hmp->common.outbuf) > 0) {
+        output = g_strdup(qstring_get_str(hmp->common.outbuf));
+    } else {
+        output = g_strdup("");
+    }
+    qemu_mutex_unlock(&hmp->common.mon_lock);
+
+    qmp_human_monitor_command_return(qret, output);
+
+    if (hmp->for_qmp_command) {
+        aio_bh_schedule_oneshot(monitor_get_aio_context(),
+                                free_hmp_monitor, hmp);
+    }
+}
+
+static void hmp_dispatch_return_cb(QmpSession *session, QDict *rsp)
+{
+    MonitorHMP *hmp = container_of(session, MonitorHMP, qmp_session);
+    QDict *err = qdict_get_qdict(rsp, "error");
+    Monitor *old_mon = cur_mon;
+
+    cur_mon = &hmp->common;
+    if (err) {
+        error_report("%s", qdict_get_str(err, "desc"));
+    } /* XXX: else, report depending on command */
+
+    if (hmp->for_qmp_command) {
+        qmp_human_monitor_command_finish(hmp, hmp->for_qmp_command);
+    } else {
+        monitor_resume(&hmp->common);
+    }
+    cur_mon = old_mon;
+}
+
 void monitor_init_hmp(Chardev *chr, bool use_readline)
 {
     MonitorHMP *mon = g_new0(MonitorHMP, 1);
@@ -1411,7 +1478,42 @@ void monitor_init_hmp(Chardev *chr, bool use_readline)
         monitor_read_command(mon, 0);
     }
 
+    qmp_session_init(&mon->qmp_session,
+                     NULL, NULL, hmp_dispatch_return_cb);
     qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read, monitor_read,
                              monitor_event, NULL, &mon->common, NULL, true);
     monitor_list_append(&mon->common);
 }
+
+void qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
+                               int64_t cpu_index, QmpReturn *qret)
+{
+    Monitor *old_mon;
+    MonitorHMP *hmp = g_new0(MonitorHMP, 1);
+
+    monitor_data_init(&hmp->common, false, true, false);
+    qmp_session_init(&hmp->qmp_session, NULL, NULL, hmp_dispatch_return_cb);
+    hmp->for_qmp_command = qret;
+
+    old_mon = cur_mon;
+    cur_mon = &hmp->common;
+
+    if (has_cpu_index) {
+        int ret = monitor_set_cpu(cpu_index);
+        if (ret < 0) {
+            Error *err = NULL;
+            error_setg(&err, QERR_INVALID_PARAMETER_VALUE, "cpu-index",
+                       "a CPU number");
+            qmp_return_error(qret, err);
+            free_hmp_monitor(hmp);
+            goto out;
+        }
+    }
+
+    if (!handle_hmp_command(hmp, command_line)) {
+        qmp_human_monitor_command_finish(hmp, qret);
+    }
+
+out:
+    cur_mon = old_mon;
+}
diff --git a/monitor/misc.c b/monitor/misc.c
index 0645667e1b..19a2919154 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -115,46 +115,6 @@ static QLIST_HEAD(, MonFdset) mon_fdsets;
 
 static HMPCommand hmp_info_cmds[];
 
-void qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
-                               int64_t cpu_index, QmpReturn *qret)
-{
-    char *output = NULL;
-    Monitor *old_mon;
-    MonitorHMP hmp = {};
-
-    monitor_data_init(&hmp.common, false, true, false);
-
-    old_mon = cur_mon;
-    cur_mon = &hmp.common;
-
-    if (has_cpu_index) {
-        int ret = monitor_set_cpu(cpu_index);
-        if (ret < 0) {
-            Error *err = NULL;
-            error_setg(&err, QERR_INVALID_PARAMETER_VALUE, "cpu-index",
-                       "a CPU number");
-            qmp_return_error(qret, err);
-            goto out;
-        }
-    }
-
-    handle_hmp_command(&hmp, command_line);
-
-    qemu_mutex_lock(&hmp.common.mon_lock);
-    if (qstring_get_length(hmp.common.outbuf) > 0) {
-        output = g_strdup(qstring_get_str(hmp.common.outbuf));
-    } else {
-        output = g_strdup("");
-    }
-    qemu_mutex_unlock(&hmp.common.mon_lock);
-
-    qmp_human_monitor_command_return(qret, output);
-
-out:
-    cur_mon = old_mon;
-    monitor_data_destroy(&hmp.common);
-}
-
 /**
  * Is @name in the '|' separated list of names @list?
  */
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 65cf668b20..cab2712187 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -73,7 +73,10 @@ typedef struct HMPCommand {
     const char *params;
     const char *help;
     const char *flags; /* p=preconfig */
-    void (*cmd)(Monitor *mon, const QDict *qdict);
+    union {
+        void (*cmd)(Monitor *mon, const QDict *qdict);
+        void (*async_cmd)(Monitor *mon, const QDict *qdict, QmpReturn *qret);
+    };
     /*
      * @sub_table is a list of 2nd level of commands. If it does not exist,
      * cmd should be used. If it exists, sub_table[?].cmd should be
@@ -81,6 +84,7 @@ typedef struct HMPCommand {
      */
     struct HMPCommand *sub_table;
     void (*command_completion)(ReadLineState *rs, int nb_args, const char 
*str);
+    bool async;
 } HMPCommand;
 
 struct Monitor {
@@ -121,6 +125,8 @@ struct MonitorHMP {
      * These members can be safely accessed without locks.
      */
     ReadLineState *rs;
+    QmpReturn *for_qmp_command;
+    QmpSession qmp_session;
 };
 
 typedef struct {
@@ -176,7 +182,6 @@ void monitor_qmp_bh_dispatcher(void *data);
 
 int get_monitor_def(int64_t *pval, const char *name);
 void help_cmd(Monitor *mon, const char *name);
-void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
 int hmp_compare_cmd(const char *name, const char *list);
 
 #endif
diff --git a/monitor/qmp.c b/monitor/qmp.c
index df8b9d8d4f..cadaf77515 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -391,13 +391,17 @@ void monitor_init_qmp(Chardev *chr, bool pretty)
     }
 }
 
-Monitor *qmp_return_get_monitor(QmpReturn *qret)
+Monitor *qmp_return_get_monitor(QmpReturn *qret, bool hmp)
 {
-    MonitorQMP *mon;
-
     if (!qret->session) {
         return NULL;
     }
-    mon = container_of(qret->session, MonitorQMP, session);
-    return &mon->common;
+
+    if (hmp) {
+        MonitorHMP *hmp = container_of(qret->session, MonitorHMP, qmp_session);
+        return &hmp->common;
+    } else {
+        MonitorQMP *qmp = container_of(qret->session, MonitorQMP, session);
+        return &qmp->common;
+    }
 }
-- 
2.22.0.428.g6d5b264208