diff mbox series

[v7,10/13] hmp: Add support for coroutine command handlers

Message ID 20200909151149.490589-11-kwolf@redhat.com
State New
Headers show
Series monitor: Optionally run handlers in coroutines | expand

Commit Message

Kevin Wolf Sept. 9, 2020, 3:11 p.m. UTC 
Often, QMP command handlers are not only called to handle QMP commands,
but also from a corresponding HMP command handler. In order to give them
a consistent environment, optionally run HMP command handlers in a
coroutine, too.

The implementation is a lot simpler than in QMP because for HMP, we
still block the VM while the coroutine is running.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 monitor/monitor-internal.h |  1 +
 monitor/hmp.c              | 37 ++++++++++++++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 5 deletions(-)

Comments

Dr. David Alan Gilbert Sept. 16, 2020, 9:46 a.m. UTC | #1 
* Kevin Wolf (kwolf@redhat.com) wrote:
> Often, QMP command handlers are not only called to handle QMP commands,
> but also from a corresponding HMP command handler. In order to give them
> a consistent environment, optionally run HMP command handlers in a
> coroutine, too.
> 
> The implementation is a lot simpler than in QMP because for HMP, we
> still block the VM while the coroutine is running.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  monitor/monitor-internal.h |  1 +
>  monitor/hmp.c              | 37 ++++++++++++++++++++++++++++++++-----
>  2 files changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index b55d6df07f..ad2e64be13 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -74,6 +74,7 @@ typedef struct HMPCommand {
>      const char *help;
>      const char *flags; /* p=preconfig */
>      void (*cmd)(Monitor *mon, const QDict *qdict);
> +    bool coroutine;
>      /*
>       * @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
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 4b66ca1cd6..b858b0dbde 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -1056,12 +1056,26 @@ fail:
>      return NULL;
>  }
>  
> +typedef struct HandleHmpCommandCo {
> +    Monitor *mon;
> +    const HMPCommand *cmd;
> +    QDict *qdict;
> +    bool done;
> +} HandleHmpCommandCo;
> +
> +static void handle_hmp_command_co(void *opaque)
> +{
> +    HandleHmpCommandCo *data = opaque;
> +    data->cmd->cmd(data->mon, data->qdict);
> +    monitor_set_cur(qemu_coroutine_self(), NULL);
> +    data->done = true;
> +}
> +
>  void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
>  {
>      QDict *qdict;
>      const HMPCommand *cmd;
>      const char *cmd_start = cmdline;
> -    Monitor *old_mon;
>  
>      trace_handle_hmp_command(mon, cmdline);
>  
> @@ -1080,10 +1094,23 @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
>          return;
>      }
>  
> -    /* old_mon is non-NULL when called from qmp_human_monitor_command() */
> -    old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
> -    cmd->cmd(&mon->common, qdict);
> -    monitor_set_cur(qemu_coroutine_self(), old_mon);
> +    if (!cmd->coroutine) {
> +        /* old_mon is non-NULL when called from qmp_human_monitor_command() */
> +        Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
> +        cmd->cmd(&mon->common, qdict);
> +        monitor_set_cur(qemu_coroutine_self(), old_mon);
> +    } else {
> +        HandleHmpCommandCo data = {
> +            .mon = &mon->common,
> +            .cmd = cmd,
> +            .qdict = qdict,
> +            .done = false,
> +        };
> +        Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data);
> +        monitor_set_cur(co, &mon->common);
> +        aio_co_enter(qemu_get_aio_context(), co);
> +        AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done);
> +    }
>  
>      qobject_unref(qdict);
>  }
> -- 
> 2.25.4
>
Markus Armbruster Oct. 2, 2020, 8:01 a.m. UTC | #2 
Additional nitpick detail on Kevin's request.

Kevin Wolf <kwolf@redhat.com> writes:

> Often, QMP command handlers are not only called to handle QMP commands,

> but also from a corresponding HMP command handler. In order to give them

> a consistent environment, optionally run HMP command handlers in a

> coroutine, too.

>

> The implementation is a lot simpler than in QMP because for HMP, we

> still block the VM while the coroutine is running.

>

> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

> ---

>  monitor/monitor-internal.h |  1 +

>  monitor/hmp.c              | 37 ++++++++++++++++++++++++++++++++-----

>  2 files changed, 33 insertions(+), 5 deletions(-)

>

> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h

> index b55d6df07f..ad2e64be13 100644

> --- a/monitor/monitor-internal.h

> +++ b/monitor/monitor-internal.h

> @@ -74,6 +74,7 @@ typedef struct HMPCommand {

>      const char *help;

>      const char *flags; /* p=preconfig */

>      void (*cmd)(Monitor *mon, const QDict *qdict);

> +    bool coroutine;

>      /*

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

> diff --git a/monitor/hmp.c b/monitor/hmp.c

> index 4b66ca1cd6..b858b0dbde 100644

> --- a/monitor/hmp.c

> +++ b/monitor/hmp.c

> @@ -1056,12 +1056,26 @@ fail:

>      return NULL;

>  }

>  

> +typedef struct HandleHmpCommandCo {

> +    Monitor *mon;

> +    const HMPCommand *cmd;

> +    QDict *qdict;

> +    bool done;

> +} HandleHmpCommandCo;

> +

> +static void handle_hmp_command_co(void *opaque)

> +{

> +    HandleHmpCommandCo *data = opaque;

> +    data->cmd->cmd(data->mon, data->qdict);

> +    monitor_set_cur(qemu_coroutine_self(), NULL);

> +    data->done = true;

> +}

> +

>  void handle_hmp_command(MonitorHMP *mon, const char *cmdline)

>  {

>      QDict *qdict;

>      const HMPCommand *cmd;

>      const char *cmd_start = cmdline;

> -    Monitor *old_mon;

>  

>      trace_handle_hmp_command(mon, cmdline);

>  

> @@ -1080,10 +1094,23 @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline)

>          return;

>      }

>  

> -    /* old_mon is non-NULL when called from qmp_human_monitor_command() */

> -    old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);

> -    cmd->cmd(&mon->common, qdict);

> -    monitor_set_cur(qemu_coroutine_self(), old_mon);

> +    if (!cmd->coroutine) {

> +        /* old_mon is non-NULL when called from qmp_human_monitor_command() */

> +        Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);


Long line.  David seems fine with it, and he's the maintainer.

> +        cmd->cmd(&mon->common, qdict);

> +        monitor_set_cur(qemu_coroutine_self(), old_mon);

> +    } else {

> +        HandleHmpCommandCo data = {

> +            .mon = &mon->common,

> +            .cmd = cmd,

> +            .qdict = qdict,

> +            .done = false,

> +        };

> +        Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data);

> +        monitor_set_cur(co, &mon->common);

> +        aio_co_enter(qemu_get_aio_context(), co);

> +        AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done);

> +    }

>  

>      qobject_unref(qdict);

>  }
diff mbox series

Patch

diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index b55d6df07f..ad2e64be13 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -74,6 +74,7 @@  typedef struct HMPCommand {
     const char *help;
     const char *flags; /* p=preconfig */
     void (*cmd)(Monitor *mon, const QDict *qdict);
+    bool coroutine;
     /*
      * @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
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 4b66ca1cd6..b858b0dbde 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1056,12 +1056,26 @@  fail:
     return NULL;
 }
 
+typedef struct HandleHmpCommandCo {
+    Monitor *mon;
+    const HMPCommand *cmd;
+    QDict *qdict;
+    bool done;
+} HandleHmpCommandCo;
+
+static void handle_hmp_command_co(void *opaque)
+{
+    HandleHmpCommandCo *data = opaque;
+    data->cmd->cmd(data->mon, data->qdict);
+    monitor_set_cur(qemu_coroutine_self(), NULL);
+    data->done = true;
+}
+
 void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
 {
     QDict *qdict;
     const HMPCommand *cmd;
     const char *cmd_start = cmdline;
-    Monitor *old_mon;
 
     trace_handle_hmp_command(mon, cmdline);
 
@@ -1080,10 +1094,23 @@  void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
         return;
     }
 
-    /* old_mon is non-NULL when called from qmp_human_monitor_command() */
-    old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
-    cmd->cmd(&mon->common, qdict);
-    monitor_set_cur(qemu_coroutine_self(), old_mon);
+    if (!cmd->coroutine) {
+        /* old_mon is non-NULL when called from qmp_human_monitor_command() */
+        Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
+        cmd->cmd(&mon->common, qdict);
+        monitor_set_cur(qemu_coroutine_self(), old_mon);
+    } else {
+        HandleHmpCommandCo data = {
+            .mon = &mon->common,
+            .cmd = cmd,
+            .qdict = qdict,
+            .done = false,
+        };
+        Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, &data);
+        monitor_set_cur(co, &mon->common);
+        aio_co_enter(qemu_get_aio_context(), co);
+        AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done);
+    }
 
     qobject_unref(qdict);
 }