[Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion

[Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion

[Luiz Capitulino]

 Hi there,

 This series is an updated version of my initial QObject conversion series,
which adds the needed infrastructure to incrementally support new style
QObject handlers without breaking the current ones.

 Some people have suggested that we should have a better error handling
in the Monitor, in the meaning that error information should be correctly
propagated and handled in order to be used by the Monitor Protocol and
the existing user protocol.

 This series introduces the MonitorError data type to solve that problem,
it's used as part of the new infrastructure.

 Additionally the following handlers are converted: do_quit(), do_stop(),
do_system_reset(), do_system_powerdown(), do_balloon(), do_info_version(),
do_info_balloon().

 I've done a full build of QEMU with this series applied on Fedora 11 x86_64
and Debian Lenny i386, also tested manually all converted commands plus some
easy ones.

 Please, review this carefully as some design decisions made here will
have impact in the Monitor Protocol.

Changelog:

V0 -> V1:

- Introduced MonitorError
- More handlers converted
- Improved patches descriptions
- Minor changes

diffstat:

 Makefile        |    2 +-
 monitor-error.c |   98 ++++++++
 monitor-error.h |   67 +++++
 monitor.c       |  631 +++++++++++++++++++++++++++++++++++++++---------
 qemu-monitor.hx |  714 +++++++++++++++++++++++++++++++++++++++++++++----------
 qobject.h       |    7 +-
 6 files changed, 1273 insertions(+), 246 deletions(-)

[Qemu-devel] [PATCH 01/14] QObject: Accept NULL

[Luiz Capitulino]

It is convenient that QDECREF() and QINCREF() accept the QObject
parameter to be NULL, so that we don't duplicate 'if' tests in
the callers.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qobject.h |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/qobject.h b/qobject.h
index 39b8649..dcc8c63 100644
--- a/qobject.h
+++ b/qobject.h
@@ -63,12 +63,10 @@ typedef struct QObject {
 
 /* High-level interface for qobject_incref() */
 #define QINCREF(obj)      \
-    assert(obj != NULL);  \
     qobject_incref(QOBJECT(obj))
 
 /* High-level interface for qobject_decref() */
 #define QDECREF(obj)              \
-    assert(obj != NULL);          \
     qobject_decref(QOBJECT(obj))
 
 /* Initialize an object to default values */
@@ -81,7 +79,8 @@ typedef struct QObject {
  */
 static inline void qobject_incref(QObject *obj)
 {
-    obj->refcnt++;
+    if (obj)
+        obj->refcnt++;
 }
 
 /**
@@ -90,7 +89,7 @@ static inline void qobject_incref(QObject *obj)
  */
 static inline void qobject_decref(QObject *obj)
 {
-    if (--obj->refcnt == 0) {
+    if (obj && --obj->refcnt == 0) {
         assert(obj->type != NULL);
         assert(obj->type->destroy != NULL);
         obj->type->destroy(obj);
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 02/14] Introduce monitor-error module

[Luiz Capitulino]

This module provides an API to be used by the Monitor and its
command handlers to store error information.

The stored information will be used by both, the future Monitor
protocol emission code and the user protocol.

The API is composed of a new data type called MonitorError and
related functions.

The MonitorError data type represents an error in the Monitor,
it has to store enough information to satisfy the future Monitor
Protocol needs and at the same time maintain compability with
existing errors.

It is composed of the following members:

- code: error code. Not used by the user protocol, but will be
  used by the Monitor Protocol

- desc: error description. This is the standard error message,
  it describes the error without details

- data: error specific data. This member can hold any kind of
  information about the error, it should be used in conjuction
  with 'desc' to provide accurate error information

For example, a MonitorError for a "unknown command" error would
look like the following:

- code: 20
- desc: "unknown command"
- data: a QString containing the (wrong) command name

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile        |    2 +-
 monitor-error.c |   92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 monitor-error.h |   62 +++++++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+), 1 deletions(-)
 create mode 100644 monitor-error.c
 create mode 100644 monitor-error.h

diff --git a/Makefile b/Makefile
index d66826c..a85d4f7 100644
--- a/Makefile
+++ b/Makefile
@@ -77,7 +77,7 @@ block-obj-y +=  $(addprefix block/, $(block-nested-y))
 # CPUs and machines.
 
 obj-y = $(block-obj-y)
-obj-y += readline.o console.o
+obj-y += monitor-error.o readline.o console.o
 
 obj-y += tcg-runtime.o host-utils.o
 obj-y += irq.o ptimer.o ioport.o
diff --git a/monitor-error.c b/monitor-error.c
new file mode 100644
index 0000000..35f61b8
--- /dev/null
+++ b/monitor-error.c
@@ -0,0 +1,92 @@
+/*
+ * MonitorError: Monitor error information.
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ *
+ * Authors:
+ *  Luiz Capitulino <lcapitulino@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+#include "qemu-common.h"
+#include "monitor-error.h"
+
+/**
+ * monitor_error_new(): Allocate a new MonitorError data type
+ */
+MonitorError *monitor_error_new(void)
+{
+    return qemu_mallocz(sizeof(MonitorError));
+}
+
+/**
+ * monitor_error_free(): Destroy a MonitorError data type
+ */
+void monitor_error_free(MonitorError *error)
+{
+    QDECREF(error->code);
+    QDECREF(error->desc);
+    qobject_decref(error->data);
+
+    qemu_free(error);
+}
+
+/**
+ * set_error_desc(): Set the error description.
+ */
+static void set_error_desc(MonitorError *error)
+{
+    const char *str;
+
+    assert(error->desc == NULL);
+    assert(error->code != NULL);
+
+    switch (qint_get_int(error->code)) {
+        case MON_ERR_UNCMD:
+            str = "unknown command";
+            break;
+        case MON_ERR_EXPFILE:
+            str = "filename expected";
+            break;
+        case MON_ERR_EXPBLK:
+            str = "block device name expected";
+            break;
+        case MON_ERR_EXPSTR:
+            str = "string expected";
+            break;
+        case MON_ERR_INVCHAR:
+            str = "invalid char in format";
+            break;
+        case MON_ERR_INVINT:
+            str = "integer is for 32-bit values";
+            break;
+        case MON_ERR_UNSOPT:
+            str = "unsopported option";
+            break;
+        case MON_ERR_UNTYPE:
+            str = "unknown type";
+            break;
+        case MON_ERR_EXTCHAR:
+            str = "extraneous characters at the end of line";
+            break;
+        default:
+            str = "unknown";
+            break;
+    }
+
+    error->desc = qstring_from_str(str);
+}
+
+/**
+ * monitor_error_set(): Setup a MonitorError data type.
+ */
+void monitor_error_set(MonitorError *error, int code, QObject *data,
+                       QString *desc)
+{
+    error->code = qint_from_int(code);
+    error->data = data;
+    error->desc = desc;
+    if (!error->desc)
+        set_error_desc(error);
+}
diff --git a/monitor-error.h b/monitor-error.h
new file mode 100644
index 0000000..cd2f82c
--- /dev/null
+++ b/monitor-error.h
@@ -0,0 +1,62 @@
+/*
+ * MonitorError header file.
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ *
+ * Authors:
+ *  Luiz Capitulino <lcapitulino@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef MONITOR_ERROR_H
+#define MONITOR_ERROR_H
+
+#include "qobject.h"
+#include "qstring.h"
+#include "qint.h"
+
+typedef enum MonitorErrCodes {
+    MON_ERR_UNCMD = 20,
+    MON_ERR_EXPFILE,
+    MON_ERR_EXPBLK,
+    MON_ERR_EXPSTR,
+    MON_ERR_INVCHAR,
+    MON_ERR_INVINT,
+    MON_ERR_UNSOPT,
+    MON_ERR_UNTYPE,
+    MON_ERR_EXTCHAR,
+} MonitorErrCodes;
+
+typedef struct MonitorError {
+    QInt *code;     /* error code */
+    QString *desc;  /* 'global' error description */
+    QObject *data;  /* error specific data */
+} MonitorError;
+
+MonitorError *monitor_error_new(void);
+void monitor_error_free(MonitorError *error);
+void monitor_error_set(MonitorError *error, int code, QObject *data,
+                       QString *desc);
+
+/**
+ * monitor_has_error(): Check if MonitorError is set.
+ *
+ * Return true if MonitorError holds an error, false otherwise.
+ */
+static inline int monitor_has_error(const MonitorError *error)
+{
+    return error->code != NULL;
+}
+
+/**
+ * monitor_error_set_code(): Set an error code.
+ *
+ * Wrapper for monitor_error_set().
+ */
+static inline void monitor_error_set_code(MonitorError *error, int code)
+{
+    monitor_error_set(error, code, NULL, NULL);
+}
+#endif /* MONITOR_ERROR_H */
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 03/14] monitor: Add new members to mon_cmd_t

[Luiz Capitulino]

Both are function pointers, which will be used to output data by
the user protocol format in success and error conditions.

They are:

 - user_print(): Print handler's data on success
 - user_error(): Print error information on error condition

user_print() is also going to be used as a flag to say if a given
handler has already been converted to the new QObject style.

Additionally, this commit converts info_cmd initialization to
the C99 way.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |  385 ++++++++++++++++++++++++------
 qemu-monitor.hx |  714 +++++++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 909 insertions(+), 190 deletions(-)

diff --git a/monitor.c b/monitor.c
index f105a2e..5cb6e81 100644
--- a/monitor.c
+++ b/monitor.c
@@ -34,6 +34,7 @@
 #include "qemu-char.h"
 #include "sysemu.h"
 #include "monitor.h"
+#include "monitor-error.h"
 #include "readline.h"
 #include "console.h"
 #include "block.h"
@@ -71,6 +72,8 @@ typedef struct mon_cmd_t {
     const char *name;
     const char *args_type;
     void *handler;
+    void (*user_print)(Monitor *mon, const QObject *data);
+    void (*user_error)(Monitor *mon, const MonitorError *error);
     const char *params;
     const char *help;
 } mon_cmd_t;
@@ -1812,82 +1815,328 @@ static const mon_cmd_t mon_cmds[] = {
 
 /* Please update qemu-monitor.hx when adding or changing commands */
 static const mon_cmd_t info_cmds[] = {
-    { "version", "", do_info_version,
-      "", "show the version of QEMU" },
-    { "network", "", do_info_network,
-      "", "show the network state" },
-    { "chardev", "", qemu_chr_info,
-      "", "show the character devices" },
-    { "block", "", bdrv_info,
-      "", "show the block devices" },
-    { "blockstats", "", bdrv_info_stats,
-      "", "show block device statistics" },
-    { "registers", "", do_info_registers,
-      "", "show the cpu registers" },
-    { "cpus", "", do_info_cpus,
-      "", "show infos for each CPU" },
-    { "history", "", do_info_history,
-      "", "show the command line history", },
-    { "irq", "", irq_info,
-      "", "show the interrupts statistics (if available)", },
-    { "pic", "", pic_info,
-      "", "show i8259 (PIC) state", },
-    { "pci", "", pci_info,
-      "", "show PCI info", },
+    {
+        .name       = "version",
+        .args_type  = "",
+        .handler    = do_info_version,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the version of QEMU"
+    },
+    {
+        .name       = "network",
+        .args_type  = "",
+        .handler    = do_info_network,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the network state"
+    },
+    {
+        .name       = "chardev",
+        .args_type  = "",
+        .handler    = qemu_chr_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the character devices"
+    },
+    {
+        .name       = "block",
+        .args_type  = "",
+        .handler    = bdrv_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the block devices"
+    },
+    {
+        .name       = "blockstats",
+        .args_type  = "",
+        .handler    = bdrv_info_stats,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show block device statistics"
+    },
+    {
+        .name       = "registers",
+        .args_type  = "",
+        .handler    = do_info_registers,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the cpu registers"
+    },
+    {
+        .name       = "cpus",
+        .args_type  = "",
+        .handler    = do_info_cpus,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show infos for each CPU"
+    },
+    {
+        .name       = "history",
+        .args_type  = "",
+        .handler    = do_info_history,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the command line history"
+    },
+    {
+        .name       = "irq",
+        .args_type  = "",
+        .handler    = irq_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the interrupts statistics (if available)"
+    },
+    {
+        .name       = "pic",
+        .args_type  = "",
+        .handler    = pic_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show i8259 (PIC) state"
+    },
+    {
+        .name       = "pci",
+        .args_type  = "",
+        .handler    = pci_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show PCI info"
+    },
 #if defined(TARGET_I386) || defined(TARGET_SH4)
-    { "tlb", "", tlb_info,
-      "", "show virtual to physical memory mappings", },
+    {
+        .name       = "tlb",
+        .args_type  = "",
+        .handler    = tlb_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show virtual to physical memory mappings"
+    },
 #endif
 #if defined(TARGET_I386)
-    { "mem", "", mem_info,
-      "", "show the active virtual memory mappings", },
-    { "hpet", "", do_info_hpet,
-      "", "show state of HPET", },
+    {
+        .name       = "mem",
+        .args_type  = "",
+        .handler    = mem_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the active virtual memory mappings"
+    },
+    {
+        .name       = "hpet",
+        .args_type  = "",
+        .handler    = do_info_hpet,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show state of HPET"
+    },
 #endif
-    { "jit", "", do_info_jit,
-      "", "show dynamic compiler info", },
-    { "kvm", "", do_info_kvm,
-      "", "show KVM information", },
-    { "numa", "", do_info_numa,
-      "", "show NUMA information", },
-    { "usb", "", usb_info,
-      "", "show guest USB devices", },
-    { "usbhost", "", usb_host_info,
-      "", "show host USB devices", },
-    { "profile", "", do_info_profile,
-      "", "show profiling information", },
-    { "capture", "", do_info_capture,
-      "", "show capture information" },
-    { "snapshots", "", do_info_snapshots,
-      "", "show the currently saved VM snapshots" },
-    { "status", "", do_info_status,
-      "", "show the current VM status (running|paused)" },
-    { "pcmcia", "", pcmcia_info,
-      "", "show guest PCMCIA status" },
-    { "mice", "", do_info_mice,
-      "", "show which guest mouse is receiving events" },
-    { "vnc", "", do_info_vnc,
-      "", "show the vnc server status"},
-    { "name", "", do_info_name,
-      "", "show the current VM name" },
-    { "uuid", "", do_info_uuid,
-      "", "show the current VM UUID" },
+    {
+        .name       = "jit",
+        .args_type  = "",
+        .handler    = do_info_jit,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show dynamic compiler info"
+    },
+    {
+        .name       = "kvm",
+        .args_type  = "",
+        .handler    = do_info_kvm,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show KVM information"
+    },
+    {
+        .name       = "numa",
+        .args_type  = "",
+        .handler    = do_info_numa,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show NUMA information"
+    },
+    {
+        .name       = "usb",
+        .args_type  = "",
+        .handler    = usb_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show guest USB devices"
+    },
+    {
+        .name       = "usbhost",
+        .args_type  = "",
+        .handler    = usb_host_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show host USB devices"
+    },
+    {
+        .name       = "profile",
+        .args_type  = "",
+        .handler    = do_info_profile,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show profiling information"
+    },
+    {
+        .name       = "capture",
+        .args_type  = "",
+        .handler    = do_info_capture,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show capture information"
+    },
+    {
+        .name       = "snapshots",
+        .args_type  = "",
+        .handler    = do_info_snapshots,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the currently saved VM snapshots"
+    },
+    {
+        .name       = "status",
+        .args_type  = "",
+        .handler    = do_info_status,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the current VM status (running|paused)"
+    },
+    {
+        .name       = "pcmcia",
+        .args_type  = "",
+        .handler    = pcmcia_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show guest PCMCIA status"
+    },
+    {
+        .name       = "mice",
+        .args_type  = "",
+        .handler    = do_info_mice,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show which guest mouse is receiving events"
+    },
+    {
+        .name       = "vnc",
+        .args_type  = "",
+        .handler    = do_info_vnc,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the vnc server status"
+    },
+    {
+        .name       = "name",
+        .args_type  = "",
+        .handler    = do_info_name,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the current VM name"
+    },
+    {
+        .name       = "uuid",
+        .args_type  = "",
+        .handler    = do_info_uuid,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show the current VM UUID"
+    },
 #if defined(TARGET_PPC)
-    { "cpustats", "", do_info_cpu_stats,
-      "", "show CPU statistics", },
+    {
+        .name       = "cpustats",
+        .args_type  = "",
+        .handler    = do_info_cpu_stats,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show CPU statistics"
+    },
 #endif
 #if defined(CONFIG_SLIRP)
-    { "usernet", "", do_info_usernet,
-      "", "show user network stack connection states", },
+    {
+        .name       = "usernet",
+        .args_type  = "",
+        .handler    = do_info_usernet,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show user network stack connection states"
+    },
 #endif
-    { "migrate", "", do_info_migrate, "", "show migration status" },
-    { "balloon", "", do_info_balloon,
-      "", "show balloon information" },
-    { "qtree", "", do_info_qtree,
-      "", "show device tree" },
-    { "qdm", "", do_info_qdm,
-      "", "show qdev device model list" },
-    { NULL, NULL, },
+    {
+        .name       = "migrate",
+        .args_type  = "",
+        .handler    = do_info_migrate,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show migration status"
+    },
+    {
+        .name       = "balloon",
+        .args_type  = "",
+        .handler    = do_info_balloon,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show balloon information"
+    },
+    {
+        .name       = "qtree",
+        .args_type  = "",
+        .handler    = do_info_qtree,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show device tree"
+    },
+    {
+        .name       = "qdm",
+        .args_type  = "",
+        .handler    = do_info_qdm,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "show qdev device model list"
+    },
+    {
+        .name       = NULL,
+        .args_type  = NULL,
+        .handler    = NULL,
+        .user_print = NULL,
+        .params     = NULL,
+        .help       = NULL
+    },
 };
 
 /*******************************************************************/
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 6aa99ce..0da810a 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -9,21 +9,46 @@ STEXI
 @table @option
 ETEXI
 
-    { "help|?", "name:s?", do_help_cmd, "[cmd]", "show the help" },
+    {
+        .name       = "help|?",
+        .args_type  = "name:s?",
+        .handler    = do_help_cmd,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[cmd]",
+        .help       = "show the help"
+    },
+
 STEXI
 @item help or ? [@var{cmd}]
 Show the help for all commands or just for command @var{cmd}.
 ETEXI
 
-    { "commit", "device:B", do_commit,
-      "device|all", "commit changes to the disk images (if -snapshot is used) or backing files" },
+    {
+        .name       = "commit",
+        .args_type  = "device:B",
+        .handler    = do_commit,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "device|all",
+        .help       = "commit changes to the disk images (if -snapshot is used) or backing files"
+    },
+
 STEXI
 @item commit
 Commit changes to the disk images (if -snapshot is used) or backing files.
 ETEXI
 
-    { "info", "item:s?", do_info,
-      "[subcommand]", "show various information about the system state" },
+    {
+        .name       = "info",
+        .args_type  = "item:s?",
+        .handler    = do_info,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[subcommand]",
+        .help       = "show various information about the system state"
+    },
+
 STEXI
 @item info @var{subcommand}
 Show various information about the system state.
@@ -94,22 +119,46 @@ show device tree
 @end table
 ETEXI
 
-    { "q|quit", "", do_quit,
-      "", "quit the emulator" },
+    {
+        .name       = "q|quit",
+        .args_type  = "",
+        .handler    = do_quit,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "quit the emulator"
+    },
+
 STEXI
 @item q or quit
 Quit the emulator.
 ETEXI
 
-    { "eject", "force:-f,filename:B", do_eject,
-      "[-f] device", "eject a removable medium (use -f to force it)" },
+    {
+        .name       = "eject",
+        .args_type  = "force:-f,filename:B",
+        .handler    = do_eject,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[-f] device",
+        .help       = "eject a removable medium (use -f to force it)"
+    },
+
 STEXI
 @item eject [-f] @var{device}
 Eject a removable medium (use -f to force it).
 ETEXI
 
-    { "change", "device:B,target:F,arg:s?", do_change,
-      "device filename [format]", "change a removable medium, optional format" },
+    {
+        .name       = "change",
+        .args_type  = "device:B,target:F,arg:s?",
+        .handler    = do_change,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "device filename [format]",
+        .help       = "change a removable medium, optional format"
+    },
+
 STEXI
 @item change @var{device} @var{setting}
 
@@ -147,29 +196,61 @@ Password: ********
 @end table
 ETEXI
 
-    { "screendump", "filename:F", do_screen_dump,
-      "filename", "save screen into PPM image 'filename'" },
+    {
+        .name       = "screendump",
+        .args_type  = "filename:F",
+        .handler    = do_screen_dump,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "filename",
+        .help       = "save screen into PPM image 'filename'"
+    },
+
 STEXI
 @item screendump @var{filename}
 Save screen into PPM image @var{filename}.
 ETEXI
 
-    { "logfile", "filename:F", do_logfile,
-      "filename", "output logs to 'filename'" },
+    {
+        .name       = "logfile",
+        .args_type  = "filename:F",
+        .handler    = do_logfile,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "filename",
+        .help       = "output logs to 'filename'"
+    },
+
 STEXI
 @item logfile @var{filename}
 Output logs to @var{filename}.
 ETEXI
 
-    { "log", "items:s", do_log,
-      "item1[,...]", "activate logging of the specified items to '/tmp/qemu.log'" },
+    {
+        .name       = "log",
+        .args_type  = "items:s",
+        .handler    = do_log,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "item1[,...]",
+        .help       = "activate logging of the specified items to '/tmp/qemu.log'"
+    },
+
 STEXI
 @item log @var{item1}[,...]
 Activate logging of the specified items to @file{/tmp/qemu.log}.
 ETEXI
 
-    { "savevm", "name:s?", do_savevm,
-      "[tag|id]", "save a VM snapshot. If no tag or id are provided, a new snapshot is created" },
+    {
+        .name       = "savevm",
+        .args_type  = "name:s?",
+        .handler    = do_savevm,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[tag|id]",
+        .help       = "save a VM snapshot. If no tag or id are provided, a new snapshot is created"
+    },
+
 STEXI
 @item savevm [@var{tag}|@var{id}]
 Create a snapshot of the whole virtual machine. If @var{tag} is
@@ -178,59 +259,123 @@ a snapshot with the same tag or ID, it is replaced. More info at
 @ref{vm_snapshots}.
 ETEXI
 
-    { "loadvm", "name:s", do_loadvm,
-      "tag|id", "restore a VM snapshot from its tag or id" },
+    {
+        .name       = "loadvm",
+        .args_type  = "name:s",
+        .handler    = do_loadvm,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "tag|id",
+        .help       = "restore a VM snapshot from its tag or id"
+    },
+
 STEXI
 @item loadvm @var{tag}|@var{id}
 Set the whole virtual machine to the snapshot identified by the tag
 @var{tag} or the unique snapshot ID @var{id}.
 ETEXI
 
-    { "delvm", "name:s", do_delvm,
-      "tag|id", "delete a VM snapshot from its tag or id" },
+    {
+        .name       = "delvm",
+        .args_type  = "name:s",
+        .handler    = do_delvm,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "tag|id",
+        .help       = "delete a VM snapshot from its tag or id"
+    },
+
 STEXI
 @item delvm @var{tag}|@var{id}
 Delete the snapshot identified by @var{tag} or @var{id}.
 ETEXI
 
-    { "singlestep", "option:s?", do_singlestep,
-      "[on|off]", "run emulation in singlestep mode or switch to normal mode", },
+    {
+        .name       = "singlestep",
+        .args_type  = "option:s?",
+        .handler    = do_singlestep,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[on|off]",
+        .help       = "run emulation in singlestep mode or switch to normal mode",
+    },
+
 STEXI
 @item singlestep [off]
 Run the emulation in single step mode.
 If called with option off, the emulation returns to normal mode.
 ETEXI
 
-    { "stop", "", do_stop,
-      "", "stop emulation", },
+    {
+        .name       = "stop",
+        .args_type  = "",
+        .handler    = do_stop,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "stop emulation",
+    },
+
 STEXI
 @item stop
 Stop emulation.
 ETEXI
 
-    { "c|cont", "", do_cont,
-      "", "resume emulation", },
+    {
+        .name       = "c|cont",
+        .args_type  = "",
+        .handler    = do_cont,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "resume emulation",
+    },
+
 STEXI
 @item c or cont
 Resume emulation.
 ETEXI
 
-    { "gdbserver", "device:s?", do_gdbserver,
-      "[device]", "start gdbserver on given device (default 'tcp::1234'), stop with 'none'", },
+    {
+        .name       = "gdbserver",
+        .args_type  = "device:s?",
+        .handler    = do_gdbserver,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[device]",
+        .help       = "start gdbserver on given device (default 'tcp::1234'), stop with 'none'",
+    },
+
 STEXI
 @item gdbserver [@var{port}]
 Start gdbserver session (default @var{port}=1234)
 ETEXI
 
-    { "x", "fmt:/,addr:l", do_memory_dump,
-      "/fmt addr", "virtual memory dump starting at 'addr'", },
+    {
+        .name       = "x",
+        .args_type  = "fmt:/,addr:l",
+        .handler    = do_memory_dump,
+        .handler    = NULL,
+        .user_error = NULL,
+        .params     = "/fmt addr",
+        .help       = "virtual memory dump starting at 'addr'",
+    },
+
 STEXI
 @item x/fmt @var{addr}
 Virtual memory dump starting at @var{addr}.
 ETEXI
 
-    { "xp", "fmt:/,addr:l", do_physical_memory_dump,
-      "/fmt addr", "physical memory dump starting at 'addr'", },
+    {
+        .name       = "xp",
+        .args_type  = "fmt:/,addr:l",
+        .handler    = do_physical_memory_dump,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "/fmt addr",
+        .help       = "physical memory dump starting at 'addr'",
+    },
+
 STEXI
 @item xp /@var{fmt} @var{addr}
 Physical memory dump starting at @var{addr}.
@@ -289,8 +434,16 @@ Dump 80 16 bit values at the start of the video memory.
 @end itemize
 ETEXI
 
-    { "p|print", "fmt:/,val:l", do_print,
-      "/fmt expr", "print expression value (use $reg for CPU register access)", },
+    {
+        .name       = "p|print",
+        .args_type  = "fmt:/,val:l",
+        .handler    = do_print,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "/fmt expr",
+        .help       = "print expression value (use $reg for CPU register access)",
+    },
+
 STEXI
 @item p or print/@var{fmt} @var{expr}
 
@@ -298,20 +451,44 @@ Print expression value. Only the @var{format} part of @var{fmt} is
 used.
 ETEXI
 
-    { "i", "fmt:/,addr:i,index:i.", do_ioport_read,
-      "/fmt addr", "I/O port read" },
+    {
+        .name       = "i",
+        .args_type  = "fmt:/,addr:i,index:i.",
+        .handler    = do_ioport_read,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "/fmt addr",
+        .help       = "I/O port read"
+    },
+
 STEXI
 Read I/O port.
 ETEXI
 
-    { "o", "fmt:/,addr:i,val:i", do_ioport_write,
-      "/fmt addr value", "I/O port write" },
+    {
+        .name       = "o",
+        .args_type  = "fmt:/,addr:i,val:i",
+        .handler    = do_ioport_write,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "/fmt addr value",
+        .help       = "I/O port write"
+    },
+
 STEXI
 Write to I/O port.
 ETEXI
 
-    { "sendkey", "string:s,hold_time:i?", do_sendkey,
-      "keys [hold_ms]", "send keys to the VM (e.g. 'sendkey ctrl-alt-f1', default hold time=100 ms)" },
+    {
+        .name       = "sendkey",
+        .args_type  = "string:s,hold_time:i?",
+        .handler    = do_sendkey,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "keys [hold_ms]",
+        .help       = "send keys to the VM (e.g. 'sendkey ctrl-alt-f1', default hold time=100 ms)"
+    },
+
 STEXI
 @item sendkey @var{keys}
 
@@ -326,32 +503,64 @@ This command is useful to send keys that your graphical user interface
 intercepts at low level, such as @code{ctrl-alt-f1} in X Window.
 ETEXI
 
-    { "system_reset", "", do_system_reset,
-      "", "reset the system" },
+    {
+        .name       = "system_reset",
+        .args_type  = "",
+        .handler    = do_system_reset,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "reset the system"
+    },
+
 STEXI
 @item system_reset
 
 Reset the system.
 ETEXI
 
-    { "system_powerdown", "", do_system_powerdown,
-      "", "send system power down event" },
+    {
+        .name       = "system_powerdown",
+        .args_type  = "",
+        .handler    = do_system_powerdown,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "send system power down event"
+    },
+
 STEXI
 @item system_powerdown
 
 Power down the system (if supported).
 ETEXI
 
-    { "sum", "start:i,size:i", do_sum,
-      "addr size", "compute the checksum of a memory region" },
+    {
+        .name       = "sum",
+        .args_type  = "start:i,size:i",
+        .handler    = do_sum,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "addr size",
+        .help       = "compute the checksum of a memory region"
+    },
+
 STEXI
 @item sum @var{addr} @var{size}
 
 Compute the checksum of a memory region.
 ETEXI
 
-    { "usb_add", "devname:s", do_usb_add,
-      "device", "add USB device (e.g. 'host:bus.addr' or 'host:vendor_id:product_id')" },
+    {
+        .name       = "usb_add",
+        .args_type  = "devname:s",
+        .handler    = do_usb_add,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "device",
+        .help       = "add USB device (e.g. 'host:bus.addr' or 'host:vendor_id:product_id')"
+    },
+
 STEXI
 @item usb_add @var{devname}
 
@@ -359,8 +568,16 @@ Add the USB device @var{devname}.  For details of available devices see
 @ref{usb_devices}
 ETEXI
 
-    { "usb_del", "devname:s", do_usb_del,
-      "device", "remove USB device 'bus.addr'" },
+    {
+        .name       = "usb_del",
+        .args_type  = "devname:s",
+        .handler    = do_usb_del,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "device",
+        .help       = "remove USB device 'bus.addr'"
+    },
+
 STEXI
 @item usb_del @var{devname}
 
@@ -369,29 +586,61 @@ hub. @var{devname} has the syntax @code{bus.addr}. Use the monitor
 command @code{info usb} to see the devices you can remove.
 ETEXI
 
-    { "cpu", "index:i", do_cpu_set,
-      "index", "set the default CPU" },
+    {
+        .name       = "cpu",
+        .args_type  = "index:i",
+        .handler    = do_cpu_set,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "index",
+        .help       = "set the default CPU"
+    },
+
 STEXI
 Set the default CPU.
 ETEXI
 
-    { "mouse_move", "dx_str:s,dy_str:s,dz_str:s?", do_mouse_move,
-      "dx dy [dz]", "send mouse move events" },
+    {
+        .name       = "mouse_move",
+        .args_type  = "dx_str:s,dy_str:s,dz_str:s?",
+        .handler    = do_mouse_move,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "dx dy [dz]",
+        .help       = "send mouse move events"
+    },
+
 STEXI
 @item mouse_move @var{dx} @var{dy} [@var{dz}]
 Move the active mouse to the specified coordinates @var{dx} @var{dy}
 with optional scroll axis @var{dz}.
 ETEXI
 
-    { "mouse_button", "button_state:i", do_mouse_button,
-      "state", "change mouse button state (1=L, 2=M, 4=R)" },
+    {
+        .name       = "mouse_button",
+        .args_type  = "button_state:i",
+        .handler    = do_mouse_button,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "state",
+        .help       = "change mouse button state (1=L, 2=M, 4=R)"
+    },
+
 STEXI
 @item mouse_button @var{val}
 Change the active mouse button state @var{val} (1=L, 2=M, 4=R).
 ETEXI
 
-    { "mouse_set", "index:i", do_mouse_set,
-      "index", "set which mouse device receives events" },
+    {
+        .name       = "mouse_set",
+        .args_type  = "index:i",
+        .handler    = do_mouse_set,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "index",
+        .help       = "set which mouse device receives events"
+    },
+
 STEXI
 @item mouse_set @var{index}
 Set which mouse device receives events at given @var{index}, index
@@ -402,10 +651,17 @@ info mice
 ETEXI
 
 #ifdef HAS_AUDIO
-    { "wavcapture", "path:F,freq:i?,bits:i?,nchannels:i?", do_wav_capture,
-      "path [frequency [bits [channels]]]",
-      "capture audio to a wave file (default frequency=44100 bits=16 channels=2)" },
+    {
+        .name       = "wavcapture",
+        .args_type  = "path:F,freq:i?,bits:i?,nchannels:i?",
+        .handler    = do_wav_capture,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "path [frequency [bits [channels]]]",
+        .help       = "capture audio to a wave file (default frequency=44100 bits=16 channels=2)"
+    },
 #endif
+
 STEXI
 @item wavcapture @var{filename} [@var{frequency} [@var{bits} [@var{channels}]]]
 Capture audio into @var{filename}. Using sample rate @var{frequency}
@@ -420,9 +676,17 @@ Defaults:
 ETEXI
 
 #ifdef HAS_AUDIO
-    { "stopcapture", "n:i", do_stop_capture,
-      "capture index", "stop capture" },
+    {
+        .name       = "stopcapture",
+        .args_type  = "n:i",
+        .handler    = do_stop_capture,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "capture index",
+        .help       = "stop capture"
+    },
 #endif
+
 STEXI
 @item stopcapture @var{index}
 Stop capture with a given @var{index}, index can be obtained with
@@ -431,22 +695,46 @@ info capture
 @end example
 ETEXI
 
-    { "memsave", "val:l,size:i,filename:s", do_memory_save,
-      "addr size file", "save to disk virtual memory dump starting at 'addr' of size 'size'", },
+    {
+        .name       = "memsave",
+        .args_type  = "val:l,size:i,filename:s",
+        .handler    = do_memory_save,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "addr size file",
+        .help       = "save to disk virtual memory dump starting at 'addr' of size 'size'",
+    },
+
 STEXI
 @item memsave @var{addr} @var{size} @var{file}
 save to disk virtual memory dump starting at @var{addr} of size @var{size}.
 ETEXI
 
-    { "pmemsave", "val:l,size:i,filename:s", do_physical_memory_save,
-      "addr size file", "save to disk physical memory dump starting at 'addr' of size 'size'", },
+    {
+        .name       = "pmemsave",
+        .args_type  = "val:l,size:i,filename:s",
+        .handler    = do_physical_memory_save,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "addr size file",
+        .help       = "save to disk physical memory dump starting at 'addr' of size 'size'",
+    },
+
 STEXI
 @item pmemsave @var{addr} @var{size} @var{file}
 save to disk physical memory dump starting at @var{addr} of size @var{size}.
 ETEXI
 
-    { "boot_set", "bootdevice:s", do_boot_set,
-      "bootdevice", "define new values for the boot device list" },
+    {
+        .name       = "boot_set",
+        .args_type  = "bootdevice:s",
+        .handler    = do_boot_set,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "bootdevice",
+        .help       = "define new values for the boot device list"
+    },
+
 STEXI
 @item boot_set @var{bootdevicelist}
 
@@ -458,37 +746,76 @@ the same that can be specified in the @code{-boot} command line option.
 ETEXI
 
 #if defined(TARGET_I386)
-    { "nmi", "cpu_index:i", do_inject_nmi,
-      "cpu", "inject an NMI on the given CPU", },
+    {
+        .name       = "nmi",
+        .args_type  = "cpu_index:i",
+        .handler    = do_inject_nmi,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "cpu",
+        .help       = "inject an NMI on the given CPU"
+    },
 #endif
+
 STEXI
 @item nmi @var{cpu}
 Inject an NMI on the given CPU (x86 only).
 ETEXI
 
-    { "migrate", "detach:-d,uri:s", do_migrate,
-      "[-d] uri", "migrate to URI (using -d to not wait for completion)" },
+    {
+        .name       = "migrate",
+        .args_type  = "detach:-d,uri:s",
+        .handler    = do_migrate,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[-d] uri",
+        .help       = "migrate to URI (using -d to not wait for completion)"
+    },
+
 STEXI
 @item migrate [-d] @var{uri}
 Migrate to @var{uri} (using -d to not wait for completion).
 ETEXI
 
-    { "migrate_cancel", "", do_migrate_cancel,
-      "", "cancel the current VM migration" },
+    {
+        .name       = "migrate_cancel",
+        .args_type  = "",
+        .handler    = do_migrate_cancel,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "",
+        .help       = "cancel the current VM migration"
+    },
+
 STEXI
 @item migrate_cancel
 Cancel the current VM migration.
 ETEXI
 
-    { "migrate_set_speed", "value:s", do_migrate_set_speed,
-      "value", "set maximum speed (in bytes) for migrations" },
+    {
+        .name       = "migrate_set_speed",
+        .args_type  = "value:s",
+        .handler    = do_migrate_set_speed,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "value",
+        .help       = "set maximum speed (in bytes) for migrations"
+    },
+
 STEXI
 @item migrate_set_speed @var{value}
 Set maximum speed to @var{value} (in bytes) for migrations.
 ETEXI
 
-    { "migrate_set_downtime", "value:s", do_migrate_set_downtime,
-      "value", "set maximum tolerated downtime (in seconds) for migrations" },
+    {
+        .name       = "migrate_set_downtime",
+        .args_type  = "value:s",
+        .handler    = do_migrate_set_downtime,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "value",
+        .help       = "set maximum tolerated downtime (in seconds) for migrations"
+    },
 
 STEXI
 @item migrate_set_downtime @var{second}
@@ -496,85 +823,170 @@ Set maximum tolerated downtime (in seconds) for migration.
 ETEXI
 
 #if defined(TARGET_I386)
-    { "drive_add", "pci_addr:s,opts:s", drive_hot_add,
-                                        "[[<domain>:]<bus>:]<slot>\n"
-                                        "[file=file][,if=type][,bus=n]\n"
-                                        "[,unit=m][,media=d][index=i]\n"
-                                        "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
-                                        "[snapshot=on|off][,cache=on|off]",
-                                        "add drive to PCI storage controller" },
+    {
+        .name       = "drive_add",
+        .args_type  = "pci_addr:s,opts:s",
+        .handler    = drive_hot_add,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[[<domain>:]<bus>:]<slot>\n"
+                      "[file=file][,if=type][,bus=n]\n"
+                      "[,unit=m][,media=d][index=i]\n"
+                      "[,cyls=c,heads=h,secs=s[,trans=t]]\n"
+                      "[snapshot=on|off][,cache=on|off]",
+        .help       = "add drive to PCI storage controller"
+    },
 #endif
+
 STEXI
 @item drive_add
 Add drive to PCI storage controller.
 ETEXI
 
 #if defined(TARGET_I386)
-    { "pci_add", "pci_addr:s,type:s,opts:s?", pci_device_hot_add, "auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...", "hot-add PCI device" },
+    {
+        .name       = "pci_add",
+        .args_type  = "pci_addr:s,type:s,opts:s?",
+        .handler    = pci_device_hot_add,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...",
+        .help       = "hot-add PCI device"
+    },
 #endif
+
 STEXI
 @item pci_add
 Hot-add PCI device.
 ETEXI
 
 #if defined(TARGET_I386)
-    { "pci_del", "pci_addr:s", do_pci_device_hot_remove, "[[<domain>:]<bus>:]<slot>", "hot remove PCI device" },
+    {
+        .name       = "pci_del",
+        .args_type  = "pci_addr:s",
+        .handler    = do_pci_device_hot_remove,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[[<domain>:]<bus>:]<slot>",
+        .help       = "hot remove PCI device"
+    },
 #endif
+
 STEXI
 @item pci_del
 Hot remove PCI device.
 ETEXI
 
-    { "host_net_add", "device:s,opts:s?", net_host_device_add,
-      "tap|user|socket|vde|dump [options]", "add host VLAN client" },
+    {
+        .name       = "host_net_add",
+        .args_type  = "device:s,opts:s?",
+        .handler    = net_host_device_add,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "tap|user|socket|vde|dump [options]",
+        .help       = "add host VLAN client"
+    },
 STEXI
 @item host_net_add
 Add host VLAN client.
 ETEXI
 
-    { "host_net_remove", "vlan_id:i,device:s", net_host_device_remove,
-      "vlan_id name", "remove host VLAN client" },
+    {
+        .name       = "host_net_remove",
+        .args_type  = "vlan_id:i,device:s",
+        .handler    = net_host_device_remove,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "vlan_id name",
+        .help       = "remove host VLAN client"
+    },
+
 STEXI
 @item host_net_remove
 Remove host VLAN client.
 ETEXI
 
 #ifdef CONFIG_SLIRP
-    { "hostfwd_add", "arg1:s,arg2:s?,arg3:s?", net_slirp_hostfwd_add,
-      "[vlan_id name] [tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport",
-      "redirect TCP or UDP connections from host to guest (requires -net user)" },
-    { "hostfwd_remove", "arg1:s,arg2:s?,arg3:s?", net_slirp_hostfwd_remove,
-      "[vlan_id name] [tcp|udp]:[hostaddr]:hostport",
-      "remove host-to-guest TCP or UDP redirection" },
+    {
+        .name       = "hostfwd_add",
+        .args_type  = "arg1:s,arg2:s?,arg3:s?",
+        .handler    = net_slirp_hostfwd_add,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[vlan_id name] [tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport",
+        .help       = "redirect TCP or UDP connections from host to guest (requires -net user)" },
+
+    {
+        .name       = "hostfwd_remove",
+        .args_type  = "arg1:s,arg2:s?,arg3:s?",
+        .handler    = net_slirp_hostfwd_remove,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[vlan_id name] [tcp|udp]:[hostaddr]:hostport",
+        .help       = "remove host-to-guest TCP or UDP redirection"
+    },
+
 #endif
 STEXI
 @item host_net_redir
 Redirect TCP or UDP connections from host to guest (requires -net user).
 ETEXI
 
-    { "balloon", "value:i", do_balloon,
-      "target", "request VM to change it's memory allocation (in MB)" },
+    {
+        .name       = "balloon",
+        .args_type  = "value:i",
+        .handler    = do_balloon,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "target",
+        .help       = "request VM to change it's memory allocation (in MB)"
+    },
+
 STEXI
 @item balloon @var{value}
 Request VM to change its memory allocation to @var{value} (in MB).
 ETEXI
 
-    { "set_link", "name:s,up_or_down:s", do_set_link,
-      "name up|down", "change the link status of a network adapter" },
+    {
+        .name       = "set_link",
+        .args_type  = "name:s,up_or_down:s",
+        .handler    = do_set_link,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "name up|down",
+        .help       = "change the link status of a network adapter"
+    },
+
 STEXI
 @item set_link @var{name} [up|down]
 Set link @var{name} up or down.
 ETEXI
 
-    { "watchdog_action", "action:s", do_watchdog_action,
-      "[reset|shutdown|poweroff|pause|debug|none]", "change watchdog action" },
+    {
+        .name       = "watchdog_action",
+        .args_type  = "action:s",
+        .handler    = do_watchdog_action,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "[reset|shutdown|poweroff|pause|debug|none]",
+        .help       = "change watchdog action"
+    },
+
 STEXI
 @item watchdog_action
 Change watchdog action.
 ETEXI
 
-    { "acl_show", "aclname:s", do_acl_show, "aclname",
-      "list rules in the access control list" },
+    {
+        .name       = "acl_show",
+        .args_type  = "aclname:s",
+        .handler    = do_acl_show,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "aclname",
+        .help       = "list rules in the access control list"
+    },
+
 STEXI
 @item acl_show @var{aclname}
 List all the matching rules in the access control list, and the default
@@ -583,8 +995,16 @@ policy. There are currently two named access control lists,
 certificate distinguished name, and SASL username respectively.
 ETEXI
 
-    { "acl_policy", "aclname:s,policy:s", do_acl_policy, "aclname allow|deny",
-      "set default access control list policy" },
+    {
+        .name       = "acl_policy",
+        .args_type  = "aclname:s,policy:s",
+        .handler    = do_acl_policy,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "aclname allow|deny",
+        .help       = "set default access control list policy"
+    },
+
 STEXI
 @item acl_policy @var{aclname} @code{allow|deny}
 Set the default access control list policy, used in the event that
@@ -592,8 +1012,16 @@ none of the explicit rules match. The default policy at startup is
 always @code{deny}.
 ETEXI
 
-    { "acl_add", "aclname:s,match:s,policy:s,index:i?", do_acl_add, "aclname match allow|deny [index]",
-      "add a match rule to the access control list" },
+    {
+        .name       = "acl_add",
+        .args_type  = "aclname:s,match:s,policy:s,index:i?",
+        .handler    = do_acl_add,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "aclname match allow|deny [index]",
+        .help       = "add a match rule to the access control list"
+    },
+
 STEXI
 @item acl_allow @var{aclname} @var{match} @code{allow|deny} [@var{index}]
 Add a match rule to the access control list, allowing or denying access.
@@ -604,15 +1032,31 @@ normally be appended to the end of the ACL, but can be inserted
 earlier in the list if the optional @var{index} parameter is supplied.
 ETEXI
 
-    { "acl_remove", "aclname:s,match:s", do_acl_remove, "aclname match",
-      "remove a match rule from the access control list" },
+    {
+        .name       = "acl_remove",
+        .args_type  = "aclname:s,match:s",
+        .handler    = do_acl_remove,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "aclname match",
+        .help       = "remove a match rule from the access control list"
+    },
+
 STEXI
 @item acl_remove @var{aclname} @var{match}
 Remove the specified match rule from the access control list.
 ETEXI
 
-    { "acl_reset", "aclname:s", do_acl_reset, "aclname",
-      "reset the access control list" },
+    {
+        .name       = "acl_reset",
+        .args_type  = "aclname:s",
+        .handler    = do_acl_reset,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "aclname",
+        .help       = "reset the access control list"
+    },
+
 STEXI
 @item acl_remove @var{aclname} @var{match}
 Remove all matches from the access control list, and set the default
@@ -620,15 +1064,33 @@ policy back to @code{deny}.
 ETEXI
 
 #if defined(TARGET_I386)
-    { "mce", "cpu_index:i,bank:i,status:l,mcg_status:l,addr:l,misc:l", do_inject_mce, "cpu bank status mcgstatus addr misc", "inject a MCE on the given CPU"},
+
+    {
+        .name       = "mce",
+        .args_type  = "cpu_index:i,bank:i,status:l,mcg_status:l,addr:l,misc:l",
+        .handler    = do_inject_mce,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "cpu bank status mcgstatus addr misc",
+        .help       = "inject a MCE on the given CPU"
+    },
+
 #endif
 STEXI
 @item mce @var{cpu} @var{bank} @var{status} @var{mcgstatus} @var{addr} @var{misc}
 Inject an MCE on the given CPU (x86 only).
 ETEXI
 
-    { "getfd", "fdname:s", do_getfd, "getfd name",
-      "receive a file descriptor via SCM rights and assign it a name" },
+    {
+        .name       = "getfd",
+        .args_type  = "fdname:s",
+        .handler    = do_getfd,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "getfd name",
+        .help       = "receive a file descriptor via SCM rights and assign it a name"
+    },
+
 STEXI
 @item getfd @var{fdname}
 If a file descriptor is passed alongside this command using the SCM_RIGHTS
@@ -636,8 +1098,16 @@ mechanism on unix sockets, it is stored using the name @var{fdname} for
 later use by other monitor commands.
 ETEXI
 
-    { "closefd", "fdname:s", do_closefd, "closefd name",
-      "close a file descriptor previously passed via SCM rights" },
+    {
+        .name       = "closefd",
+        .args_type  = "fdname:s",
+        .handler    = do_closefd,
+        .user_print = NULL,
+        .user_error = NULL,
+        .params     = "closefd name",
+        .help       = "close a file descriptor previously passed via SCM rights"
+    },
+
 STEXI
 @item closefd @var{fdname}
 Close the file descriptor previously assigned to @var{fdname} using the
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 04/14] monitor: Handle new and old style handlers

[Luiz Capitulino]

This commit changes monitor_handle_command() to support old style
_and_ new style handlers.

New style handlers are protocol independent, they return their data
to the Monitor, which in turn decides how to print them
(ie. user protocol vs. machine protocol).

Old style handlers are called as usual.

Converted handlers will use both the 'user_print' and 'user_error'
members of 'mon_cmd_t' to define its user protocol functions, which
will be called to print data in the user protocol format.

Also note that we are allocating a MonitoError but not using it
yet, this will be done by the next commits.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   38 ++++++++++++++++++++++++++++++++------
 1 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/monitor.c b/monitor.c
index 5cb6e81..3f97926 100644
--- a/monitor.c
+++ b/monitor.c
@@ -212,6 +212,11 @@ static int monitor_fprintf(FILE *stream, const char *fmt, ...)
     return 0;
 }
 
+static int monitor_handler_ported(const mon_cmd_t *cmd)
+{
+    return cmd->user_print != NULL;
+}
+
 static int compare_cmd(const char *name, const char *list)
 {
     const char *p, *pstart;
@@ -3075,23 +3080,44 @@ fail:
 static void monitor_handle_command(Monitor *mon, const char *cmdline)
 {
     QDict *qdict;
+    MonitorError *error;
     const mon_cmd_t *cmd;
 
     qdict = qdict_new();
+    error = monitor_error_new();
 
     cmd = monitor_parse_command(mon, cmdline, qdict);
-    if (cmd) {
-        void (*handler)(Monitor *mon, const QDict *qdict);
+    if (!cmd)
+        goto out;
 
-        qemu_errors_to_mon(mon);
+    qemu_errors_to_mon(mon);
 
-        handler = cmd->handler;
-        handler(mon, qdict);
+    if (monitor_handler_ported(cmd)) {
+        QObject *data = NULL;
+        void (*handler_new)(Monitor *mon, const QDict *params,
+                            QObject **ret_data, MonitorError *error);
 
-        qemu_errors_to_previous();
+        handler_new = cmd->handler;
+        handler_new(mon, qdict, &data, error);
+
+        if (monitor_has_error(error)) {
+            cmd->user_error(mon, error);
+        } else {
+            cmd->user_print(mon, data);
+        }
+
+        qobject_decref(data);
+    } else {
+        void (*handler_old)(Monitor *mon, const QDict *qdict);
+        handler_old = cmd->handler;
+        handler_old(mon, qdict);
     }
 
+   qemu_errors_to_previous();
+
+out:
     QDECREF(qdict);
+    monitor_error_free(error);
 }
 
 static void cmd_completion(const char *name, const char *list)
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 05/14] monitor: Initial MonitorError usage

[Luiz Capitulino]

This commit converts errors generated by monitor_parse_command()
to use the new MonitorError style.

The MonitorError used is setup by the newly introduced
monitor_parse_error(). It allocates a QDict to store the command
name which trigged the error and any 'extra information'.

The error is printed by monitor_print_parsing_err(), it knows
how to format the error message properly.

As a result of this change errors are always returned and printed
in only one place. Also, parsing error messages will the following
standard format:

<command-name>: <error-description> <extra-info>

This is good, because current code does not have a defined "standard".
There is a side effect though: some error messages will change.

NOTE: Not all errors have been converted.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   79 ++++++++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/monitor.c b/monitor.c
index 3f97926..2bfc592 100644
--- a/monitor.c
+++ b/monitor.c
@@ -217,6 +217,32 @@ static int monitor_handler_ported(const mon_cmd_t *cmd)
     return cmd->user_print != NULL;
 }
 
+static void monitor_print_parsing_err(Monitor *mon, MonitorError *error)
+{
+    QDict *qdict;
+
+    if (!monitor_has_error(error))
+        return;
+
+    assert(qobject_type(error->data) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(error->data);
+
+    /* Handle exceptions first */
+    if (qint_get_int(error->code) == MON_ERR_UNCMD) {
+        monitor_printf(mon, "%s: '%s'\n", qstring_get_str(error->desc),
+                                          qdict_get_str(qdict, "name"));
+        return;
+    }
+
+    /* standard format */
+    monitor_printf(mon, "%s: ", qdict_get_str(qdict, "name"));
+    monitor_printf(mon, "%s", qstring_get_str(error->desc));
+    if (qdict_haskey(qdict, "extra"))
+        monitor_printf(mon, " '%c'", (int) qdict_get_int(qdict, "extra"));
+
+    monitor_puts(mon, "\n");
+}
+
 static int compare_cmd(const char *name, const char *list)
 {
     const char *p, *pstart;
@@ -2834,6 +2860,19 @@ static char *key_get_info(const char *type, char **key)
     return ++p;
 }
 
+static void monitor_parse_error(MonitorError *error, int code,
+                                const char *cmdname, int extra)
+{
+    QDict *qdict;
+
+    qdict = qdict_new();
+    qdict_put(qdict, "name", qstring_from_str(cmdname));
+    if (extra != -1)
+        qdict_put(qdict, "extra", qint_from_int(extra));
+
+    monitor_error_set(error, code, QOBJECT(qdict), NULL);
+}
+
 static int default_fmt_format = 'x';
 static int default_fmt_size = 4;
 
@@ -2841,7 +2880,8 @@ static int default_fmt_size = 4;
 
 static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                                               const char *cmdline,
-                                              QDict *qdict)
+                                              QDict *qdict,
+                                              MonitorError *error)
 {
     const char *p, *typestr;
     int c;
@@ -2866,7 +2906,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
     }
 
     if (cmd->name == NULL) {
-        monitor_printf(mon, "unknown command: '%s'\n", cmdname);
+        monitor_parse_error(error, MON_ERR_UNCMD, cmdname, -1);
         return NULL;
     }
 
@@ -2883,7 +2923,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
         case 'B':
         case 's':
             {
-                int ret;
+                int err;
 
                 while (qemu_isspace(*p))
                     p++;
@@ -2894,21 +2934,20 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                         break;
                     }
                 }
-                ret = get_str(buf, sizeof(buf), &p);
-                if (ret < 0) {
+                err = get_str(buf, sizeof(buf), &p);
+                if (err < 0) {
                     switch(c) {
                     case 'F':
-                        monitor_printf(mon, "%s: filename expected\n",
-                                       cmdname);
+                        err = MON_ERR_EXPFILE;
                         break;
                     case 'B':
-                        monitor_printf(mon, "%s: block device name expected\n",
-                                       cmdname);
+                        err = MON_ERR_EXPBLK;
                         break;
                     default:
-                        monitor_printf(mon, "%s: string expected\n", cmdname);
+                        err = MON_ERR_EXPSTR;
                         break;
                     }
+                    monitor_parse_error(error, err, cmdname, -1);
                     goto fail;
                 }
                 qdict_put(qdict, key, qstring_from_str(buf));
@@ -2966,8 +3005,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     }
                 next:
                     if (*p != '\0' && !qemu_isspace(*p)) {
-                        monitor_printf(mon, "invalid char in format: '%c'\n",
-                                       *p);
+                        monitor_parse_error(error, MON_ERR_INVCHAR, cmdname,*p);
                         goto fail;
                     }
                     if (format < 0)
@@ -3022,8 +3060,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     goto fail;
                 /* Check if 'i' is greater than 32-bit */
                 if ((c == 'i') && ((val >> 32) & 0xffffffff)) {
-                    monitor_printf(mon, "\'%s\' has failed: ", cmdname);
-                    monitor_printf(mon, "integer is for 32-bit values\n");
+                    monitor_parse_error(error, MON_ERR_INVINT, cmdname, -1);
                     goto fail;
                 }
                 qdict_put(qdict, key, qint_from_int(val));
@@ -3043,8 +3080,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                 if (*p == '-') {
                     p++;
                     if (*p != c) {
-                        monitor_printf(mon, "%s: unsupported option -%c\n",
-                                       cmdname, *p);
+                        monitor_parse_error(error, MON_ERR_UNSOPT, cmdname, *p);
                         goto fail;
                     }
                     p++;
@@ -3055,7 +3091,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
             break;
         default:
         bad_type:
-            monitor_printf(mon, "%s: unknown type '%c'\n", cmdname, c);
+            monitor_parse_error(error, MON_ERR_UNTYPE, cmdname, c);
             goto fail;
         }
         qemu_free(key);
@@ -3065,8 +3101,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
     while (qemu_isspace(*p))
         p++;
     if (*p != '\0') {
-        monitor_printf(mon, "%s: extraneous characters at the end of line\n",
-                       cmdname);
+        monitor_parse_error(error, MON_ERR_EXTCHAR, cmdname, -1);
         goto fail;
     }
 
@@ -3086,9 +3121,11 @@ static void monitor_handle_command(Monitor *mon, const char *cmdline)
     qdict = qdict_new();
     error = monitor_error_new();
 
-    cmd = monitor_parse_command(mon, cmdline, qdict);
-    if (!cmd)
+    cmd = monitor_parse_command(mon, cmdline, qdict, error);
+    if (!cmd) {
+        monitor_print_parsing_err(mon, error);
         goto out;
+    }
 
     qemu_errors_to_mon(mon);
 
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 06/14] monitor: do_info(): handle new and old info handlers

[Luiz Capitulino]

do_info() is special, its job is to call 'info handlers'.
This is similar to what monitor_handle_command() does,
therefore do_info() also has to distinguish among new and
old style info handlers.

This commit converts do_info() to the new QObject style and
makes the appropriate changes so that it can handle both
info handlers styles.

In the future, when all handlers are converted to QObject's
style, it will be possible to share more code with
monitor_handle_command().

This commit also introduces two new functions that should
be used by handlers which do not have data to print:

- monitor_user_noop()
- monitor_error_noop()

This is the case of do_info().

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |   43 ++++++++++++++++++++++++++++++++++---------
 qemu-monitor.hx |    4 ++--
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/monitor.c b/monitor.c
index 2bfc592..fecdc63 100644
--- a/monitor.c
+++ b/monitor.c
@@ -212,6 +212,10 @@ static int monitor_fprintf(FILE *stream, const char *fmt, ...)
     return 0;
 }
 
+static void monitor_user_noop(Monitor *mon, const QObject *data) { }
+
+static void monitor_error_noop(Monitor *mon, const MonitorError *error) { }
+
 static int monitor_handler_ported(const mon_cmd_t *cmd)
 {
     return cmd->user_print != NULL;
@@ -312,24 +316,45 @@ static void do_commit(Monitor *mon, const QDict *qdict)
     }
 }
 
-static void do_info(Monitor *mon, const QDict *qdict)
+static void do_info(Monitor *mon, const QDict *qdict, QObject **ret_data,
+                    MonitorError *error)
 {
     const mon_cmd_t *cmd;
     const char *item = qdict_get_try_str(qdict, "item");
-    void (*handler)(Monitor *);
 
     if (!item)
         goto help;
-    for(cmd = info_cmds; cmd->name != NULL; cmd++) {
+
+    for (cmd = info_cmds; cmd->name != NULL; cmd++) {
         if (compare_cmd(item, cmd->name))
-            goto found;
+            break;
     }
- help:
-    help_cmd(mon, "info");
+
+    if (cmd->name == NULL)
+        goto help;
+
+    if (monitor_handler_ported(cmd)) {
+        void (*handler_new)(Monitor *mon, QObject **ret_data,
+                            MonitorError *error);
+
+        handler_new = cmd->handler;
+        handler_new(mon, ret_data, error);
+
+        if (monitor_has_error(error)) {
+            cmd->user_error(mon, error);
+        } else {
+            cmd->user_print(mon, *ret_data);
+        }
+    } else {
+        void (*handler_old)(Monitor *mon);
+        handler_old = cmd->handler;
+        handler_old(mon);
+    }
+
     return;
- found:
-    handler = cmd->handler;
-    handler(mon);
+
+help:
+    help_cmd(mon, "info");
 }
 
 static void do_info_version(Monitor *mon)
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 0da810a..b9c33e5 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -43,8 +43,8 @@ ETEXI
         .name       = "info",
         .args_type  = "item:s?",
         .handler    = do_info,
-        .user_print = NULL,
-        .user_error = NULL,
+        .user_print = monitor_user_noop,
+        .user_error = monitor_error_noop,
         .params     = "[subcommand]",
         .help       = "show various information about the system state"
     },
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 07/14] monitor: Convert do_quit() do QObject

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |    6 +++++-
 qemu-monitor.hx |    4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index fecdc63..411c843 100644
--- a/monitor.c
+++ b/monitor.c
@@ -492,7 +492,11 @@ static void do_info_cpu_stats(Monitor *mon)
 }
 #endif
 
-static void do_quit(Monitor *mon, const QDict *qdict)
+/**
+ * do_quit(): Quit QEMU execution
+ */
+static void do_quit(Monitor *mon, const QDict *qdict, QObject **ret_data,
+                    MonitorError *error)
 {
     exit(0);
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index b9c33e5..7d5af9f 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -123,8 +123,8 @@ ETEXI
         .name       = "q|quit",
         .args_type  = "",
         .handler    = do_quit,
-        .user_print = NULL,
-        .user_error = NULL,
+        .user_print = monitor_user_noop,
+        .user_error = monitor_error_noop,
         .params     = "",
         .help       = "quit the emulator"
     },
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 08/14] monitor: Convert do_stop() to QObject

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |    6 +++++-
 qemu-monitor.hx |    4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index 411c843..3e96792 100644
--- a/monitor.c
+++ b/monitor.c
@@ -635,7 +635,11 @@ static void do_singlestep(Monitor *mon, const QDict *qdict)
     }
 }
 
-static void do_stop(Monitor *mon, const QDict *qdict)
+/**
+ * do_stop(): Stop VM execution
+ */
+static void do_stop(Monitor *mon, const QDict *qdict, QObject **ret_data,
+                    MonitorError *error)
 {
     vm_stop(EXCP_INTERRUPT);
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 7d5af9f..91bfe37 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -310,8 +310,8 @@ ETEXI
         .name       = "stop",
         .args_type  = "",
         .handler    = do_stop,
-        .user_print = NULL,
-        .user_error = NULL,
+        .user_print = monitor_user_noop,
+        .user_error = monitor_error_noop,
         .params     = "",
         .help       = "stop emulation",
     },
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 09/14] monitor: Convert do_system_reset() to QObject

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |    6 +++++-
 qemu-monitor.hx |    4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index 3e96792..f2f0c74 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1329,7 +1329,11 @@ static void do_boot_set(Monitor *mon, const QDict *qdict)
     }
 }
 
-static void do_system_reset(Monitor *mon, const QDict *qdict)
+/**
+ * do_system_reset(): Issue a machine reset
+ */
+static void do_system_reset(Monitor *mon, const QDict *qdict,
+                            QObject **ret_data, MonitorError *error)
 {
     qemu_system_reset_request();
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 91bfe37..4f8e678 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -507,8 +507,8 @@ ETEXI
         .name       = "system_reset",
         .args_type  = "",
         .handler    = do_system_reset,
-        .user_print = NULL,
-        .user_error = NULL,
+        .user_print = monitor_user_noop,
+        .user_error = monitor_error_noop,
         .params     = "",
         .help       = "reset the system"
     },
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 10/14] monitor: Convert do_system_powerdown() to QObject

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |    6 +++++-
 qemu-monitor.hx |    4 ++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index f2f0c74..87fd91c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1338,7 +1338,11 @@ static void do_system_reset(Monitor *mon, const QDict *qdict,
     qemu_system_reset_request();
 }
 
-static void do_system_powerdown(Monitor *mon, const QDict *qdict)
+/**
+ * do_system_powerdown(): Issue a machine powerdown
+ */
+static void do_system_powerdown(Monitor *mon, const QDict *qdict,
+                                QObject **ret_data, MonitorError *error)
 {
     qemu_system_powerdown_request();
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 4f8e678..e848b1d 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -523,8 +523,8 @@ ETEXI
         .name       = "system_powerdown",
         .args_type  = "",
         .handler    = do_system_powerdown,
-        .user_print = NULL,
-        .user_error = NULL,
+        .user_print = monitor_user_noop,
+        .user_error = monitor_error_noop,
         .params     = "",
         .help       = "send system power down event"
     },
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 11/14] monitor: Convert do_balloon() to QObject

[Luiz Capitulino]

It is important to observe that it never fails, as big refactoring
of the virtio code would be needed to get the proper error code.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c       |    7 +++++--
 qemu-monitor.hx |    4 ++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 87fd91c..852e558 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1636,8 +1636,11 @@ static void do_info_status(Monitor *mon)
        monitor_printf(mon, "VM status: paused\n");
 }
 
-
-static void do_balloon(Monitor *mon, const QDict *qdict)
+/**
+ * do_balloon(): Request VM to change its memory allocation
+ */
+static void do_balloon(Monitor *mon, const QDict *qdict, QObject **ret_data,
+                       MonitorError *error)
 {
     int value = qdict_get_int(qdict, "value");
     ram_addr_t target = value;
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index e848b1d..7cde9d9 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -936,8 +936,8 @@ ETEXI
         .name       = "balloon",
         .args_type  = "value:i",
         .handler    = do_balloon,
-        .user_print = NULL,
-        .user_error = NULL,
+        .user_print = monitor_user_noop,
+        .user_error = monitor_error_noop,
         .params     = "target",
         .help       = "request VM to change it's memory allocation (in MB)"
     },
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 12/14] monitor: Convert do_info_version() to QObject

[Luiz Capitulino]

The returned data is always a QString.

Also introduces monitor_print_qobject(), which can be used as
a standard way to print QObjects in the user protocol format.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   30 ++++++++++++++++++++++++++----
 1 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 852e558..1b52355 100644
--- a/monitor.c
+++ b/monitor.c
@@ -247,6 +247,24 @@ static void monitor_print_parsing_err(Monitor *mon, MonitorError *error)
     monitor_puts(mon, "\n");
 }
 
+static void monitor_print_qobject(Monitor *mon, const QObject *data)
+{
+    switch (qobject_type(data)) {
+        case QTYPE_QSTRING:
+            monitor_printf(mon, "%s",qstring_get_str(qobject_to_qstring(data)));
+            break;
+        case QTYPE_QINT:
+            monitor_printf(mon, "%" PRId64,qint_get_int(qobject_to_qint(data)));
+            break;
+        default:
+            monitor_printf(mon, "ERROR: unsupported type: %d",
+                                                        qobject_type(data));
+            break;
+    }
+
+    monitor_puts(mon, "\n");
+}
+
 static int compare_cmd(const char *name, const char *list)
 {
     const char *p, *pstart;
@@ -357,9 +375,13 @@ help:
     help_cmd(mon, "info");
 }
 
-static void do_info_version(Monitor *mon)
+/**
+ * do_info_version(): Show QEMU version
+ */
+static void do_info_version(Monitor *mon, QObject **ret_data,
+                            MonitorError *error)
 {
-    monitor_printf(mon, "%s\n", QEMU_VERSION QEMU_PKGVERSION);
+    *ret_data = QOBJECT(qstring_from_str(QEMU_VERSION QEMU_PKGVERSION));
 }
 
 static void do_info_name(Monitor *mon)
@@ -1894,8 +1916,8 @@ static const mon_cmd_t info_cmds[] = {
         .name       = "version",
         .args_type  = "",
         .handler    = do_info_version,
-        .user_print = NULL,
-        .user_error = NULL,
+        .user_print = monitor_print_qobject,
+        .user_error = monitor_error_noop,
         .params     = "",
         .help       = "show the version of QEMU"
     },
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 13/14] monitor-error: Add do_info_balloon() errors

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor-error.c |    6 ++++++
 monitor-error.h |    5 +++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/monitor-error.c b/monitor-error.c
index 35f61b8..078bae2 100644
--- a/monitor-error.c
+++ b/monitor-error.c
@@ -70,6 +70,12 @@ static void set_error_desc(MonitorError *error)
         case MON_ERR_EXTCHAR:
             str = "extraneous characters at the end of line";
             break;
+        case MON_ERR_BAL_MMU:
+            str = "Using KVM without synchronous MMU, ballooning disabled";
+            break;
+        case MON_ERR_BAL_DIS:
+            str = "Ballooning not activated in VM";
+            break;
         default:
             str = "unknown";
             break;
diff --git a/monitor-error.h b/monitor-error.h
index cd2f82c..36299d4 100644
--- a/monitor-error.h
+++ b/monitor-error.h
@@ -29,6 +29,11 @@ typedef enum MonitorErrCodes {
     MON_ERR_EXTCHAR,
 } MonitorErrCodes;
 
+typedef enum MonitorHndErrCodes {
+    MON_ERR_BAL_MMU = 30,
+    MON_ERR_BAL_DIS,
+} MonitorHndErrCodes;
+
 typedef struct MonitorError {
     QInt *code;     /* error code */
     QString *desc;  /* 'global' error description */
-- 
1.6.5.rc2

[Qemu-devel] [PATCH 14/14] monitor: Convert do_info_balloon() to QObject

[Luiz Capitulino]

On success return a QInt with the balloon's value, on error
MonitorError is properly filled.

This also introduces monitor_print_balloon() to print the
balloon information in the user protocol format and
monitor_print_error(), which can be used as a standard way
to print error descriptions in the user protocol format.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   35 +++++++++++++++++++++++++----------
 1 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/monitor.c b/monitor.c
index 1b52355..2cf9f7e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -265,6 +265,11 @@ static void monitor_print_qobject(Monitor *mon, const QObject *data)
     monitor_puts(mon, "\n");
 }
 
+static void monitor_print_error(Monitor *mon, const MonitorError *error)
+{
+    monitor_print_qobject(mon, QOBJECT(error->desc));
+}
+
 static int compare_cmd(const char *name, const char *list)
 {
     const char *p, *pstart;
@@ -1669,18 +1674,28 @@ static void do_balloon(Monitor *mon, const QDict *qdict, QObject **ret_data,
     qemu_balloon(target << 20);
 }
 
-static void do_info_balloon(Monitor *mon)
+static void monitor_print_balloon(Monitor *mon, const QObject *data)
+{
+    monitor_printf(mon, "balloon: actual=%d\n",
+                                     (int)qint_get_int(qobject_to_qint(data)));
+}
+
+/**
+ * do_info_balloon(): Balloon information
+ */
+static void do_info_balloon(Monitor *mon, QObject **ret_data,
+                            MonitorError *error)
 {
     ram_addr_t actual;
 
     actual = qemu_balloon_status();
-    if (kvm_enabled() && !kvm_has_sync_mmu())
-        monitor_printf(mon, "Using KVM without synchronous MMU, "
-                       "ballooning disabled\n");
-    else if (actual == 0)
-        monitor_printf(mon, "Ballooning not activated in VM\n");
-    else
-        monitor_printf(mon, "balloon: actual=%d\n", (int)(actual >> 20));
+    if (kvm_enabled() && !kvm_has_sync_mmu()) {
+        monitor_error_set_code(error, MON_ERR_BAL_MMU);
+    } else if (actual == 0) {
+        monitor_error_set_code(error, MON_ERR_BAL_DIS);
+    } else {
+        *ret_data = QOBJECT(qint_from_int((int)(actual >> 20)));
+    }
 }
 
 static qemu_acl *find_acl(Monitor *mon, const char *name)
@@ -2203,8 +2218,8 @@ static const mon_cmd_t info_cmds[] = {
         .name       = "balloon",
         .args_type  = "",
         .handler    = do_info_balloon,
-        .user_print = NULL,
-        .user_error = NULL,
+        .user_print = monitor_print_balloon,
+        .user_error = monitor_print_error,
         .params     = "",
         .help       = "show balloon information"
     },
-- 
1.6.5.rc2

[Qemu-devel] Re: [PATCH v1 00/14]: Initial QObject conversion

[Avi Kivity]

On 10/01/2009 05:50 PM, Luiz Capitulino wrote:
>   Hi there,
>
>   This series is an updated version of my initial QObject conversion series,
> which adds the needed infrastructure to incrementally support new style
> QObject handlers without breaking the current ones.
>
>   Some people have suggested that we should have a better error handling
> in the Monitor, in the meaning that error information should be correctly
> propagated and handled in order to be used by the Monitor Protocol and
> the existing user protocol.
>
>   This series introduces the MonitorError data type to solve that problem,
> it's used as part of the new infrastructure.
>
>   Additionally the following handlers are converted: do_quit(), do_stop(),
> do_system_reset(), do_system_powerdown(), do_balloon(), do_info_version(),
> do_info_balloon().
>
>   I've done a full build of QEMU with this series applied on Fedora 11 x86_64
> and Debian Lenny i386, also tested manually all converted commands plus some
> easy ones.
>
>   Please, review this carefully as some design decisions made here will
> have impact in the Monitor Protocol.
>
>    

Looks good to me.

Regarding info, I think the machine protocol should manage it as 
separate commands ("info-cpus" instead of "info"("cpus")).  Having a 
function which returns wildly different return types (a string for 
version, a device tree for qdev) is unwieldy.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

[Qemu-devel] Re: [PATCH v1 00/14]: Initial QObject conversion

[Luiz Capitulino]

On Thu, 01 Oct 2009 18:01:20 +0200
Avi Kivity <avi@redhat.com> wrote:

> On 10/01/2009 05:50 PM, Luiz Capitulino wrote:
> >   Hi there,
> >
> >   This series is an updated version of my initial QObject conversion series,
> > which adds the needed infrastructure to incrementally support new style
> > QObject handlers without breaking the current ones.
> >
> >   Some people have suggested that we should have a better error handling
> > in the Monitor, in the meaning that error information should be correctly
> > propagated and handled in order to be used by the Monitor Protocol and
> > the existing user protocol.
> >
> >   This series introduces the MonitorError data type to solve that problem,
> > it's used as part of the new infrastructure.
> >
> >   Additionally the following handlers are converted: do_quit(), do_stop(),
> > do_system_reset(), do_system_powerdown(), do_balloon(), do_info_version(),
> > do_info_balloon().
> >
> >   I've done a full build of QEMU with this series applied on Fedora 11 x86_64
> > and Debian Lenny i386, also tested manually all converted commands plus some
> > easy ones.
> >
> >   Please, review this carefully as some design decisions made here will
> > have impact in the Monitor Protocol.
> >
> >    
> 
> Looks good to me.
> 
> Regarding info, I think the machine protocol should manage it as 
> separate commands ("info-cpus" instead of "info"("cpus")).  Having a 
> function which returns wildly different return types (a string for 
> version, a device tree for qdev) is unwieldy.

 I agree and I think this approach would make my life easier.

 The problem though is how to properly refactor the code so that
we don't conflict with the user's 'info cpus' command.

 I will think about it, but would be good to decide this before
mass conversion.

[Qemu-devel] Re: [PATCH v1 00/14]: Initial QObject conversion

[Avi Kivity]

On 10/01/2009 11:21 PM, Luiz Capitulino wrote:
>> Regarding info, I think the machine protocol should manage it as
>> separate commands ("info-cpus" instead of "info"("cpus")).  Having a
>> function which returns wildly different return types (a string for
>> version, a device tree for qdev) is unwieldy.
>>      
>   I agree and I think this approach would make my life easier.
>    

I was thinking more of client writers.

>   The problem though is how to properly refactor the code so that
> we don't conflict with the user's 'info cpus' command.
>
>   I will think about it, but would be good to decide this before
> mass conversion.
>    

One approach would be to have two command tables, one for ordinary 
commands and one for info commands.  The machine monitor can scan the 
two tables sequentially, while the human monitor will only scan the 
first table, and the second as a response to an 'info' command.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

[Qemu-devel] Re: [PATCH v1 00/14]: Initial QObject conversion

[Luiz Capitulino]

On Sat, 03 Oct 2009 09:59:39 +0200
Avi Kivity <avi@redhat.com> wrote:

> On 10/01/2009 11:21 PM, Luiz Capitulino wrote:
> >> Regarding info, I think the machine protocol should manage it as
> >> separate commands ("info-cpus" instead of "info"("cpus")).  Having a
> >> function which returns wildly different return types (a string for
> >> version, a device tree for qdev) is unwieldy.
> >>      
> >   I agree and I think this approach would make my life easier.
> >    
> 
> I was thinking more of client writers.

 I see.

> >   The problem though is how to properly refactor the code so that
> > we don't conflict with the user's 'info cpus' command.
> >
> >   I will think about it, but would be good to decide this before
> > mass conversion.
> >    
> 
> One approach would be to have two command tables, one for ordinary 
> commands and one for info commands.  The machine monitor can scan the 
> two tables sequentially, while the human monitor will only scan the 
> first table, and the second as a response to an 'info' command.

 I thought about doing something similar, as we already have a
different table for 'info' commands.

 The disadvantage of this approach though, is that we would have
to handle 'info' handlers differently, ie, we would have to add
special cases in common code.

 Today, a 'info' command is treated as any other command, but
its handler (do_info()) calls the correct handler.

 Not sure if this is a big deal, though.

Re: [Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion

[Gerd Hoffmann]

   Hi,

>   Some people have suggested that we should have a better error handling
> in the Monitor, in the meaning that error information should be correctly
> propagated and handled in order to be used by the Monitor Protocol and
> the existing user protocol.

A bunch of code paths can be called from both monitor and non-monitor 
contexts (network configuration, device hotplug).  Right now there are 
the qemu_error*() functions to make sure the error messages appear on 
the correct place (monitor or stderr) without having to pass through a 
Monitor pointer all the way down.

How do you plan to handle this in the new world of monitor error reporting?

cheers,
   Gerd

Re: [Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion

[Luiz Capitulino]

On Fri, 02 Oct 2009 14:47:02 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>    Hi,
> 
> >   Some people have suggested that we should have a better error handling
> > in the Monitor, in the meaning that error information should be correctly
> > propagated and handled in order to be used by the Monitor Protocol and
> > the existing user protocol.
> 
> A bunch of code paths can be called from both monitor and non-monitor 
> contexts (network configuration, device hotplug).  Right now there are 
> the qemu_error*() functions to make sure the error messages appear on 
> the correct place (monitor or stderr) without having to pass through a 
> Monitor pointer all the way down.
> 
> How do you plan to handle this in the new world of monitor error reporting?

 Good question.

 The first thing to bear in mind is that MonitorError is not just about
error reporting, but more importantly, it makes errors have a common
structure so that they can be emitted by the Monitor Protocol.

 One way to solve the problem could be to move the MonitorError pointer
to the Monitor struct. This way, when there's a ERR_SINK_MONITOR
qemu_error() would fill MonitorError instead of calling monitor_vprintf().

 The bad news though, is that we would have to change all qemu_error()
calls to have "structured" errors instead of pretty printing, that is,
create macros, define its errors data and write functions to do
pretty printing....

 Looks hard, doesn't it?

 The real problem behind all QMP work is that we're starting from
a big amount of human-targeted functionality and trying to make it
common so that it's useful for machines as well.

Re: [Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion

[Gerd Hoffmann]

On 10/02/09 15:47, Luiz Capitulino wrote:
> On Fri, 02 Oct 2009 14:47:02 +0200
> Gerd Hoffmann<kraxel@redhat.com>  wrote:
>
>>     Hi,
>>
>>>    Some people have suggested that we should have a better error handling
>>> in the Monitor, in the meaning that error information should be correctly
>>> propagated and handled in order to be used by the Monitor Protocol and
>>> the existing user protocol.
>>
>> A bunch of code paths can be called from both monitor and non-monitor
>> contexts (network configuration, device hotplug).  Right now there are
>> the qemu_error*() functions to make sure the error messages appear on
>> the correct place (monitor or stderr) without having to pass through a
>> Monitor pointer all the way down.
>>
>> How do you plan to handle this in the new world of monitor error reporting?
>
>   Good question.
>
>   The first thing to bear in mind is that MonitorError is not just about
> error reporting, but more importantly, it makes errors have a common
> structure so that they can be emitted by the Monitor Protocol.

So maybe they shouldn't be named MonitorError in the first place?
And make sure the design works for non-monitor contexts too?
Having the user_error callback in struct mon_cmd_t doesn't make sense 
from that point of view for example.

Why user_error is needed in the first place btw?  To maintain 
backward-compatible error message formating?

>   One way to solve the problem could be to move the MonitorError pointer
> to the Monitor struct.

I think that is a good idea nevertheless.

> This way, when there's a ERR_SINK_MONITOR
> qemu_error() would fill MonitorError instead of calling monitor_vprintf().
>
>   The bad news though, is that we would have to change all qemu_error()
> calls to have "structured" errors instead of pretty printing, that is,
> create macros, define its errors data and write functions to do
> pretty printing....

Or add a qemu_error_structed() variant for smooth switchover and have 
qemu_error() use a generic error code, so you don't have to switch over 
all at once.  There are not *that* many qemu_error users though, so just 
converting them all in one go might not be too hard.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion

[Luiz Capitulino]

On Fri, 02 Oct 2009 16:17:35 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> On 10/02/09 15:47, Luiz Capitulino wrote:
> > On Fri, 02 Oct 2009 14:47:02 +0200
> > Gerd Hoffmann<kraxel@redhat.com>  wrote:
> >
> >>     Hi,
> >>
> >>>    Some people have suggested that we should have a better error handling
> >>> in the Monitor, in the meaning that error information should be correctly
> >>> propagated and handled in order to be used by the Monitor Protocol and
> >>> the existing user protocol.
> >>
> >> A bunch of code paths can be called from both monitor and non-monitor
> >> contexts (network configuration, device hotplug).  Right now there are
> >> the qemu_error*() functions to make sure the error messages appear on
> >> the correct place (monitor or stderr) without having to pass through a
> >> Monitor pointer all the way down.
> >>
> >> How do you plan to handle this in the new world of monitor error reporting?
> >
> >   Good question.
> >
> >   The first thing to bear in mind is that MonitorError is not just about
> > error reporting, but more importantly, it makes errors have a common
> > structure so that they can be emitted by the Monitor Protocol.
> 
> So maybe they shouldn't be named MonitorError in the first place?

 What do you suggest?

 Hm.. This could be QError, meaning that it's also a QObject,
so that we can put it in dicts, lists, etc.

> And make sure the design works for non-monitor contexts too?

 Yes, this is a good point.

> Having the user_error callback in struct mon_cmd_t doesn't make sense 
> from that point of view for example.

 If we make it qemu-wide, yes, I agree. There should be an error
table in QError with at least:

1. Error number/macro
2. Error description string
3. user_error() function

> Why user_error is needed in the first place btw?  To maintain 
> backward-compatible error message formating?

 Yes and to do pretty printing too.

 For example:

qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
           driver);

 Makes sense for humans, but for QMP it would probably look like:

{ "error": { "code": 1234,
             "desc": "device not found",
             "data": { "name": "foobar-device" } } }

 So, I would change the qemu_error() call to something like:

qemu_error_structed(QEMU_ERR_QDEV_NDEV, driver);

 This call would build the right QObjects in MonitorError (or QError),
and we would also need an user_error() function, like:

void qemu_err_qdev_nodev(const QError *error)
{
    const char *driver = qstring_get_str(error->data);
    qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
                driver);
}

 So, when QMP is disabled that function will be called to print the error
for humans. When QMP is enabled this is never called and the protocol
emission code will use QError to emit something like the { "error" }
dict above.

> >   One way to solve the problem could be to move the MonitorError pointer
> > to the Monitor struct.
> 
> I think that is a good idea nevertheless.

 Yes.

> > This way, when there's a ERR_SINK_MONITOR
> > qemu_error() would fill MonitorError instead of calling monitor_vprintf().
> >
> >   The bad news though, is that we would have to change all qemu_error()
> > calls to have "structured" errors instead of pretty printing, that is,
> > create macros, define its errors data and write functions to do
> > pretty printing....
> 
> Or add a qemu_error_structed() variant for smooth switchover and have 
> qemu_error() use a generic error code, so you don't have to switch over 
> all at once.  There are not *that* many qemu_error users though, so just 
> converting them all in one go might not be too hard.

 Ok.

Re: [Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion

[Gerd Hoffmann]

   Hi,
>
>> So maybe they shouldn't be named MonitorError in the first place?
>
>   What do you suggest?
>
>   Hm.. This could be QError, meaning that it's also a QObject,
> so that we can put it in dicts, lists, etc.

Makes sense to me.

Attaching the pretty-print callback to the error struct not to the 
monitor command is a good move too.  Makes it easier to reuse these 
functions.

Hmm, maybe it would be even better to link the pretty-print functions to 
error codes?

>> Why user_error is needed in the first place btw?  To maintain
>> backward-compatible error message formating?
>
>   Yes and to do pretty printing too.
>
>   For example:
>
> qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
>             driver);
>
>   Makes sense for humans, but for QMP it would probably look like:
>
> { "error": { "code": 1234,
>               "desc": "device not found",
>               "data": { "name": "foobar-device" } } }

You could have a generic function which makes a message like this:

   'device not found: name=foobar-device'

out of the QError.  Not too bad.  You'll loose the '-device ?' hint though.

I think it is reasonable to make the pretty-print callback optional and 
use the generic function in case it isn't present.  So you have 
something working by default and can override it of the message isn't 
userfriendly or backward-compatible enougth.

>   So, when QMP is disabled that function will be called to print the error
> for humans. When QMP is enabled this is never called and the protocol
> emission code will use QError to emit something like the { "error" }
> dict above.

Thanks for the explanation.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH v1 00/14]: Initial QObject conversion

[Luiz Capitulino]

On Fri, 02 Oct 2009 17:36:13 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

>    Hi,
> >
> >> So maybe they shouldn't be named MonitorError in the first place?
> >
> >   What do you suggest?
> >
> >   Hm.. This could be QError, meaning that it's also a QObject,
> > so that we can put it in dicts, lists, etc.
> 
> Makes sense to me.
> 
> Attaching the pretty-print callback to the error struct not to the 
> monitor command is a good move too.  Makes it easier to reuse these 
> functions.
> 
> Hmm, maybe it would be even better to link the pretty-print functions to 
> error codes?

 Yes, I'll try that.

> >> Why user_error is needed in the first place btw?  To maintain
> >> backward-compatible error message formating?
> >
> >   Yes and to do pretty printing too.
> >
> >   For example:
> >
> > qemu_error("Device \"%s\" not found.  Try -device '?' for a list.\n",
> >             driver);
> >
> >   Makes sense for humans, but for QMP it would probably look like:
> >
> > { "error": { "code": 1234,
> >               "desc": "device not found",
> >               "data": { "name": "foobar-device" } } }
> 
> You could have a generic function which makes a message like this:
> 
>    'device not found: name=foobar-device'
> 
> out of the QError.  Not too bad.  You'll loose the '-device ?' hint though.
> 
> I think it is reasonable to make the pretty-print callback optional and 
> use the generic function in case it isn't present.  So you have 
> something working by default and can override it of the message isn't 
> userfriendly or backward-compatible enougth.

 Yeah, it's a good idea.

> >   So, when QMP is disabled that function will be called to print the error
> > for humans. When QMP is enabled this is never called and the protocol
> > emission code will use QError to emit something like the { "error" }
> > dict above.
> 
> Thanks for the explanation.

 Thanks for the feedback!