From 03a63484a6978f68caff087bbaabcd1d383563af Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Wed, 16 Jun 2010 00:38:33 +0200 Subject: [PATCH 01/23] monitor: Fix leakage during completion processing Given too many arguments or an invalid command, we were leaking the duplicated argument strings. Signed-off-by: Jan Kiszka Signed-off-by: Luiz Capitulino --- monitor.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/monitor.c b/monitor.c index 170b26971f..42ae1546db 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) From 3b6dbf277232920a6463fc01d630e4d9dc378ef6 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Wed, 16 Jun 2010 00:38:34 +0200 Subject: [PATCH 02/23] monitor: Fix command completion vs. boolean switches 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 Signed-off-by: Luiz Capitulino --- monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index 42ae1546db..b375f102b7 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': From 8ac470c1f945601de9f1f577791c48e95d5340db Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Wed, 16 Jun 2010 00:38:39 +0200 Subject: [PATCH 03/23] monitor: Establish cmd flags and convert the async tag As we want to add more flags to monitor commands, convert the only so far existing one accordingly. Signed-off-by: Jan Kiszka Signed-off-by: Luiz Capitulino --- 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 b375f102b7..980e98de06 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 ea15469f25..9582b9cf1f 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 9f62b94862..2af3de6c22 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 From 8d7e84571bd3f44c106ed12cb316b1ca30915fc7 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Wed, 16 Jun 2010 00:38:45 +0200 Subject: [PATCH 04/23] QMP: Teach basic capability negotiation to python example As sending "qmp_capabilities" on session start became mandatory, both python examples were broken. Signed-off-by: Jan Kiszka Signed-off-by: Luiz Capitulino --- QMP/qmp-shell | 1 + QMP/vm-info | 1 + 2 files changed, 2 insertions(+) diff --git a/QMP/qmp-shell b/QMP/qmp-shell index f89b9af87e..a5b72d15d1 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 8ebaeb38c0..be5b03843c 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)) From bbafc7a8798bc9ed1380e75033544e0614d344c7 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Wed, 16 Jun 2010 00:38:46 +0200 Subject: [PATCH 05/23] QMP: Fix python helper /wrt long return strings Remove the arbitrary limitation of 1024 characters per return string and read complete lines instead. Required for device_show. Signed-off-by: Jan Kiszka Signed-off-by: Luiz Capitulino --- QMP/qmp.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/QMP/qmp.py b/QMP/qmp.py index d9da603bec..4062f84f36 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() From 410cbafebc7168a278a23c856b4f5ff276ef1c85 Mon Sep 17 00:00:00 2001 From: Yoshiaki Tamura Date: Mon, 21 Jun 2010 10:41:36 +0900 Subject: [PATCH 06/23] net: delete QemuOpts when net_client_init() fails. 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 Signed-off-by: Yoshiaki Tamura Signed-off-by: Luiz Capitulino --- net.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net.c b/net.c index 90bd5a9047..8ddf872a6f 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; } From 5af7bbae0ca45962d0bcd19753a947aabee6f7f1 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Tue, 22 Jun 2010 19:10:46 -0300 Subject: [PATCH 07/23] QMP: Fix error reporting in the async API 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 --- monitor.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/monitor.c b/monitor.c index 980e98de06..58b060bdaa 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); } From 8754c81e11ab046c445425753f52cbb505f60a50 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Wed, 23 Jun 2010 12:37:47 -0300 Subject: [PATCH 08/23] QError: Enhance QERR_DEVICE_NOT_ACTIVE's user desc 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 --- qerror.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qerror.c b/qerror.c index 44d0bf82b4..cce1e7befa 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, From 83aba69ec05e17ff34708a5b7a3b719dac5c8fc0 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Fri, 4 Jun 2010 19:20:54 -0300 Subject: [PATCH 09/23] QDict: Rename 'err_value' A missing key is not an error. Signed-off-by: Luiz Capitulino --- qdict.c | 6 +++--- qdict.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/qdict.c b/qdict.c index 175bc178f0..c974d6f37e 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 5e5902caea..72ea563d19 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 */ From c8bc3cd72b4c530721d5be1bf9f599edb5d72160 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Mon, 7 Jun 2010 15:45:22 -0300 Subject: [PATCH 10/23] QDict: Small terminology change 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 --- 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 2c3089fa66..1b070f4864 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 c974d6f37e..71be2ebcd8 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 72ea563d19..dcd2b29780 100644 --- a/qdict.h +++ b/qdict.h @@ -18,7 +18,7 @@ #include "qemu-queue.h" #include -#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 */ From 0d078b2adec72fd74b00defb260724b33e87e184 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Mon, 7 Jun 2010 16:53:51 -0300 Subject: [PATCH 11/23] QDict: Introduce functions to retrieve QDictEntry values 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 --- qdict.c | 21 +++++++++++++++++++++ qdict.h | 2 ++ 2 files changed, 23 insertions(+) diff --git a/qdict.c b/qdict.c index 71be2ebcd8..c4677636e3 100644 --- a/qdict.c +++ b/qdict.c @@ -82,6 +82,27 @@ static QDictEntry *alloc_entry(const char *key, QObject *value) return entry; } +/** + * 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 */ diff --git a/qdict.h b/qdict.h index dcd2b29780..0c8de3c9c2 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); From f2b07f35d2ee1c6a501d1e3c68c0db42c70751fd Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Mon, 7 Jun 2010 16:07:29 -0300 Subject: [PATCH 12/23] QDict: Introduce new iteration API It's composed of functions qdict_first() and qdict_next(), plus functions to access QDictEntry values. This API was suggested by Markus Armbruster 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 --- qdict.c | 37 +++++++++++++++++++++++++++++++++++++ qdict.h | 2 ++ 2 files changed, 39 insertions(+) diff --git a/qdict.c b/qdict.c index c4677636e3..a28a0a9f97 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 0c8de3c9c2..0e7a43fcb9 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) \ From d02c6bd428ddebd11c6e8f1bf13013b97c397628 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Mon, 7 Jun 2010 15:29:58 -0300 Subject: [PATCH 13/23] check-qdict: Introduce test for the new iteration API Signed-off-by: Luiz Capitulino --- check-qdict.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/check-qdict.c b/check-qdict.c index 1b070f4864..6afce5a5ca 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); From 35006ac856d6f0593c2185dd84ad004852053cf9 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Mon, 7 Jun 2010 17:25:04 -0300 Subject: [PATCH 14/23] QDict: Introduce qdict_get_try_bool() Signed-off-by: Luiz Capitulino --- qdict.c | 18 ++++++++++++++++++ qdict.h | 1 + 2 files changed, 19 insertions(+) diff --git a/qdict.c b/qdict.c index a28a0a9f97..dee0fb447c 100644 --- a/qdict.c +++ b/qdict.c @@ -307,6 +307,24 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key, return qint_get_int(qobject_to_qint(obj)); } +/** + * 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 0e7a43fcb9..929d8d22f5 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 */ From eb159d13ee36a9ef2a83e3ab66f1b2ae1cc2d9f1 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Fri, 28 May 2010 15:25:24 -0300 Subject: [PATCH 15/23] Monitor: handle optional '-' arg as a bool 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 --- 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 3b8c6067c7..4dcfad89c5 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 b49964c5e3..650eb78d26 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 58b060bdaa..568bfb2f95 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: From 2dbc8db0ba9e34b168c3b79a6c5435f770d3796e Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Wed, 26 May 2010 16:13:09 -0300 Subject: [PATCH 16/23] QMP: New argument checker (first part) 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 --- monitor.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/monitor.c b/monitor.c index 568bfb2f95..222622b772 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; From 4af9193ae954f87225e1ba5d527f6a13e37b1e0e Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Tue, 22 Jun 2010 11:44:05 -0300 Subject: [PATCH 17/23] QMP: New argument checker (second part) 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 --- monitor.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index 222622b772..2fd8005e35 100644 --- a/monitor.c +++ b/monitor.c @@ -4150,6 +4150,95 @@ static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name) return (qmp_cmd_mode(mon) ? is_cap : !is_cap); } +/* + * 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); From f6b4fc8b23b1154577c72937b70e565716bb0a60 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Fri, 28 May 2010 17:24:49 -0300 Subject: [PATCH 18/23] QMP: Drop old client argument checker 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 --- monitor.c | 176 ------------------------------------------------------ 1 file changed, 176 deletions(-) diff --git a/monitor.c b/monitor.c index 2fd8005e35..172dc2133d 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) { From 60d76d7b073f6f25c9dbe4d68df7d012f73db27f Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Tue, 1 Jun 2010 16:15:23 -0300 Subject: [PATCH 19/23] QError: Introduce QERR_QMP_EXTRA_MEMBER Signed-off-by: Luiz Capitulino --- qerror.c | 4 ++++ qerror.h | 3 +++ 2 files changed, 7 insertions(+) diff --git a/qerror.c b/qerror.c index cce1e7befa..2f6f59061f 100644 --- a/qerror.c +++ b/qerror.c @@ -176,6 +176,10 @@ static const QErrorStringTable qerror_table[] = { .error_fmt = QERR_QMP_BAD_INPUT_OBJECT_MEMBER, .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 77ae57464e..9ad00b4b82 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': {} }" From c917c8f3d07816f018a96121422de3b53f525fe4 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Mon, 31 May 2010 17:28:01 -0300 Subject: [PATCH 20/23] QMP: Introduce qmp_check_input_obj() 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 --- monitor.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/monitor.c b/monitor.c index 172dc2133d..22e0650eb2 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); From 0bbab46db67bd5a059aadabc498be3c371551506 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Mon, 31 May 2010 17:32:50 -0300 Subject: [PATCH 21/23] QMP: Drop old input object checking Previous commit added qmp_check_input_obj(), it does all the checking we need. Signed-off-by: Luiz Capitulino --- monitor.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/monitor.c b/monitor.c index 22e0650eb2..1c8992bd24 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); From e4940c603a209c82c2ea80cfeb244c5dec8e3118 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Thu, 24 Jun 2010 17:58:20 -0300 Subject: [PATCH 22/23] QMP: handle_qmp_command(): Small cleanup Drop a unneeded label and QDECREF() call. Signed-off-by: Luiz Capitulino --- monitor.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/monitor.c b/monitor.c index 1c8992bd24..00a627acfa 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); } From a6c4d36425871fafc55ce3937bebd05e86f5ea81 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Mon, 28 Jun 2010 18:27:47 +0200 Subject: [PATCH 23/23] monitor: Allow to exclude commands from QMP 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 Signed-off-by: Luiz Capitulino --- monitor.c | 18 +++++++++++++++--- monitor.h | 1 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/monitor.c b/monitor.c index 00a627acfa..f319fee791 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 9582b9cf1f..38b22a4b28 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 {