[Qemu-devel] [PATCH 00/23][PULL]: QMP/Monitor queue

[Qemu-devel] [PATCH 00/23][PULL]: QMP/Monitor queue

[Luiz Capitulino]

Hi Anthony,

The following QMP/Monitor patches have been sent to the list and look good
to me. I also did some basic testing on them.

Most of these changes are bug fixes, the only exception is my new argument
checker series, which is a complete rewrite (that also fix bugs...).

The changes (since 449041d4db1f82f281fe097e832f07cd9ee1e864) are available
in the following repository:

    git://repo.or.cz/qemu/qmp-unstable.git for-anthony

Jan Kiszka (6):
      monitor: Fix leakage during completion processing
      monitor: Fix command completion vs. boolean switches
      monitor: Establish cmd flags and convert the async tag
      QMP: Teach basic capability negotiation to python example
      QMP: Fix python helper /wrt long return strings
      monitor: Allow to exclude commands from QMP

Luiz Capitulino (16):
      QMP: Fix error reporting in the async API
      QError: Enhance QERR_DEVICE_NOT_ACTIVE's user desc
      QDict: Rename 'err_value'
      QDict: Small terminology change
      QDict: Introduce functions to retrieve QDictEntry values
      QDict: Introduce new iteration API
      check-qdict: Introduce test for the new iteration API
      QDict: Introduce qdict_get_try_bool()
      Monitor: handle optional '-' arg as a bool
      QMP: New argument checker (first part)
      QMP: New argument checker (second part)
      QMP: Drop old client argument checker
      QError: Introduce QERR_QMP_EXTRA_MEMBER
      QMP: Introduce qmp_check_input_obj()
      QMP: Drop old input object checking
      QMP: handle_qmp_command(): Small cleanup

Yoshiaki Tamura (1):
      net: delete QemuOpts when net_client_init() fails.

 QMP/qmp-shell   |    1 +
 QMP/qmp.py      |    6 +-
 QMP/vm-info     |    1 +
 blockdev.c      |    2 +-
 check-qdict.c   |   33 ++++-
 migration.c     |   16 +-
 monitor.c       |  435 ++++++++++++++++++++++++++++++++-----------------------
 monitor.h       |    4 +
 net.c           |    4 +
 qdict.c         |  106 ++++++++++++--
 qdict.h         |   11 +-
 qemu-monitor.hx |    2 +-
 qerror.c        |    6 +-
 qerror.h        |    3 +
 14 files changed, 418 insertions(+), 212 deletions(-)

[Qemu-devel] [PATCH 01/23] monitor: Fix leakage during completion processing

[Luiz Capitulino]

From: Jan Kiszka <jan.kiszka@siemens.com>

Given too many arguments or an invalid command, we were leaking the
duplicated argument strings.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/monitor.c b/monitor.c
index 170b269..42ae154 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3882,8 +3882,9 @@ static void monitor_find_completion(const char *cmdline)
        next arg */
     len = strlen(cmdline);
     if (len > 0 && qemu_isspace(cmdline[len - 1])) {
-        if (nb_args >= MAX_ARGS)
-            return;
+        if (nb_args >= MAX_ARGS) {
+            goto cleanup;
+        }
         args[nb_args++] = qemu_strdup("");
     }
     if (nb_args <= 1) {
@@ -3898,12 +3899,15 @@ static void monitor_find_completion(const char *cmdline)
         }
     } else {
         /* find the command */
-        for(cmd = mon_cmds; cmd->name != NULL; cmd++) {
-            if (compare_cmd(args[0], cmd->name))
-                goto found;
+        for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
+            if (compare_cmd(args[0], cmd->name)) {
+                break;
+            }
         }
-        return;
-    found:
+        if (!cmd->name) {
+            goto cleanup;
+        }
+
         ptype = next_arg_type(cmd->args_type);
         for(i = 0; i < nb_args - 2; i++) {
             if (*ptype != '\0') {
@@ -3953,8 +3957,11 @@ static void monitor_find_completion(const char *cmdline)
             break;
         }
     }
-    for(i = 0; i < nb_args; i++)
+
+cleanup:
+    for (i = 0; i < nb_args; i++) {
         qemu_free(args[i]);
+    }
 }
 
 static int monitor_can_read(void *opaque)
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 02/23] monitor: Fix command completion vs. boolean switches

[Luiz Capitulino]

From: Jan Kiszka <jan.kiszka@siemens.com>

We now have to move forward to the next argument type via next_arg_type.
This patch fixes completion for 'eject' and maybe also other commands.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 42ae154..b375f10 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3918,7 +3918,7 @@ static void monitor_find_completion(const char *cmdline)
         }
         str = args[nb_args - 1];
         if (*ptype == '-' && ptype[1] != '\0') {
-            ptype += 2;
+            ptype = next_arg_type(ptype);
         }
         switch(*ptype) {
         case 'F':
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 03/23] monitor: Establish cmd flags and convert the async tag

[Luiz Capitulino]

From: Jan Kiszka <jan.kiszka@siemens.com>

As we want to add more flags to monitor commands, convert the only so
far existing one accordingly.

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

diff --git a/monitor.c b/monitor.c
index b375f10..980e98d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -112,7 +112,7 @@ typedef struct mon_cmd_t {
         int  (*cmd_async)(Monitor *mon, const QDict *params,
                           MonitorCompletion *cb, void *opaque);
     } mhandler;
-    int async;
+    int flags;
 } mon_cmd_t;
 
 /* file descriptors passed via SCM_RIGHTS */
@@ -327,7 +327,7 @@ static inline int monitor_handler_ported(const mon_cmd_t *cmd)
 
 static inline bool monitor_handler_is_async(const mon_cmd_t *cmd)
 {
-    return cmd->async != 0;
+    return cmd->flags & MONITOR_CMD_ASYNC;
 }
 
 static inline int monitor_has_error(const Monitor *mon)
@@ -2536,7 +2536,7 @@ static const mon_cmd_t info_cmds[] = {
         .help       = "show balloon information",
         .user_print = monitor_print_balloon,
         .mhandler.info_async = do_info_balloon,
-        .async      = 1,
+        .flags      = MONITOR_CMD_ASYNC,
     },
     {
         .name       = "qtree",
diff --git a/monitor.h b/monitor.h
index ea15469..9582b9c 100644
--- a/monitor.h
+++ b/monitor.h
@@ -15,6 +15,9 @@ extern Monitor *default_mon;
 #define MONITOR_USE_READLINE  0x02
 #define MONITOR_USE_CONTROL   0x04
 
+/* flags for monitor commands */
+#define MONITOR_CMD_ASYNC       0x0001
+
 /* QMP events */
 typedef enum MonitorEvent {
     QEVENT_SHUTDOWN,
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 9f62b94..2af3de6 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1287,7 +1287,7 @@ ETEXI
         .help       = "request VM to change its memory allocation (in MB)",
         .user_print = monitor_user_noop,
         .mhandler.cmd_async = do_balloon,
-        .async      = 1,
+        .flags      = MONITOR_CMD_ASYNC,
     },
 
 STEXI
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 04/23] QMP: Teach basic capability negotiation to python example

[Luiz Capitulino]

From: Jan Kiszka <jan.kiszka@siemens.com>

As sending "qmp_capabilities" on session start became mandatory, both
python examples were broken.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp-shell |    1 +
 QMP/vm-info   |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-shell b/QMP/qmp-shell
index f89b9af..a5b72d1 100755
--- a/QMP/qmp-shell
+++ b/QMP/qmp-shell
@@ -42,6 +42,7 @@ def main():
 
     qemu = qmp.QEMUMonitorProtocol(argv[1])
     qemu.connect()
+    qemu.send("qmp_capabilities")
 
     print 'Connected!'
 
diff --git a/QMP/vm-info b/QMP/vm-info
index 8ebaeb3..be5b038 100755
--- a/QMP/vm-info
+++ b/QMP/vm-info
@@ -24,6 +24,7 @@ def main():
 
     qemu = qmp.QEMUMonitorProtocol(argv[1])
     qemu.connect()
+    qemu.send("qmp_capabilities")
 
     for cmd in [ 'version', 'kvm', 'status', 'uuid', 'balloon' ]:
         print cmd + ': ' + str(qemu.send('query-' + cmd))
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 05/23] QMP: Fix python helper /wrt long return strings

[Luiz Capitulino]

From: Jan Kiszka <jan.kiszka@siemens.com>

Remove the arbitrary limitation of 1024 characters per return string and
read complete lines instead. Required for device_show.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 QMP/qmp.py |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/QMP/qmp.py b/QMP/qmp.py
index d9da603..4062f84 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -63,10 +63,14 @@ class QEMUMonitorProtocol:
 
     def __json_read(self):
         try:
-            return json.loads(self.sock.recv(1024))
+            while True:
+                line = json.loads(self.sockfile.readline())
+                if not 'event' in line:
+                    return line
         except ValueError:
             return
 
     def __init__(self, filename):
         self.filename = filename
         self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+        self.sockfile = self.sock.makefile()
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 06/23] net: delete QemuOpts when net_client_init() fails.

[Luiz Capitulino]

From: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>

This fixes the following scenario using QMP.

First, put a bogus argument "foo" to "type", which results in an error.
{"execute": "netdev_add", "arguments": { "type": "foo", "id": "netdev1" } }
Then, call it again with correct argument "user".
{"execute": "netdev_add", "arguments": { "type": "user", "id": "netdev1" } }
This results in "DuplicatedId" error.

Because the first command was invalid, it should be able to reuse the
same "id", and the second command should work.

Reported-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 net.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net.c b/net.c
index 90bd5a9..8ddf872 100644
--- a/net.c
+++ b/net.c
@@ -1208,6 +1208,10 @@ int do_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
     }
 
     res = net_client_init(mon, opts, 1);
+    if (res < 0) {
+        qemu_opts_del(opts);
+    }
+
     return res;
 }
 
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 07/23] QMP: Fix error reporting in the async API

[Luiz Capitulino]

The current asynchronous command API doesn't return a QMP response
when the async command fails.

This is easy to reproduce with the balloon command (the sole async
command we have so far): run qemu w/o the '-balloon virtio' option
and try to issue the balloon command via QMP: no response will be
sent to the client.

This commit fixes the problem by making qmp_async_cmd_handler()
return the handler's error code and then calling
monitor_protocol_emitter() if the handler has returned an error.

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

diff --git a/monitor.c b/monitor.c
index 980e98d..58b060b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -546,10 +546,10 @@ static void qmp_monitor_complete(void *opaque, QObject *ret_data)
     monitor_protocol_emitter(opaque, ret_data);
 }
 
-static void qmp_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
-                                  const QDict *params)
+static int qmp_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd,
+                                 const QDict *params)
 {
-    cmd->mhandler.cmd_async(mon, params, qmp_monitor_complete, mon);
+    return cmd->mhandler.cmd_async(mon, params, qmp_monitor_complete, mon);
 }
 
 static void qmp_async_info_handler(Monitor *mon, const mon_cmd_t *cmd)
@@ -4239,7 +4239,11 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     }
 
     if (monitor_handler_is_async(cmd)) {
-        qmp_async_cmd_handler(mon, cmd, args);
+        err = qmp_async_cmd_handler(mon, cmd, args);
+        if (err) {
+            /* emit the error response */
+            goto err_out;
+        }
     } else {
         monitor_call_handler(mon, cmd, args);
     }
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 08/23] QError: Enhance QERR_DEVICE_NOT_ACTIVE's user desc

[Luiz Capitulino]

The 'by the guest' part is misleading, it could be disabled by
the host too.

We will likely need more surgery if we care for the distinction,
just dropping the problematic part is good enough for now.

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

diff --git a/qerror.c b/qerror.c
index 44d0bf8..cce1e7b 100644
--- a/qerror.c
+++ b/qerror.c
@@ -82,7 +82,7 @@ static const QErrorStringTable qerror_table[] = {
     },
     {
         .error_fmt = QERR_DEVICE_NOT_ACTIVE,
-        .desc      = "Device '%(device)' has not been activated by the guest",
+        .desc      = "Device '%(device)' has not been activated",
     },
     {
         .error_fmt = QERR_DEVICE_NOT_ENCRYPTED,
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 09/23] QDict: Rename 'err_value'

[Luiz Capitulino]

A missing key is not an error.

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

diff --git a/qdict.c b/qdict.c
index 175bc17..c974d6f 100644
--- a/qdict.c
+++ b/qdict.c
@@ -272,16 +272,16 @@ const char *qdict_get_str(const QDict *qdict, const char *key)
  *
  * Return integer mapped by 'key', if it is not present in
  * the dictionary or if the stored object is not of QInt type
- * 'err_value' will be returned.
+ * 'def_value' will be returned.
  */
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
-                          int64_t err_value)
+                          int64_t def_value)
 {
     QObject *obj;
 
     obj = qdict_get(qdict, key);
     if (!obj || qobject_type(obj) != QTYPE_QINT)
-        return err_value;
+        return def_value;
 
     return qint_get_int(qobject_to_qint(obj));
 }
diff --git a/qdict.h b/qdict.h
index 5e5902c..72ea563 100644
--- a/qdict.h
+++ b/qdict.h
@@ -56,7 +56,7 @@ QList *qdict_get_qlist(const QDict *qdict, const char *key);
 QDict *qdict_get_qdict(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);
+                          int64_t def_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 #endif /* QDICT_H */
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 10/23] QDict: Small terminology change

[Luiz Capitulino]

Let's call a 'hash' only what is returned by our hash function,
anything else is a 'bucket'.

This helps avoiding confusion with regard to how we traverse
our table.

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

diff --git a/check-qdict.c b/check-qdict.c
index 2c3089f..1b070f4 100644
--- a/check-qdict.c
+++ b/check-qdict.c
@@ -50,7 +50,7 @@ START_TEST(qdict_put_obj_test)
     qdict_put_obj(qdict, "", QOBJECT(qint_from_int(num)));
 
     fail_unless(qdict_size(qdict) == 1);
-    ent = QLIST_FIRST(&qdict->table[12345 % QDICT_HASH_SIZE]);
+    ent = QLIST_FIRST(&qdict->table[12345 % QDICT_BUCKET_MAX]);
     qi = qobject_to_qint(ent->value);
     fail_unless(qint_get_int(qi) == num);
 
diff --git a/qdict.c b/qdict.c
index c974d6f..71be2eb 100644
--- a/qdict.c
+++ b/qdict.c
@@ -86,11 +86,11 @@ static QDictEntry *alloc_entry(const char *key, QObject *value)
  * qdict_find(): List lookup function
  */
 static QDictEntry *qdict_find(const QDict *qdict,
-                              const char *key, unsigned int hash)
+                              const char *key, unsigned int bucket)
 {
     QDictEntry *entry;
 
-    QLIST_FOREACH(entry, &qdict->table[hash], next)
+    QLIST_FOREACH(entry, &qdict->table[bucket], next)
         if (!strcmp(entry->key, key))
             return entry;
 
@@ -110,11 +110,11 @@ static QDictEntry *qdict_find(const QDict *qdict,
  */
 void qdict_put_obj(QDict *qdict, const char *key, QObject *value)
 {
-    unsigned int hash;
+    unsigned int bucket;
     QDictEntry *entry;
 
-    hash = tdb_hash(key) % QDICT_HASH_SIZE;
-    entry = qdict_find(qdict, key, hash);
+    bucket = tdb_hash(key) % QDICT_BUCKET_MAX;
+    entry = qdict_find(qdict, key, bucket);
     if (entry) {
         /* replace key's value */
         qobject_decref(entry->value);
@@ -122,7 +122,7 @@ void qdict_put_obj(QDict *qdict, const char *key, QObject *value)
     } else {
         /* allocate a new entry */
         entry = alloc_entry(key, value);
-        QLIST_INSERT_HEAD(&qdict->table[hash], entry, next);
+        QLIST_INSERT_HEAD(&qdict->table[bucket], entry, next);
         qdict->size++;
     }
 }
@@ -137,7 +137,7 @@ QObject *qdict_get(const QDict *qdict, const char *key)
 {
     QDictEntry *entry;
 
-    entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_HASH_SIZE);
+    entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX);
     return (entry == NULL ? NULL : entry->value);
 }
 
@@ -148,8 +148,8 @@ QObject *qdict_get(const QDict *qdict, const char *key)
  */
 int qdict_haskey(const QDict *qdict, const char *key)
 {
-    unsigned int hash = tdb_hash(key) % QDICT_HASH_SIZE;
-    return (qdict_find(qdict, key, hash) == NULL ? 0 : 1);
+    unsigned int bucket = tdb_hash(key) % QDICT_BUCKET_MAX;
+    return (qdict_find(qdict, key, bucket) == NULL ? 0 : 1);
 }
 
 /**
@@ -318,7 +318,7 @@ void qdict_iter(const QDict *qdict,
     int i;
     QDictEntry *entry;
 
-    for (i = 0; i < QDICT_HASH_SIZE; i++) {
+    for (i = 0; i < QDICT_BUCKET_MAX; i++) {
         QLIST_FOREACH(entry, &qdict->table[i], next)
             iter(entry->key, entry->value, opaque);
     }
@@ -347,7 +347,7 @@ void qdict_del(QDict *qdict, const char *key)
 {
     QDictEntry *entry;
 
-    entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_HASH_SIZE);
+    entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX);
     if (entry) {
         QLIST_REMOVE(entry, next);
         qentry_destroy(entry);
@@ -366,7 +366,7 @@ static void qdict_destroy_obj(QObject *obj)
     assert(obj != NULL);
     qdict = qobject_to_qdict(obj);
 
-    for (i = 0; i < QDICT_HASH_SIZE; i++) {
+    for (i = 0; i < QDICT_BUCKET_MAX; i++) {
         QDictEntry *entry = QLIST_FIRST(&qdict->table[i]);
         while (entry) {
             QDictEntry *tmp = QLIST_NEXT(entry, next);
diff --git a/qdict.h b/qdict.h
index 72ea563..dcd2b29 100644
--- a/qdict.h
+++ b/qdict.h
@@ -18,7 +18,7 @@
 #include "qemu-queue.h"
 #include <stdint.h>
 
-#define QDICT_HASH_SIZE 512
+#define QDICT_BUCKET_MAX 512
 
 typedef struct QDictEntry {
     char *key;
@@ -29,7 +29,7 @@ typedef struct QDictEntry {
 typedef struct QDict {
     QObject_HEAD;
     size_t size;
-    QLIST_HEAD(,QDictEntry) table[QDICT_HASH_SIZE];
+    QLIST_HEAD(,QDictEntry) table[QDICT_BUCKET_MAX];
 } QDict;
 
 /* Object API */
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 11/23] QDict: Introduce functions to retrieve QDictEntry values

[Luiz Capitulino]

Next commit will introduce a new QDict iteration API which
returns QDictEntry entries, but we don't want users to directly
access its members since QDictEntry should be private to QDict.

In the near future this kind of data type will be turned into a
forward reference.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qdict.c |   21 +++++++++++++++++++++
 qdict.h |    2 ++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index 71be2eb..c467763 100644
--- a/qdict.c
+++ b/qdict.c
@@ -83,6 +83,27 @@ static QDictEntry *alloc_entry(const char *key, QObject *value)
 }
 
 /**
+ * qdict_entry_value(): Return qdict entry value
+ *
+ * Return weak reference.
+ */
+QObject *qdict_entry_value(const QDictEntry *entry)
+{
+    return entry->value;
+}
+
+/**
+ * qdict_entry_key(): Return qdict entry key
+ *
+ * Return a *pointer* to the string, it has to be duplicated before being
+ * stored.
+ */
+const char *qdict_entry_key(const QDictEntry *entry)
+{
+    return entry->key;
+}
+
+/**
  * qdict_find(): List lookup function
  */
 static QDictEntry *qdict_find(const QDict *qdict,
diff --git a/qdict.h b/qdict.h
index dcd2b29..0c8de3c 100644
--- a/qdict.h
+++ b/qdict.h
@@ -34,6 +34,8 @@ typedef struct QDict {
 
 /* Object API */
 QDict *qdict_new(void);
+const char *qdict_entry_key(const QDictEntry *entry);
+QObject *qdict_entry_value(const QDictEntry *entry);
 size_t qdict_size(const QDict *qdict);
 void qdict_put_obj(QDict *qdict, const char *key, QObject *value);
 void qdict_del(QDict *qdict, const char *key);
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 12/23] QDict: Introduce new iteration API

[Luiz Capitulino]

It's composed of functions qdict_first() and qdict_next(), plus
functions to access QDictEntry values.

This API was suggested by Markus Armbruster <armbru@redhat.com> and
it offers full control over the iteration process.

The usage is simple, the following example prints all keys in 'qdict'
(it's hopefully better than any English description):

   QDict *qdict;
   const QDictEntry *ent;

   [...]

   for (ent = qdict_first(qdict); ent; ent = qdict_next(qdict, ent)) {
        printf("%s ", qdict_entry_key(ent));
    }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qdict.c |   37 +++++++++++++++++++++++++++++++++++++
 qdict.h |    2 ++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index c467763..a28a0a9 100644
--- a/qdict.c
+++ b/qdict.c
@@ -345,6 +345,43 @@ void qdict_iter(const QDict *qdict,
     }
 }
 
+static QDictEntry *qdict_next_entry(const QDict *qdict, int first_bucket)
+{
+    int i;
+
+    for (i = first_bucket; i < QDICT_BUCKET_MAX; i++) {
+        if (!QLIST_EMPTY(&qdict->table[i])) {
+            return QLIST_FIRST(&qdict->table[i]);
+        }
+    }
+
+    return NULL;
+}
+
+/**
+ * qdict_first(): Return first qdict entry for iteration.
+ */
+const QDictEntry *qdict_first(const QDict *qdict)
+{
+    return qdict_next_entry(qdict, 0);
+}
+
+/**
+ * qdict_next(): Return next qdict entry in an iteration.
+ */
+const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry)
+{
+    QDictEntry *ret;
+
+    ret = QLIST_NEXT(entry, next);
+    if (!ret) {
+        unsigned int bucket = tdb_hash(entry->key) % QDICT_BUCKET_MAX;
+        ret = qdict_next_entry(qdict, bucket + 1);
+    }
+
+    return ret;
+}
+
 /**
  * qentry_destroy(): Free all the memory allocated by a QDictEntry
  */
diff --git a/qdict.h b/qdict.h
index 0c8de3c..0e7a43f 100644
--- a/qdict.h
+++ b/qdict.h
@@ -45,6 +45,8 @@ QDict *qobject_to_qdict(const QObject *obj);
 void qdict_iter(const QDict *qdict,
                 void (*iter)(const char *key, QObject *obj, void *opaque),
                 void *opaque);
+const QDictEntry *qdict_first(const QDict *qdict);
+const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry);
 
 /* Helper to qdict_put_obj(), accepts any object */
 #define qdict_put(qdict, key, obj) \
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 13/23] check-qdict: Introduce test for the new iteration API

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 check-qdict.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/check-qdict.c b/check-qdict.c
index 1b070f4..6afce5a 100644
--- a/check-qdict.c
+++ b/check-qdict.c
@@ -194,6 +194,36 @@ START_TEST(qobject_to_qdict_test)
 }
 END_TEST
 
+START_TEST(qdict_iterapi_test)
+{
+    int count;
+    const QDictEntry *ent;
+
+    fail_unless(qdict_first(tests_dict) == NULL);
+
+    qdict_put(tests_dict, "key1", qint_from_int(1));
+    qdict_put(tests_dict, "key2", qint_from_int(2));
+    qdict_put(tests_dict, "key3", qint_from_int(3));
+
+    count = 0;
+    for (ent = qdict_first(tests_dict); ent; ent = qdict_next(tests_dict, ent)){
+        fail_unless(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1);
+        count++;
+    }
+
+    fail_unless(count == qdict_size(tests_dict));
+
+    /* Do it again to test restarting */
+    count = 0;
+    for (ent = qdict_first(tests_dict); ent; ent = qdict_next(tests_dict, ent)){
+        fail_unless(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1);
+        count++;
+    }
+
+    fail_unless(count == qdict_size(tests_dict));
+}
+END_TEST
+
 /*
  * Errors test-cases
  */
@@ -338,6 +368,7 @@ static Suite *qdict_suite(void)
     tcase_add_test(qdict_public2_tcase, qdict_haskey_test);
     tcase_add_test(qdict_public2_tcase, qdict_del_test);
     tcase_add_test(qdict_public2_tcase, qobject_to_qdict_test);
+    tcase_add_test(qdict_public2_tcase, qdict_iterapi_test);
 
     qdict_errors_tcase = tcase_create("Errors");
     suite_add_tcase(s, qdict_errors_tcase);
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 14/23] QDict: Introduce qdict_get_try_bool()

[Luiz Capitulino]

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

diff --git a/qdict.c b/qdict.c
index a28a0a9..dee0fb4 100644
--- a/qdict.c
+++ b/qdict.c
@@ -308,6 +308,24 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
 }
 
 /**
+ * qdict_get_try_bool(): Try to get a bool mapped by 'key'
+ *
+ * Return bool mapped by 'key', if it is not present in the
+ * dictionary or if the stored object is not of QBool type
+ * 'def_value' will be returned.
+ */
+int qdict_get_try_bool(const QDict *qdict, const char *key, int def_value)
+{
+    QObject *obj;
+
+    obj = qdict_get(qdict, key);
+    if (!obj || qobject_type(obj) != QTYPE_QBOOL)
+        return def_value;
+
+    return qbool_get_int(qobject_to_qbool(obj));
+}
+
+/**
  * qdict_get_try_str(): Try to get a pointer to the stored string
  * mapped by 'key'
  *
diff --git a/qdict.h b/qdict.h
index 0e7a43f..929d8d2 100644
--- a/qdict.h
+++ b/qdict.h
@@ -61,6 +61,7 @@ QDict *qdict_get_qdict(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 def_value);
+int qdict_get_try_bool(const QDict *qdict, const char *key, int def_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 #endif /* QDICT_H */
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 15/23] Monitor: handle optional '-' arg as a bool

[Luiz Capitulino]

Historically, user monitor arguments beginning with '-' (eg. '-f')
were passed as integers down to handlers.

I've maintained this behavior in the new monitor because we didn't
have a boolean type at the very beginning of QMP. Today we have it
and this behavior is causing trouble to QMP's argument checker.

This commit fixes the problem by doing the following changes:

1. User Monitor

   Before: the optional arg was represented as a QInt, we'd pass 1
           down to handlers if the user specified the argument or
           0 otherwise

   This commit: the optional arg is represented as a QBool, we pass
                true down to handlers if the user specified the
                argument, otherwise _nothing_ is passed

2. QMP

   Before: the client was required to pass the arg as QBool, but we'd
           convert it to QInt internally. If the argument wasn't passed,
           we'd pass 0 down

   This commit: still require a QBool, but doesn't do any conversion and
                doesn't pass any default value

3. Convert existing handlers (do_eject()/do_migrate()) to the new way

   Before: Both handlers would expect a QInt value, either 0 or 1

   This commit: Change the handlers to accept a QBool, they handle the
                following cases:

                   A) true is passed: the option is enabled
                   B) false is passed: the option is disabled
                   C) nothing is passed: option not specified, use
                                         default behavior

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 blockdev.c  |    2 +-
 migration.c |   16 +++++++---------
 monitor.c   |   17 +++--------------
 3 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3b8c606..4dcfad8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -521,7 +521,7 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
 int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     BlockDriverState *bs;
-    int force = qdict_get_int(qdict, "force");
+    int force = qdict_get_try_bool(qdict, "force", 0);
     const char *filename = qdict_get_str(qdict, "device");
 
     bs = bdrv_find(filename);
diff --git a/migration.c b/migration.c
index b49964c..650eb78 100644
--- a/migration.c
+++ b/migration.c
@@ -75,7 +75,9 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = NULL;
     const char *p;
-    int detach = qdict_get_int(qdict, "detach");
+    int detach = qdict_get_try_bool(qdict, "detach", 0);
+    int blk = qdict_get_try_bool(qdict, "blk", 0);
+    int inc = qdict_get_try_bool(qdict, "inc", 0);
     const char *uri = qdict_get_str(qdict, "uri");
 
     if (current_migration &&
@@ -86,21 +88,17 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 
     if (strstart(uri, "tcp:", &p)) {
         s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
-                                         (int)qdict_get_int(qdict, "blk"), 
-                                         (int)qdict_get_int(qdict, "inc"));
+                                         blk, inc);
 #if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         s = exec_start_outgoing_migration(mon, p, max_throttle, detach,
-                                          (int)qdict_get_int(qdict, "blk"), 
-                                          (int)qdict_get_int(qdict, "inc"));
+                                          blk, inc);
     } else if (strstart(uri, "unix:", &p)) {
         s = unix_start_outgoing_migration(mon, p, max_throttle, detach,
-					  (int)qdict_get_int(qdict, "blk"), 
-                                          (int)qdict_get_int(qdict, "inc"));
+                                          blk, inc);
     } else if (strstart(uri, "fd:", &p)) {
         s = fd_start_outgoing_migration(mon, p, max_throttle, detach, 
-                                        (int)qdict_get_int(qdict, "blk"), 
-                                        (int)qdict_get_int(qdict, "inc"));
+                                        blk, inc);
 #endif
     } else {
         monitor_printf(mon, "unknown migration protocol: %s\n", uri);
diff --git a/monitor.c b/monitor.c
index 58b060b..568bfb2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3565,7 +3565,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
         case '-':
             {
                 const char *tmp = p;
-                int has_option, skip_key = 0;
+                int skip_key = 0;
                 /* option */
 
                 c = *typestr++;
@@ -3573,7 +3573,6 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     goto bad_type;
                 while (qemu_isspace(*p))
                     p++;
-                has_option = 0;
                 if (*p == '-') {
                     p++;
                     if(c != *p) {
@@ -3589,11 +3588,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     if(skip_key) {
                         p = tmp;
                     } else {
+                        /* has option */
                         p++;
-                        has_option = 1;
+                        qdict_put(qdict, key, qbool_from_int(1));
                     }
                 }
-                qdict_put(qdict, key, qint_from_int(has_option));
             }
             break;
         default:
@@ -3985,11 +3984,6 @@ static int check_opt(const CmdArgs *cmd_args, const char *name, QDict *args)
         return -1;
     }
 
-    if (cmd_args->type == '-') {
-        /* handlers expect a value, they need to be changed */
-        qdict_put(args, name, qint_from_int(0));
-    }
-
     return 0;
 }
 
@@ -4062,11 +4056,6 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
                 qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
                 return -1;
             }
-            if (qobject_type(value) == QTYPE_QBOOL) {
-                /* handlers expect a QInt, they need to be changed */
-                qdict_put(args, name,
-                         qint_from_int(qbool_get_int(qobject_to_qbool(value))));
-            }
             break;
         case 'O':
         default:
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 16/23] QMP: New argument checker (first part)

[Luiz Capitulino]

Current QMP's argument checker is more complex than it should be
and has (at least) one serious bug: it ignores unknown arguments.

To solve both problems we introduce a new argument checker. It's
added on top of the existing one, so that there are no regressions
during the transition.

This commit introduces the first part of the new checker, which
is run by qmp_check_client_args() and does the following:

  1. Check if all mandatory arguments were provided
  2. Set flags for argument validation

In order to do that, we transform the args_type string (from
qemu-montor.hx) into a qdict and iterate over it.

Next commit adds the new checker's second part: type checking and
invalid argument detection.

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

diff --git a/monitor.c b/monitor.c
index 568bfb2..222622b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -177,6 +177,9 @@ static inline void mon_print_count_init(Monitor *mon) { }
 static inline int mon_print_count_get(const Monitor *mon) { return 0; }
 #endif /* CONFIG_DEBUG_MONITOR */
 
+/* QMP checker flags */
+#define QMP_ACCEPT_UNKNOWNS 1
+
 static QLIST_HEAD(mon_list, Monitor) mon_list;
 
 static const mon_cmd_t mon_cmds[];
@@ -4147,6 +4150,104 @@ static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name)
     return (qmp_cmd_mode(mon) ? is_cap : !is_cap);
 }
 
+/*
+ * - Check if the client has passed all mandatory args
+ * - Set special flags for argument validation
+ */
+static int check_mandatory_args(const QDict *cmd_args,
+                                const QDict *client_args, int *flags)
+{
+    const QDictEntry *ent;
+
+    for (ent = qdict_first(cmd_args); ent; ent = qdict_next(cmd_args, ent)) {
+        const char *cmd_arg_name = qdict_entry_key(ent);
+        QString *type = qobject_to_qstring(qdict_entry_value(ent));
+        assert(type != NULL);
+
+        if (qstring_get_str(type)[0] == 'O') {
+            assert((*flags & QMP_ACCEPT_UNKNOWNS) == 0);
+            *flags |= QMP_ACCEPT_UNKNOWNS;
+        } else if (qstring_get_str(type)[0] != '-' &&
+                   qstring_get_str(type)[1] != '?' &&
+                   !qdict_haskey(client_args, cmd_arg_name)) {
+            qerror_report(QERR_MISSING_PARAMETER, cmd_arg_name);
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+static QDict *qdict_from_args_type(const char *args_type)
+{
+    int i;
+    QDict *qdict;
+    QString *key, *type, *cur_qs;
+
+    assert(args_type != NULL);
+
+    qdict = qdict_new();
+
+    if (args_type == NULL || args_type[0] == '\0') {
+        /* no args, empty qdict */
+        goto out;
+    }
+
+    key = qstring_new();
+    type = qstring_new();
+
+    cur_qs = key;
+
+    for (i = 0;; i++) {
+        switch (args_type[i]) {
+            case ',':
+            case '\0':
+                qdict_put(qdict, qstring_get_str(key), type);
+                QDECREF(key);
+                if (args_type[i] == '\0') {
+                    goto out;
+                }
+                type = qstring_new(); /* qdict has ref */
+                cur_qs = key = qstring_new();
+                break;
+            case ':':
+                cur_qs = type;
+                break;
+            default:
+                qstring_append_chr(cur_qs, args_type[i]);
+                break;
+        }
+    }
+
+out:
+    return qdict;
+}
+
+/*
+ * Client argument checking rules:
+ *
+ * 1. Client must provide all mandatory arguments
+ */
+static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
+{
+    int flags, err;
+    QDict *cmd_args;
+
+    cmd_args = qdict_from_args_type(cmd->args_type);
+
+    flags = 0;
+    err = check_mandatory_args(cmd_args, client_args, &flags);
+    if (err) {
+        goto out;
+    }
+
+    /* TODO: Check client args type */
+
+out:
+    QDECREF(cmd_args);
+    return err;
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 {
     int err;
@@ -4222,6 +4323,11 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 
     QDECREF(input);
 
+    err = qmp_check_client_args(cmd, args);
+    if (err < 0) {
+        goto err_out;
+    }
+
     err = monitor_check_qmp_args(cmd, args);
     if (err < 0) {
         goto err_out;
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 17/23] QMP: New argument checker (second part)

[Luiz Capitulino]

This commit introduces the second (and last) part of QMP's new
argument checker.

The job is done by check_client_args_type(), it iterates over
the client's argument qdict and for for each argument it checks
if it exists and if its type is valid.

It's important to observe the following changes from the existing
argument checker:

  - If the handler accepts an O-type argument, unknown arguments
    are passed down to it. It's up to O-type handlers to validate
    their arguments

  - Boolean types (eg. 'b' and '-') don't accept integers anymore,
    only json-bool

  - Argument types '/' and '.' are currently unsupported under QMP,
    thus they're not handled

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

diff --git a/monitor.c b/monitor.c
index 222622b..2fd8005 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4151,6 +4151,95 @@ static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name)
 }
 
 /*
+ * Argument validation rules:
+ *
+ * 1. The argument must exist in cmd_args qdict
+ * 2. The argument type must be the expected one
+ *
+ * Special case: If the argument doesn't exist in cmd_args and
+ *               the QMP_ACCEPT_UNKNOWNS flag is set, then the
+ *               checking is skipped for it.
+ */
+static int check_client_args_type(const QDict *client_args,
+                                  const QDict *cmd_args, int flags)
+{
+    const QDictEntry *ent;
+
+    for (ent = qdict_first(client_args); ent;ent = qdict_next(client_args,ent)){
+        QObject *obj;
+        QString *arg_type;
+        const QObject *client_arg = qdict_entry_value(ent);
+        const char *client_arg_name = qdict_entry_key(ent);
+
+        obj = qdict_get(cmd_args, client_arg_name);
+        if (!obj) {
+            if (flags & QMP_ACCEPT_UNKNOWNS) {
+                /* handler accepts unknowns */
+                continue;
+            }
+            /* client arg doesn't exist */
+            qerror_report(QERR_INVALID_PARAMETER, client_arg_name);
+            return -1;
+        }
+
+        arg_type = qobject_to_qstring(obj);
+        assert(arg_type != NULL);
+
+        /* check if argument's type is correct */
+        switch (qstring_get_str(arg_type)[0]) {
+        case 'F':
+        case 'B':
+        case 's':
+            if (qobject_type(client_arg) != QTYPE_QSTRING) {
+                qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+                              "string");
+                return -1;
+            }
+        break;
+        case 'i':
+        case 'l':
+        case 'M':
+            if (qobject_type(client_arg) != QTYPE_QINT) {
+                qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+                              "int");
+                return -1; 
+            }
+            break;
+        case 'f':
+        case 'T':
+            if (qobject_type(client_arg) != QTYPE_QINT &&
+                qobject_type(client_arg) != QTYPE_QFLOAT) {
+                qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+                              "number");
+               return -1; 
+            }
+            break;
+        case 'b':
+        case '-':
+            if (qobject_type(client_arg) != QTYPE_QBOOL) {
+                qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+                              "bool");
+               return -1; 
+            }
+            break;
+        case 'O':
+            assert(flags & QMP_ACCEPT_UNKNOWNS);
+            break;
+        case '/':
+        case '.':
+            /*
+             * These types are not supported by QMP and thus are not
+             * handled here. Fall through.
+             */
+        default:
+            abort();
+        }
+    }
+
+    return 0;
+}
+
+/*
  * - Check if the client has passed all mandatory args
  * - Set special flags for argument validation
  */
@@ -4227,6 +4316,9 @@ out:
  * Client argument checking rules:
  *
  * 1. Client must provide all mandatory arguments
+ * 2. Each argument provided by the client must be expected
+ * 3. Each argument provided by the client must have the type expected
+ *    by the command
  */
 static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
 {
@@ -4241,7 +4333,7 @@ static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
         goto out;
     }
 
-    /* TODO: Check client args type */
+    err = check_client_args_type(client_args, cmd_args, flags);
 
 out:
     QDECREF(cmd_args);
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 18/23] QMP: Drop old client argument checker

[Luiz Capitulino]

Previous two commits added qmp_check_client_args(), which
fully replaces this code and is way better.

It's important to note that the new checker doesn't support
the '/' arg type. As we don't have any of those handlers
converted to QMP, this is just dead code.

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

diff --git a/monitor.c b/monitor.c
index 2fd8005..172dc21 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3973,177 +3973,6 @@ static int monitor_can_read(void *opaque)
     return (mon->suspend_cnt == 0) ? 1 : 0;
 }
 
-typedef struct CmdArgs {
-    QString *name;
-    int type;
-    int flag;
-    int optional;
-} CmdArgs;
-
-static int check_opt(const CmdArgs *cmd_args, const char *name, QDict *args)
-{
-    if (!cmd_args->optional) {
-        qerror_report(QERR_MISSING_PARAMETER, name);
-        return -1;
-    }
-
-    return 0;
-}
-
-static int check_arg(const CmdArgs *cmd_args, QDict *args)
-{
-    QObject *value;
-    const char *name;
-
-    name = qstring_get_str(cmd_args->name);
-
-    if (!args) {
-        return check_opt(cmd_args, name, args);
-    }
-
-    value = qdict_get(args, name);
-    if (!value) {
-        return check_opt(cmd_args, name, args);
-    }
-
-    switch (cmd_args->type) {
-        case 'F':
-        case 'B':
-        case 's':
-            if (qobject_type(value) != QTYPE_QSTRING) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "string");
-                return -1;
-            }
-            break;
-        case '/': {
-            int i;
-            const char *keys[] = { "count", "format", "size", NULL };
-
-            for (i = 0; keys[i]; i++) {
-                QObject *obj = qdict_get(args, keys[i]);
-                if (!obj) {
-                    qerror_report(QERR_MISSING_PARAMETER, name);
-                    return -1;
-                }
-                if (qobject_type(obj) != QTYPE_QINT) {
-                    qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "int");
-                    return -1;
-                }
-            }
-            break;
-        }
-        case 'i':
-        case 'l':
-        case 'M':
-            if (qobject_type(value) != QTYPE_QINT) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "int");
-                return -1;
-            }
-            break;
-        case 'f':
-        case 'T':
-            if (qobject_type(value) != QTYPE_QINT && qobject_type(value) != QTYPE_QFLOAT) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "number");
-                return -1;
-            }
-            break;
-        case 'b':
-            if (qobject_type(value) != QTYPE_QBOOL) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
-                return -1;
-            }
-            break;
-        case '-':
-            if (qobject_type(value) != QTYPE_QINT &&
-                qobject_type(value) != QTYPE_QBOOL) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
-                return -1;
-            }
-            break;
-        case 'O':
-        default:
-            /* impossible */
-            abort();
-    }
-
-    return 0;
-}
-
-static void cmd_args_init(CmdArgs *cmd_args)
-{
-    cmd_args->name = qstring_new();
-    cmd_args->type = cmd_args->flag = cmd_args->optional = 0;
-}
-
-static int check_opts(QemuOptsList *opts_list, QDict *args)
-{
-    assert(!opts_list->desc->name);
-    return 0;
-}
-
-/*
- * This is not trivial, we have to parse Monitor command's argument
- * type syntax to be able to check the arguments provided by clients.
- *
- * In the near future we will be using an array for that and will be
- * able to drop all this parsing...
- */
-static int monitor_check_qmp_args(const mon_cmd_t *cmd, QDict *args)
-{
-    int err;
-    const char *p;
-    CmdArgs cmd_args;
-    QemuOptsList *opts_list;
-
-    if (cmd->args_type == NULL) {
-        return (qdict_size(args) == 0 ? 0 : -1);
-    }
-
-    err = 0;
-    cmd_args_init(&cmd_args);
-    opts_list = NULL;
-
-    for (p = cmd->args_type;; p++) {
-        if (*p == ':') {
-            cmd_args.type = *++p;
-            p++;
-            if (cmd_args.type == '-') {
-                cmd_args.flag = *p++;
-                cmd_args.optional = 1;
-            } else if (cmd_args.type == 'O') {
-                opts_list = qemu_find_opts(qstring_get_str(cmd_args.name));
-                assert(opts_list);
-            } else if (*p == '?') {
-                cmd_args.optional = 1;
-                p++;
-            }
-
-            assert(*p == ',' || *p == '\0');
-            if (opts_list) {
-                err = check_opts(opts_list, args);
-                opts_list = NULL;
-            } else {
-                err = check_arg(&cmd_args, args);
-                QDECREF(cmd_args.name);
-                cmd_args_init(&cmd_args);
-            }
-
-            if (err < 0) {
-                break;
-            }
-        } else {
-            qstring_append_chr(cmd_args.name, *p);
-        }
-
-        if (*p == '\0') {
-            break;
-        }
-    }
-
-    QDECREF(cmd_args.name);
-    return err;
-}
-
 static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name)
 {
     int is_cap = compare_cmd(cmd_name, "qmp_capabilities");
@@ -4420,11 +4249,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         goto err_out;
     }
 
-    err = monitor_check_qmp_args(cmd, args);
-    if (err < 0) {
-        goto err_out;
-    }
-
     if (monitor_handler_is_async(cmd)) {
         err = qmp_async_cmd_handler(mon, cmd, args);
         if (err) {
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 19/23] QError: Introduce QERR_QMP_EXTRA_MEMBER

[Luiz Capitulino]

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

diff --git a/qerror.c b/qerror.c
index cce1e7b..2f6f590 100644
--- a/qerror.c
+++ b/qerror.c
@@ -177,6 +177,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "QMP input object member '%(member)' expects '%(expected)'",
     },
     {
+        .error_fmt = QERR_QMP_EXTRA_MEMBER,
+        .desc      = "QMP input object member '%(member)' is unexpected",
+    },
+    {
         .error_fmt = QERR_SET_PASSWD_FAILED,
         .desc      = "Could not set password",
     },
diff --git a/qerror.h b/qerror.h
index 77ae574..9ad00b4 100644
--- a/qerror.h
+++ b/qerror.h
@@ -148,6 +148,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
     "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
 
+#define QERR_QMP_EXTRA_MEMBER \
+    "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
+
 #define QERR_SET_PASSWD_FAILED \
     "{ 'class': 'SetPasswdFailed', 'data': {} }"
 
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 20/23] QMP: Introduce qmp_check_input_obj()

[Luiz Capitulino]

This is similar to qmp_check_client_args(), but it checks if
the input object follows the specification (QMP/qmp-spec.txt
section 2.3).

As we're limited to three keys, the work here is quite simple:
we iterate over the input object, checking each time if the
current argument complies to the specification.

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

diff --git a/monitor.c b/monitor.c
index 172dc21..22e0650 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4169,6 +4169,62 @@ out:
     return err;
 }
 
+/*
+ * Input object checking rules
+ *
+ * 1. Input object must be a dict
+ * 2. The "execute" key must exist
+ * 3. The "execute" key must be a string
+ * 4. If the "arguments" key exists, it must be a dict
+ * 5. If the "id" key exists, it can be anything (ie. json-value)
+ * 6. Any argument not listed above is considered invalid
+ */
+static QDict *qmp_check_input_obj(QObject *input_obj)
+{
+    const QDictEntry *ent;
+    int has_exec_key = 0;
+    QDict *input_dict;
+
+    if (qobject_type(input_obj) != QTYPE_QDICT) {
+        qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "object");
+        return NULL;
+    }
+
+    input_dict = qobject_to_qdict(input_obj);
+
+    for (ent = qdict_first(input_dict); ent; ent = qdict_next(input_dict, ent)){
+        const char *arg_name = qdict_entry_key(ent);
+        const QObject *arg_obj = qdict_entry_value(ent);
+
+        if (!strcmp(arg_name, "execute")) {
+            if (qobject_type(arg_obj) != QTYPE_QSTRING) {
+                qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "execute",
+                              "string");
+                return NULL;
+            }
+            has_exec_key = 1;
+        } else if (!strcmp(arg_name, "arguments")) {
+            if (qobject_type(arg_obj) != QTYPE_QDICT) {
+                qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "arguments",
+                              "object");
+                return NULL;
+            }
+        } else if (!strcmp(arg_name, "id")) {
+            /* FIXME: check duplicated IDs for async commands */
+        } else {
+            qerror_report(QERR_QMP_EXTRA_MEMBER, arg_name);
+            return NULL;
+        }
+    }
+
+    if (!has_exec_key) {
+        qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "execute");
+        return NULL;
+    }
+
+    return input_dict;
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 {
     int err;
@@ -4191,7 +4247,11 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         goto err_out;
     }
 
-    input = qobject_to_qdict(obj);
+    input = qmp_check_input_obj(obj);
+    if (!input) {
+        qobject_decref(obj);
+        goto err_out;
+    }
 
     mon->mc->id = qdict_get(input, "id");
     qobject_incref(mon->mc->id);
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 21/23] QMP: Drop old input object checking

[Luiz Capitulino]

Previous commit added qmp_check_input_obj(), it does all the
checking we need.

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

diff --git a/monitor.c b/monitor.c
index 22e0650..1c8992b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4241,10 +4241,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         // FIXME: should be triggered in json_parser_parse()
         qerror_report(QERR_JSON_PARSING);
         goto err_out;
-    } else if (qobject_type(obj) != QTYPE_QDICT) {
-        qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "object");
-        qobject_decref(obj);
-        goto err_out;
     }
 
     input = qmp_check_input_obj(obj);
@@ -4256,17 +4252,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     mon->mc->id = qdict_get(input, "id");
     qobject_incref(mon->mc->id);
 
-    obj = qdict_get(input, "execute");
-    if (!obj) {
-        qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "execute");
-        goto err_input;
-    } else if (qobject_type(obj) != QTYPE_QSTRING) {
-        qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "execute", "string");
-        goto err_input;
-    }
-
-    cmd_name = qstring_get_str(qobject_to_qstring(obj));
-
+    cmd_name = qdict_get_str(input, "execute");
     if (invalid_qmp_mode(mon, cmd_name)) {
         qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
         goto err_input;
@@ -4294,9 +4280,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     obj = qdict_get(input, "arguments");
     if (!obj) {
         args = qdict_new();
-    } else if (qobject_type(obj) != QTYPE_QDICT) {
-        qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "arguments", "object");
-        goto err_input;
     } else {
         args = qobject_to_qdict(obj);
         QINCREF(args);
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 22/23] QMP: handle_qmp_command(): Small cleanup

[Luiz Capitulino]

Drop a unneeded label and QDECREF() call.

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

diff --git a/monitor.c b/monitor.c
index 1c8992b..00a627a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4234,7 +4234,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     Monitor *mon = cur_mon;
     const char *cmd_name, *info_item;
 
-    args = NULL;
+    args = input = NULL;
 
     obj = json_parser_parse(tokens, NULL);
     if (!obj) {
@@ -4255,7 +4255,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     cmd_name = qdict_get_str(input, "execute");
     if (invalid_qmp_mode(mon, cmd_name)) {
         qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-        goto err_input;
+        goto err_out;
     }
 
     /*
@@ -4264,7 +4264,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
      */
     if (compare_cmd(cmd_name, "info")) {
         qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-        goto err_input;
+        goto err_out;
     } else if (strstart(cmd_name, "query-", &info_item)) {
         cmd = monitor_find_command("info");
         qdict_put_obj(input, "arguments",
@@ -4273,7 +4273,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         cmd = monitor_find_command(cmd_name);
         if (!cmd || !monitor_handler_ported(cmd)) {
             qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-            goto err_input;
+            goto err_out;
         }
     }
 
@@ -4285,8 +4285,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         QINCREF(args);
     }
 
-    QDECREF(input);
-
     err = qmp_check_client_args(cmd, args);
     if (err < 0) {
         goto err_out;
@@ -4301,13 +4299,13 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     } else {
         monitor_call_handler(mon, cmd, args);
     }
+
     goto out;
 
-err_input:
-    QDECREF(input);
 err_out:
     monitor_protocol_emitter(mon, NULL);
 out:
+    QDECREF(input);
     QDECREF(args);
 }
 
-- 
1.7.2.rc0

[Qemu-devel] [PATCH 23/23] monitor: Allow to exclude commands from QMP

[Luiz Capitulino]

From: Jan Kiszka <jan.kiszka@siemens.com>

Ported commands that are marked 'user_only' will not be considered for
QMP monitor sessions. This allows to implement new commands that do not
(yet) provide a sufficiently stable interface for QMP use.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   18 +++++++++++++++---
 monitor.h |    1 +
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/monitor.c b/monitor.c
index 00a627a..f319fee 100644
--- a/monitor.c
+++ b/monitor.c
@@ -333,6 +333,11 @@ static inline bool monitor_handler_is_async(const mon_cmd_t *cmd)
     return cmd->flags & MONITOR_CMD_ASYNC;
 }
 
+static inline bool monitor_cmd_user_only(const mon_cmd_t *cmd)
+{
+    return (cmd->flags & MONITOR_CMD_USER_ONLY);
+}
+
 static inline int monitor_has_error(const Monitor *mon)
 {
     return mon->error != NULL;
@@ -615,6 +620,11 @@ static int do_info(Monitor *mon, const QDict *qdict, QObject **ret_data)
         goto help;
     }
 
+    if (monitor_ctrl_mode(mon) && monitor_cmd_user_only(cmd)) {
+        qerror_report(QERR_COMMAND_NOT_FOUND, item);
+        return -1;
+    }
+
     if (monitor_handler_is_async(cmd)) {
         if (monitor_ctrl_mode(mon)) {
             qmp_async_info_handler(mon, cmd);
@@ -712,13 +722,14 @@ static void do_info_commands(Monitor *mon, QObject **ret_data)
     cmd_list = qlist_new();
 
     for (cmd = mon_cmds; cmd->name != NULL; cmd++) {
-        if (monitor_handler_ported(cmd) && !compare_cmd(cmd->name, "info")) {
+        if (monitor_handler_ported(cmd) && !monitor_cmd_user_only(cmd) &&
+            !compare_cmd(cmd->name, "info")) {
             qlist_append_obj(cmd_list, get_cmd_dict(cmd->name));
         }
     }
 
     for (cmd = info_cmds; cmd->name != NULL; cmd++) {
-        if (monitor_handler_ported(cmd)) {
+        if (monitor_handler_ported(cmd) && !monitor_cmd_user_only(cmd)) {
             char buf[128];
             snprintf(buf, sizeof(buf), "query-%s", cmd->name);
             qlist_append_obj(cmd_list, get_cmd_dict(buf));
@@ -4271,7 +4282,8 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
                       qobject_from_jsonf("{ 'item': %s }", info_item));
     } else {
         cmd = monitor_find_command(cmd_name);
-        if (!cmd || !monitor_handler_ported(cmd)) {
+        if (!cmd || !monitor_handler_ported(cmd)
+            || monitor_cmd_user_only(cmd)) {
             qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
             goto err_out;
         }
diff --git a/monitor.h b/monitor.h
index 9582b9c..38b22a4 100644
--- a/monitor.h
+++ b/monitor.h
@@ -17,6 +17,7 @@ extern Monitor *default_mon;
 
 /* flags for monitor commands */
 #define MONITOR_CMD_ASYNC       0x0001
+#define MONITOR_CMD_USER_ONLY   0x0002
 
 /* QMP events */
 typedef enum MonitorEvent {
-- 
1.7.2.rc0

Re: [Qemu-devel] [PATCH 00/23][PULL]: QMP/Monitor queue

[Anthony Liguori]

On 07/01/2010 02:21 PM, Luiz Capitulino wrote:
> Hi Anthony,
>
> The following QMP/Monitor patches have been sent to the list and look good
> to me. I also did some basic testing on them.
>
> Most of these changes are bug fixes, the only exception is my new argument
> checker series, which is a complete rewrite (that also fix bugs...).
>
> The changes (since 449041d4db1f82f281fe097e832f07cd9ee1e864) are available
> in the following repository:
>
>      git://repo.or.cz/qemu/qmp-unstable.git for-anthony
>    

Pulled.  Thanks.

Regards,

Anthony Liguori

> Jan Kiszka (6):
>        monitor: Fix leakage during completion processing
>        monitor: Fix command completion vs. boolean switches
>        monitor: Establish cmd flags and convert the async tag
>        QMP: Teach basic capability negotiation to python example
>        QMP: Fix python helper /wrt long return strings
>        monitor: Allow to exclude commands from QMP
>
> Luiz Capitulino (16):
>        QMP: Fix error reporting in the async API
>        QError: Enhance QERR_DEVICE_NOT_ACTIVE's user desc
>        QDict: Rename 'err_value'
>        QDict: Small terminology change
>        QDict: Introduce functions to retrieve QDictEntry values
>        QDict: Introduce new iteration API
>        check-qdict: Introduce test for the new iteration API
>        QDict: Introduce qdict_get_try_bool()
>        Monitor: handle optional '-' arg as a bool
>        QMP: New argument checker (first part)
>        QMP: New argument checker (second part)
>        QMP: Drop old client argument checker
>        QError: Introduce QERR_QMP_EXTRA_MEMBER
>        QMP: Introduce qmp_check_input_obj()
>        QMP: Drop old input object checking
>        QMP: handle_qmp_command(): Small cleanup
>
> Yoshiaki Tamura (1):
>        net: delete QemuOpts when net_client_init() fails.
>
>   QMP/qmp-shell   |    1 +
>   QMP/qmp.py      |    6 +-
>   QMP/vm-info     |    1 +
>   blockdev.c      |    2 +-
>   check-qdict.c   |   33 ++++-
>   migration.c     |   16 +-
>   monitor.c       |  435 ++++++++++++++++++++++++++++++++-----------------------
>   monitor.h       |    4 +
>   net.c           |    4 +
>   qdict.c         |  106 ++++++++++++--
>   qdict.h         |   11 +-
>   qemu-monitor.hx |    2 +-
>   qerror.c        |    6 +-
>   qerror.h        |    3 +
>   14 files changed, 418 insertions(+), 212 deletions(-)
>
>
>