Patchwork New API for asynchronous monitor commands (V2)

login
register
mail settings
Submitter Adam Litke
Date Jan. 25, 2010, 6:18 p.m.
Message ID <1264443524.2890.3.camel@aglitke>
Download mbox | patch
Permalink /patch/43653/
State New
Headers show

Comments

Adam Litke - Jan. 25, 2010, 6:18 p.m.
Changes since V1:
 - Miscellaneous code cleanups (Thanks Luiz)

Qemu has a number of commands that can operate asynchronously (savevm, migrate,
etc) and it will be getting more.  For these commands, the user monitor needs
to be suspended, but QMP monitors could continue to to accept other commands.
This patch introduces a new command API that isolates the details of handling
Luiz Capitulino - Jan. 26, 2010, 12:16 p.m.
On Mon, 25 Jan 2010 12:18:44 -0600
Adam Litke <agl@us.ibm.com> wrote:

> Changes since V1:
>  - Miscellaneous code cleanups (Thanks Luiz)
> 
> Qemu has a number of commands that can operate asynchronously (savevm, migrate,
> etc) and it will be getting more.  For these commands, the user monitor needs
> to be suspended, but QMP monitors could continue to to accept other commands.
> This patch introduces a new command API that isolates the details of handling
> different monitor types from the actual command execution.
> 
> A monitor command can use this API by implementing the mhandler.cmd_async
> handler (or info_async if appropriate).  This function is responsible for
> submitting the command and does not return any data although it may raise
> errors.  When the command completes, the QMPCompletion callback should be
> invoked with its opaque data and the command result.
> 
> The process for submitting and completing an asynchronous command is different
> for QMP and user monitors.  A user monitor must be suspended at submit time and
> resumed at completion time.  The user_print() function must be passed to the
> QMPCompletion callback so the result can be displayed properly.  QMP monitors
> are simpler.  No submit time setup is required.  When the command completes,
> monitor_protocol_emitter() writes the result in JSON format.

 s/QMPCompletion/MonitorCompletion

 Otherwise looks good to me.
Anthony Liguori - Jan. 27, 2010, 12:07 a.m.
On 01/25/2010 12:18 PM, Adam Litke wrote:
> Changes since V1:
>   - Miscellaneous code cleanups (Thanks Luiz)
>
> Qemu has a number of commands that can operate asynchronously (savevm, migrate,
> etc) and it will be getting more.  For these commands, the user monitor needs
> to be suspended, but QMP monitors could continue to to accept other commands.
> This patch introduces a new command API that isolates the details of handling
> different monitor types from the actual command execution.
>
> A monitor command can use this API by implementing the mhandler.cmd_async
> handler (or info_async if appropriate).  This function is responsible for
> submitting the command and does not return any data although it may raise
> errors.  When the command completes, the QMPCompletion callback should be
> invoked with its opaque data and the command result.
>
> The process for submitting and completing an asynchronous command is different
> for QMP and user monitors.  A user monitor must be suspended at submit time and
> resumed at completion time.  The user_print() function must be passed to the
> QMPCompletion callback so the result can be displayed properly.  QMP monitors
> are simpler.  No submit time setup is required.  When the command completes,
> monitor_protocol_emitter() writes the result in JSON format.
>
> This API can also be used to implement synchronous commands.  In this case, the
> cmd_async handler should immediately call the QMPCompletion callback.  It is my
> hope that this new interface will work for all commands, leading to a
> drastically simplified monitor.c once all commands are ported.
>
> Thanks to Anthony for helping me out with the initial design.
>
> Signed-off-by: Adam Litke<agl@us.ibm.com>
> To: Anthony Liguori<anthony@codemonkey.ws>
> cc: Luiz Capitulino<lcapitulino@redhat.com>
> Cc: Avi Kivity<avi@redhat.com>
> Cc: qemu-devel@nongnu.org
>    

Applied.  Thanks.

Regards,

Anthony Liguori
> diff --git a/monitor.c b/monitor.c
> index cadf422..58cd02c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -76,6 +76,12 @@
>    *
>    */
>
> +typedef struct MonitorCompletionData MonitorCompletionData;
> +struct MonitorCompletionData {
> +    Monitor *mon;
> +    void (*user_print)(Monitor *mon, const QObject *data);
> +};
> +
>   typedef struct mon_cmd_t {
>       const char *name;
>       const char *args_type;
> @@ -85,9 +91,13 @@ typedef struct mon_cmd_t {
>       union {
>           void (*info)(Monitor *mon);
>           void (*info_new)(Monitor *mon, QObject **ret_data);
> +        int  (*info_async)(Monitor *mon, MonitorCompletion *cb, void *opaque);
>           void (*cmd)(Monitor *mon, const QDict *qdict);
>           void (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data);
> +        int  (*cmd_async)(Monitor *mon, const QDict *params,
> +                          MonitorCompletion *cb, void *opaque);
>       } mhandler;
> +    int async;
>   } mon_cmd_t;
>
>   /* file descriptors passed via SCM_RIGHTS */
> @@ -255,6 +265,11 @@ static inline int monitor_handler_ported(const mon_cmd_t *cmd)
>       return cmd->user_print != NULL;
>   }
>
> +static inline bool monitor_handler_is_async(const mon_cmd_t *cmd)
> +{
> +    return cmd->async != 0;
> +}
> +
>   static inline int monitor_has_error(const Monitor *mon)
>   {
>       return mon->error != NULL;
> @@ -453,6 +468,65 @@ static void do_commit(Monitor *mon, const QDict *qdict)
>       }
>   }
>
> +static void user_monitor_complete(void *opaque, QObject *ret_data)
> +{
> +    MonitorCompletionData *data = (MonitorCompletionData *)opaque;
> +
> +    if (ret_data) {
> +        data->user_print(data->mon, ret_data);
> +    }
> +    monitor_resume(data->mon);
> +    qemu_free(data);
> +}
> +
> +static void qmp_monitor_complete(void *opaque, QObject *ret_data)
> +{
> +    monitor_protocol_emitter(opaque, ret_data);
> +}
> +
> +static void qmp_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
> +                                  const QDict *params)
> +{
> +    cmd->mhandler.cmd_async(mon, params, qmp_monitor_complete, mon);
> +}
> +
> +static void qmp_async_info_handler(Monitor *mon, const mon_cmd_t *cmd)
> +{
> +    cmd->mhandler.info_async(mon, qmp_monitor_complete, mon);
> +}
> +
> +static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
> +                                   const QDict *params)
> +{
> +    int ret;
> +
> +    MonitorCompletionData *cb_data = qemu_malloc(sizeof(*cb_data));
> +    cb_data->mon = mon;
> +    cb_data->user_print = cmd->user_print;
> +    monitor_suspend(mon);
> +    ret = cmd->mhandler.cmd_async(mon, params,
> +                                  user_monitor_complete, cb_data);
> +    if (ret<  0) {
> +        monitor_resume(mon);
> +        qemu_free(cb_data);
> +    }
> +}
> +
> +static void user_async_info_handler(Monitor *mon, const mon_cmd_t *cmd)
> +{
> +    int ret;
> +
> +    MonitorCompletionData *cb_data = qemu_malloc(sizeof(*cb_data));
> +    cb_data->mon = mon;
> +    cb_data->user_print = cmd->user_print;
> +    monitor_suspend(mon);
> +    ret = cmd->mhandler.info_async(mon, user_monitor_complete, cb_data);
> +    if (ret<  0) {
> +        monitor_resume(mon);
> +        qemu_free(cb_data);
> +    }
> +}
> +
>   static void do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
>   {
>       const mon_cmd_t *cmd;
> @@ -476,7 +550,19 @@ static void do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
>           goto help;
>       }
>
> -    if (monitor_handler_ported(cmd)) {
> +    if (monitor_handler_is_async(cmd)) {
> +        if (monitor_ctrl_mode(mon)) {
> +            qmp_async_info_handler(mon, cmd);
> +        } else {
> +            user_async_info_handler(mon, cmd);
> +        }
> +        /*
> +         * Indicate that this command is asynchronous and will not return any
> +         * data (not even empty).  Instead, the data will be returned via a
> +         * completion callback.
> +         */
> +        *ret_data = qobject_from_jsonf("{ '__mon_async': 'return' }");
> +    } else if (monitor_handler_ported(cmd)) {
>           cmd->mhandler.info_new(mon, ret_data);
>
>           if (!monitor_ctrl_mode(mon)) {
> @@ -3612,6 +3698,11 @@ static void monitor_print_error(Monitor *mon)
>       mon->error = NULL;
>   }
>
> +static int is_async_return(const QObject *data)
> +{
> +    return data&&  qdict_haskey(qobject_to_qdict(data), "__mon_async");
> +}
> +
>   static void monitor_call_handler(Monitor *mon, const mon_cmd_t *cmd,
>                                    const QDict *params)
>   {
> @@ -3619,7 +3710,15 @@ static void monitor_call_handler(Monitor *mon, const mon_cmd_t *cmd,
>
>       cmd->mhandler.cmd_new(mon, params,&data);
>
> -    if (monitor_ctrl_mode(mon)) {
> +    if (is_async_return(data)) {
> +        /*
> +         * Asynchronous commands have no initial return data but they can
> +         * generate errors.  Data is returned via the async completion handler.
> +         */
> +        if (monitor_ctrl_mode(mon)&&  monitor_has_error(mon)) {
> +            monitor_protocol_emitter(mon, NULL);
> +        }
> +    } else if (monitor_ctrl_mode(mon)) {
>           /* Monitor Protocol */
>           monitor_protocol_emitter(mon, data);
>       } else {
> @@ -3644,7 +3743,9 @@ static void handle_user_command(Monitor *mon, const char *cmdline)
>
>       qemu_errors_to_mon(mon);
>
> -    if (monitor_handler_ported(cmd)) {
> +    if (monitor_handler_is_async(cmd)) {
> +        user_async_cmd_handler(mon, cmd, qdict);
> +    } else if (monitor_handler_ported(cmd)) {
>           monitor_call_handler(mon, cmd, qdict);
>       } else {
>           cmd->mhandler.cmd(mon, qdict);
> @@ -4106,7 +4207,11 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
>           goto err_out;
>       }
>
> -    monitor_call_handler(mon, cmd, args);
> +    if (monitor_handler_is_async(cmd)) {
> +        qmp_async_cmd_handler(mon, cmd, args);
> +    } else {
> +        monitor_call_handler(mon, cmd, args);
> +    }
>       goto out;
>
>   err_input:
> diff --git a/monitor.h b/monitor.h
> index 2da30e8..b0f9270 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -44,4 +44,6 @@ void monitor_printf(Monitor *mon, const char *fmt, ...)
>   void monitor_print_filename(Monitor *mon, const char *filename);
>   void monitor_flush(Monitor *mon);
>
> +typedef void (MonitorCompletion)(void *opaque, QObject *ret_data);
> +
>   #endif /* !MONITOR_H */
>
>
>
Luiz Capitulino - Jan. 28, 2010, 6:37 p.m.
On Thu, 28 Jan 2010 19:29:22 +0100
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 25.01.2010 19:18, schrieb Adam Litke:
> > Changes since V1:
> >  - Miscellaneous code cleanups (Thanks Luiz)
> > 
> > Qemu has a number of commands that can operate asynchronously (savevm, migrate,
> > etc) and it will be getting more.  For these commands, the user monitor needs
> > to be suspended, but QMP monitors could continue to to accept other commands.
> > This patch introduces a new command API that isolates the details of handling
> > different monitor types from the actual command execution.
> > 
> > A monitor command can use this API by implementing the mhandler.cmd_async
> > handler (or info_async if appropriate).  This function is responsible for
> > submitting the command and does not return any data although it may raise
> > errors.  When the command completes, the QMPCompletion callback should be
> > invoked with its opaque data and the command result.
> > 
> > The process for submitting and completing an asynchronous command is different
> > for QMP and user monitors.  A user monitor must be suspended at submit time and
> > resumed at completion time.  The user_print() function must be passed to the
> > QMPCompletion callback so the result can be displayed properly.  QMP monitors
> > are simpler.  No submit time setup is required.  When the command completes,
> > monitor_protocol_emitter() writes the result in JSON format.
> > 
> > This API can also be used to implement synchronous commands.  In this case, the
> > cmd_async handler should immediately call the QMPCompletion callback.  It is my
> > hope that this new interface will work for all commands, leading to a
> > drastically simplified monitor.c once all commands are ported.
> > 
> > Thanks to Anthony for helping me out with the initial design.
> > 
> > Signed-off-by: Adam Litke <agl@us.ibm.com>
> 
> This commit introduces a segfault on info pci for me. The following
> patch seems to make it work again. Is it correct or is rather info pci
> doing something wrong?

 Breaks others 'info' commands as well, I've submitted a proper fix already:

http://lists.gnu.org/archive/html/qemu-devel/2010-01/msg01965.html

Patch

different monitor types from the actual command execution.

A monitor command can use this API by implementing the mhandler.cmd_async
handler (or info_async if appropriate).  This function is responsible for
submitting the command and does not return any data although it may raise
errors.  When the command completes, the QMPCompletion callback should be
invoked with its opaque data and the command result.

The process for submitting and completing an asynchronous command is different
for QMP and user monitors.  A user monitor must be suspended at submit time and
resumed at completion time.  The user_print() function must be passed to the
QMPCompletion callback so the result can be displayed properly.  QMP monitors
are simpler.  No submit time setup is required.  When the command completes,
monitor_protocol_emitter() writes the result in JSON format.

This API can also be used to implement synchronous commands.  In this case, the
cmd_async handler should immediately call the QMPCompletion callback.  It is my
hope that this new interface will work for all commands, leading to a
drastically simplified monitor.c once all commands are ported.

Thanks to Anthony for helping me out with the initial design.

Signed-off-by: Adam Litke <agl@us.ibm.com>
To: Anthony Liguori <anthony@codemonkey.ws>
cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Avi Kivity <avi@redhat.com>
Cc: qemu-devel@nongnu.org

diff --git a/monitor.c b/monitor.c
index cadf422..58cd02c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -76,6 +76,12 @@ 
  *
  */
 
+typedef struct MonitorCompletionData MonitorCompletionData;
+struct MonitorCompletionData {
+    Monitor *mon;
+    void (*user_print)(Monitor *mon, const QObject *data);
+};
+
 typedef struct mon_cmd_t {
     const char *name;
     const char *args_type;
@@ -85,9 +91,13 @@  typedef struct mon_cmd_t {
     union {
         void (*info)(Monitor *mon);
         void (*info_new)(Monitor *mon, QObject **ret_data);
+        int  (*info_async)(Monitor *mon, MonitorCompletion *cb, void *opaque);
         void (*cmd)(Monitor *mon, const QDict *qdict);
         void (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data);
+        int  (*cmd_async)(Monitor *mon, const QDict *params,
+                          MonitorCompletion *cb, void *opaque);
     } mhandler;
+    int async;
 } mon_cmd_t;
 
 /* file descriptors passed via SCM_RIGHTS */
@@ -255,6 +265,11 @@  static inline int monitor_handler_ported(const mon_cmd_t *cmd)
     return cmd->user_print != NULL;
 }
 
+static inline bool monitor_handler_is_async(const mon_cmd_t *cmd)
+{
+    return cmd->async != 0;
+}
+
 static inline int monitor_has_error(const Monitor *mon)
 {
     return mon->error != NULL;
@@ -453,6 +468,65 @@  static void do_commit(Monitor *mon, const QDict *qdict)
     }
 }
 
+static void user_monitor_complete(void *opaque, QObject *ret_data)
+{
+    MonitorCompletionData *data = (MonitorCompletionData *)opaque; 
+
+    if (ret_data) {
+        data->user_print(data->mon, ret_data);
+    }
+    monitor_resume(data->mon);
+    qemu_free(data);
+}
+
+static void qmp_monitor_complete(void *opaque, QObject *ret_data)
+{
+    monitor_protocol_emitter(opaque, ret_data);
+}
+
+static void qmp_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
+                                  const QDict *params)
+{
+    cmd->mhandler.cmd_async(mon, params, qmp_monitor_complete, mon);
+}
+
+static void qmp_async_info_handler(Monitor *mon, const mon_cmd_t *cmd)
+{
+    cmd->mhandler.info_async(mon, qmp_monitor_complete, mon);
+}
+
+static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
+                                   const QDict *params)
+{
+    int ret;
+
+    MonitorCompletionData *cb_data = qemu_malloc(sizeof(*cb_data));
+    cb_data->mon = mon;
+    cb_data->user_print = cmd->user_print;
+    monitor_suspend(mon);
+    ret = cmd->mhandler.cmd_async(mon, params,
+                                  user_monitor_complete, cb_data);
+    if (ret < 0) {
+        monitor_resume(mon);
+        qemu_free(cb_data);
+    }
+}
+
+static void user_async_info_handler(Monitor *mon, const mon_cmd_t *cmd)
+{
+    int ret;
+
+    MonitorCompletionData *cb_data = qemu_malloc(sizeof(*cb_data));
+    cb_data->mon = mon;
+    cb_data->user_print = cmd->user_print;
+    monitor_suspend(mon);
+    ret = cmd->mhandler.info_async(mon, user_monitor_complete, cb_data);
+    if (ret < 0) {
+        monitor_resume(mon);
+        qemu_free(cb_data);
+    }
+}
+
 static void do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     const mon_cmd_t *cmd;
@@ -476,7 +550,19 @@  static void do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
         goto help;
     }
 
-    if (monitor_handler_ported(cmd)) {
+    if (monitor_handler_is_async(cmd)) {
+        if (monitor_ctrl_mode(mon)) {
+            qmp_async_info_handler(mon, cmd);
+        } else {
+            user_async_info_handler(mon, cmd);
+        }
+        /*
+         * Indicate that this command is asynchronous and will not return any
+         * data (not even empty).  Instead, the data will be returned via a
+         * completion callback.
+         */
+        *ret_data = qobject_from_jsonf("{ '__mon_async': 'return' }");
+    } else if (monitor_handler_ported(cmd)) {
         cmd->mhandler.info_new(mon, ret_data);
 
         if (!monitor_ctrl_mode(mon)) {
@@ -3612,6 +3698,11 @@  static void monitor_print_error(Monitor *mon)
     mon->error = NULL;
 }
 
+static int is_async_return(const QObject *data)
+{
+    return data && qdict_haskey(qobject_to_qdict(data), "__mon_async");
+}
+
 static void monitor_call_handler(Monitor *mon, const mon_cmd_t *cmd,
                                  const QDict *params)
 {
@@ -3619,7 +3710,15 @@  static void monitor_call_handler(Monitor *mon, const mon_cmd_t *cmd,
 
     cmd->mhandler.cmd_new(mon, params, &data);
 
-    if (monitor_ctrl_mode(mon)) {
+    if (is_async_return(data)) {
+        /*
+         * Asynchronous commands have no initial return data but they can
+         * generate errors.  Data is returned via the async completion handler.
+         */
+        if (monitor_ctrl_mode(mon) && monitor_has_error(mon)) {
+            monitor_protocol_emitter(mon, NULL);
+        }
+    } else if (monitor_ctrl_mode(mon)) {
         /* Monitor Protocol */
         monitor_protocol_emitter(mon, data);
     } else {
@@ -3644,7 +3743,9 @@  static void handle_user_command(Monitor *mon, const char *cmdline)
 
     qemu_errors_to_mon(mon);
 
-    if (monitor_handler_ported(cmd)) {
+    if (monitor_handler_is_async(cmd)) {
+        user_async_cmd_handler(mon, cmd, qdict);
+    } else if (monitor_handler_ported(cmd)) {
         monitor_call_handler(mon, cmd, qdict);
     } else {
         cmd->mhandler.cmd(mon, qdict);
@@ -4106,7 +4207,11 @@  static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         goto err_out;
     }
 
-    monitor_call_handler(mon, cmd, args);
+    if (monitor_handler_is_async(cmd)) {
+        qmp_async_cmd_handler(mon, cmd, args);
+    } else {
+        monitor_call_handler(mon, cmd, args);
+    }
     goto out;
 
 err_input:
diff --git a/monitor.h b/monitor.h
index 2da30e8..b0f9270 100644
--- a/monitor.h
+++ b/monitor.h
@@ -44,4 +44,6 @@  void monitor_printf(Monitor *mon, const char *fmt, ...)
 void monitor_print_filename(Monitor *mon, const char *filename);
 void monitor_flush(Monitor *mon);
 
+typedef void (MonitorCompletion)(void *opaque, QObject *ret_data);
+
 #endif /* !MONITOR_H */