[Qemu-devel] [PATCH v2 00/15]: Initial QObject conversion

[Qemu-devel] [PATCH v2 00/15]: Initial QObject conversion

[Luiz Capitulino]

 Hi there,

 Here goes another new version of my initial QObject conversion work.

 I'm taking a different approach in this series. Instead of introducing
the error handling code along with the initial conversions, I'm only
doing the QObject conversion part.

 This means that error information is still directly printed by the
command handler, but 'regular' data is returned to the Monitor in the
form of QObjects.

 This approach makes the series easier to:

1. Review, as we can concentrate only on the QObject conversion
2. Convert & merge, as we can do only one thing at a time
3. Test, for the same reason as in '2'

 The disadvantage is that we will need a 'second pass conversion' to
convert the error handling part, which can only be done when I finish
the QError data type I'm writing.

 This is not a big issue anyway, as only two of nine conversions in
this series can fail.

 The following handlers are being converted: do_quit(), do_stop(),
do_system_reset(), do_system_powerdown(), do_cont(), do_balloon(),
do_info_version(), do_info_balloon(), do_info_cpus().

 Also, I'm introducing QList as it's needed by the do_info_cpus()
conversion.

 I've done a full build of this series on F11 (x86_64) and
Debian Lenny (i386), manually tested all conversions and some other
commands.

Changelog:

V1 -> V2:

- Drop error handling code
- More handlers converted

V0 -> V1:

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

diffstat:

 Makefile        |    3 +-
 check-qlist.c   |  153 +++++++++++++
 configure       |    2 +-
 monitor.c       |  599 +++++++++++++++++++++++++++++++++++++++---------
 qemu-monitor.hx |  674 ++++++++++++++++++++++++++++++++++++++++++++----------
 qlist.c         |  100 ++++++++
 qlist.h         |   27 +++
 qobject.h       |    8 +-
 8 files changed, 1322 insertions(+), 244 deletions(-)

[Qemu-devel] [PATCH 01/15] 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.17.gdbc1b

[Qemu-devel] [PATCH 02/15] Introduce QList

[Luiz Capitulino]

QList is a high-level data type that can be used to store QObjects
in a singly-linked list.

The following functions are available:

- qlist_new()    Create a new QList
- qlist_append() Append a QObject to the list
- qlist_iter()   Iterate over stored QObjects

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile  |    2 +-
 qlist.c   |  100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qlist.h   |   27 ++++++++++++++++
 qobject.h |    1 +
 4 files changed, 129 insertions(+), 1 deletions(-)
 create mode 100644 qlist.c
 create mode 100644 qlist.h

diff --git a/Makefile b/Makefile
index c552739..17bcbe4 100644
--- a/Makefile
+++ b/Makefile
@@ -100,7 +100,7 @@ obj-y += buffered_file.o migration.o migration-tcp.o net.o qemu-sockets.o
 obj-y += qemu-char.o aio.o net-checksum.o savevm.o
 obj-y += msmouse.o ps2.o
 obj-y += qdev.o qdev-properties.o ssi.o
-obj-y += qint.o qstring.o qdict.o qemu-config.o
+obj-y += qint.o qstring.o qdict.o qlist.o qemu-config.o
 
 obj-$(CONFIG_BRLAPI) += baum.o
 obj-$(CONFIG_WIN32) += tap-win32.o
diff --git a/qlist.c b/qlist.c
new file mode 100644
index 0000000..9c753ce
--- /dev/null
+++ b/qlist.c
@@ -0,0 +1,100 @@
+/*
+ * QList data type.
+ *
+ * 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 "qlist.h"
+#include "qobject.h"
+#include "qemu-queue.h"
+#include "qemu-common.h"
+
+static const QType qlist_type;
+ 
+/**
+ * qlist_new(): Create a new QList
+ *
+ * Return strong reference.
+ */
+QList *qlist_new(void)
+{
+    QList *qlist;
+
+    qlist = qemu_malloc(sizeof(*qlist));
+    QTAILQ_INIT(&qlist->head);
+    QOBJECT_INIT(qlist, &qlist_type);
+
+    return qlist;
+}
+
+/**
+ * qlist_append_obj(): Append an QObject into QList
+ *
+ * NOTE: ownership of 'value' is transferred to the QList
+ */
+void qlist_append_obj(QList *qlist, QObject *value)
+{
+    QListEntry *entry;
+
+    entry = qemu_malloc(sizeof(*entry));
+    entry->value = value;
+
+    QTAILQ_INSERT_TAIL(&qlist->head, entry, next);
+}
+
+/**
+ * qlist_iter(): Iterate over all the list's stored values.
+ *
+ * This function allows the user to provide an iterator, which will be
+ * called for each stored value in the list.
+ */
+void qlist_iter(const QList *qlist,
+                void (*iter)(QObject *obj, void *opaque), void *opaque)
+{
+    QListEntry *entry;
+
+    QTAILQ_FOREACH(entry, &qlist->head, next)
+        iter(entry->value, opaque);
+}
+
+/**
+ * qobject_to_qlist(): Convert a QObject into a QList
+ */
+QList *qobject_to_qlist(const QObject *obj)
+{
+    if (qobject_type(obj) != QTYPE_QLIST)
+        return NULL;
+
+    return container_of(obj, QList, base);
+}
+
+/**
+ * qlist_destroy_obj(): Free all the memory allocated by a QList
+ */
+static void qlist_destroy_obj(QObject *obj)
+{
+    QList *qlist;
+    QListEntry *entry, *next_entry;
+
+    assert(obj != NULL);
+    qlist = qobject_to_qlist(obj);
+
+    QTAILQ_FOREACH_SAFE(entry, &qlist->head, next, next_entry) {
+        QTAILQ_REMOVE(&qlist->head, entry, next);
+        qobject_decref(entry->value);
+        qemu_free(entry);
+    }
+
+    qemu_free(qlist);
+}
+
+static const QType qlist_type = {
+    .code = QTYPE_QLIST,
+    .destroy = qlist_destroy_obj,
+};
diff --git a/qlist.h b/qlist.h
new file mode 100644
index 0000000..b38786e
--- /dev/null
+++ b/qlist.h
@@ -0,0 +1,27 @@
+#ifndef QLIST
+#define QLIST
+
+#include "qobject.h"
+#include "qemu-queue.h"
+#include "qemu-common.h"
+
+typedef struct QListEntry {
+    QObject *value;
+    QTAILQ_ENTRY(QListEntry) next;
+} QListEntry;
+
+typedef struct QList {
+    QObject_HEAD;
+    QTAILQ_HEAD(,QListEntry) head;
+} QList;
+
+#define qlist_append(qlist, obj) \
+        qlist_append_obj(qlist, QOBJECT(obj))
+
+QList *qlist_new(void);
+void qlist_append_obj(QList *qlist, QObject *obj);
+void qlist_iter(const QList *qlist,
+                void (*iter)(QObject *obj, void *opaque), void *opaque);
+QList *qobject_to_qlist(const QObject *obj);
+
+#endif /* QLIST */
diff --git a/qobject.h b/qobject.h
index dcc8c63..4cc9287 100644
--- a/qobject.h
+++ b/qobject.h
@@ -40,6 +40,7 @@ typedef enum {
     QTYPE_QINT,
     QTYPE_QSTRING,
     QTYPE_QDICT,
+    QTYPE_QLIST,
 } qtype_code;
 
 struct QObject;
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] Re: [PATCH 02/15] Introduce QList

[Anthony Liguori]

Luiz Capitulino wrote:
> QList is a high-level data type that can be used to store QObjects
> in a singly-linked list.
>
> The following functions are available:
>
> - qlist_new()    Create a new QList
> - qlist_append() Append a QObject to the list
> - qlist_iter()   Iterate over stored QObjects
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  Makefile  |    2 +-
>  qlist.c   |  100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qlist.h   |   27 ++++++++++++++++
>  qobject.h |    1 +
>  4 files changed, 129 insertions(+), 1 deletions(-)
>  create mode 100644 qlist.c
>  create mode 100644 qlist.h
>   
<snip>
> +/**
> + * qlist_iter(): Iterate over all the list's stored values.
> + *
> + * This function allows the user to provide an iterator, which will be
> + * called for each stored value in the list.
> + */
> +void qlist_iter(const QList *qlist,
> +                void (*iter)(QObject *obj, void *opaque), void *opaque)
> +{
> +    QListEntry *entry;
> +
> +    QTAILQ_FOREACH(entry, &qlist->head, next)
> +        iter(entry->value, opaque);
> +}
> +
> +/**
> + * qobject_to_qlist(): Convert a QObject into a QList
> + */
> +QList *qobject_to_qlist(const QObject *obj)
> +{
> +    if (qobject_type(obj) != QTYPE_QLIST)
> +        return NULL;
> +
> +    return container_of(obj, QList, base);
> +}
>   

Missing {} around ifs.

> +
> +/**
> + * qlist_destroy_obj(): Free all the memory allocated by a QList
> + */
> +static void qlist_destroy_obj(QObject *obj)
> +{
> +    QList *qlist;
> +    QListEntry *entry, *next_entry;
> +
> +    assert(obj != NULL);
>   

Usually accepting NULL in a free function makes for nicer exit paths in 
function.

> +    qlist = qobject_to_qlist(obj);
> +
> +    QTAILQ_FOREACH_SAFE(entry, &qlist->head, next, next_entry) {
> +        QTAILQ_REMOVE(&qlist->head, entry, next);
> +        qobject_decref(entry->value);
> +        qemu_free(entry);
> +    }
> +
> +    qemu_free(qlist);
> +}
> +
> +static const QType qlist_type = {
> +    .code = QTYPE_QLIST,
> +    .destroy = qlist_destroy_obj,
> +};
>   

Forward definitions of statics usually indicate code motion is in 
order.  In this case, I think it's probably a good idea as my first 
reaction to this was, gees, this has to be dead code since it's a static 
at the end of a file.

> diff --git a/qlist.h b/qlist.h
> new file mode 100644
> index 0000000..b38786e
> --- /dev/null
> +++ b/qlist.h
> @@ -0,0 +1,27 @@
>   

Missing copyright/license.

> +#ifndef QLIST
> +#define QLIST
>   

Should probably at least do QLIST_H to avoid namespace polution.

-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 02/15] Introduce QList

[Luiz Capitulino]

On Tue, 06 Oct 2009 20:37:43 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Luiz Capitulino wrote:

[...]

> > +/**
> > + * qlist_destroy_obj(): Free all the memory allocated by a QList
> > + */
> > +static void qlist_destroy_obj(QObject *obj)
> > +{
> > +    QList *qlist;
> > +    QListEntry *entry, *next_entry;
> > +
> > +    assert(obj != NULL);
> >   
> 
> Usually accepting NULL in a free function makes for nicer exit paths in 
> function.

 The destroy_obj() functions are always called by qobject_decref()
and it will never pass a NULL qobject.

 But qobject_decref() accepts NULL.

 I've applied the other changes, some of them also need to be done
in the others data types (will send patches).

[Qemu-devel] [PATCH 03/15] Introduce QList unit-tests

[Luiz Capitulino]

This suite contains tests to assure that QList API works as expected.

To execute it you should have check installed and build QEMU with
check support enabled (--enable-check-utests) and then run:

$ ./check-qlist

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile      |    1 +
 check-qlist.c |  153 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure     |    2 +-
 3 files changed, 155 insertions(+), 1 deletions(-)
 create mode 100644 check-qlist.c

diff --git a/Makefile b/Makefile
index 17bcbe4..d96fb4b 100644
--- a/Makefile
+++ b/Makefile
@@ -187,6 +187,7 @@ qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 check-qint: check-qint.o qint.o qemu-malloc.o
 check-qstring: check-qstring.o qstring.o qemu-malloc.o
 check-qdict: check-qdict.o qdict.o qint.o qstring.o qemu-malloc.o
+check-qlist: check-qlist.o qlist.o qint.o qemu-malloc.o
 
 clean:
 # avoid old build problems by removing potentially incorrect old files
diff --git a/check-qlist.c b/check-qlist.c
new file mode 100644
index 0000000..0117ef3
--- /dev/null
+++ b/check-qlist.c
@@ -0,0 +1,153 @@
+/*
+ * QList unit-tests.
+ *
+ * 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 <check.h>
+
+#include "qint.h"
+#include "qlist.h"
+
+/*
+ * Public Interface test-cases
+ *
+ * (with some violations to access 'private' data)
+ */
+
+START_TEST(qlist_new_test)
+{
+    QList *qlist;
+
+    qlist = qlist_new();
+    fail_unless(qlist != NULL);
+    fail_unless(qlist->base.refcnt == 1);
+    fail_unless(qobject_type(QOBJECT(qlist)) == QTYPE_QLIST);
+
+    // destroy doesn't exist yet
+    qemu_free(qlist);
+}
+END_TEST
+
+START_TEST(qlist_append_test)
+{
+    QInt *qi;
+    QList *qlist;
+    QListEntry *entry;
+
+    qi = qint_from_int(42);
+
+    qlist = qlist_new();
+    qlist_append(qlist, qi);
+
+    entry = QTAILQ_FIRST(&qlist->head);
+    fail_unless(entry != NULL);
+    fail_unless(entry->value == QOBJECT(qi));
+
+    // destroy doesn't exist yet
+    QDECREF(qi);
+    qemu_free(entry);
+    qemu_free(qlist);
+}
+END_TEST
+
+START_TEST(qobject_to_qlist_test)
+{
+    QList *qlist;
+
+    qlist = qlist_new();
+
+    fail_unless(qobject_to_qlist(QOBJECT(qlist)) == qlist);
+
+    // destroy doesn't exist yet
+    qemu_free(qlist);
+}
+END_TEST
+
+START_TEST(qlist_destroy_test)
+{
+    int i;
+    QList *qlist;
+
+    qlist = qlist_new();
+
+    for (i = 0; i < 42; i++)
+        qlist_append(qlist, qint_from_int(i));
+
+    QDECREF(qlist);
+}
+END_TEST
+
+static int iter_called;
+static const int iter_max = 42;
+
+static void iter_func(QObject *obj, void *opaque)
+{
+    QInt *qi;
+
+    fail_unless(opaque == NULL);
+
+    qi = qobject_to_qint(obj);
+    fail_unless(qi != NULL);
+    fail_unless((qint_get_int(qi) >= 0) && (qint_get_int(qi) <= iter_max));
+
+    iter_called++;
+}
+
+START_TEST(qlist_iter_test)
+{
+    int i;
+    QList *qlist;
+
+    qlist = qlist_new();
+
+    for (i = 0; i < iter_max; i++)
+        qlist_append(qlist, qint_from_int(i));
+
+    iter_called = 0;
+    qlist_iter(qlist, iter_func, NULL);
+
+    fail_unless(iter_called == iter_max);
+
+    QDECREF(qlist);
+}
+END_TEST
+
+static Suite *QList_suite(void)
+{
+    Suite *s;
+    TCase *qlist_public_tcase;
+
+    s = suite_create("QList suite");
+
+    qlist_public_tcase = tcase_create("Public Interface");
+    suite_add_tcase(s, qlist_public_tcase);
+    tcase_add_test(qlist_public_tcase, qlist_new_test);
+    tcase_add_test(qlist_public_tcase, qlist_append_test);
+    tcase_add_test(qlist_public_tcase, qobject_to_qlist_test);
+    tcase_add_test(qlist_public_tcase, qlist_destroy_test);
+    tcase_add_test(qlist_public_tcase, qlist_iter_test);
+
+    return s;
+}
+
+int main(void)
+{
+	int nf;
+	Suite *s;
+	SRunner *sr;
+
+	s = QList_suite();
+	sr = srunner_create(s);
+
+	srunner_run_all(sr, CK_NORMAL);
+	nf = srunner_ntests_failed(sr);
+	srunner_free(sr);
+
+	return (nf == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
diff --git a/configure b/configure
index ad04880..4ea5483 100755
--- a/configure
+++ b/configure
@@ -2017,7 +2017,7 @@ if test `expr "$target_list" : ".*softmmu.*"` != 0 ; then
   if [ "$linux" = "yes" ] ; then
       tools="qemu-nbd\$(EXESUF) qemu-io\$(EXESUF) $tools"
     if [ "$check_utests" = "yes" ]; then
-      tools="check-qint check-qstring check-qdict $tools"
+      tools="check-qint check-qstring check-qdict check-qlist $tools"
     fi
   elif test "$mingw32" = "yes" ; then
       tools="qemu-io\$(EXESUF) $tools"
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 04/15] monitor: Add user_print() to mon_cmd_t

[Luiz Capitulino]

This new struct member will store a pointer to a function that
should be used to output data in the user protocol format.

It will also serve as a flag to say if a given handler has already
been converted to the new QObject style.

Additionally, this commit converts mon_cmd_t static initializations
to the C99 way.

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

diff --git a/monitor.c b/monitor.c
index f79b833..9d4c168 100644
--- a/monitor.c
+++ b/monitor.c
@@ -72,6 +72,7 @@ typedef struct mon_cmd_t {
     const char *name;
     const char *args_type;
     void *handler;
+    void (*user_print)(Monitor *mon, const QObject *data);
     const char *params;
     const char *help;
 } mon_cmd_t;
@@ -1813,84 +1814,302 @@ 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,
+        .params     = "",
+        .help       = "show the version of QEMU"
+    },
+    {
+        .name       = "network",
+        .args_type  = "",
+        .handler    = do_info_network,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the network state"
+    },
+    {
+        .name       = "chardev",
+        .args_type  = "",
+        .handler    = qemu_chr_info,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the character devices"
+    },
+    {
+        .name       = "block",
+        .args_type  = "",
+        .handler    = bdrv_info,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the block devices"
+    },
+    {
+        .name       = "blockstats",
+        .args_type  = "",
+        .handler    = bdrv_info_stats,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show block device statistics"
+    },
+    {
+        .name       = "registers",
+        .args_type  = "",
+        .handler    = do_info_registers,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the cpu registers"
+    },
+    {
+        .name       = "cpus",
+        .args_type  = "",
+        .handler    = do_info_cpus,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show infos for each CPU"
+    },
+    {
+        .name       = "history",
+        .args_type  = "",
+        .handler    = do_info_history,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the command line history"
+    },
+    {
+        .name       = "irq",
+        .args_type  = "",
+        .handler    = irq_info,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the interrupts statistics (if available)"
+    },
+    {
+        .name       = "pic",
+        .args_type  = "",
+        .handler    = pic_info,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show i8259 (PIC) state"
+    },
+    {
+        .name       = "pci",
+        .args_type  = "",
+        .handler    = pci_info,
+        .user_print = 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,
+        .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,
+        .params     = "",
+        .help       = "show the active virtual memory mappings"
+    },
+    {
+        .name       = "hpet",
+        .args_type  = "",
+        .handler    = do_info_hpet,
+        .user_print = 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,
+        .params     = "",
+        .help       = "show dynamic compiler info"
+    },
+    {
+        .name       = "kvm",
+        .args_type  = "",
+        .handler    = do_info_kvm,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show KVM information"
+    },
+    {
+        .name       = "numa",
+        .args_type  = "",
+        .handler    = do_info_numa,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show NUMA information"
+    },
+    {
+        .name       = "usb",
+        .args_type  = "",
+        .handler    = usb_info,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show guest USB devices"
+    },
+    {
+        .name       = "usbhost",
+        .args_type  = "",
+        .handler    = usb_host_info,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show host USB devices"
+    },
+    {
+        .name       = "profile",
+        .args_type  = "",
+        .handler    = do_info_profile,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show profiling information"
+    },
+    {
+        .name       = "capture",
+        .args_type  = "",
+        .handler    = do_info_capture,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show capture information"
+    },
+    {
+        .name       = "snapshots",
+        .args_type  = "",
+        .handler    = do_info_snapshots,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the currently saved VM snapshots"
+    },
+    {
+        .name       = "status",
+        .args_type  = "",
+        .handler    = do_info_status,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the current VM status (running|paused)"
+    },
+    {
+        .name       = "pcmcia",
+        .args_type  = "",
+        .handler    = pcmcia_info,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show guest PCMCIA status"
+    },
+    {
+        .name       = "mice",
+        .args_type  = "",
+        .handler    = do_info_mice,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show which guest mouse is receiving events"
+    },
+    {
+        .name       = "vnc",
+        .args_type  = "",
+        .handler    = do_info_vnc,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the vnc server status"
+    },
+    {
+        .name       = "name",
+        .args_type  = "",
+        .handler    = do_info_name,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show the current VM name"
+    },
+    {
+        .name       = "uuid",
+        .args_type  = "",
+        .handler    = do_info_uuid,
+        .user_print = 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,
+        .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,
+        .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" },
-    { "roms", "", do_info_roms,
-      "", "show roms" },
-    { NULL, NULL, },
+    {
+        .name       = "migrate",
+        .args_type  = "",
+        .handler    = do_info_migrate,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show migration status"
+    },
+    {
+        .name       = "balloon",
+        .args_type  = "",
+        .handler    = do_info_balloon,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show balloon information"
+    },
+    {
+        .name       = "qtree",
+        .args_type  = "",
+        .handler    = do_info_qtree,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show device tree"
+    },
+    {
+        .name       = "qdm",
+        .args_type  = "",
+        .handler    = do_info_qdm,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show qdev device model list"
+    },
+    {
+        .name       = "roms",
+        .args_type  = "",
+        .handler    = do_info_roms,
+        .user_print = NULL,
+        .params     = "",
+        .help       = "show roms"
+    },
+    {
+        .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 784d30e..dfd0951 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -9,21 +9,43 @@ 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,
+        .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,
+        .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,
+        .params     = "[subcommand]",
+        .help       = "show various information about the system state"
+    },
+
 STEXI
 @item info @var{subcommand}
 Show various information about the system state.
@@ -94,22 +116,43 @@ show device tree
 @end table
 ETEXI
 
-    { "q|quit", "", do_quit,
-      "", "quit the emulator" },
+    {
+        .name       = "q|quit",
+        .args_type  = "",
+        .handler    = do_quit,
+        .user_print = 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,
+        .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,
+        .params     = "device filename [format]",
+        .help       = "change a removable medium, optional format"
+    },
+
 STEXI
 @item change @var{device} @var{setting}
 
@@ -147,29 +190,57 @@ 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,
+        .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,
+        .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,
+        .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,
+        .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 +249,115 @@ 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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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 +416,15 @@ 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,
+        .params     = "/fmt expr",
+        .help       = "print expression value (use $reg for CPU register access)",
+    },
+
 STEXI
 @item p or print/@var{fmt} @var{expr}
 
@@ -298,20 +432,41 @@ 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,
+        .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,
+        .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,
+        .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 +481,60 @@ 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,
+        .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,
+        .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,
+        .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,
+        .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 +542,15 @@ 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,
+        .params     = "device",
+        .help       = "remove USB device 'bus.addr'"
+    },
+
 STEXI
 @item usb_del @var{devname}
 
@@ -369,45 +559,87 @@ hub. @var{devname} has the syntax @code{bus.addr}. Use the monitor
 command @code{info usb} to see the devices you can remove.
 ETEXI
 
-    { "device_add", "config:s", do_device_add,
-      "device", "add device, like -device on the command line" },
+    {
+        .name       = "device_add",
+        .args_type  = "config:s",
+        .handler    = do_device_add,
+        .user_print = NULL,
+        .params     = "device",
+        .help       = "add device, like -device on the command line"
+    },
+
 STEXI
 @item device_add @var{config}
 
 Add device.
 ETEXI
 
-    { "device_del", "id:s", do_device_del,
-      "device", "remove device" },
+    {
+        .name       = "device_del",
+        .args_type  = "id:s",
+        .handler    = do_device_del,
+        .user_print = NULL,
+        .params     = "device",
+        .help       = "remove device"
+    },
+
 STEXI
 @item device_del @var{id}
 
 Remove device @var{id}.
 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,
+        .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,
+        .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,
+        .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,
+        .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
@@ -418,9 +650,14 @@ 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,
+        .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}]]]
@@ -436,8 +673,14 @@ 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,
+        .params     = "capture index",
+        .help       = "stop capture"
+    },
 #endif
 STEXI
 @item stopcapture @var{index}
@@ -447,22 +690,43 @@ 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,
+        .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,
+        .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,
+        .params     = "bootdevice",
+        .help       = "define new values for the boot device list"
+    },
+
 STEXI
 @item boot_set @var{bootdevicelist}
 
@@ -474,37 +738,70 @@ 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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .params     = "value",
+        .help       = "set maximum tolerated downtime (in seconds) for migrations"
+    },
 
 STEXI
 @item migrate_set_downtime @var{second}
@@ -512,85 +809,161 @@ 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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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,
+        .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
@@ -599,8 +972,15 @@ 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,
+        .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
@@ -608,8 +988,15 @@ 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,
+        .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.
@@ -620,15 +1007,29 @@ 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,
+        .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,
+        .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
@@ -636,15 +1037,31 @@ 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,
+        .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,
+        .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
@@ -652,8 +1069,15 @@ 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,
+        .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.17.gdbc1b

[Qemu-devel] Re: [PATCH 04/15] monitor: Add user_print() to mon_cmd_t

[Anthony Liguori]

Luiz Capitulino wrote:
> This new struct member will store a pointer to a function that
> should be used to output data in the user protocol format.
>
> It will also serve as a flag to say if a given handler has already
> been converted to the new QObject style.
>
> Additionally, this commit converts mon_cmd_t static initializations
> to the C99 way.
>   

This should really be two patches.  One that does the mon_cmd_t init 
conversion and then another that introduces user_print.  This would be 
easier if...

<snip>
> +    {
> +        .name       = "version",
> +        .args_type  = "",
> +        .handler    = do_info_version,
> +        .user_print = NULL,
> +        .params     = "",
> +        .help       = "show the version of QEMU"
> +    },
>   

You didn't explicitly initialize things to NULL and made sure that 
args_types = "" worked the same as args_type = NULL (and likewise for 
params.  This would help de-uglify what's happening in this file :-)


-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 04/15] monitor: Add user_print() to mon_cmd_t

[Luiz Capitulino]

On Tue, 06 Oct 2009 20:40:31 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Luiz Capitulino wrote:
> > This new struct member will store a pointer to a function that
> > should be used to output data in the user protocol format.
> >
> > It will also serve as a flag to say if a given handler has already
> > been converted to the new QObject style.
> >
> > Additionally, this commit converts mon_cmd_t static initializations
> > to the C99 way.
> >   
> 
> This should really be two patches.  One that does the mon_cmd_t init 
> conversion and then another that introduces user_print.  This would be 
> easier if...

 Ok, will split.

> <snip>
> > +    {
> > +        .name       = "version",
> > +        .args_type  = "",
> > +        .handler    = do_info_version,
> > +        .user_print = NULL,
> > +        .params     = "",
> > +        .help       = "show the version of QEMU"
> > +    },
> >   
> 
> You didn't explicitly initialize things to NULL and made sure that 
> args_types = "" worked the same as args_type = NULL (and likewise for 
> params.  This would help de-uglify what's happening in this file :-)

 I think this work is part of the patch series which will change
args_type to be an array, as suggested by Markus.

 This is series is just an initial help.

[Qemu-devel] [PATCH 05/15] 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).

Converted handlers will use the 'user_print' member of 'mon_cmd_t'
to define its user protocol function, which will be called to print
data in the user protocol format.

Handlers which don't have 'user_print' defined are not converted
and are handled as usual.

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

diff --git a/monitor.c b/monitor.c
index 9d4c168..c0ba5ee 100644
--- a/monitor.c
+++ b/monitor.c
@@ -211,6 +211,11 @@ static int monitor_fprintf(FILE *stream, const char *fmt, ...)
     return 0;
 }
 
+static inline 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;
@@ -3053,17 +3058,32 @@ static void monitor_handle_command(Monitor *mon, const char *cmdline)
     qdict = qdict_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);
+    if (monitor_handler_ported(cmd)) {
+        QObject *data = NULL;
+        void (*handler_new)(Monitor *mon, const QDict *params,
+                            QObject **ret_data);
 
-        handler = cmd->handler;
-        handler(mon, qdict);
+        handler_new = cmd->handler;
+        handler_new(mon, qdict, &data);
 
-        qemu_errors_to_previous();
+        if (data)
+            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);
 }
 
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] Re: [PATCH 05/15] monitor: Handle new and old style handlers

[Anthony Liguori]

Luiz Capitulino wrote:
> 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).
>
> Converted handlers will use the 'user_print' member of 'mon_cmd_t'
> to define its user protocol function, which will be called to print
> data in the user protocol format.
>
> Handlers which don't have 'user_print' defined are not converted
> and are handled as usual.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  monitor.c |   32 ++++++++++++++++++++++++++------
>  1 files changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 9d4c168..c0ba5ee 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -211,6 +211,11 @@ static int monitor_fprintf(FILE *stream, const char *fmt, ...)
>      return 0;
>  }
>
> +static inline 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;
> @@ -3053,17 +3058,32 @@ static void monitor_handle_command(Monitor *mon, const char *cmdline)
>      qdict = qdict_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);
> +    if (monitor_handler_ported(cmd)) {
> +        QObject *data = NULL;
> +        void (*handler_new)(Monitor *mon, const QDict *params,
> +                            QObject **ret_data);
>   

Screams to be a typedef.

> -        handler = cmd->handler;
> -        handler(mon, qdict);
> +        handler_new = cmd->handler;
> +        handler_new(mon, qdict, &data);
>   

At this point, couldn't cmd->handler be a union so we had a bit more 
type safety?

-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 05/15] monitor: Handle new and old style handlers

[Luiz Capitulino]

On Tue, 06 Oct 2009 20:42:47 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Luiz Capitulino wrote:
> > 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).
> >
> > Converted handlers will use the 'user_print' member of 'mon_cmd_t'
> > to define its user protocol function, which will be called to print
> > data in the user protocol format.
> >
> > Handlers which don't have 'user_print' defined are not converted
> > and are handled as usual.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  monitor.c |   32 ++++++++++++++++++++++++++------
> >  1 files changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 9d4c168..c0ba5ee 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -211,6 +211,11 @@ static int monitor_fprintf(FILE *stream, const char *fmt, ...)
> >      return 0;
> >  }
> >
> > +static inline 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;
> > @@ -3053,17 +3058,32 @@ static void monitor_handle_command(Monitor *mon, const char *cmdline)
> >      qdict = qdict_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);
> > +    if (monitor_handler_ported(cmd)) {
> > +        QObject *data = NULL;
> > +        void (*handler_new)(Monitor *mon, const QDict *params,
> > +                            QObject **ret_data);
> >   
> 
> Screams to be a typedef.
> 
> > -        handler = cmd->handler;
> > -        handler(mon, qdict);
> > +        handler_new = cmd->handler;
> > +        handler_new(mon, qdict, &data);
> >   
> 
> At this point, couldn't cmd->handler be a union so we had a bit more 
> type safety?

 Yes, will do.

[Qemu-devel] [PATCH 06/15] 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 a new function called
monitor_user_noop(), it should be used by handlers which do
not have data to print.

This is the case of do_info().

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

diff --git a/monitor.c b/monitor.c
index c0ba5ee..81f3785 100644
--- a/monitor.c
+++ b/monitor.c
@@ -211,6 +211,8 @@ static int monitor_fprintf(FILE *stream, const char *fmt, ...)
     return 0;
 }
 
+static void monitor_user_noop(Monitor *mon, const QObject *data) { }
+
 static inline int monitor_handler_ported(const mon_cmd_t *cmd)
 {
     return cmd->user_print != NULL;
@@ -285,24 +287,40 @@ 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)
 {
     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);
+
+        handler_new = cmd->handler;
+        handler_new(mon, ret_data);
+
+        if (*ret_data)
+            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 dfd0951..170c6ae 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -41,7 +41,7 @@ ETEXI
         .name       = "info",
         .args_type  = "item:s?",
         .handler    = do_info,
-        .user_print = NULL,
+        .user_print = monitor_user_noop,
         .params     = "[subcommand]",
         .help       = "show various information about the system state"
     },
-- 
1.6.5.rc2.17.gdbc1b

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

[Luiz Capitulino]

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

diff --git a/monitor.c b/monitor.c
index 81f3785..9046c5c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -458,7 +458,10 @@ 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)
 {
     exit(0);
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 170c6ae..eb5e728 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -120,7 +120,7 @@ ETEXI
         .name       = "q|quit",
         .args_type  = "",
         .handler    = do_quit,
-        .user_print = NULL,
+        .user_print = monitor_user_noop,
         .params     = "",
         .help       = "quit the emulator"
     },
-- 
1.6.5.rc2.17.gdbc1b

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

[Luiz Capitulino]

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

diff --git a/monitor.c b/monitor.c
index 9046c5c..71e659d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -600,7 +600,10 @@ 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)
 {
     vm_stop(EXCP_INTERRUPT);
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index eb5e728..a3e4b54 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -297,7 +297,7 @@ ETEXI
         .name       = "stop",
         .args_type  = "",
         .handler    = do_stop,
-        .user_print = NULL,
+        .user_print = monitor_user_noop,
         .params     = "",
         .help       = "stop emulation",
     },
-- 
1.6.5.rc2.17.gdbc1b

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

[Luiz Capitulino]

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

diff --git a/monitor.c b/monitor.c
index 71e659d..d1396c3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1293,7 +1293,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)
 {
     qemu_system_reset_request();
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index a3e4b54..4b18680 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -485,7 +485,7 @@ ETEXI
         .name       = "system_reset",
         .args_type  = "",
         .handler    = do_system_reset,
-        .user_print = NULL,
+        .user_print = monitor_user_noop,
         .params     = "",
         .help       = "reset the system"
     },
-- 
1.6.5.rc2.17.gdbc1b

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

[Luiz Capitulino]

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

diff --git a/monitor.c b/monitor.c
index d1396c3..198f0fc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1302,7 +1302,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)
 {
     qemu_system_powerdown_request();
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 4b18680..eef0c8e 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -500,7 +500,7 @@ ETEXI
         .name       = "system_powerdown",
         .args_type  = "",
         .handler    = do_system_powerdown,
-        .user_print = NULL,
+        .user_print = monitor_user_noop,
         .params     = "",
         .help       = "send system power down event"
     },
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 11/15] monitor: Convert do_cont() to QObject

[Luiz Capitulino]

Appropriate error handling support will be needed to have
encrypted images working under the future machine protocol,
but this initial conversion will work with the current
user protocol.

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

diff --git a/monitor.c b/monitor.c
index 198f0fc..094b090 100644
--- a/monitor.c
+++ b/monitor.c
@@ -615,7 +615,10 @@ struct bdrv_iterate_context {
     int err;
 };
 
-static void do_cont(Monitor *mon, const QDict *qdict)
+/**
+ * do_cont(): Resume emulation.
+ */
+static void do_cont(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     struct bdrv_iterate_context context = { mon, 0 };
 
@@ -631,7 +634,7 @@ static void bdrv_key_cb(void *opaque, int err)
 
     /* another key was set successfully, retry to continue */
     if (!err)
-        do_cont(mon, NULL);
+        do_cont(mon, NULL, NULL);
 }
 
 static void encrypted_bdrv_it(void *opaque, BlockDriverState *bs)
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index eef0c8e..935e8ea 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -311,7 +311,7 @@ ETEXI
         .name       = "c|cont",
         .args_type  = "",
         .handler    = do_cont,
-        .user_print = NULL,
+        .user_print = monitor_user_noop,
         .params     = "",
         .help       = "resume emulation",
     },
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 12/15] monitor: Convert do_balloon() to QObject

[Luiz Capitulino]

It is important to note 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       |    6 ++++--
 qemu-monitor.hx |    2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index 094b090..cb28ffb 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1603,8 +1603,10 @@ 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)
 {
     int value = qdict_get_int(qdict, "value");
     ram_addr_t target = value;
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 935e8ea..efaabea 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -917,7 +917,7 @@ ETEXI
         .name       = "balloon",
         .args_type  = "value:i",
         .handler    = do_balloon,
-        .user_print = NULL,
+        .user_print = monitor_user_noop,
         .params     = "target",
         .help       = "request VM to change it's memory allocation (in MB)"
     },
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 13/15] 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 |   27 ++++++++++++++++++++++++---
 1 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index cb28ffb..2a1e4d7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -218,6 +218,24 @@ static inline int monitor_handler_ported(const mon_cmd_t *cmd)
     return cmd->user_print != NULL;
 }
 
+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;
@@ -323,9 +341,12 @@ 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)
 {
-    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)
@@ -1860,7 +1881,7 @@ static const mon_cmd_t info_cmds[] = {
         .name       = "version",
         .args_type  = "",
         .handler    = do_info_version,
-        .user_print = NULL,
+        .user_print = monitor_print_qobject,
         .params     = "",
         .help       = "show the version of QEMU"
     },
-- 
1.6.5.rc2.17.gdbc1b

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

[Luiz Capitulino]

On success return a QInt with the balloon's value.

This also introduces monitor_print_balloon() to print the
balloon information in the user protocol.

Please, note that errors are not being converted yet.

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

diff --git a/monitor.c b/monitor.c
index 2a1e4d7..0dcf478 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1634,7 +1634,16 @@ 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)
 {
     ram_addr_t actual;
 
@@ -1645,7 +1654,7 @@ static void do_info_balloon(Monitor *mon)
     else if (actual == 0)
         monitor_printf(mon, "Ballooning not activated in VM\n");
     else
-        monitor_printf(mon, "balloon: actual=%d\n", (int)(actual >> 20));
+        *ret_data = QOBJECT(qint_from_int((int)(actual >> 20)));
 }
 
 static qemu_acl *find_acl(Monitor *mon, const char *name)
@@ -2137,7 +2146,7 @@ static const mon_cmd_t info_cmds[] = {
         .name       = "balloon",
         .args_type  = "",
         .handler    = do_info_balloon,
-        .user_print = NULL,
+        .user_print = monitor_print_balloon,
         .params     = "",
         .help       = "show balloon information"
     },
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] [PATCH 15/15] monitor: Convert do_info_cpus() to QObject

[Luiz Capitulino]

Each CPU information is stored in a QDict and the returned
QObject is a QList of all CPUs.

The QDict contains the following information:

- "CPU": cpu index
- "current": "yes" or "no"
- "pc": current PC
- "halted": "yes" or "no"

The user output in the Monitor should not change and the
future monitor protocol is expected to emit something like:

[ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
{ "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]

which corresponds to the following user output:

* CPU #0: pc=0x00000000fffffff0
  CPU #1: pc=0x00000000fffffff0 (halted)

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

diff --git a/monitor.c b/monitor.c
index 0dcf478..1487b72 100644
--- a/monitor.c
+++ b/monitor.c
@@ -46,6 +46,7 @@
 #include "kvm.h"
 #include "acl.h"
 #include "qint.h"
+#include "qlist.h"
 #include "qdict.h"
 #include "qstring.h"
 
@@ -410,33 +411,98 @@ static void do_info_registers(Monitor *mon)
 #endif
 }
 
-static void do_info_cpus(Monitor *mon)
+static void print_cpu_iter(QObject *obj, void *opaque)
+{
+    QDict *cpu;
+    int active = ' ';
+    Monitor *mon = opaque;
+
+    assert(qobject_type(obj) == QTYPE_QDICT);
+    cpu = qobject_to_qdict(obj);
+
+    if (strcmp(qdict_get_str(cpu, "current"), "yes") == 0)
+        active = '*';
+
+    monitor_printf(mon, "%c CPU #%d: ", active, (int)qdict_get_int(cpu, "CPU"));
+
+#if defined(TARGET_I386)
+    monitor_printf(mon, "pc=0x" TARGET_FMT_lx,
+                   (target_ulong) qdict_get_int(cpu, "pc"));
+#elif defined(TARGET_PPC)
+    monitor_printf(mon, "nip=0x" TARGET_FMT_lx,
+                   (target_long) qdict_get_int(cpu, "nip"));
+#elif defined(TARGET_SPARC)
+    monitor_printf(mon, "pc=0x " TARGET_FMT_lx,
+                   (target_long) qdict_get_int(cpu, "pc"));
+    monitor_printf(mon, "npc=0x" TARGET_FMT_lx,
+                   (target_long) qdict_get_int(cpu, "npc"));
+#elif defined(TARGET_MIPS)
+    monitor_printf(mon, "PC=0x" TARGET_FMT_lx,
+                   (target_long) qdict_get_int(cpu, "PC"));
+#endif
+
+    if (strcmp(qdict_get_str(cpu, "halted"), "yes") == 0)
+        monitor_printf(mon, " (halted)");
+
+    monitor_printf(mon, "\n");
+}
+
+static void monitor_print_cpus(Monitor *mon, const QObject *data)
+{
+    QList *cpu_list;
+
+    assert(qobject_type(data) == QTYPE_QLIST);
+    cpu_list = qobject_to_qlist(data);
+    qlist_iter(cpu_list, print_cpu_iter, mon);
+}
+
+/**
+ * do_info_cpus(): Show CPU information
+ *
+ * Return a QList with a QDict for each CPU.
+ *
+ * For example:
+ *
+ * [ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
+ *   { "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]
+ */
+static void do_info_cpus(Monitor *mon, QObject **ret_data)
 {
     CPUState *env;
+    QList *cpu_list;
+
+    cpu_list = qlist_new();
 
     /* just to set the default cpu if not already done */
     mon_get_cpu();
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
+        const char *answer;
+        QDict *cpu = qdict_new();
+
         cpu_synchronize_state(env);
-        monitor_printf(mon, "%c CPU #%d:",
-                       (env == mon->mon_cpu) ? '*' : ' ',
-                       env->cpu_index);
+
+        qdict_put(cpu, "CPU", qint_from_int(env->cpu_index));
+        answer = (env == mon->mon_cpu) ? "yes" : "no";
+        qdict_put(cpu, "current", qstring_from_str(answer));
+
 #if defined(TARGET_I386)
-        monitor_printf(mon, " pc=0x" TARGET_FMT_lx,
-                       env->eip + env->segs[R_CS].base);
+        qdict_put(cpu, "pc", qint_from_int(env->eip + env->segs[R_CS].base));
 #elif defined(TARGET_PPC)
-        monitor_printf(mon, " nip=0x" TARGET_FMT_lx, env->nip);
+        qdict_put(cpu, "nip", qint_from_int(env->nip));
 #elif defined(TARGET_SPARC)
-        monitor_printf(mon, " pc=0x" TARGET_FMT_lx " npc=0x" TARGET_FMT_lx,
-                       env->pc, env->npc);
+        qdict_put(cpu, "pc", qint_from_int(env->pc));
+        qdict_put(cpu, "npc", qint_from_int(env->npc));
 #elif defined(TARGET_MIPS)
-        monitor_printf(mon, " PC=0x" TARGET_FMT_lx, env->active_tc.PC);
+        qdict_put(cpu, "PC", qint_from_int(env->active_tc.PC));
 #endif
-        if (env->halted)
-            monitor_printf(mon, " (halted)");
-        monitor_printf(mon, "\n");
+        answer = env->halted ? "yes" : "no";
+        qdict_put(cpu, "halted", qstring_from_str(answer));
+
+        qlist_append(cpu_list, cpu);
     }
+
+    *ret_data = QOBJECT(cpu_list);
 }
 
 static void do_cpu_set(Monitor *mon, const QDict *qdict)
@@ -1938,7 +2004,7 @@ static const mon_cmd_t info_cmds[] = {
         .name       = "cpus",
         .args_type  = "",
         .handler    = do_info_cpus,
-        .user_print = NULL,
+        .user_print = monitor_print_cpus,
         .params     = "",
         .help       = "show infos for each CPU"
     },
-- 
1.6.5.rc2.17.gdbc1b

[Qemu-devel] Re: [PATCH 15/15] monitor: Convert do_info_cpus() to QObject

[Anthony Liguori]

Luiz Capitulino wrote:
> Each CPU information is stored in a QDict and the returned
> QObject is a QList of all CPUs.
>
> The QDict contains the following information:
>
> - "CPU": cpu index
> - "current": "yes" or "no"
> - "pc": current PC
> - "halted": "yes" or "no"
>
> The user output in the Monitor should not change and the
> future monitor protocol is expected to emit something like:
>
> [ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
> { "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]
>
> which corresponds to the following user output:
>
> * CPU #0: pc=0x00000000fffffff0
>   CPU #1: pc=0x00000000fffffff0 (halted)
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  monitor.c |   94 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 80 insertions(+), 14 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 0dcf478..1487b72 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -46,6 +46,7 @@
>  #include "kvm.h"
>  #include "acl.h"
>  #include "qint.h"
> +#include "qlist.h"
>  #include "qdict.h"
>  #include "qstring.h"
>
> @@ -410,33 +411,98 @@ static void do_info_registers(Monitor *mon)
>  #endif
>  }
>
> -static void do_info_cpus(Monitor *mon)
> +static void print_cpu_iter(QObject *obj, void *opaque)
> +{
> +    QDict *cpu;
> +    int active = ' ';
> +    Monitor *mon = opaque;
> +
> +    assert(qobject_type(obj) == QTYPE_QDICT);
> +    cpu = qobject_to_qdict(obj);
> +
> +    if (strcmp(qdict_get_str(cpu, "current"), "yes") == 0)
> +        active = '*';
> +
> +    monitor_printf(mon, "%c CPU #%d: ", active, (int)qdict_get_int(cpu, "CPU"));
> +
> +#if defined(TARGET_I386)
> +    monitor_printf(mon, "pc=0x" TARGET_FMT_lx,
> +                   (target_ulong) qdict_get_int(cpu, "pc"));
> +#elif defined(TARGET_PPC)
> +    monitor_printf(mon, "nip=0x" TARGET_FMT_lx,
> +                   (target_long) qdict_get_int(cpu, "nip"));
> +#elif defined(TARGET_SPARC)
> +    monitor_printf(mon, "pc=0x " TARGET_FMT_lx,
> +                   (target_long) qdict_get_int(cpu, "pc"));
> +    monitor_printf(mon, "npc=0x" TARGET_FMT_lx,
> +                   (target_long) qdict_get_int(cpu, "npc"));
> +#elif defined(TARGET_MIPS)
> +    monitor_printf(mon, "PC=0x" TARGET_FMT_lx,
> +                   (target_long) qdict_get_int(cpu, "PC"));
> +#endif
> +
> +    if (strcmp(qdict_get_str(cpu, "halted"), "yes") == 0)
> +        monitor_printf(mon, " (halted)");
> +
> +    monitor_printf(mon, "\n");
> +}
> +
> +static void monitor_print_cpus(Monitor *mon, const QObject *data)
> +{
> +    QList *cpu_list;
> +
> +    assert(qobject_type(data) == QTYPE_QLIST);
> +    cpu_list = qobject_to_qlist(data);
> +    qlist_iter(cpu_list, print_cpu_iter, mon);
> +}
> +
> +/**
> + * do_info_cpus(): Show CPU information
> + *
> + * Return a QList with a QDict for each CPU.
> + *
> + * For example:
> + *
> + * [ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
> + *   { "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]
> + */
> +static void do_info_cpus(Monitor *mon, QObject **ret_data)
>  {
>      CPUState *env;
> +    QList *cpu_list;
> +
> +    cpu_list = qlist_new();
>
>      /* just to set the default cpu if not already done */
>      mon_get_cpu();
>
>      for(env = first_cpu; env != NULL; env = env->next_cpu) {
> +        const char *answer;
> +        QDict *cpu = qdict_new();
> +
>          cpu_synchronize_state(env);
> -        monitor_printf(mon, "%c CPU #%d:",
> -                       (env == mon->mon_cpu) ? '*' : ' ',
> -                       env->cpu_index);
> +
> +        qdict_put(cpu, "CPU", qint_from_int(env->cpu_index));
>   

I'd almost think that a json parser that supported varadics would make 
this much easier..

-- 
Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 15/15] monitor: Convert do_info_cpus() to QObject

[Luiz Capitulino]

On Tue, 06 Oct 2009 20:46:53 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Luiz Capitulino wrote:
> > Each CPU information is stored in a QDict and the returned
> > QObject is a QList of all CPUs.
> >
> > The QDict contains the following information:
> >
> > - "CPU": cpu index
> > - "current": "yes" or "no"
> > - "pc": current PC
> > - "halted": "yes" or "no"
> >
> > The user output in the Monitor should not change and the
> > future monitor protocol is expected to emit something like:
> >
> > [ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
> > { "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]
> >
> > which corresponds to the following user output:
> >
> > * CPU #0: pc=0x00000000fffffff0
> >   CPU #1: pc=0x00000000fffffff0 (halted)
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  monitor.c |   94 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
> >  1 files changed, 80 insertions(+), 14 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 0dcf478..1487b72 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -46,6 +46,7 @@
> >  #include "kvm.h"
> >  #include "acl.h"
> >  #include "qint.h"
> > +#include "qlist.h"
> >  #include "qdict.h"
> >  #include "qstring.h"
> >
> > @@ -410,33 +411,98 @@ static void do_info_registers(Monitor *mon)
> >  #endif
> >  }
> >
> > -static void do_info_cpus(Monitor *mon)
> > +static void print_cpu_iter(QObject *obj, void *opaque)
> > +{
> > +    QDict *cpu;
> > +    int active = ' ';
> > +    Monitor *mon = opaque;
> > +
> > +    assert(qobject_type(obj) == QTYPE_QDICT);
> > +    cpu = qobject_to_qdict(obj);
> > +
> > +    if (strcmp(qdict_get_str(cpu, "current"), "yes") == 0)
> > +        active = '*';
> > +
> > +    monitor_printf(mon, "%c CPU #%d: ", active, (int)qdict_get_int(cpu, "CPU"));
> > +
> > +#if defined(TARGET_I386)
> > +    monitor_printf(mon, "pc=0x" TARGET_FMT_lx,
> > +                   (target_ulong) qdict_get_int(cpu, "pc"));
> > +#elif defined(TARGET_PPC)
> > +    monitor_printf(mon, "nip=0x" TARGET_FMT_lx,
> > +                   (target_long) qdict_get_int(cpu, "nip"));
> > +#elif defined(TARGET_SPARC)
> > +    monitor_printf(mon, "pc=0x " TARGET_FMT_lx,
> > +                   (target_long) qdict_get_int(cpu, "pc"));
> > +    monitor_printf(mon, "npc=0x" TARGET_FMT_lx,
> > +                   (target_long) qdict_get_int(cpu, "npc"));
> > +#elif defined(TARGET_MIPS)
> > +    monitor_printf(mon, "PC=0x" TARGET_FMT_lx,
> > +                   (target_long) qdict_get_int(cpu, "PC"));
> > +#endif
> > +
> > +    if (strcmp(qdict_get_str(cpu, "halted"), "yes") == 0)
> > +        monitor_printf(mon, " (halted)");
> > +
> > +    monitor_printf(mon, "\n");
> > +}
> > +
> > +static void monitor_print_cpus(Monitor *mon, const QObject *data)
> > +{
> > +    QList *cpu_list;
> > +
> > +    assert(qobject_type(data) == QTYPE_QLIST);
> > +    cpu_list = qobject_to_qlist(data);
> > +    qlist_iter(cpu_list, print_cpu_iter, mon);
> > +}
> > +
> > +/**
> > + * do_info_cpus(): Show CPU information
> > + *
> > + * Return a QList with a QDict for each CPU.
> > + *
> > + * For example:
> > + *
> > + * [ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
> > + *   { "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]
> > + */
> > +static void do_info_cpus(Monitor *mon, QObject **ret_data)
> >  {
> >      CPUState *env;
> > +    QList *cpu_list;
> > +
> > +    cpu_list = qlist_new();
> >
> >      /* just to set the default cpu if not already done */
> >      mon_get_cpu();
> >
> >      for(env = first_cpu; env != NULL; env = env->next_cpu) {
> > +        const char *answer;
> > +        QDict *cpu = qdict_new();
> > +
> >          cpu_synchronize_state(env);
> > -        monitor_printf(mon, "%c CPU #%d:",
> > -                       (env == mon->mon_cpu) ? '*' : ' ',
> > -                       env->cpu_index);
> > +
> > +        qdict_put(cpu, "CPU", qint_from_int(env->cpu_index));
> >   
> 
> I'd almost think that a json parser that supported varadics would make 
> this much easier..

 In this specific case most values are conditionally inserted into the dict,
so I think it's easier to insert them separately.

 But I've a function called qobject_from_fmt() which allows one to
do something like:

QObject *obj = qobject_from_fmt("{ s: [ i, s ], s: i }", ... );

 I'm not sure I've implemented it correctly, but will submit as
part of the series that will use it.