[Qemu-devel] [FOR 0.12 v3 00/19]: info handlers conversions to QObject

[Qemu-devel] [FOR 0.12 v3 00/19]: info handlers conversions to QObject

[Luiz Capitulino]

 Hi,

 This series covers almost half of the info handlers conversions to the
QObject style.

 The biggest change in this version is that now info handlers always
return a QDict.

changelog
---------

v2 -> v3

- Change all info handlers to return a dict
- Fix bad do_info_migrate() rebase
- do_info_cpus(): Use QBool
- do_info_version(): Return QDict

v1 -> v2

- Rebase
- do_info_vnc(): drop keys with 'none' strings
- Fix do_info_balloon() to return bytes

v0 -> v1

- Minor fixes (indentation, casts, etc)
- Improved some documentation and changelogs
- Dropped do_info_network() conversion, it still needs work

[Qemu-devel] [PATCH 01/19] Introduce qemu-objects.h header file

[Luiz Capitulino]

An easy way to include all QEMU objects.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qemu-objects.h |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)
 create mode 100644 qemu-objects.h

diff --git a/qemu-objects.h b/qemu-objects.h
new file mode 100644
index 0000000..e1d1e0c
--- /dev/null
+++ b/qemu-objects.h
@@ -0,0 +1,24 @@
+/*
+ * Include all QEMU objects.
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ *
+ * Authors:
+ *  Luiz Capitulino <lcapitulino@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+#ifndef QEMU_OBJECTS_H
+#define QEMU_OBJECTS_H
+
+#include "qobject.h"
+#include "qint.h"
+#include "qfloat.h"
+#include "qbool.h"
+#include "qstring.h"
+#include "qdict.h"
+#include "qlist.h"
+#include "qjson.h"
+
+#endif /* QEMU_OBJECTS_H */
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 02/19] Makefile: move QObject objs to their own entry

[Luiz Capitulino]

Other subsystems will need to link against them.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 85ad688..ea90c23 100644
--- a/Makefile
+++ b/Makefile
@@ -82,6 +82,12 @@ ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
 recurse-all: $(SUBDIR_RULES) $(ROMSUBDIR_RULES)
 
 #######################################################################
+# QObject
+qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
+qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
+qobject-obj-y += qerror.o
+
+#######################################################################
 # block-obj-y is code used by both qemu system emulation and qemu-img
 
 block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
@@ -120,6 +126,7 @@ net-obj-y += $(addprefix net/, $(net-nested-y))
 
 obj-y = $(block-obj-y)
 obj-y += $(net-obj-y)
+obj-y += $(qobject-obj-y)
 obj-y += readline.o console.o
 
 obj-y += tcg-runtime.o host-utils.o
@@ -152,8 +159,6 @@ obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o
 obj-y += qemu-char.o aio.o savevm.o
 obj-y += msmouse.o ps2.o
 obj-y += qdev.o qdev-properties.o
-obj-y += qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o json-lexer.o
-obj-y += json-streamer.o json-parser.o qjson.o qerror.o
 obj-y += qemu-config.o block-migration.o
 
 obj-$(CONFIG_BRLAPI) += baum.o
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 03/19] QDict: Introduce qdict_get_qbool()

[Luiz Capitulino]

This is a helper function that does type checking before retrieving
a QBool from the dictionary.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile |    2 +-
 qdict.c  |   15 +++++++++++++++
 qdict.h  |    1 +
 3 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index ea90c23..b4356ae 100644
--- a/Makefile
+++ b/Makefile
@@ -246,7 +246,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-qdict: check-qdict.o qdict.o qint.o qstring.o qbool.o qemu-malloc.o
 check-qlist: check-qlist.o qlist.o qint.o qemu-malloc.o
 check-qfloat: check-qfloat.o qfloat.o qemu-malloc.o
 check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o qemu-malloc.o
diff --git a/qdict.c b/qdict.c
index 0e04cb1..45e08be 100644
--- a/qdict.c
+++ b/qdict.c
@@ -12,6 +12,7 @@
 
 #include "qint.h"
 #include "qdict.h"
+#include "qbool.h"
 #include "qstring.h"
 #include "qobject.h"
 #include "qemu-queue.h"
@@ -189,6 +190,20 @@ int64_t qdict_get_int(const QDict *qdict, const char *key)
 }
 
 /**
+ * qdict_get_bool(): Get a bool mapped by 'key'
+ *
+ * This function assumes that 'key' exists and it stores a
+ * QBool object.
+ *
+ * Return bool mapped by 'key'.
+ */
+int qdict_get_bool(const QDict *qdict, const char *key)
+{
+    QObject *obj = qdict_get_obj(qdict, key, QTYPE_QBOOL);
+    return qbool_get_int(qobject_to_qbool(obj));
+}
+
+/**
  * qdict_get_str(): Get a pointer to the stored string mapped
  * by 'key'
  *
diff --git a/qdict.h b/qdict.h
index 14b2633..1473c04 100644
--- a/qdict.h
+++ b/qdict.h
@@ -37,6 +37,7 @@ void qdict_iter(const QDict *qdict,
 
 /* High level helpers */
 int64_t qdict_get_int(const QDict *qdict, const char *key);
+int qdict_get_bool(const QDict *qdict, const char *key);
 const char *qdict_get_str(const QDict *qdict, const char *key);
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
                           int64_t err_value);
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 04/19] QDict: Introduce qdict_get_qlist()

[Luiz Capitulino]

A helper function to get a QList from a QDict.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile |    2 +-
 qdict.c  |   13 +++++++++++++
 qdict.h  |    2 ++
 3 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index b4356ae..f1b0131 100644
--- a/Makefile
+++ b/Makefile
@@ -246,7 +246,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 qbool.o qemu-malloc.o
+check-qdict: check-qdict.o qdict.o qint.o qstring.o qbool.o qemu-malloc.o qlist.o
 check-qlist: check-qlist.o qlist.o qint.o qemu-malloc.o
 check-qfloat: check-qfloat.o qfloat.o qemu-malloc.o
 check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o qemu-malloc.o
diff --git a/qdict.c b/qdict.c
index 45e08be..ef73265 100644
--- a/qdict.c
+++ b/qdict.c
@@ -204,6 +204,19 @@ int qdict_get_bool(const QDict *qdict, const char *key)
 }
 
 /**
+ * qdict_get_qlist(): Get the QList mapped by 'key'
+ *
+ * This function assumes that 'key' exists and it stores a
+ * QList object.
+ *
+ * Return QList mapped by 'key'.
+ */
+QList *qdict_get_qlist(const QDict *qdict, const char *key)
+{
+    return qobject_to_qlist(qdict_get_obj(qdict, key, QTYPE_QLIST));
+}
+
+/**
  * qdict_get_str(): Get a pointer to the stored string mapped
  * by 'key'
  *
diff --git a/qdict.h b/qdict.h
index 1473c04..5fef1ea 100644
--- a/qdict.h
+++ b/qdict.h
@@ -2,6 +2,7 @@
 #define QDICT_H
 
 #include "qobject.h"
+#include "qlist.h"
 #include "qemu-queue.h"
 #include <stdint.h>
 
@@ -38,6 +39,7 @@ void qdict_iter(const QDict *qdict,
 /* High level helpers */
 int64_t qdict_get_int(const QDict *qdict, const char *key);
 int qdict_get_bool(const QDict *qdict, const char *key);
+QList *qdict_get_qlist(const QDict *qdict, const char *key);
 const char *qdict_get_str(const QDict *qdict, const char *key);
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
                           int64_t err_value);
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 05/19] monitor: Fix do_info_balloon() output

[Luiz Capitulino]

Monitor commands should always return values in bytes and info
commands should always return a QDict.

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

diff --git a/monitor.c b/monitor.c
index a38a103..aa56ec7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1919,12 +1919,24 @@ static void do_balloon(Monitor *mon, const QDict *qdict, QObject **ret_data)
 
 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)));
+    QDict *qdict;
+
+    qdict = qobject_to_qdict(data);
+
+    monitor_printf(mon, "balloon: actual=%" PRId64 "\n",
+                        qdict_get_int(qdict, "balloon") >> 20);
 }
 
 /**
  * do_info_balloon(): Balloon information
+ *
+ * Return a QDict with the following information:
+ *
+ * - "balloon": current balloon value in bytes
+ *
+ * Example:
+ *
+ * { "balloon": 1073741824 }
  */
 static void do_info_balloon(Monitor *mon, QObject **ret_data)
 {
@@ -1936,7 +1948,8 @@ static void do_info_balloon(Monitor *mon, QObject **ret_data)
     else if (actual == 0)
         qemu_error_new(QERR_DEVICE_NOT_ACTIVE, "balloon");
     else
-        *ret_data = QOBJECT(qint_from_int((int)(actual >> 20)));
+        *ret_data = qobject_from_jsonf("{ 'balloon': %" PRId64 "}",
+                                       (int64_t) actual);
 }
 
 static qemu_acl *find_acl(Monitor *mon, const char *name)
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 06/19] monitor: do_info_cpus(): Use QBool

[Luiz Capitulino]

While there update the documentation as well.

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

diff --git a/monitor.c b/monitor.c
index aa56ec7..8729535 100644
--- a/monitor.c
+++ b/monitor.c
@@ -611,8 +611,9 @@ static void print_cpu_iter(QObject *obj, void *opaque)
     assert(qobject_type(obj) == QTYPE_QDICT);
     cpu = qobject_to_qdict(obj);
 
-    if (strcmp(qdict_get_str(cpu, "current"), "yes") == 0)
+    if (qdict_get_bool(cpu, "current")) {
         active = '*';
+    }
 
     monitor_printf(mon, "%c CPU #%d: ", active, (int)qdict_get_int(cpu, "CPU"));
 
@@ -632,8 +633,9 @@ static void print_cpu_iter(QObject *obj, void *opaque)
                    (target_long) qdict_get_int(cpu, "PC"));
 #endif
 
-    if (strcmp(qdict_get_str(cpu, "halted"), "yes") == 0)
+    if (qdict_get_bool(cpu, "halted")) {
         monitor_printf(mon, " (halted)");
+    }
 
     monitor_printf(mon, "\n");
 }
@@ -650,12 +652,22 @@ static void monitor_print_cpus(Monitor *mon, const QObject *data)
 /**
  * do_info_cpus(): Show CPU information
  *
- * Return a QList with a QDict for each CPU.
+ * Return a QList. Each CPU is represented by a QDict, which contains:
  *
- * For example:
+ * - "cpu": CPU index
+ * - "current": true if this is the current CPU, false otherwise
+ * - "halted": true if the cpu is halted, false otherwise
+ * - Current program counter, in decimal. The key name depends on
+ *   the architecture:
+ *      "pc": i386/x86)64
+ *      "nip": PPC
+ *      "pc" and "npc": sparc
+ *      "PC": mips
  *
- * [ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
- *   { "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]
+ * Example:
+ *
+ * [ { "CPU": 0, "current": true, "halted": false, "pc": 3227107138 },
+ *   { "CPU": 1, "current": false, "halted": true, "pc": 7108165 } ]
  */
 static void do_info_cpus(Monitor *mon, QObject **ret_data)
 {
@@ -668,14 +680,17 @@ static void do_info_cpus(Monitor *mon, QObject **ret_data)
     mon_get_cpu();
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
-        const char *answer;
-        QDict *cpu = qdict_new();
+        QDict *cpu;
+        QObject *obj;
 
         cpu_synchronize_state(env);
 
-        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));
+        obj = qobject_from_jsonf("{ 'CPU': %d, 'current': %i, 'halted': %i }",
+                                 env->cpu_index, env == mon->mon_cpu,
+                                 env->halted);
+        assert(obj != NULL);
+
+        cpu = qobject_to_qdict(obj);
 
 #if defined(TARGET_I386)
         qdict_put(cpu, "pc", qint_from_int(env->eip + env->segs[R_CS].base));
@@ -687,8 +702,6 @@ static void do_info_cpus(Monitor *mon, QObject **ret_data)
 #elif defined(TARGET_MIPS)
         qdict_put(cpu, "PC", qint_from_int(env->active_tc.PC));
 #endif
-        answer = env->halted ? "yes" : "no";
-        qdict_put(cpu, "halted", qstring_from_str(answer));
 
         qlist_append(cpu_list, cpu);
     }
-- 
1.6.6.rc1.39.g9a42

Re: [Qemu-devel] [PATCH 06/19] monitor: do_info_cpus(): Use QBool

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> While there update the documentation as well.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  monitor.c |   39 ++++++++++++++++++++++++++-------------
>  1 files changed, 26 insertions(+), 13 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index aa56ec7..8729535 100644
> --- a/monitor.c
> +++ b/monitor.c
[...]
> @@ -650,12 +652,22 @@ static void monitor_print_cpus(Monitor *mon, const QObject *data)
>  /**
>   * do_info_cpus(): Show CPU information
>   *
> - * Return a QList with a QDict for each CPU.
> + * Return a QList. Each CPU is represented by a QDict, which contains:
>   *
> - * For example:
> + * - "cpu": CPU index
> + * - "current": true if this is the current CPU, false otherwise
> + * - "halted": true if the cpu is halted, false otherwise
> + * - Current program counter, in decimal. The key name depends on

Do we want to specify the base for numbers in the JSON?

If yes, why not use decimal everywhere?

Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
to be hexadecimal.

> + *   the architecture:
> + *      "pc": i386/x86)64
> + *      "nip": PPC
> + *      "pc" and "npc": sparc
> + *      "PC": mips
>   *
> - * [ { "CPU": 0, "current": "yes", "pc": 0x..., "halted": "no" },
> - *   { "CPU": 1, "current": "no",  "pc": 0x..., "halted": "yes" } ]
> + * Example:
> + *
> + * [ { "CPU": 0, "current": true, "halted": false, "pc": 3227107138 },
> + *   { "CPU": 1, "current": false, "halted": true, "pc": 7108165 } ]
>   */
>  static void do_info_cpus(Monitor *mon, QObject **ret_data)
[...]

Re: [Qemu-devel] [PATCH 06/19] monitor: do_info_cpus(): Use QBool

[Luiz Capitulino]

On Thu, 10 Dec 2009 11:44:49 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > While there update the documentation as well.
> >
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  monitor.c |   39 ++++++++++++++++++++++++++-------------
> >  1 files changed, 26 insertions(+), 13 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index aa56ec7..8729535 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> [...]
> > @@ -650,12 +652,22 @@ static void monitor_print_cpus(Monitor *mon, const QObject *data)
> >  /**
> >   * do_info_cpus(): Show CPU information
> >   *
> > - * Return a QList with a QDict for each CPU.
> > + * Return a QList. Each CPU is represented by a QDict, which contains:
> >   *
> > - * For example:
> > + * - "cpu": CPU index
> > + * - "current": true if this is the current CPU, false otherwise
> > + * - "halted": true if the cpu is halted, false otherwise
> > + * - Current program counter, in decimal. The key name depends on
> 
> Do we want to specify the base for numbers in the JSON?
> 
> If yes, why not use decimal everywhere?
> 
> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
> to be hexadecimal.

 Makes sense, I was in doubt about how I should represent this but addresses
should always be hexadecimal.

 What about adding a section to do the spec (or creating a new
document) describing the protocol style?

Re: [Qemu-devel] [PATCH 06/19] monitor: do_info_cpus(): Use QBool

[Anthony Liguori]

Markus Armbruster wrote:
> Do we want to specify the base for numbers in the JSON?
>
> If yes, why not use decimal everywhere?
>
> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
> to be hexadecimal.
>   

Hexadecimal is not valid to represent numbers in JSON.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 06/19] monitor: do_info_cpus(): Use QBool

[Luiz Capitulino]

On Thu, 10 Dec 2009 07:01:22 -0600
Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:

> Markus Armbruster wrote:
> > Do we want to specify the base for numbers in the JSON?
> >
> > If yes, why not use decimal everywhere?
> >
> > Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
> > to be hexadecimal.
> >   
> 
> Hexadecimal is not valid to represent numbers in JSON.

 We can return a string.

Re: [Qemu-devel] [PATCH 06/19] monitor: do_info_cpus(): Use QBool

[Anthony Liguori]

Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 07:01:22 -0600
> Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:
>
>   
>> Markus Armbruster wrote:
>>     
>>> Do we want to specify the base for numbers in the JSON?
>>>
>>> If yes, why not use decimal everywhere?
>>>
>>> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
>>> to be hexadecimal.
>>>   
>>>       
>> Hexadecimal is not valid to represent numbers in JSON.
>>     
>
>  We can return a string.
>   
Returning a hex string is a bad idea.  It overcomplicates clients.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 06/19] monitor: do_info_cpus(): Use QBool

[Anthony Liguori]

Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 07:01:22 -0600
> Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:
>
>   
>> Markus Armbruster wrote:
>>     
>>> Do we want to specify the base for numbers in the JSON?
>>>
>>> If yes, why not use decimal everywhere?
>>>
>>> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
>>> to be hexadecimal.
>>>   
>>>       
>> Hexadecimal is not valid to represent numbers in JSON.
>>     
>
>  We can return a string.
>   

When it comes to returning u64s, we'll have to send these over the wire 
as s64s.  It's not ideal but it's the only way to support languages that 
don't distinguish between signed and unsigned types.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 06/19] monitor: do_info_cpus(): Use QBool

[Markus Armbruster]

Anthony Liguori <aliguori@linux.vnet.ibm.com> writes:

> Markus Armbruster wrote:
>> Do we want to specify the base for numbers in the JSON?
>>
>> If yes, why not use decimal everywhere?
>>
>> Aside, if we use hexadecimal in JSON at all, then I'd prefer addresses
>> to be hexadecimal.
>>   
>
> Hexadecimal is not valid to represent numbers in JSON.

In that case, there's no need to document the format of numbers for
every command.

[Qemu-devel] [PATCH 07/19] monitor: do_info_version(): Use QDict

[Luiz Capitulino]

All 'info' commands should use QDict, this commit also kills
monitor_print_qobject() as do_info_version() doesn't use it
anymore (and no handler will).

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

diff --git a/monitor.c b/monitor.c
index 8729535..775e687 100644
--- a/monitor.c
+++ b/monitor.c
@@ -257,24 +257,6 @@ static inline int monitor_has_error(const Monitor *mon)
     return mon->error != 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 void monitor_json_emitter(Monitor *mon, const QObject *data)
 {
     QString *json;
@@ -504,12 +486,32 @@ help:
     help_cmd(mon, "info");
 }
 
+static void do_info_version_print(Monitor *mon, const QObject *data)
+{
+    QDict *qdict;
+
+    qdict = qobject_to_qdict(data);
+
+    monitor_printf(mon, "%s%s\n", qdict_get_str(qdict, "qemu"),
+                                  qdict_get_str(qdict, "package"));
+}
+
 /**
  * do_info_version(): Show QEMU version
+ *
+ * Return a QDict with the following information:
+ *
+ * - "qemu": QEMU's version
+ * - "package": package's version
+ *
+ * Example:
+ *
+ * { "qemu": "0.11.50", "package": "" }
  */
 static void do_info_version(Monitor *mon, QObject **ret_data)
 {
-    *ret_data = QOBJECT(qstring_from_str(QEMU_VERSION QEMU_PKGVERSION));
+    *ret_data = qobject_from_jsonf("{ 'qemu': %s, 'package': %s }",
+                                   QEMU_VERSION, QEMU_PKGVERSION);
 }
 
 static void do_info_name(Monitor *mon)
@@ -2200,7 +2202,7 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the version of QEMU",
-        .user_print = monitor_print_qobject,
+        .user_print = do_info_version_print,
         .mhandler.info_new = do_info_version,
     },
     {
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 08/19] monitor: Convert do_info_status() to QObject

[Luiz Capitulino]

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

diff --git a/monitor.c b/monitor.c
index 775e687..e7a6294 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1910,16 +1910,41 @@ static void do_inject_nmi(Monitor *mon, const QDict *qdict)
 }
 #endif
 
-static void do_info_status(Monitor *mon)
+static void do_info_status_print(Monitor *mon, const QObject *data)
 {
-    if (vm_running) {
-        if (singlestep) {
-            monitor_printf(mon, "VM status: running (single step mode)\n");
-        } else {
-            monitor_printf(mon, "VM status: running\n");
+    QDict *qdict;
+
+    qdict = qobject_to_qdict(data);
+
+    monitor_printf(mon, "VM status: ");
+    if (qdict_get_bool(qdict, "running")) {
+        monitor_printf(mon, "running");
+        if (qdict_get_bool(qdict, "single-step")) {
+            monitor_printf(mon, " (single step mode)");
         }
-    } else
-       monitor_printf(mon, "VM status: paused\n");
+    } else {
+        monitor_printf(mon, "paused");
+    }
+
+    monitor_printf(mon, "\n");
+}
+
+/**
+ * do_info_status(): VM status
+ *
+ * Return a QDict with the following information:
+ *
+ * - "running": true if the VM is running, or false if it is paused
+ * - "single-step": true if the VM is in single step mode, false otherwise
+ *
+ * Example:
+ *
+ * { "running": true, "single-step": false }
+ */
+static void do_info_status(Monitor *mon, QObject **ret_data)
+{
+    *ret_data = qobject_from_jsonf("{ 'running': %i, 'single-step': %i }",
+                                    vm_running, singlestep);
 }
 
 /**
@@ -2370,7 +2395,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the current VM status (running|paused)",
-        .mhandler.info = do_info_status,
+        .user_print = do_info_status_print,
+        .mhandler.info_new = do_info_status,
     },
     {
         .name       = "pcmcia",
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 09/19] monitor: Convert do_info_kvm() to QObject

[Luiz Capitulino]

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

diff --git a/monitor.c b/monitor.c
index e7a6294..47f794d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1783,16 +1783,40 @@ static void tlb_info(Monitor *mon)
 
 #endif
 
-static void do_info_kvm(Monitor *mon)
+static void do_info_kvm_print(Monitor *mon, const QObject *data)
 {
-#ifdef CONFIG_KVM
+    QDict *qdict;
+
+    qdict = qobject_to_qdict(data);
+
     monitor_printf(mon, "kvm support: ");
-    if (kvm_enabled())
-        monitor_printf(mon, "enabled\n");
-    else
-        monitor_printf(mon, "disabled\n");
+    if (qdict_get_bool(qdict, "present")) {
+        monitor_printf(mon, "%s\n", qdict_get_bool(qdict, "enabled") ?
+                                    "enabled" : "disabled");
+    } else {
+        monitor_printf(mon, "not compiled\n");
+    }
+}
+
+/**
+ * do_info_kvm(): Show KVM information
+ *
+ * Return a QDict with the following information:
+ *
+ * - "enabled": true if KVM support is enabled, false otherwise
+ * - "present": true if QEMU has KVM support, false otherwise
+ *
+ * Example:
+ *
+ * { "enabled": true, "present": true }
+ */
+static void do_info_kvm(Monitor *mon, QObject **ret_data)
+{
+#ifdef CONFIG_KVM
+    *ret_data = qobject_from_jsonf("{ 'enabled': %i, 'present': true }",
+                                   kvm_enabled());
 #else
-    monitor_printf(mon, "kvm support: not compiled\n");
+    *ret_data = qobject_from_jsonf("{ 'enabled': false, 'present': false }");
 #endif
 }
 
@@ -2346,7 +2370,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show KVM information",
-        .mhandler.info = do_info_kvm,
+        .user_print = do_info_kvm_print,
+        .mhandler.info_new = do_info_kvm,
     },
     {
         .name       = "numa",
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Luiz Capitulino]

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

diff --git a/monitor.c b/monitor.c
index 47f794d..3d33bd8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -514,10 +514,30 @@ static void do_info_version(Monitor *mon, QObject **ret_data)
                                    QEMU_VERSION, QEMU_PKGVERSION);
 }
 
-static void do_info_name(Monitor *mon)
+static void do_info_name_print(Monitor *mon, const QObject *data)
 {
-    if (qemu_name)
-        monitor_printf(mon, "%s\n", qemu_name);
+    const char *str;
+
+    str = qdict_get_str(qobject_to_qdict(data), "name");
+    if (strlen(str) > 0) {
+        monitor_printf(mon, "%s\n", str);
+    }
+}
+
+/**
+ * do_info_name(): Show VM name
+ *
+ * Return a QDict with the following information:
+ *
+ * - "name": VM's name. If the VM has no name, the string will be empty
+ *
+ * Example:
+ *
+ * { "name": "qemu-name" }
+ */
+static void do_info_name(Monitor *mon, QObject **ret_data)
+{
+    *ret_data = qobject_from_jsonf("{'name': %s}", qemu_name ? qemu_name : "");
 }
 
 /**
@@ -2449,7 +2469,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the current VM name",
-        .mhandler.info = do_info_name,
+        .user_print = do_info_name_print,
+        .mhandler.info_new = do_info_name,
     },
     {
         .name       = "uuid",
-- 
1.6.6.rc1.39.g9a42

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  monitor.c |   29 +++++++++++++++++++++++++----
>  1 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 47f794d..3d33bd8 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -514,10 +514,30 @@ static void do_info_version(Monitor *mon, QObject **ret_data)
>                                     QEMU_VERSION, QEMU_PKGVERSION);
>  }
>  
> -static void do_info_name(Monitor *mon)
> +static void do_info_name_print(Monitor *mon, const QObject *data)
>  {
> -    if (qemu_name)
> -        monitor_printf(mon, "%s\n", qemu_name);
> +    const char *str;
> +
> +    str = qdict_get_str(qobject_to_qdict(data), "name");
> +    if (strlen(str) > 0) {
> +        monitor_printf(mon, "%s\n", str);
> +    }
> +}
> +
> +/**
> + * do_info_name(): Show VM name
> + *
> + * Return a QDict with the following information:
> + *
> + * - "name": VM's name. If the VM has no name, the string will be empty

So you can't distinguish name "" from unnamed.  Do we care?

Monitor output for unnamed guests changes from

    (qemu) info name
    (qemu) 

to

(qemu) info name

(qemu) 

> + *
> + * Example:
> + *
> + * { "name": "qemu-name" }
> + */
> +static void do_info_name(Monitor *mon, QObject **ret_data)
> +{
> +    *ret_data = qobject_from_jsonf("{'name': %s}", qemu_name ? qemu_name : "");
>  }
>  
>  /**
> @@ -2449,7 +2469,8 @@ static const mon_cmd_t info_cmds[] = {
>          .args_type  = "",
>          .params     = "",
>          .help       = "show the current VM name",
> -        .mhandler.info = do_info_name,
> +        .user_print = do_info_name_print,
> +        .mhandler.info_new = do_info_name,
>      },
>      {
>          .name       = "uuid",

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Luiz Capitulino]

On Thu, 10 Dec 2009 11:09:53 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  monitor.c |   29 +++++++++++++++++++++++++----
> >  1 files changed, 25 insertions(+), 4 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 47f794d..3d33bd8 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -514,10 +514,30 @@ static void do_info_version(Monitor *mon, QObject **ret_data)
> >                                     QEMU_VERSION, QEMU_PKGVERSION);
> >  }
> >  
> > -static void do_info_name(Monitor *mon)
> > +static void do_info_name_print(Monitor *mon, const QObject *data)
> >  {
> > -    if (qemu_name)
> > -        monitor_printf(mon, "%s\n", qemu_name);
> > +    const char *str;
> > +
> > +    str = qdict_get_str(qobject_to_qdict(data), "name");
> > +    if (strlen(str) > 0) {
> > +        monitor_printf(mon, "%s\n", str);
> > +    }
> > +}
> > +
> > +/**
> > + * do_info_name(): Show VM name
> > + *
> > + * Return a QDict with the following information:
> > + *
> > + * - "name": VM's name. If the VM has no name, the string will be empty
> 
> So you can't distinguish name "" from unnamed.  Do we care?

 I don't think so, but if we do the best way to deal with the fact
that qemu_name can be NULL would be to return null, like:

{ "name": null }

 But we don't support json-null yet... There are other two
ways to solve this, but they seem workarounds for not supporting
null: return an empty dict or return { "name": false }.

> Monitor output for unnamed guests changes from
> 
>     (qemu) info name
>     (qemu) 
> 
> to
> 
> (qemu) info name
> 
> (qemu) 

 The strlen() call doesn't let this happen. Although the
other way around does happen: if you call qemu with -name '',
then output would be:

(qemu) info name

(qemu)

 This won't happen anymore, goto do_we_care.

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Anthony Liguori]

Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 11:09:53 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>   
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>>
>>     
>>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>>> ---
>>>  monitor.c |   29 +++++++++++++++++++++++++----
>>>  1 files changed, 25 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index 47f794d..3d33bd8 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -514,10 +514,30 @@ static void do_info_version(Monitor *mon, QObject **ret_data)
>>>                                     QEMU_VERSION, QEMU_PKGVERSION);
>>>  }
>>>  
>>> -static void do_info_name(Monitor *mon)
>>> +static void do_info_name_print(Monitor *mon, const QObject *data)
>>>  {
>>> -    if (qemu_name)
>>> -        monitor_printf(mon, "%s\n", qemu_name);
>>> +    const char *str;
>>> +
>>> +    str = qdict_get_str(qobject_to_qdict(data), "name");
>>> +    if (strlen(str) > 0) {
>>> +        monitor_printf(mon, "%s\n", str);
>>> +    }
>>> +}
>>> +
>>> +/**
>>> + * do_info_name(): Show VM name
>>> + *
>>> + * Return a QDict with the following information:
>>> + *
>>> + * - "name": VM's name. If the VM has no name, the string will be empty
>>>       
>> So you can't distinguish name "" from unnamed.  Do we care?
>>     
>
>  I don't think so, but if we do the best way to deal with the fact
> that qemu_name can be NULL would be to return null, like:
>
> { "name": null }
>
>  But we don't support json-null yet... There are other two
> ways to solve this, but they seem workarounds for not supporting
> null: return an empty dict or return { "name": false }.
>   

I'd prefer an empty dict.  I actually prefer that over null.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Avi Kivity]

On 12/10/2009 02:56 PM, Anthony Liguori wrote:
>
> I'd prefer an empty dict.  I actually prefer that over null.
>

Depends.  Key not present suggests the feature is not implemented.  
Value is null suggests the feature is not used.  Both key and value 
present suggest the feature is in implemented and active.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Avi Kivity]

On 12/10/2009 05:55 PM, Avi Kivity wrote:
> On 12/10/2009 02:56 PM, Anthony Liguori wrote:
>>
>> I'd prefer an empty dict.  I actually prefer that over null.
>>
>
> Depends.  Key not present suggests the feature is not implemented.  
> Value is null suggests the feature is not used.  Both key and value 
> present suggest the feature is in implemented and active.
>

And, as with hex numbers, we need to prefer usability to machine clients 
to readability to user beings.  This is a machine protocol, so unless 
you're a machine, aesthetic considerations take second place.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Anthony Liguori]

Avi Kivity wrote:
> On 12/10/2009 02:56 PM, Anthony Liguori wrote:
>>
>> I'd prefer an empty dict.  I actually prefer that over null.
>>
>
> Depends.  Key not present suggests the feature is not implemented.  
> Value is null suggests the feature is not used.  Both key and value 
> present suggest the feature is in implemented and active.

What I suggested to Luiz was that all info commands return a 
dictionary.  The main reason was that for most commands, we can extend 
the commands in a compatible way by adding new fields to the dictionary.

My expectation is that there will be a lot of client code that does:

hpet_info = qmp.info_hpet()
if hpet_info.has_key('period'):
   period = hpet_info['period']
else:
   period = 100 # old qemu's has a fixed period of 100ns

So in keeping with this idiom, I think that checking 
name_info.has_key('name') is a more meaningful way to determine whether 
the virtual machine has been given a name vs comparing name_info['name'] 
== None.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Avi Kivity]

On 12/10/2009 06:03 PM, Anthony Liguori wrote:
> Avi Kivity wrote:
>> On 12/10/2009 02:56 PM, Anthony Liguori wrote:
>>>
>>> I'd prefer an empty dict.  I actually prefer that over null.
>>>
>>
>> Depends.  Key not present suggests the feature is not implemented.  
>> Value is null suggests the feature is not used.  Both key and value 
>> present suggest the feature is in implemented and active.
>
> What I suggested to Luiz was that all info commands return a 
> dictionary.  The main reason was that for most commands, we can extend 
> the commands in a compatible way by adding new fields to the dictionary.

Oh yes.

>
> My expectation is that there will be a lot of client code that does:
>
> hpet_info = qmp.info_hpet()
> if hpet_info.has_key('period'):
>   period = hpet_info['period']
> else:
>   period = 100 # old qemu's has a fixed period of 100ns
>
> So in keeping with this idiom, I think that checking 
> name_info.has_key('name') is a more meaningful way to determine 
> whether the virtual machine has been given a name vs comparing 
> name_info['name'] == None.

But we have two null conditions to check for, in an extendible dictionary:

1. The feature is unknown to qemu
2. The feature is known to qemu, but disabled

So, "if 'field' in result:" tests the former, and "if result['field']:" 
tests the latter.

In your example, a period of None makes no sense, so it would be 
sufficient to

   period = hpet_info.get('period', 0.100)



-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Anthony Liguori]

Avi Kivity wrote:
> But we have two null conditions to check for, in an extendible 
> dictionary:
>
> 1. The feature is unknown to qemu
> 2. The feature is known to qemu, but disabled
>
> So, "if 'field' in result:" tests the former, and "if 
> result['field']:" tests the latter.
>
> In your example, a period of None makes no sense, so it would be 
> sufficient to
>
>   period = hpet_info.get('period', 0.100)

By the same token, wouldn't you probably do:

name = hpet_info.get('name', None)

Let me put it another way, I don't think adding null to the json parser 
and incorporating it into this command is a good idea at this stage in 
the release so if we want to do something like this, we need to defer it 
to 0.13.

I agree there are some instances where null could be useful.  I think we 
can get away without it here though.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Avi Kivity]

On 12/10/2009 06:20 PM, Anthony Liguori wrote:
>
> By the same token, wouldn't you probably do:
>
> name = hpet_info.get('name', None)
>

For name, yes.  For an optional feature where you're interested in 
knowing both its existence and its value (if it exists), no.

> Let me put it another way, I don't think adding null to the json 
> parser and incorporating it into this command is a good idea at this 
> stage in the release so if we want to do something like this, we need 
> to defer it to 0.13.
>
> I agree there are some instances where null could be useful.  I think 
> we can get away without it here though.

For 'name', definitely, since it's known to exist.  It would be nice to 
have consistency in how features are presented, though.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Luiz Capitulino]

On Thu, 10 Dec 2009 18:24:38 +0200
Avi Kivity <avi@redhat.com> wrote:

> > Let me put it another way, I don't think adding null to the json 
> > parser and incorporating it into this command is a good idea at this 
> > stage in the release so if we want to do something like this, we need 
> > to defer it to 0.13.
> >
> > I agree there are some instances where null could be useful.  I think 
> > we can get away without it here though.
> 
> For 'name', definitely, since it's known to exist.  It would be nice to 
> have consistency in how features are presented, though.

 Following what you propose, if it's known to exist then we should
never return an empty dict.

 There are other commands that might require adjustments, for example
'info kvm' has a 'present' key. If QEMU is built w/o KVM support, then
this key will be 'false'. Should we return an empty dict then?

 HPET is another example, currently it's only compiled in if the
target is i386. Otherwise the command won't even be available, and
we have more commands with conditional features/compilation.

 So, what I arguably did wrong here was starting the conversion
work before defining all these rules.

 An option we have is: libvirt actually uses four or five of those
info commands. So, we could drop all the rest and guarantee that
only those libvirt ones are 100% correct.

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Avi Kivity]

On 12/10/2009 06:54 PM, Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 18:24:38 +0200
> Avi Kivity<avi@redhat.com>  wrote:
>
>    
>>> Let me put it another way, I don't think adding null to the json
>>> parser and incorporating it into this command is a good idea at this
>>> stage in the release so if we want to do something like this, we need
>>> to defer it to 0.13.
>>>
>>> I agree there are some instances where null could be useful.  I think
>>> we can get away without it here though.
>>>        
>> For 'name', definitely, since it's known to exist.  It would be nice to
>> have consistency in how features are presented, though.
>>      
>   Following what you propose, if it's known to exist then we should
> never return an empty dict.
>    

Right, but if we can't support null (why?) then we can make an exception 
for 'name'.

>   There are other commands that might require adjustments, for example
> 'info kvm' has a 'present' key. If QEMU is built w/o KVM support, then
> this key will be 'false'. Should we return an empty dict then?
>    

No.

>   HPET is another example, currently it's only compiled in if the
> target is i386. Otherwise the command won't even be available, and
> we have more commands with conditional features/compilation.
>
>    

That's fine, as long as command availability is discoverable.

>   So, what I arguably did wrong here was starting the conversion
> work before defining all these rules.
>    

On the other hand, we can often only find out something by implementing 
it incorrectly first.

>   An option we have is: libvirt actually uses four or five of those
> info commands. So, we could drop all the rest and guarantee that
> only those libvirt ones are 100% correct.
>    

My worry is with the commands that parse or emit comma-separated option 
strings.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Luiz Capitulino]

On Thu, 10 Dec 2009 19:02:37 +0200
Avi Kivity <avi@redhat.com> wrote:

> On 12/10/2009 06:54 PM, Luiz Capitulino wrote:
> > On Thu, 10 Dec 2009 18:24:38 +0200
> > Avi Kivity<avi@redhat.com>  wrote:
> >
> >    
> >>> Let me put it another way, I don't think adding null to the json
> >>> parser and incorporating it into this command is a good idea at this
> >>> stage in the release so if we want to do something like this, we need
> >>> to defer it to 0.13.
> >>>
> >>> I agree there are some instances where null could be useful.  I think
> >>> we can get away without it here though.
> >>>        
> >> For 'name', definitely, since it's known to exist.  It would be nice to
> >> have consistency in how features are presented, though.
> >>      
> >   Following what you propose, if it's known to exist then we should
> > never return an empty dict.
> >    
> 
> Right, but if we can't support null (why?) then we can make an exception 
> for 'name'.

 I'm ok with the exception too and the problem with adding null support
now is that it requires some changes to the parser which seem a bit
late to be done.

> >   There are other commands that might require adjustments, for example
> > 'info kvm' has a 'present' key. If QEMU is built w/o KVM support, then
> > this key will be 'false'. Should we return an empty dict then?
> >    
> 
> No.
> 
> >   HPET is another example, currently it's only compiled in if the
> > target is i386. Otherwise the command won't even be available, and
> > we have more commands with conditional features/compilation.
> >
> >    
> 
> That's fine, as long as command availability is discoverable.

 Yes, it's.

> >   So, what I arguably did wrong here was starting the conversion
> > work before defining all these rules.
> >    
> 
> On the other hand, we can often only find out something by implementing 
> it incorrectly first.

 Sure and even being inconsistent a bit I'm quite sure that it's a lot
better than parsing the old user monitor. :)

> >   An option we have is: libvirt actually uses four or five of those
> > info commands. So, we could drop all the rest and guarantee that
> > only those libvirt ones are 100% correct.
> >    
> 
> My worry is with the commands that parse or emit comma-separated option 
> strings.

 I tried to fix all emissions, but we accept it like in the uri
argument of migrate.

 I don't think it's a big deal though, as it's easy to add a new
key for that (if needed) w/o breaking compatibility.

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 02:54:57PM -0200, Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 18:24:38 +0200
> Avi Kivity <avi@redhat.com> wrote:
> 
> > > Let me put it another way, I don't think adding null to the json 
> > > parser and incorporating it into this command is a good idea at this 
> > > stage in the release so if we want to do something like this, we need 
> > > to defer it to 0.13.
> > >
> > > I agree there are some instances where null could be useful.  I think 
> > > we can get away without it here though.
> > 
> > For 'name', definitely, since it's known to exist.  It would be nice to 
> > have consistency in how features are presented, though.
> 
>  Following what you propose, if it's known to exist then we should
> never return an empty dict.
> 
>  There are other commands that might require adjustments, for example
> 'info kvm' has a 'present' key. If QEMU is built w/o KVM support, then
> this key will be 'false'. Should we return an empty dict then?
> 
>  HPET is another example, currently it's only compiled in if the
> target is i386. Otherwise the command won't even be available, and
> we have more commands with conditional features/compilation.
> 
>  So, what I arguably did wrong here was starting the conversion
> work before defining all these rules.
> 
>  An option we have is: libvirt actually uses four or five of those
> info commands. So, we could drop all the rest and guarantee that
> only those libvirt ones are 100% correct.

Please don't do that. libvirt is adding support for new features all the
time. I don't want to be in the situation where we can't add a new feature
because it is missing in the JSON impl. If we're going to provide a supported
JSON monitor it needs to have all the commands converted, otherwise we'll
have to stick with using the text based monitor.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Luiz Capitulino]

On Thu, 10 Dec 2009 17:38:13 +0000
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Thu, Dec 10, 2009 at 02:54:57PM -0200, Luiz Capitulino wrote:

> >  An option we have is: libvirt actually uses four or five of those
> > info commands. So, we could drop all the rest and guarantee that
> > only those libvirt ones are 100% correct.
> 
> Please don't do that.

 I won't.

> libvirt is adding support for new features all the
> time. I don't want to be in the situation where we can't add a new feature
> because it is missing in the JSON impl. If we're going to provide a supported
> JSON monitor it needs to have all the commands converted, otherwise we'll
> have to stick with using the text based monitor.

 But 0.12 won't have all commands converted, this page has a listing
of what we're going to have:

http://www.linux-kvm.org/page/MonitorProtocol#Conversion_work_detailed_status

 'converted' and 'merged' are guaranteed, Markus did some work and enabled
one or two which are not there.

 I know that the usb and net ones are vital for you, I've patches enabling
them but their error handling is very difficult to get right.

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 03:49:20PM -0200, Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 17:38:13 +0000
> "Daniel P. Berrange" <berrange@redhat.com> wrote:
> 
> > On Thu, Dec 10, 2009 at 02:54:57PM -0200, Luiz Capitulino wrote:
> 
> > >  An option we have is: libvirt actually uses four or five of those
> > > info commands. So, we could drop all the rest and guarantee that
> > > only those libvirt ones are 100% correct.
> > 
> > Please don't do that.
> 
>  I won't.
> 
> > libvirt is adding support for new features all the
> > time. I don't want to be in the situation where we can't add a new feature
> > because it is missing in the JSON impl. If we're going to provide a supported
> > JSON monitor it needs to have all the commands converted, otherwise we'll
> > have to stick with using the text based monitor.
> 
>  But 0.12 won't have all commands converted, this page has a listing
> of what we're going to have:
> 
> http://www.linux-kvm.org/page/MonitorProtocol#Conversion_work_detailed_status
> 
>  'converted' and 'merged' are guaranteed, Markus did some work and enabled
> one or two which are not there.
> 
>  I know that the usb and net ones are vital for you, I've patches enabling
> them but their error handling is very difficult to get right.

The list of what libvirt uses is also outdated. In addition to those in
yellow, we also now use, or will likely use in near future

  device_add
  device_del
  info pci
  set_link
  migrate_cancel
  migrate_set_downtime
  drive_add
  info usb

So given the 0.12 release target, it sounds like we won't be able to
use the JSON monitor :-(

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Luiz Capitulino]

On Thu, 10 Dec 2009 18:00:31 +0000
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Thu, Dec 10, 2009 at 03:49:20PM -0200, Luiz Capitulino wrote:
> > On Thu, 10 Dec 2009 17:38:13 +0000
> > "Daniel P. Berrange" <berrange@redhat.com> wrote:
> > 
> > > On Thu, Dec 10, 2009 at 02:54:57PM -0200, Luiz Capitulino wrote:
> > 
> > > >  An option we have is: libvirt actually uses four or five of those
> > > > info commands. So, we could drop all the rest and guarantee that
> > > > only those libvirt ones are 100% correct.
> > > 
> > > Please don't do that.
> > 
> >  I won't.
> > 
> > > libvirt is adding support for new features all the
> > > time. I don't want to be in the situation where we can't add a new feature
> > > because it is missing in the JSON impl. If we're going to provide a supported
> > > JSON monitor it needs to have all the commands converted, otherwise we'll
> > > have to stick with using the text based monitor.
> > 
> >  But 0.12 won't have all commands converted, this page has a listing
> > of what we're going to have:
> > 
> > http://www.linux-kvm.org/page/MonitorProtocol#Conversion_work_detailed_status
> > 
> >  'converted' and 'merged' are guaranteed, Markus did some work and enabled
> > one or two which are not there.
> > 
> >  I know that the usb and net ones are vital for you, I've patches enabling
> > them but their error handling is very difficult to get right.
> 
> The list of what libvirt uses is also outdated. In addition to those in
> yellow, we also now use, or will likely use in near future
> 
>   device_add
>   device_del
>   info pci
>   set_link
>   migrate_cancel
>   migrate_set_downtime
>   drive_add
>   info usb
> 
> So given the 0.12 release target, it sounds like we won't be able to
> use the JSON monitor :-(

 Well, given that we have only a few days before the final release it'll
be very hard to properly convert those.

 But there's something we could do if libvirt early adoption is a hard
requirement.

 You need two info handlers and some command handlers. The latter is
trivial to 'enable for usage' if it doesn't have user output, which is
the case for most command handlers.

 The problem though is error handling. Enabling a handler for QMP usage
w/o adding proper error handling support means that you won't be able
to get most error conditions. In the best case you'll get a generic error
message, in the worst QEMU won't even detect the error.

 This is doable, but doesn't sound like a good idea. Specially if today
libvirt is able to handle error conditions with the user protocol.

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Anthony Liguori]

Luiz Capitulino wrote:
>> The list of what libvirt uses is also outdated. In addition to those in
>> yellow, we also now use, or will likely use in near future
>>
>>   device_add
>>   device_del
>>   info pci
>>   set_link
>>   migrate_cancel
>>   migrate_set_downtime
>>   drive_add
>>   info usb
>>
>> So given the 0.12 release target, it sounds like we won't be able to
>> use the JSON monitor :-(
>>     
>
>  Well, given that we have only a few days before the final release it'll
> be very hard to properly convert those.
>
>  But there's something we could do if libvirt early adoption is a hard
> requirement.
>   

It's too late.  I don't want to see anything but bug fixes go in post -rc2.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Anthony Liguori]

Daniel P. Berrange wrote:
> Please don't do that. libvirt is adding support for new features all the
> time. I don't want to be in the situation where we can't add a new feature
> because it is missing in the JSON impl. If we're going to provide a supported
> JSON monitor it needs to have all the commands converted, otherwise we'll
> have to stick with using the text based monitor.
>   

I would suggest that for 0.12, libvirt not use QMP.  It should wait 
until 0.13 to convert over to QMP since we should have a full conversion 
by then.

> Daniel
>   


-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Daniel P. Berrange]

On Fri, Dec 11, 2009 at 07:20:12AM -0600, Anthony Liguori wrote:
> Daniel P. Berrange wrote:
> >Please don't do that. libvirt is adding support for new features all the
> >time. I don't want to be in the situation where we can't add a new feature
> >because it is missing in the JSON impl. If we're going to provide a 
> >supported
> >JSON monitor it needs to have all the commands converted, otherwise we'll
> >have to stick with using the text based monitor.
> >  
> 
> I would suggest that for 0.12, libvirt not use QMP.  It should wait 
> until 0.13 to convert over to QMP since we should have a full conversion 
> by then.

I agree, given the time lines involved its not practical for us to
switch to QMP in 0.12, so we'll wait until 0.13. On the plus side
we've got all the QMP code in libvirt GIT now, so we can make sure
that 0.13 is very well tested when time comes for its release :-)

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Anthony Liguori]

Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 18:24:38 +0200
> Avi Kivity <avi@redhat.com> wrote:
>
>   
>>> Let me put it another way, I don't think adding null to the json 
>>> parser and incorporating it into this command is a good idea at this 
>>> stage in the release so if we want to do something like this, we need 
>>> to defer it to 0.13.
>>>
>>> I agree there are some instances where null could be useful.  I think 
>>> we can get away without it here though.
>>>       
>> For 'name', definitely, since it's known to exist.  It would be nice to 
>> have consistency in how features are presented, though.
>>     
>
>  Following what you propose, if it's known to exist then we should
> never return an empty dict.
>
>  There are other commands that might require adjustments, for example
> 'info kvm' has a 'present' key. If QEMU is built w/o KVM support, then
> this key will be 'false'. Should we return an empty dict then?
>
>  HPET is another example, currently it's only compiled in if the
> target is i386. Otherwise the command won't even be available, and
> we have more commands with conditional features/compilation.
>
>  So, what I arguably did wrong here was starting the conversion
> work before defining all these rules.
>   

The monitor in general is very ad-hoc.  The fact that the info commands 
don't behave consistently shouldn't be a surprise.

Let's focus on converting the remaining monitor commands.  The goal is 
that any user of the monitor today can convert over to QMP.

Once we've achieved that goal, let's start looking at introducing proper 
commands that make sense and are consistent.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject

[Luiz Capitulino]

On Fri, 11 Dec 2009 07:18:27 -0600
Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:

> Let's focus on converting the remaining monitor commands.  The goal is 
> that any user of the monitor today can convert over to QMP.
> 
> Once we've achieved that goal, let's start looking at introducing proper 
> commands that make sense and are consistent.

 ACK.

[Qemu-devel] [PATCH 11/19] monitor: Convert do_info_hpet() to QObject

[Luiz Capitulino]

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

diff --git a/monitor.c b/monitor.c
index 3d33bd8..72fd573 100644
--- a/monitor.c
+++ b/monitor.c
@@ -570,10 +570,27 @@ static void do_info_commands(Monitor *mon, QObject **ret_data)
 }
 
 #if defined(TARGET_I386)
-static void do_info_hpet(Monitor *mon)
+static void do_info_hpet_print(Monitor *mon, const QObject *data)
 {
     monitor_printf(mon, "HPET is %s by QEMU\n",
-                   (no_hpet) ? "disabled" : "enabled");
+                   qdict_get_bool(qobject_to_qdict(data), "enabled") ?
+                   "enabled" : "disabled");
+}
+
+/**
+ * do_info_hpet(): Show HPET state
+ *
+ * Return a QDict with the following information:
+ *
+ * - "enabled": true if hpet if enabled, false otherwise
+ *
+ * Example:
+ *
+ * { "enabled": true }
+ */
+static void do_info_hpet(Monitor *mon, QObject **ret_data)
+{
+    *ret_data = qobject_from_jsonf("{ 'enabled': %i }", !no_hpet);
 }
 #endif
 
@@ -2375,7 +2392,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show state of HPET",
-        .mhandler.info = do_info_hpet,
+        .user_print = do_info_hpet_print,
+        .mhandler.info_new = do_info_hpet,
     },
 #endif
     {
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 12/19] monitor: Convert do_info_uuid() to QObject

[Luiz Capitulino]

snprintf() is used because the UUID_FMT is too complex for
qobject_from_jsonf().

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

diff --git a/monitor.c b/monitor.c
index 72fd573..f0cb759 100644
--- a/monitor.c
+++ b/monitor.c
@@ -594,13 +594,32 @@ static void do_info_hpet(Monitor *mon, QObject **ret_data)
 }
 #endif
 
-static void do_info_uuid(Monitor *mon)
+static void do_info_uuid_print(Monitor *mon, const QObject *data)
 {
-    monitor_printf(mon, UUID_FMT "\n", qemu_uuid[0], qemu_uuid[1],
+    monitor_printf(mon, "%s\n", qdict_get_str(qobject_to_qdict(data), "UUID"));
+}
+
+/**
+ * do_info_uuid(): Show VM UUID
+ *
+ * Return a QDict with the following information:
+ *
+ * - "UUID": Universally Unique Identifier
+ *
+ * Example:
+ *
+ * { "UUID": "550e8400-e29b-41d4-a716-446655440000" }
+ */
+static void do_info_uuid(Monitor *mon, QObject **ret_data)
+{
+    char uuid[64];
+
+    snprintf(uuid, sizeof(uuid), UUID_FMT, qemu_uuid[0], qemu_uuid[1],
                    qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], qemu_uuid[5],
                    qemu_uuid[6], qemu_uuid[7], qemu_uuid[8], qemu_uuid[9],
                    qemu_uuid[10], qemu_uuid[11], qemu_uuid[12], qemu_uuid[13],
                    qemu_uuid[14], qemu_uuid[15]);
+    *ret_data = qobject_from_jsonf("{ 'UUID': %s }", uuid);
 }
 
 /* get the current CPU defined by the user */
@@ -2495,7 +2514,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the current VM UUID",
-        .mhandler.info = do_info_uuid,
+        .user_print = do_info_uuid_print,
+        .mhandler.info_new = do_info_uuid,
     },
 #if defined(TARGET_PPC)
     {
-- 
1.6.6.rc1.39.g9a42

Re: [Qemu-devel] [PATCH 12/19] monitor: Convert do_info_uuid() to QObject

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> snprintf() is used because the UUID_FMT is too complex for
> qobject_from_jsonf().

Put this note in the source instead?

[Qemu-devel] [PATCH 13/19] monitor: Convert do_info_mice() to QObject

[Luiz Capitulino]

Each mouse is represented by a QDict, the returned QObject is a QList of
all mice.

This commit should not change user output.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 console.h |    3 +-
 monitor.c |    3 +-
 vl.c      |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/console.h b/console.h
index 9615f56..c7172f6 100644
--- a/console.h
+++ b/console.h
@@ -44,7 +44,8 @@ struct MouseTransformInfo {
     int a[7];
 };
 
-void do_info_mice(Monitor *mon);
+void do_info_mice_print(Monitor *mon, const QObject *data);
+void do_info_mice(Monitor *mon, QObject **ret_data);
 void do_mouse_set(Monitor *mon, const QDict *qdict);
 
 /* keysym is a unicode code except for special keys (see QEMU_KEY_xxx
diff --git a/monitor.c b/monitor.c
index f0cb759..d5e8dda 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2492,7 +2492,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show which guest mouse is receiving events",
-        .mhandler.info = do_info_mice,
+        .user_print = do_info_mice_print,
+        .mhandler.info_new = do_info_mice,
     },
     {
         .name       = "vnc",
diff --git a/vl.c b/vl.c
index 09a0ec5..73737ee 100644
--- a/vl.c
+++ b/vl.c
@@ -156,6 +156,7 @@ int main(int argc, char **argv)
 #include "balloon.h"
 #include "qemu-option.h"
 #include "qemu-config.h"
+#include "qemu-objects.h"
 
 #include "disas.h"
 
@@ -458,25 +459,72 @@ int kbd_mouse_is_absolute(void)
     return qemu_put_mouse_event_current->qemu_put_mouse_event_absolute;
 }
 
-void do_info_mice(Monitor *mon)
+static void info_mice_iter(QObject *data, void *opaque)
+{
+    QDict *mouse;
+    Monitor *mon = opaque;
+
+    mouse = qobject_to_qdict(data);
+    monitor_printf(mon, "%c Mouse #%" PRId64 ": %s\n",
+                  (qdict_get_bool(mouse, "current") ? '*' : ' '),
+                  qdict_get_int(mouse, "index"), qdict_get_str(mouse, "name"));
+}
+
+void do_info_mice_print(Monitor *mon, const QObject *data)
+{
+    QList *mice_list;
+
+    mice_list = qobject_to_qlist(data);
+    if (qlist_empty(mice_list)) {
+        monitor_printf(mon, "No mouse devices connected\n");
+        return;
+    }
+
+    qlist_iter(mice_list, info_mice_iter, mon);
+}
+
+/**
+ * do_info_mice(): Show VM mice information
+ *
+ * Each mouse is represented by a QDict, the returned QObject is a QList of
+ * all mice.
+ *
+ * The mouse QDict contains the following:
+ *
+ * - "name": mouse's name
+ * - "index": mouse's index
+ * - "current": true if this mouse is receiving events, false otherwise
+ *
+ * Example:
+ *
+ * [ { "name": "QEMU Microsoft Mouse", "index": 0, "current": false },
+ *   { "name": "QEMU PS/2 Mouse", "index": 1, "current": true } ]
+ */
+void do_info_mice(Monitor *mon, QObject **ret_data)
 {
     QEMUPutMouseEntry *cursor;
+    QList *mice_list;
     int index = 0;
 
+    mice_list = qlist_new();
+
     if (!qemu_put_mouse_event_head) {
-        monitor_printf(mon, "No mouse devices connected\n");
-        return;
+        goto out;
     }
 
-    monitor_printf(mon, "Mouse devices available:\n");
     cursor = qemu_put_mouse_event_head;
     while (cursor != NULL) {
-        monitor_printf(mon, "%c Mouse #%d: %s\n",
-                       (cursor == qemu_put_mouse_event_current ? '*' : ' '),
-                       index, cursor->qemu_put_mouse_event_name);
+        QObject *obj;
+        obj = qobject_from_jsonf("{ 'name': %s, 'index': %d, 'current': %i }",
+                                 cursor->qemu_put_mouse_event_name,
+                                 index, cursor == qemu_put_mouse_event_current);
+        qlist_append_obj(mice_list, obj);
         index++;
         cursor = cursor->next;
     }
+
+out:
+    *ret_data = QOBJECT(mice_list);
 }
 
 void do_mouse_set(Monitor *mon, const QDict *qdict)
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 14/19] migration: Convert do_info_migrate() to QObject

[Luiz Capitulino]

Return a QDict, which may contain more two QDicts, depending on
the type of migration we're performing.

IMPORTANT: as a QInt stores a int64_t integer, RAM values are going
to be stored as int64_t and not as uint64_t as they are today. If
this is a problem QInt will have to be changed.

This commit should not change user output.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 migration.c |  117 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
 migration.h |    4 ++-
 monitor.c   |    3 +-
 3 files changed, 107 insertions(+), 17 deletions(-)

diff --git a/migration.c b/migration.c
index d6a3e26..44e37e2 100644
--- a/migration.c
+++ b/migration.c
@@ -19,6 +19,7 @@
 #include "block.h"
 #include "qemu_socket.h"
 #include "block-migration.h"
+#include "qemu-objects.h"
 
 //#define DEBUG_MIGRATION
 
@@ -163,37 +164,123 @@ void do_migrate_set_downtime(Monitor *mon, const QDict *qdict)
     max_downtime = (uint64_t)d;
 }
 
-void do_info_migrate(Monitor *mon)
+static void migrate_print_status(Monitor *mon, const char *name,
+                                 const QDict *status_dict)
 {
+    QDict *qdict;
+
+    qdict = qobject_to_qdict(qdict_get(status_dict, name));
+
+    monitor_printf(mon, "transferred %s: %" PRIu64 " kbytes\n", name,
+                        qdict_get_int(qdict, "transferred") >> 10);
+    monitor_printf(mon, "remaining %s: %" PRIu64 " kbytes\n", name,
+                        qdict_get_int(qdict, "remaining") >> 10);
+    monitor_printf(mon, "total %s: %" PRIu64 " kbytes\n", name,
+                        qdict_get_int(qdict, "total") >> 10);
+}
+
+void do_info_migrate_print(Monitor *mon, const QObject *data)
+{
+    QDict *qdict;
+
+    qdict = qobject_to_qdict(data);
+
+    monitor_printf(mon, "Migration status: %s\n",
+                   qdict_get_str(qdict, "status"));
+
+    if (qdict_haskey(qdict, "ram")) {
+        migrate_print_status(mon, "ram", qdict);
+    }
+
+    if (qdict_haskey(qdict, "disk")) {
+        migrate_print_status(mon, "disk", qdict);
+    }
+}
+
+static void migrate_put_status(QDict *qdict, const char *name,
+                               uint64_t trans, uint64_t rem, uint64_t total)
+{
+    QObject *obj;
+
+    obj = qobject_from_jsonf("{ 'transferred': %" PRId64 ", "
+                               "'remaining': %" PRId64 ", "
+                               "'total': %" PRId64 " }", trans, rem, total);
+    assert(obj != NULL);
+
+    qdict_put_obj(qdict, name, obj);
+}
+
+/**
+ * do_info_migrate(): Migration status
+ *
+ * Return a QDict. If migration is active there will be another
+ * QDict with RAM migration status and if block migration is active
+ * another one with block migration status.
+ *
+ * The main QDict contains the following:
+ *
+ * - "status": migration status
+ * - "ram": only present if "status" is "active", it is a QDict with the
+ *   following RAM information (in bytes):
+ *          - "transferred": amount transferred
+ *          - "remaining": amount remaining
+ *          - "total": total
+ * - "disk": only present if it is a block migration, it is a QDict with
+ *   the following disk information (in bytes):
+ *          - "transferred": amount transferred
+ *          - "remaining": amount remaining
+ *          - "total": total
+ *
+ * Examples:
+ *
+ * 1. Migration is "completed":
+ *
+ * { "status": "completed" }
+ *
+ * 2. Migration is "active" and it is not a block migration:
+ *
+ * { "status": "active",
+ *            "ram": { "transferred": 123, "remaining": 123, "total": 246 } }
+ *
+ * 3. Migration is "active" and it is a block migration:
+ *
+ * { "status": "active",
+ *   "ram": { "total": 1057024, "remaining": 1053304, "transferred": 3720 },
+ *   "disk": { "total": 20971520, "remaining": 20880384, "transferred": 91136 }}
+ */
+void do_info_migrate(Monitor *mon, QObject **ret_data)
+{
+    QDict *qdict;
     MigrationState *s = current_migration;
 
     if (s) {
-        monitor_printf(mon, "Migration status: ");
         switch (s->get_status(s)) {
         case MIG_STATE_ACTIVE:
-            monitor_printf(mon, "active\n");
-            monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n", ram_bytes_transferred() >> 10);
-            monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n", ram_bytes_remaining() >> 10);
-            monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n", ram_bytes_total() >> 10);
+            qdict = qdict_new();
+            qdict_put(qdict, "status", qstring_from_str("active"));
+
+            migrate_put_status(qdict, "ram", ram_bytes_transferred(),
+                               ram_bytes_remaining(), ram_bytes_total());
+
             if (blk_mig_active()) {
-                monitor_printf(mon, "transferred disk: %" PRIu64 " kbytes\n",
-                               blk_mig_bytes_transferred() >> 10);
-                monitor_printf(mon, "remaining disk: %" PRIu64 " kbytes\n",
-                               blk_mig_bytes_remaining() >> 10);
-                monitor_printf(mon, "total disk: %" PRIu64 " kbytes\n",
-                               blk_mig_bytes_total() >> 10);
+                migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
+                                   blk_mig_bytes_remaining(),
+                                   blk_mig_bytes_total());
             }
+
+            *ret_data = QOBJECT(qdict);
             break;
         case MIG_STATE_COMPLETED:
-            monitor_printf(mon, "completed\n");
+            *ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
             break;
         case MIG_STATE_ERROR:
-            monitor_printf(mon, "failed\n");
+            *ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
             break;
         case MIG_STATE_CANCELLED:
-            monitor_printf(mon, "cancelled\n");
+            *ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
             break;
         }
+        assert(*ret_data != NULL);
     }
 }
 
diff --git a/migration.h b/migration.h
index 3f2b3df..3ac208b 100644
--- a/migration.h
+++ b/migration.h
@@ -62,7 +62,9 @@ uint64_t migrate_max_downtime(void);
 
 void do_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 
-void do_info_migrate(Monitor *mon);
+void do_info_migrate_print(Monitor *mon, const QObject *data);
+
+void do_info_migrate(Monitor *mon, QObject **ret_data);
 
 int exec_start_incoming_migration(const char *host_port);
 
diff --git a/monitor.c b/monitor.c
index d5e8dda..7e05765 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2541,7 +2541,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show migration status",
-        .mhandler.info = do_info_migrate,
+        .user_print = do_info_migrate_print,
+        .mhandler.info_new = do_info_migrate,
     },
     {
         .name       = "balloon",
-- 
1.6.6.rc1.39.g9a42

Re: [Qemu-devel] [PATCH 14/19] migration: Convert do_info_migrate() to QObject

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Return a QDict, which may contain more two QDicts, depending on

s/more two/up to two more/

> the type of migration we're performing.
>
> IMPORTANT: as a QInt stores a int64_t integer, RAM values are going
> to be stored as int64_t and not as uint64_t as they are today. If
> this is a problem QInt will have to be changed.
>
> This commit should not change user output.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  migration.c |  117 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  migration.h |    4 ++-
>  monitor.c   |    3 +-
>  3 files changed, 107 insertions(+), 17 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index d6a3e26..44e37e2 100644
> --- a/migration.c
> +++ b/migration.c
[...]
> +/**
> + * do_info_migrate(): Migration status
> + *
> + * Return a QDict. If migration is active there will be another
> + * QDict with RAM migration status and if block migration is active
> + * another one with block migration status.
> + *
> + * The main QDict contains the following:
> + *
> + * - "status": migration status
> + * - "ram": only present if "status" is "active", it is a QDict with the
> + *   following RAM information (in bytes):
> + *          - "transferred": amount transferred
> + *          - "remaining": amount remaining
> + *          - "total": total
> + * - "disk": only present if it is a block migration, it is a QDict with

... if "status" is "active" and it is a block migration ...

> + *   the following disk information (in bytes):
> + *          - "transferred": amount transferred
> + *          - "remaining": amount remaining
> + *          - "total": total
> + *
> + * Examples:
> + *
> + * 1. Migration is "completed":
> + *
> + * { "status": "completed" }
> + *
> + * 2. Migration is "active" and it is not a block migration:
> + *
> + * { "status": "active",
> + *            "ram": { "transferred": 123, "remaining": 123, "total": 246 } }
> + *
> + * 3. Migration is "active" and it is a block migration:
> + *
> + * { "status": "active",
> + *   "ram": { "total": 1057024, "remaining": 1053304, "transferred": 3720 },
> + *   "disk": { "total": 20971520, "remaining": 20880384, "transferred": 91136 }}
> + */
> +void do_info_migrate(Monitor *mon, QObject **ret_data)
[...]

[Qemu-devel] [PATCH 15/19] block: Convert bdrv_info() to QObject

[Luiz Capitulino]

Each block device information is stored in a QDict and the
returned QObject is a QList of all devices.

This commit should not change user output.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 Makefile  |    6 +-
 block.c   |  123 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
 block.h   |    4 +-
 monitor.c |    3 +-
 4 files changed, 111 insertions(+), 25 deletions(-)

diff --git a/Makefile b/Makefile
index f1b0131..a662d96 100644
--- a/Makefile
+++ b/Makefile
@@ -235,11 +235,11 @@ libqemu_common.a: $(obj-y)
 
 qemu-img.o: qemu-img-cmds.h
 
-qemu-img$(EXESUF): qemu-img.o qemu-tool.o $(block-obj-y)
+qemu-img$(EXESUF): qemu-img.o qemu-tool.o $(block-obj-y) $(qobject-obj-y)
 
-qemu-nbd$(EXESUF):  qemu-nbd.o qemu-tool.o $(block-obj-y)
+qemu-nbd$(EXESUF):  qemu-nbd.o qemu-tool.o $(block-obj-y) $(qobject-obj-y)
 
-qemu-io$(EXESUF):  qemu-io.o qemu-tool.o cmd.o $(block-obj-y)
+qemu-io$(EXESUF):  qemu-io.o qemu-tool.o cmd.o $(block-obj-y) $(qobject-obj-y)
 
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 	$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@,"  GEN   $@")
diff --git a/block.c b/block.c
index 853f025..d7eccb7 100644
--- a/block.c
+++ b/block.c
@@ -26,6 +26,7 @@
 #include "monitor.h"
 #include "block_int.h"
 #include "module.h"
+#include "qemu-objects.h"
 
 #ifdef CONFIG_BSD
 #include <sys/types.h>
@@ -1139,43 +1140,125 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
     return bs->drv->bdrv_is_allocated(bs, sector_num, nb_sectors, pnum);
 }
 
-void bdrv_info(Monitor *mon)
+static void bdrv_print_dict(QObject *obj, void *opaque)
 {
+    QDict *bs_dict;
+    Monitor *mon = opaque;
+
+    bs_dict = qobject_to_qdict(obj);
+
+    monitor_printf(mon, "%s: type=%s removable=%d",
+                        qdict_get_str(bs_dict, "device"),
+                        qdict_get_str(bs_dict, "type"),
+                        qdict_get_bool(bs_dict, "removable"));
+
+    if (qdict_get_bool(bs_dict, "removable")) {
+        monitor_printf(mon, " locked=%d", qdict_get_bool(bs_dict, "locked"));
+    }
+
+    if (qdict_haskey(bs_dict, "inserted")) {
+        QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted"));
+
+        monitor_printf(mon, " file=");
+        monitor_print_filename(mon, qdict_get_str(qdict, "file"));
+        if (qdict_haskey(qdict, "backing_file")) {
+            monitor_printf(mon, " backing_file=");
+            monitor_print_filename(mon, qdict_get_str(qdict, "backing_file"));
+        }
+        monitor_printf(mon, " ro=%d drv=%s encrypted=%d",
+                            qdict_get_bool(qdict, "ro"),
+                            qdict_get_str(qdict, "drv"),
+                            qdict_get_bool(qdict, "encrypted"));
+    } else {
+        monitor_printf(mon, " [not inserted]");
+    }
+
+    monitor_printf(mon, "\n");
+}
+
+void bdrv_info_print(Monitor *mon, const QObject *data)
+{
+    qlist_iter(qobject_to_qlist(data), bdrv_print_dict, mon);
+}
+
+/**
+ * bdrv_info(): Block devices information
+ *
+ * Each block device information is stored in a QDict and the
+ * returned QObject is a QList of all devices.
+ *
+ * The QDict contains the following:
+ *
+ * - "device": device name
+ * - "type": device type
+ * - "removable": true if the device is removable, false otherwise
+ * - "locked": true if the device is locked, false otherwise
+ * - "inserted": only present if the device is inserted, it is a QDict
+ *    containing the following:
+ *          - "file": device file name
+ *          - "ro": true if read-only, false otherwise
+ *          - "drv": driver format name
+ *          - "backing_file": backing file name if one is used
+ *          - "encrypted": true if encrypted, false otherwise
+ *
+ * Example:
+ *
+ * [ { "device": "ide0-hd0", "type": "hd", "removable": false, "locked": false,
+ *     "inserted": { "file": "/tmp/foobar", "ro": false, "drv": "qcow2" } },
+ *   { "device": "floppy0", "type": "floppy", "removable": true,
+ *     "locked": false } ]
+ */
+void bdrv_info(Monitor *mon, QObject **ret_data)
+{
+    QList *bs_list;
     BlockDriverState *bs;
 
+    bs_list = qlist_new();
+
     for (bs = bdrv_first; bs != NULL; bs = bs->next) {
-        monitor_printf(mon, "%s:", bs->device_name);
-        monitor_printf(mon, " type=");
+        QObject *bs_obj;
+        const char *type = "unknown";
+
         switch(bs->type) {
         case BDRV_TYPE_HD:
-            monitor_printf(mon, "hd");
+            type = "hd";
             break;
         case BDRV_TYPE_CDROM:
-            monitor_printf(mon, "cdrom");
+            type = "cdrom";
             break;
         case BDRV_TYPE_FLOPPY:
-            monitor_printf(mon, "floppy");
+            type = "floppy";
             break;
         }
-        monitor_printf(mon, " removable=%d", bs->removable);
-        if (bs->removable) {
-            monitor_printf(mon, " locked=%d", bs->locked);
-        }
+
+        bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': %s, "
+                                    "'removable': %i, 'locked': %i }",
+                                    bs->device_name, type, bs->removable,
+                                    bs->locked);
+        assert(bs_obj != NULL);
+
         if (bs->drv) {
-            monitor_printf(mon, " file=");
-            monitor_print_filename(mon, bs->filename);
+            QObject *obj;
+            QDict *bs_dict = qobject_to_qdict(bs_obj);
+
+            obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, "
+                                     "'encrypted': %i }",
+                                     bs->filename, bs->read_only,
+                                     bs->drv->format_name,
+                                     bdrv_is_encrypted(bs));
+            assert(obj != NULL);
             if (bs->backing_file[0] != '\0') {
-                monitor_printf(mon, " backing_file=");
-                monitor_print_filename(mon, bs->backing_file);
+                QDict *qdict = qobject_to_qdict(obj);
+                qdict_put(qdict, "backing_file",
+                          qstring_from_str(bs->backing_file));
             }
-            monitor_printf(mon, " ro=%d", bs->read_only);
-            monitor_printf(mon, " drv=%s", bs->drv->format_name);
-            monitor_printf(mon, " encrypted=%d", bdrv_is_encrypted(bs));
-        } else {
-            monitor_printf(mon, " [not inserted]");
+
+            qdict_put_obj(bs_dict, "inserted", obj);
         }
-        monitor_printf(mon, "\n");
+        qlist_append_obj(bs_list, bs_obj);
     }
+
+    *ret_data = QOBJECT(bs_list);
 }
 
 /* The "info blockstats" command. */
diff --git a/block.h b/block.h
index 4a8b628..3282dd2 100644
--- a/block.h
+++ b/block.h
@@ -4,6 +4,7 @@
 #include "qemu-aio.h"
 #include "qemu-common.h"
 #include "qemu-option.h"
+#include "qobject.h"
 
 /* block.c */
 typedef struct BlockDriver BlockDriver;
@@ -45,7 +46,8 @@ typedef struct QEMUSnapshotInfo {
 #define BDRV_SECTOR_SIZE   (1 << BDRV_SECTOR_BITS)
 #define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1);
 
-void bdrv_info(Monitor *mon);
+void bdrv_info_print(Monitor *mon, const QObject *data);
+void bdrv_info(Monitor *mon, QObject **ret_data);
 void bdrv_info_stats(Monitor *mon);
 
 void bdrv_init(void);
diff --git a/monitor.c b/monitor.c
index 7e05765..db596a9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2337,7 +2337,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the block devices",
-        .mhandler.info = bdrv_info,
+        .user_print = bdrv_info_print,
+        .mhandler.info_new = bdrv_info,
     },
     {
         .name       = "blockstats",
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 16/19] block: Convert bdrv_info_stats() to QObject

[Luiz Capitulino]

Each device statistic information is stored in a QDict and
the returned QObject is a QList of all devices.

This commit should not change user output.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 block.c   |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 block.h   |    3 +-
 monitor.c |    3 +-
 3 files changed, 75 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index d7eccb7..3f3496e 100644
--- a/block.c
+++ b/block.c
@@ -1261,22 +1261,82 @@ void bdrv_info(Monitor *mon, QObject **ret_data)
     *ret_data = QOBJECT(bs_list);
 }
 
-/* The "info blockstats" command. */
-void bdrv_info_stats(Monitor *mon)
+static void bdrv_stats_iter(QObject *data, void *opaque)
 {
+    QDict *qdict;
+    Monitor *mon = opaque;
+
+    qdict = qobject_to_qdict(data);
+    monitor_printf(mon, "%s:", qdict_get_str(qdict, "device"));
+
+    qdict = qobject_to_qdict(qdict_get(qdict, "stats"));
+    monitor_printf(mon, " rd_bytes=%" PRId64
+                        " wr_bytes=%" PRId64
+                        " rd_operations=%" PRId64
+                        " wr_operations=%" PRId64
+                        "\n",
+                        qdict_get_int(qdict, "rd_bytes"),
+                        qdict_get_int(qdict, "wr_bytes"),
+                        qdict_get_int(qdict, "rd_operations"),
+                        qdict_get_int(qdict, "wr_operations"));
+}
+
+void bdrv_stats_print(Monitor *mon, const QObject *data)
+{
+    qlist_iter(qobject_to_qlist(data), bdrv_stats_iter, mon);
+}
+
+/**
+ * bdrv_info_stats(): show block device statistics
+ *
+ * Each device statistic information is stored in a QDict and
+ * the returned QObject is a QList of all devices.
+ *
+ * The QDict contains the following:
+ *
+ * - "device": device name
+ * - "stats": A QDict with the statistics information, it contains:
+ *     - "rd_bytes": bytes read
+ *     - "wr_bytes": bytes written
+ *     - "rd_operations": read operations
+ *     - "wr_operations": write operations
+ * 
+ * Example:
+ *
+ * [ { "device": "ide0-hd0",
+ *               "stats": { "rd_bytes": 512,
+ *                          "wr_bytes": 0,
+ *                          "rd_operations": 1,
+ *                          "wr_operations": 0 } },
+ *   { "device": "ide1-cd0",
+ *               "stats": { "rd_bytes": 0,
+ *                          "wr_bytes": 0,
+ *                          "rd_operations": 0,
+ *                          "wr_operations": 0 } } ]
+ */
+void bdrv_info_stats(Monitor *mon, QObject **ret_data)
+{
+    QObject *obj;
+    QList *devices;
     BlockDriverState *bs;
 
+    devices = qlist_new();
+
     for (bs = bdrv_first; bs != NULL; bs = bs->next) {
-        monitor_printf(mon, "%s:"
-                       " rd_bytes=%" PRIu64
-                       " wr_bytes=%" PRIu64
-                       " rd_operations=%" PRIu64
-                       " wr_operations=%" PRIu64
-                       "\n",
-                       bs->device_name,
-                       bs->rd_bytes, bs->wr_bytes,
-                       bs->rd_ops, bs->wr_ops);
-    }
+        obj = qobject_from_jsonf("{ 'device': %s, 'stats': {"
+                                 "'rd_bytes': %" PRId64 ","
+                                 "'wr_bytes': %" PRId64 ","
+                                 "'rd_operations': %" PRId64 ","
+                                 "'wr_operations': %" PRId64
+                                 "} }",
+                                 bs->device_name,
+                                 bs->rd_bytes, bs->wr_bytes,
+                                 bs->rd_ops, bs->wr_ops);
+        assert(obj != NULL);
+        qlist_append_obj(devices, obj);
+    }
+
+    *ret_data = QOBJECT(devices);
 }
 
 const char *bdrv_get_encrypted_filename(BlockDriverState *bs)
diff --git a/block.h b/block.h
index 3282dd2..fa51ddf 100644
--- a/block.h
+++ b/block.h
@@ -48,7 +48,8 @@ typedef struct QEMUSnapshotInfo {
 
 void bdrv_info_print(Monitor *mon, const QObject *data);
 void bdrv_info(Monitor *mon, QObject **ret_data);
-void bdrv_info_stats(Monitor *mon);
+void bdrv_stats_print(Monitor *mon, const QObject *data);
+void bdrv_info_stats(Monitor *mon, QObject **ret_data);
 
 void bdrv_init(void);
 void bdrv_init_with_whitelist(void);
diff --git a/monitor.c b/monitor.c
index db596a9..e63a262 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2345,7 +2345,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show block device statistics",
-        .mhandler.info = bdrv_info_stats,
+        .user_print = bdrv_stats_print,
+        .mhandler.info_new = bdrv_info_stats,
     },
     {
         .name       = "registers",
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 17/19] char: Convert qemu_chr_info() to QObject

[Luiz Capitulino]

Each device is represented by a QDict. The returned QObject is a QList
of all devices.

This commit should not change user output.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c   |    3 ++-
 qemu-char.c |   43 +++++++++++++++++++++++++++++++++++++++++--
 qemu-char.h |    4 +++-
 3 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index e63a262..b2d0384 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2330,7 +2330,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the character devices",
-        .mhandler.info = qemu_chr_info,
+        .user_print = qemu_chr_info_print,
+        .mhandler.info_new = qemu_chr_info,
     },
     {
         .name       = "block",
diff --git a/qemu-char.c b/qemu-char.c
index da5c15c..c0d1cb0 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -32,6 +32,7 @@
 #include "hw/usb.h"
 #include "hw/baum.h"
 #include "hw/msmouse.h"
+#include "qemu-objects.h"
 
 #include <unistd.h>
 #include <fcntl.h>
@@ -2469,13 +2470,51 @@ void qemu_chr_close(CharDriverState *chr)
     qemu_free(chr);
 }
 
-void qemu_chr_info(Monitor *mon)
+static void qemu_chr_qlist_iter(QObject *obj, void *opaque)
 {
+    QDict *chr_dict;
+    Monitor *mon = opaque;
+
+    chr_dict = qobject_to_qdict(obj);
+    monitor_printf(mon, "%s: filename=%s\n", qdict_get_str(chr_dict, "label"),
+                                         qdict_get_str(chr_dict, "filename"));
+}
+
+void qemu_chr_info_print(Monitor *mon, const QObject *ret_data)
+{
+    qlist_iter(qobject_to_qlist(ret_data), qemu_chr_qlist_iter, mon);
+}
+
+/**
+ * qemu_chr_info(): Character devices information
+ *
+ * Each device is represented by a QDict. The returned QObject is a QList
+ * of all devices.
+ *
+ * The QDict contains the following:
+ *
+ * - "label": device's label
+ * - "filename": device's file
+ *
+ * Example:
+ *
+ * [ { "label": "monitor", "filename", "stdio" },
+ *   { "label": "serial0", "filename": "vc" } ]
+ */
+void qemu_chr_info(Monitor *mon, QObject **ret_data)
+{
+    QList *chr_list;
     CharDriverState *chr;
 
+    chr_list = qlist_new();
+
     QTAILQ_FOREACH(chr, &chardevs, next) {
-        monitor_printf(mon, "%s: filename=%s\n", chr->label, chr->filename);
+        QObject *obj = qobject_from_jsonf("{ 'label': %s, 'filename': %s }",
+                                          chr->label, chr->filename);
+        qlist_append_obj(chr_list, obj);
     }
+
+    *ret_data = QOBJECT(chr_list);
 }
 
 CharDriverState *qemu_chr_find(const char *name)
diff --git a/qemu-char.h b/qemu-char.h
index 9957db1..48f54d8 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -5,6 +5,7 @@
 #include "qemu-queue.h"
 #include "qemu-option.h"
 #include "qemu-config.h"
+#include "qobject.h"
 
 /* character device */
 
@@ -87,7 +88,8 @@ int qemu_chr_can_read(CharDriverState *s);
 void qemu_chr_read(CharDriverState *s, uint8_t *buf, int len);
 int qemu_chr_get_msgfd(CharDriverState *s);
 void qemu_chr_accept_input(CharDriverState *s);
-void qemu_chr_info(Monitor *mon);
+void qemu_chr_info_print(Monitor *mon, const QObject *ret_data);
+void qemu_chr_info(Monitor *mon, QObject **ret_data);
 CharDriverState *qemu_chr_find(const char *name);
 
 extern int term_escape_char;
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 18/19] PCI: Convert pci_device_hot_add() to QObject

[Luiz Capitulino]

Return a QDict with information about the just added device.

This commit should not change user output.

Please, note that this patch does not do error handling
conversion. In error conditions the handler still calls
monitor_printf().

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 hw/pci-hotplug.c |   40 ++++++++++++++++++++++++++++++++++++----
 qemu-monitor.hx  |    3 ++-
 sysemu.h         |    3 ++-
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 081d6d1..455fedd 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -33,6 +33,7 @@
 #include "scsi.h"
 #include "virtio-blk.h"
 #include "qemu-config.h"
+#include "qemu-objects.h"
 
 #if defined(TARGET_I386)
 static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
@@ -212,7 +213,36 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
     return dev;
 }
 
-void pci_device_hot_add(Monitor *mon, const QDict *qdict)
+void pci_device_hot_add_print(Monitor *mon, const QObject *data)
+{
+    QDict *qdict;
+
+    assert(qobject_type(data) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(data);
+
+    monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
+                   (int) qdict_get_int(qdict, "domain"),
+                   (int) qdict_get_int(qdict, "bus"),
+                   (int) qdict_get_int(qdict, "slot"),
+                   (int) qdict_get_int(qdict, "function"));
+
+}
+
+/**
+ * pci_device_hot_add(): Hot add a PCI device
+ *
+ * Return a QDict with the following device information:
+ *
+ * - "domain": domain number
+ * - "bus": bus number
+ * - "slot": slot number
+ * - "function": function number
+ *
+ * Example:
+ *
+ * { "domain": 0, "bus": 0, "slot": 5, "function": 0 }
+ */
+void pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     PCIDevice *dev = NULL;
     const char *pci_addr = qdict_get_str(qdict, "pci_addr");
@@ -239,9 +269,11 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
         monitor_printf(mon, "invalid type: %s\n", type);
 
     if (dev) {
-        monitor_printf(mon, "OK domain %d, bus %d, slot %d, function %d\n",
-                       0, pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
-                       PCI_FUNC(dev->devfn));
+        *ret_data =
+        qobject_from_jsonf("{ 'domain': 0, 'bus': %d, 'slot': %d, "
+                           "'function': %d }", pci_bus_num(dev->bus),
+                           PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
+        assert(*ret_data != NULL);
     } else
         monitor_printf(mon, "failed to add %s\n", opts);
 }
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 0657b2d..c788c73 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -810,7 +810,8 @@ ETEXI
         .args_type  = "pci_addr:s,type:s,opts:s?",
         .params     = "auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...",
         .help       = "hot-add PCI device",
-        .mhandler.cmd = pci_device_hot_add,
+        .user_print = pci_device_hot_add_print,
+        .mhandler.cmd_new = pci_device_hot_add,
     },
 #endif
 
diff --git a/sysemu.h b/sysemu.h
index efed771..9d80bb2 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -212,7 +212,8 @@ extern DriveInfo *drive_init(QemuOpts *arg, void *machine, int *fatal_error);
 DriveInfo *add_init_drive(const char *opts);
 
 /* pci-hotplug */
-void pci_device_hot_add(Monitor *mon, const QDict *qdict);
+void pci_device_hot_add_print(Monitor *mon, const QObject *data);
+void pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 void drive_hot_add(Monitor *mon, const QDict *qdict);
 void pci_device_hot_remove(Monitor *mon, const char *pci_addr);
 void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict,
-- 
1.6.6.rc1.39.g9a42

[Qemu-devel] [PATCH 19/19] VNC: Convert do_info_vnc() to QObject

[Luiz Capitulino]

Return a QDict with server information. Connected clients are returned
as a QList of QDicts.

The new functions (vnc_qdict_remote_addr(), vnc_qdict_local_addr() and
put_addr_qdict()) are used to insert 'host' and 'service' information
in the returned QDict.

This patch is big, but I don't see how to split it.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 console.h |    3 +-
 monitor.c |    3 +-
 vnc.c     |  191 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
 3 files changed, 164 insertions(+), 33 deletions(-)

diff --git a/console.h b/console.h
index c7172f6..dfc8ae4 100644
--- a/console.h
+++ b/console.h
@@ -323,7 +323,8 @@ void vnc_display_init(DisplayState *ds);
 void vnc_display_close(DisplayState *ds);
 int vnc_display_open(DisplayState *ds, const char *display);
 int vnc_display_password(DisplayState *ds, const char *password);
-void do_info_vnc(Monitor *mon);
+void do_info_vnc_print(Monitor *mon, const QObject *data);
+void do_info_vnc(Monitor *mon, QObject **ret_data);
 char *vnc_display_local_addr(DisplayState *ds);
 
 /* curses.c */
diff --git a/monitor.c b/monitor.c
index b2d0384..ddf9eac 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2503,7 +2503,8 @@ static const mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the vnc server status",
-        .mhandler.info = do_info_vnc,
+        .user_print = do_info_vnc_print,
+        .mhandler.info_new = do_info_vnc,
     },
     {
         .name       = "name",
diff --git a/vnc.c b/vnc.c
index 32c4678..f0fea6a 100644
--- a/vnc.c
+++ b/vnc.c
@@ -29,6 +29,7 @@
 #include "qemu_socket.h"
 #include "qemu-timer.h"
 #include "acl.h"
+#include "qemu-objects.h"
 
 #define VNC_REFRESH_INTERVAL_BASE 30
 #define VNC_REFRESH_INTERVAL_INC  50
@@ -99,6 +100,52 @@ char *vnc_socket_remote_addr(const char *format, int fd) {
     return addr_to_string(format, &sa, salen);
 }
 
+static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa,
+                          socklen_t salen)
+{
+    char host[NI_MAXHOST];
+    char serv[NI_MAXSERV];
+    int err;
+
+    if ((err = getnameinfo((struct sockaddr *)sa, salen,
+                           host, sizeof(host),
+                           serv, sizeof(serv),
+                           NI_NUMERICHOST | NI_NUMERICSERV)) != 0) {
+        VNC_DEBUG("Cannot resolve address %d: %s\n",
+                  err, gai_strerror(err));
+        return -1;
+    }
+
+    qdict_put(qdict, "host", qstring_from_str(host));
+    qdict_put(qdict, "service", qstring_from_str(serv));
+
+    return 0;
+}
+
+static int vnc_qdict_local_addr(QDict *qdict, int fd)
+{
+    struct sockaddr_storage sa;
+    socklen_t salen;
+
+    salen = sizeof(sa);
+    if (getsockname(fd, (struct sockaddr*)&sa, &salen) < 0)
+        return -1;
+
+    return put_addr_qdict(qdict, &sa, salen);
+}
+
+static int vnc_qdict_remote_addr(QDict *qdict, int fd)
+{
+    struct sockaddr_storage sa;
+    socklen_t salen;
+
+    salen = sizeof(sa);
+    if (getpeername(fd, (struct sockaddr*)&sa, &salen) < 0)
+        return -1;
+
+    return put_addr_qdict(qdict, &sa, salen);
+}
+
 static const char *vnc_auth_name(VncDisplay *vd) {
     switch (vd->auth) {
     case VNC_AUTH_INVALID:
@@ -150,58 +197,140 @@ static const char *vnc_auth_name(VncDisplay *vd) {
     return "unknown";
 }
 
-static void do_info_vnc_client(Monitor *mon, VncState *client)
+static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
 {
-    char *clientAddr =
-        vnc_socket_remote_addr("     address: %s:%s\n",
-                               client->csock);
-    if (!clientAddr)
-        return;
+    QDict *qdict;
 
-    monitor_printf(mon, "Client:\n");
-    monitor_printf(mon, "%s", clientAddr);
-    free(clientAddr);
+    qdict = qdict_new();
+    if (vnc_qdict_remote_addr(qdict, client->csock) < 0) {
+        QDECREF(qdict);
+        return NULL;
+    }
 
 #ifdef CONFIG_VNC_TLS
     if (client->tls.session &&
-        client->tls.dname)
-        monitor_printf(mon, "  x509 dname: %s\n", client->tls.dname);
-    else
-        monitor_printf(mon, "  x509 dname: none\n");
+        client->tls.dname) {
+        qdict_put(qdict, "x509 dname", qstring_from_str(client->tls.dname));
+    }
 #endif
 #ifdef CONFIG_VNC_SASL
     if (client->sasl.conn &&
-        client->sasl.username)
-        monitor_printf(mon, "    username: %s\n", client->sasl.username);
-    else
-        monitor_printf(mon, "    username: none\n");
+        client->sasl.username) {
+        qdict_put(qdict, "username", qstring_from_str(client->sasl.username));
+    }
 #endif
+
+    return qdict;
 }
 
-void do_info_vnc(Monitor *mon)
+static void info_vnc_iter(QObject *obj, void *opaque)
 {
-    if (vnc_display == NULL || vnc_display->display == NULL) {
+    QDict *client;
+    Monitor *mon = opaque;
+
+    client = qobject_to_qdict(obj);
+    monitor_printf(mon, "Client:\n");
+    monitor_printf(mon, "     address: %s:%s\n",
+                   qdict_get_str(client, "host"),
+                   qdict_get_str(client, "service"));
+
+#ifdef CONFIG_VNC_TLS
+    monitor_printf(mon, "  x509 dname: %s\n",
+        qdict_haskey(client, "x509 dname") ?
+        qdict_get_str(client, "x509 dname") : "none");
+#endif
+#ifdef CONFIG_VNC_SASL
+    monitor_printf(mon, "    username: %s\n",
+        qdict_haskey(client, "username") ?
+        qdict_get_str(client, "username") : "none");
+#endif
+}
+
+void do_info_vnc_print(Monitor *mon, const QObject *data)
+{
+    QDict *server;
+    QList *clients;
+
+    server = qobject_to_qdict(data);
+    if (strcmp(qdict_get_str(server, "status"), "disabled") == 0) {
         monitor_printf(mon, "Server: disabled\n");
-    } else {
-        char *serverAddr = vnc_socket_local_addr("     address: %s:%s\n",
-                                                 vnc_display->lsock);
+        return;
+    }
 
-        if (!serverAddr)
-            return;
+    monitor_printf(mon, "Server:\n");
+    monitor_printf(mon, "     address: %s:%s\n",
+                   qdict_get_str(server, "host"),
+                   qdict_get_str(server, "service"));
+    monitor_printf(mon, "        auth: %s\n",
+        qdict_haskey(server, "auth") ? qdict_get_str(server, "auth") : "none");
+
+    clients = qdict_get_qlist(server, "clients");
+    if (qlist_empty(clients)) {
+        monitor_printf(mon, "Client: none\n");
+    } else {
+        qlist_iter(clients, info_vnc_iter, mon);
+    }
+}
 
-        monitor_printf(mon, "Server:\n");
-        monitor_printf(mon, "%s", serverAddr);
-        free(serverAddr);
-        monitor_printf(mon, "        auth: %s\n", vnc_auth_name(vnc_display));
+/**
+ * do_info_vnc(): Show VNC server information
+ *
+ * Return a QDict with server information. Connected clients are returned
+ * as a QList of QDicts.
+ *
+ * The main QDict contains the following:
+ *
+ * - "status": "disabled" or "enabled"
+ * - "host": server's IP address
+ * - "service": server's port number
+ * - "auth": authentication method (optional)
+ * - "clients": a QList of all connected clients
+ *
+ * Clients are described by a QDict, with the following information:
+ *
+ * - "host": client's IP address
+ * - "service": client's port number
+ * - "x509 dname": TLS dname (optional)
+ * - "username": SASL username (optional)
+ *
+ * Example:
+ *
+ * { "status": "enabled", "host": "0.0.0.0", "service": "50402", "auth": "vnc",
+ *   "clients": [ { "host": "127.0.0.1", "service": "50401" } ] }
+ */
+void do_info_vnc(Monitor *mon, QObject **ret_data)
+{
+    if (vnc_display == NULL || vnc_display->display == NULL) {
+        *ret_data = qobject_from_jsonf("{ 'status': 'disabled' }");
+    } else {
+        QDict *qdict;
+        QList *clist;
 
+        clist = qlist_new();
         if (vnc_display->clients) {
             VncState *client = vnc_display->clients;
             while (client) {
-                do_info_vnc_client(mon, client);
+                qdict = do_info_vnc_client(mon, client);
+                if (qdict)
+                    qlist_append(clist, qdict);
                 client = client->next;
             }
-        } else {
-            monitor_printf(mon, "Client: none\n");
+        }
+
+        *ret_data = qobject_from_jsonf("{ 'status': 'enabled', 'clients': %p }",
+                                       QOBJECT(clist));
+        assert(*ret_data != NULL);
+
+        qdict = qobject_to_qdict(*ret_data);
+
+        if (vnc_display->auth != VNC_AUTH_NONE) {
+            qdict_put(qdict, "auth",
+                      qstring_from_str(vnc_auth_name(vnc_display)));
+        }
+
+        if (vnc_qdict_local_addr(qdict, vnc_display->lsock) < 0) {
+            qobject_decref(*ret_data);
+            *ret_data = NULL;
         }
     }
 }
-- 
1.6.6.rc1.39.g9a42

Re: [Qemu-devel] [PATCH 19/19] VNC: Convert do_info_vnc() to QObject

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> Return a QDict with server information. Connected clients are returned
> as a QList of QDicts.
>
> The new functions (vnc_qdict_remote_addr(), vnc_qdict_local_addr() and
> put_addr_qdict()) are used to insert 'host' and 'service' information
> in the returned QDict.
>
> This patch is big, but I don't see how to split it.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  console.h |    3 +-
>  monitor.c |    3 +-
>  vnc.c     |  191 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
>  3 files changed, 164 insertions(+), 33 deletions(-)
>
[...]
> diff --git a/vnc.c b/vnc.c
> index 32c4678..f0fea6a 100644
> --- a/vnc.c
> +++ b/vnc.c
[...]
> +/**
> + * do_info_vnc(): Show VNC server information
> + *
> + * Return a QDict with server information. Connected clients are returned
> + * as a QList of QDicts.
> + *
> + * The main QDict contains the following:
> + *
> + * - "status": "disabled" or "enabled"
> + * - "host": server's IP address
> + * - "service": server's port number
> + * - "auth": authentication method (optional)
> + * - "clients": a QList of all connected clients
> + *
> + * Clients are described by a QDict, with the following information:
> + *
> + * - "host": client's IP address
> + * - "service": client's port number
> + * - "x509 dname": TLS dname (optional)

Sure you want dict keys with spaces?  I'd prefer "x509-dname".

> + * - "username": SASL username (optional)
> + *
> + * Example:
> + *
> + * { "status": "enabled", "host": "0.0.0.0", "service": "50402", "auth": "vnc",
> + *   "clients": [ { "host": "127.0.0.1", "service": "50401" } ] }
> + */
> +void do_info_vnc(Monitor *mon, QObject **ret_data)
[...]

Re: [Qemu-devel] [PATCH 19/19] VNC: Convert do_info_vnc() to QObject

[Luiz Capitulino]

On Thu, 10 Dec 2009 11:34:37 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> > +/**
> > + * do_info_vnc(): Show VNC server information
> > + *
> > + * Return a QDict with server information. Connected clients are returned
> > + * as a QList of QDicts.
> > + *
> > + * The main QDict contains the following:
> > + *
> > + * - "status": "disabled" or "enabled"
> > + * - "host": server's IP address
> > + * - "service": server's port number
> > + * - "auth": authentication method (optional)
> > + * - "clients": a QList of all connected clients
> > + *
> > + * Clients are described by a QDict, with the following information:
> > + *
> > + * - "host": client's IP address
> > + * - "service": client's port number
> > + * - "x509 dname": TLS dname (optional)
> 
> Sure you want dict keys with spaces?  I'd prefer "x509-dname".

 I don't think it's a big deal because this string will never
change, but would be good to come with a standard style for
dict keys (at least for the protocol).

 I'm ok with either way.

Re: [Qemu-devel] [PATCH 19/19] VNC: Convert do_info_vnc() to QObject

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 09:56:06AM -0200, Luiz Capitulino wrote:
> On Thu, 10 Dec 2009 11:34:37 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
> 
> > > +/**
> > > + * do_info_vnc(): Show VNC server information
> > > + *
> > > + * Return a QDict with server information. Connected clients are returned
> > > + * as a QList of QDicts.
> > > + *
> > > + * The main QDict contains the following:
> > > + *
> > > + * - "status": "disabled" or "enabled"
> > > + * - "host": server's IP address
> > > + * - "service": server's port number
> > > + * - "auth": authentication method (optional)
> > > + * - "clients": a QList of all connected clients
> > > + *
> > > + * Clients are described by a QDict, with the following information:
> > > + *
> > > + * - "host": client's IP address
> > > + * - "service": client's port number
> > > + * - "x509 dname": TLS dname (optional)
> > 
> > Sure you want dict keys with spaces?  I'd prefer "x509-dname".
> 
>  I don't think it's a big deal because this string will never
> change, but would be good to come with a standard style for
> dict keys (at least for the protocol).

I agree with Markus, that it is nicer to avoid spaces in the dict keys,
even if technically it is allowed. I'd vote for x509-dname too.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [PATCH 19/19] VNC: Convert do_info_vnc() to QObject

[Anthony Liguori]

Markus Armbruster wrote:
> Luiz Capitulino <lcapitulino@redhat.com> writes:
>
>   
>> Return a QDict with server information. Connected clients are returned
>> as a QList of QDicts.
>>
>> The new functions (vnc_qdict_remote_addr(), vnc_qdict_local_addr() and
>> put_addr_qdict()) are used to insert 'host' and 'service' information
>> in the returned QDict.
>>
>> This patch is big, but I don't see how to split it.
>>
>> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
>> ---
>>  console.h |    3 +-
>>  monitor.c |    3 +-
>>  vnc.c     |  191 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
>>  3 files changed, 164 insertions(+), 33 deletions(-)
>>
>>     
> [...]
>   
>> diff --git a/vnc.c b/vnc.c
>> index 32c4678..f0fea6a 100644
>> --- a/vnc.c
>> +++ b/vnc.c
>>     
> [...]
>   
>> +/**
>> + * do_info_vnc(): Show VNC server information
>> + *
>> + * Return a QDict with server information. Connected clients are returned
>> + * as a QList of QDicts.
>> + *
>> + * The main QDict contains the following:
>> + *
>> + * - "status": "disabled" or "enabled"
>> + * - "host": server's IP address
>> + * - "service": server's port number
>> + * - "auth": authentication method (optional)
>> + * - "clients": a QList of all connected clients
>> + *
>> + * Clients are described by a QDict, with the following information:
>> + *
>> + * - "host": client's IP address
>> + * - "service": client's port number
>> + * - "x509 dname": TLS dname (optional)
>>     
>
> Sure you want dict keys with spaces?  I'd prefer "x509-dname".
>   

+1

Actually, x509_dname is preferable.  While the JSON spec doesn't say 
this, in JavaScript, dictionaries are indistinguishable from objects.  
It's better for the key names to be valid identifiers.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [FOR 0.12 v3 00/19]: info handlers conversions to QObject

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

>  Hi,
>
>  This series covers almost half of the info handlers conversions to the
> QObject style.
>
>  The biggest change in this version is that now info handlers always
> return a QDict.

Apart from a few nits, I like this series.  It's not
backward-compatible, so getting it in 0.12 would be preferable.

PATCH 05/19 is a must-have, because it fixes a regression in info
monitor.