[Qemu-devel] [PATCH v14 03/19] option: allow qemu_opts_to_qdict to merge repeated options

["Daniel P. Berrange" <berrange@redhat.com>]

If given an option string such as

  size=1024,nodes=10,nodes=4-5,nodes=1-2,policy=bind

the qemu_opts_to_qdict() method will currently overwrite
the values for repeated option keys, so only the last
value is in the returned dict:

    size=1024
    nodes=1-2
    policy=bind

This adds the ability for the caller to ask that the
repeated keys be turned into list indexes:

    size=1024
    nodes.0=10
    nodes.1=4-5
    nodes.2=1-2
    policy=bind

Note that the conversion has no way of knowing whether
any given key is expected to be a list upfront - it can
only figure that out when seeing the first duplicated
key. Thus the caller has to be prepared to deal with the
fact that if a key 'foo' is a list, then the returned
qdict may contain either 'foo' (if only a single instance
of the key was seen) or 'foo.NN' (if multiple instances
of the key were seen).

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 blockdev.c               |  7 ++++---
 include/qemu/option.h    |  8 +++++++-
 monitor.c                |  3 ++-
 qemu-img.c               |  4 +++-
 qemu-io-cmds.c           |  3 ++-
 qemu-io.c                |  6 ++++--
 qemu-nbd.c               |  3 ++-
 qom/object_interfaces.c  |  3 ++-
 tests/test-qemu-opts.c   | 40 ++++++++++++++++++++++++++++++++++++++++
 tests/test-replication.c |  9 ++++++---
 util/qemu-option.c       | 41 ++++++++++++++++++++++++++++++++++++++---
 11 files changed, 110 insertions(+), 17 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3010393..5ef3193 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -911,7 +911,8 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
 
     /* Get a QDict for processing the options */
     bs_opts = qdict_new();
-    qemu_opts_to_qdict(all_opts, bs_opts);
+    qemu_opts_to_qdict(all_opts, bs_opts,
+                       QEMU_OPTS_REPEAT_POLICY_LAST);
 
     legacy_opts = qemu_opts_create(&qemu_legacy_drive_opts, NULL, 0,
                                    &error_abort);
@@ -3758,8 +3759,8 @@ void hmp_drive_add_node(Monitor *mon, const char *optstr)
         return;
     }
 
-    qdict = qemu_opts_to_qdict(opts, NULL);
-
+    qdict = qemu_opts_to_qdict(opts, NULL,
+                               QEMU_OPTS_REPEAT_POLICY_LAST);
     if (!qdict_get_try_str(qdict, "node-name")) {
         QDECREF(qdict);
         error_report("'node-name' needs to be specified");
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 2a5266f..328c468 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -125,7 +125,13 @@ void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
                             int permit_abbrev);
 QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
                                Error **errp);
-QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict);
+typedef enum {
+    QEMU_OPTS_REPEAT_POLICY_LAST,
+    QEMU_OPTS_REPEAT_POLICY_LIST,
+} QemuOptsRepeatPolicy;
+
+QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict,
+                          QemuOptsRepeatPolicy repeatPolicy);
 void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp);
 
 typedef int (*qemu_opts_loopfunc)(void *opaque, QemuOpts *opts, Error **errp);
diff --git a/monitor.c b/monitor.c
index 83c4edf..7dcd66b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2642,7 +2642,8 @@ static QDict *monitor_parse_arguments(Monitor *mon,
                 if (!opts) {
                     goto fail;
                 }
-                qemu_opts_to_qdict(opts, qdict);
+                qemu_opts_to_qdict(opts, qdict,
+                                   QEMU_OPTS_REPEAT_POLICY_LAST);
                 qemu_opts_del(opts);
             }
             break;
diff --git a/qemu-img.c b/qemu-img.c
index ceffefe..b399ae5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -273,7 +273,9 @@ static BlockBackend *img_open_opts(const char *optstr,
     QDict *options;
     Error *local_err = NULL;
     BlockBackend *blk;
-    options = qemu_opts_to_qdict(opts, NULL);
+    options = qemu_opts_to_qdict(opts, NULL,
+                                 QEMU_OPTS_REPEAT_POLICY_LAST);
+
     blk = blk_new_open(NULL, NULL, options, flags, &local_err);
     if (!blk) {
         error_reportf_err(local_err, "Could not open '%s': ", optstr);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 3a3838a..e14beed 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1952,7 +1952,8 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
     }
 
     qopts = qemu_opts_find(&reopen_opts, NULL);
-    opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
+    opts = qopts ? qemu_opts_to_qdict(qopts, NULL,
+                                      QEMU_OPTS_REPEAT_POLICY_LAST) : NULL;
     qemu_opts_reset(&reopen_opts);
 
     brq = bdrv_reopen_queue(NULL, bs, opts, flags);
diff --git a/qemu-io.c b/qemu-io.c
index db129ea..b295afa 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -207,7 +207,8 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
     }
 
     qopts = qemu_opts_find(&empty_opts, NULL);
-    opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
+    opts = qopts ? qemu_opts_to_qdict(qopts, NULL,
+                                      QEMU_OPTS_REPEAT_POLICY_LAST) : NULL;
     qemu_opts_reset(&empty_opts);
 
     if (optind == argc - 1) {
@@ -593,7 +594,8 @@ int main(int argc, char **argv)
             if (!qopts) {
                 exit(1);
             }
-            opts = qemu_opts_to_qdict(qopts, NULL);
+            opts = qemu_opts_to_qdict(qopts, NULL,
+                                      QEMU_OPTS_REPEAT_POLICY_LAST);
             openfile(NULL, flags, writethrough, opts);
         } else {
             if (format) {
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 99297a5..54eb3ce 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -859,7 +859,8 @@ int main(int argc, char **argv)
             qemu_opts_reset(&file_opts);
             exit(EXIT_FAILURE);
         }
-        options = qemu_opts_to_qdict(opts, NULL);
+        options = qemu_opts_to_qdict(opts, NULL,
+                                      QEMU_OPTS_REPEAT_POLICY_LAST);
         qemu_opts_reset(&file_opts);
         blk = blk_new_open(NULL, NULL, options, flags, &local_err);
     } else {
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index bf59846..48edf2f 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -161,7 +161,8 @@ Object *user_creatable_add_opts(QemuOpts *opts, Error **errp)
     Object *obj = NULL;
 
     v = opts_visitor_new(opts);
-    pdict = qemu_opts_to_qdict(opts, NULL);
+    pdict = qemu_opts_to_qdict(opts, NULL,
+                               QEMU_OPTS_REPEAT_POLICY_LAST);
 
     obj = user_creatable_add(pdict, v, errp);
     visit_free(v);
diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
index a505a3e..b68ef74 100644
--- a/tests/test-qemu-opts.c
+++ b/tests/test-qemu-opts.c
@@ -421,6 +421,45 @@ static void test_qemu_opts_set(void)
     g_assert(opts == NULL);
 }
 
+
+static void test_qemu_opts_to_qdict(void)
+{
+    QemuOpts *opts;
+    QDict *dict;
+
+    /* dynamically initialized (parsed) opts */
+    opts = qemu_opts_parse(&opts_list_03,
+                           "size=1024,nodes=10,nodes=4-5,nodes=1-2,policy=bind",
+                           false, NULL);
+    g_assert(opts);
+
+    dict = qemu_opts_to_qdict(opts, NULL,
+                              QEMU_OPTS_REPEAT_POLICY_LAST);
+    g_assert(dict);
+
+    g_assert_cmpstr(qdict_get_str(dict, "size"), ==, "1024");
+    g_assert_cmpstr(qdict_get_str(dict, "nodes"), ==, "1-2");
+    g_assert(!qdict_haskey(dict, "nodes.0"));
+    g_assert(!qdict_haskey(dict, "nodes.1"));
+    g_assert(!qdict_haskey(dict, "nodes.2"));
+    g_assert_cmpstr(qdict_get_str(dict, "policy"), ==, "bind");
+    QDECREF(dict);
+
+    dict = qemu_opts_to_qdict(opts, NULL,
+                              QEMU_OPTS_REPEAT_POLICY_LIST);
+    g_assert(dict);
+
+    g_assert_cmpstr(qdict_get_str(dict, "size"), ==, "1024");
+    g_assert(!qdict_haskey(dict, "nodes"));
+    g_assert_cmpstr(qdict_get_str(dict, "nodes.0"), ==, "10");
+    g_assert_cmpstr(qdict_get_str(dict, "nodes.1"), ==, "4-5");
+    g_assert_cmpstr(qdict_get_str(dict, "nodes.2"), ==, "1-2");
+    g_assert_cmpstr(qdict_get_str(dict, "policy"), ==, "bind");
+    QDECREF(dict);
+
+    qemu_opts_del(opts);
+}
+
 int main(int argc, char *argv[])
 {
     register_opts();
@@ -435,6 +474,7 @@ int main(int argc, char *argv[])
     g_test_add_func("/qemu-opts/opt_unset", test_qemu_opt_unset);
     g_test_add_func("/qemu-opts/opts_reset", test_qemu_opts_reset);
     g_test_add_func("/qemu-opts/opts_set", test_qemu_opts_set);
+    g_test_add_func("/qemu-opts/to_qdict", test_qemu_opts_to_qdict);
     g_test_run();
     return 0;
 }
diff --git a/tests/test-replication.c b/tests/test-replication.c
index 0997bd8..6165ac9 100644
--- a/tests/test-replication.c
+++ b/tests/test-replication.c
@@ -181,7 +181,8 @@ static BlockBackend *start_primary(void)
     opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
     g_free(cmdline);
 
-    qdict = qemu_opts_to_qdict(opts, NULL);
+    qdict = qemu_opts_to_qdict(opts, NULL,
+                               QEMU_OPTS_REPEAT_POLICY_LAST);
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");
 
@@ -311,7 +312,8 @@ static BlockBackend *start_secondary(void)
     opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
     g_free(cmdline);
 
-    qdict = qemu_opts_to_qdict(opts, NULL);
+    qdict = qemu_opts_to_qdict(opts, NULL,
+                               QEMU_OPTS_REPEAT_POLICY_LAST);
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");
 
@@ -336,7 +338,8 @@ static BlockBackend *start_secondary(void)
     opts = qemu_opts_parse_noisily(&qemu_drive_opts, cmdline, false);
     g_free(cmdline);
 
-    qdict = qemu_opts_to_qdict(opts, NULL);
+    qdict = qemu_opts_to_qdict(opts, NULL,
+                               QEMU_OPTS_REPEAT_POLICY_LAST);
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");
 
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 41b356c..ad28d4e 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -1058,10 +1058,12 @@ void qemu_opts_absorb_qdict(QemuOpts *opts, QDict *qdict, Error **errp)
  * TODO We'll want to use types appropriate for opt->desc->type, but
  * this is enough for now.
  */
-QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
+QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict,
+                          QemuOptsRepeatPolicy repeatPolicy)
 {
     QemuOpt *opt;
-    QObject *val;
+    QObject *val, *prevval;
+    QDict *lists = qdict_new();
 
     if (!qdict) {
         qdict = qdict_new();
@@ -1070,9 +1072,42 @@ QDict *qemu_opts_to_qdict(QemuOpts *opts, QDict *qdict)
         qdict_put(qdict, "id", qstring_from_str(opts->id));
     }
     QTAILQ_FOREACH(opt, &opts->head, next) {
+        gchar *key = NULL;
         val = QOBJECT(qstring_from_str(opt->str));
-        qdict_put_obj(qdict, opt->name, val);
+        switch (repeatPolicy) {
+        case QEMU_OPTS_REPEAT_POLICY_LIST:
+            if (qdict_haskey(lists, opt->name)) {
+                /* Current val goes into 'foo.N' */
+                int64_t max = qdict_get_int(lists, opt->name);
+                max++;
+                key = g_strdup_printf("%s.%" PRId64, opt->name, max);
+                qdict_put_obj(lists, opt->name, QOBJECT(qint_from_int(max)));
+                qdict_put_obj(qdict, key, val);
+            } else if (qdict_haskey(qdict, opt->name)) {
+                /* Move previous val from 'foo' to 'foo.0' */
+                prevval = qdict_get(qdict, opt->name);
+                qobject_incref(prevval);
+                qdict_del(qdict, opt->name);
+                key = g_strdup_printf("%s.0", opt->name);
+                qdict_put_obj(qdict, key, prevval);
+                g_free(key);
+
+                /* Current val goes into 'foo.1' */
+                key = g_strdup_printf("%s.1", opt->name);
+                qdict_put_obj(lists, opt->name, QOBJECT(qint_from_int(1)));
+                qdict_put_obj(qdict, key, val);
+            } else {
+                qdict_put_obj(qdict, key ? key : opt->name, val);
+            }
+            break;
+
+        case QEMU_OPTS_REPEAT_POLICY_LAST:
+            qdict_put_obj(qdict, key ? key : opt->name, val);
+            break;
+        }
+        g_free(key);
     }
+    QDECREF(lists);
     return qdict;
 }
 
-- 
2.7.4