[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v14 12/21] option: allow qemu_opts_to_qdict to m
|
From: |
Markus Armbruster |
|
Subject: |
Re: [Qemu-devel] [PATCH v14 12/21] option: allow qemu_opts_to_qdict to merge repeated options |
|
Date: |
Thu, 13 Oct 2016 10:31:37 +0200 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
"Daniel P. Berrange" <address@hidden> writes:
> 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=QString("1024")
> nodes=QString("1-2")
> policy=QString("bind")
>
> With this change the caller can optionally ask for all
> the repeated values to be stored in a QList. In the
> above example that would result in 'nodes' being a
> QList, so the returned dict would contain
>
> size=QString("1024")
> nodes=QList([QString("10"),
> QString("4-5"),
> QString("1-2")])
> policy=QString("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 a QString or a QList for the
> key 'foo'.
Actually three cases, not two:
0. qdict does not contain the key means empty list.
1. qdict contains the key with a QString value means list of one
element.
2. qdict contains the key with a QList value means list of more than one
element.
I consider this weird. However, it's usefully weird with at least your
QObject input visitor.
> In a third mode, it is possible to ask for repeated
> options to be reported as an error, rather than silently
> dropping all but the last one.
Got users for this policy in the pipeline?
> All existing callers are all converted to explicitly
> request the historical behaviour of only reporting the
> last key. Later patches will make use of the new modes.
>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
> blockdev.c | 7 ++-
> include/qemu/option.h | 13 ++++-
> monitor.c | 3 +-
> qapi/qobject-input-visitor.c | 4 +-
> 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 | 132
> +++++++++++++++++++++++++++++++++++++++++++
> tests/test-replication.c | 9 ++-
> util/qemu-option.c | 64 ++++++++++++++++++---
> 12 files changed, 229 insertions(+), 22 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 814d49f..a999d46 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -914,7 +914,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,
> + &error_abort);
>
> legacy_opts = qemu_opts_create(&qemu_legacy_drive_opts, NULL, 0,
> &error_abort);
> @@ -3804,8 +3805,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,
> + &error_abort);
> 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 29f3f18..ffd841d 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -125,7 +125,18 @@ 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(const QemuOpts *opts, QDict *qdict);
Blank line here, please.
> +typedef enum {
> + /* Last occurrence of a duplicate option silently replaces earlier */
> + QEMU_OPTS_REPEAT_POLICY_LAST,
> + /* Each occurrence of a duplicate option converts value to a QList */
> + QEMU_OPTS_REPEAT_POLICY_ALL,
> + /* First occurrence of a duplicate option causes an error */
> + QEMU_OPTS_REPEAT_POLICY_ERROR,
> +} QemuOptsRepeatPolicy;
> +
> +QDict *qemu_opts_to_qdict(const QemuOpts *opts, QDict *qdict,
> + QemuOptsRepeatPolicy repeatPolicy,
> + Error **errp);
> 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 14089cc..84e79d4 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,
> + &error_abort);
> qemu_opts_del(opts);
> }
> break;
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 6b3d0f2..2287d11 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -759,7 +759,9 @@ Visitor *qobject_input_visitor_new_opts(const QemuOpts
> *opts,
> QObject *pobj = NULL;
> Visitor *v = NULL;
>
> - pdict = qemu_opts_to_qdict(opts, NULL);
> + pdict = qemu_opts_to_qdict(opts, NULL,
> + QEMU_OPTS_REPEAT_POLICY_LAST,
Join the previous two lines, please.
> + errp);
Unlike the other callers, this one doesn't pass &error_abort. The error
handling is actually dead code. Let's pass &error_abort and be done
with it.
> if (!pdict) {
> goto cleanup;
> }
> diff --git a/qemu-img.c b/qemu-img.c
> index 851422a..f47ea75 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,
> + &error_abort);
> +
> 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..ce654f4 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,
> + &error_abort) : NULL;
Short ternaries are more legible than if statements, but this one is
pushing the limit now.
> 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..bb374a6 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,
> + &error_abort) : NULL;
Likewise.
> 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,
> + &error_abort);
Even this line's getting ugly. Can we find a prefix that's less
loquacious than QEMU_OPTS_REPEAT_POLICY_, but still self-explaining?
> openfile(NULL, flags, writethrough, opts);
> } else {
> if (format) {
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 99297a5..73b40b1 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,
> + &error_abort);
> 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 ded4d84..fdc406b 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,
> + &error_abort);
>
> 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..3b59978 100644
> --- a/tests/test-qemu-opts.c
> +++ b/tests/test-qemu-opts.c
> @@ -421,6 +421,130 @@ static void test_qemu_opts_set(void)
> g_assert(opts == NULL);
> }
>
> +
> +static void test_qemu_opts_to_qdict_repeat_last(void)
> +{
> + QemuOpts *opts;
> + QDict *dict;
> +
> + /* dynamically initialized (parsed) opts */
What are you trying to say with this comment? Hmm, you merely copied it
from the existing tests. I'd drop the existing instances of this
comment instead.
> + opts = qemu_opts_parse(&opts_list_03,
> +
> "size=1024,nodes=10,nodes=4-5,nodes=1-2,policy=bind",
> + false, NULL);
> + g_assert(opts);
You could pass &error_abort.
> +
> + dict = qemu_opts_to_qdict(opts, NULL, QEMU_OPTS_REPEAT_POLICY_LAST,
> + &error_abort);
> + g_assert(dict);
> +
> +
One blank line, not two, please.
> + 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");
Checking for absence of "nodes.0"... feels odd. A better idea is to
check we got exactly the keys we expect, and no more. Here's a simple
way to do that:
g_assert(qdict_size(dict) == 3);
> + QDECREF(dict);
> +
> + qemu_opts_del(opts);
> + qemu_opts_reset(&opts_list_03);
> +}
> +
> +
> +static void test_qemu_opts_to_qdict_repeat_all(void)
> +{
> + QemuOpts *opts;
> + QDict *dict;
> + QList *list;
> + const QListEntry *ent;
> + QString *str;
> +
> + /* 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_ALL,
> + &error_abort);
> + g_assert(dict);
> +
> + g_assert_cmpstr(qdict_get_str(dict, "size"), ==, "1024");
> + g_assert(qdict_haskey(dict, "nodes"));
> + list = qobject_to_qlist(qdict_get(dict, "nodes"));
> + g_assert(list);
Works, but I'd do
obj = qdict_get(dict, "nodes");
g_assert(obj);
list = qobject_to_qlist(obj);
g_assert(list);
> +
> + ent = qlist_first(list);
> + g_assert(ent);
> + str = qobject_to_qstring(ent->value);
> + g_assert(str);
> + g_assert_cmpstr(qstring_get_str(str), ==, "10");
Since g_assert_cmpstr() does the right thing when @str is null,
g_assert(str) is redundant. More of the same below.
> +
> + ent = qlist_next(ent);
> + g_assert(ent);
> + str = qobject_to_qstring(ent->value);
> + g_assert(str);
> + g_assert_cmpstr(qstring_get_str(str), ==, "4-5");
> +
> + ent = qlist_next(ent);
> + g_assert(ent);
> + str = qobject_to_qstring(ent->value);
> + g_assert(str);
> + g_assert_cmpstr(qstring_get_str(str), ==, "1-2");
> +
> + ent = qlist_next(ent);
> + g_assert(!ent);
> +
> + g_assert_cmpstr(qdict_get_str(dict, "policy"), ==, "bind");
g_assert(qdict_size(dict) == 3);
> + QDECREF(dict);
> +
> + qemu_opts_del(opts);
> + qemu_opts_reset(&opts_list_03);
> +}
> +
> +static void test_qemu_opts_to_qdict_repeat_err_fail(void)
> +{
> + QemuOpts *opts;
> + QDict *dict;
> + Error *err = NULL;
> +
> + /* 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_ERROR,
> + &err);
> + error_free_or_abort(&err);
> + g_assert(!dict);
> +
> + qemu_opts_del(opts);
> + qemu_opts_reset(&opts_list_03);
> +}
> +
> +static void test_qemu_opts_to_qdict_repeat_err_ok(void)
> +{
> + QemuOpts *opts;
> + QDict *dict;
> +
> + /* dynamically initialized (parsed) opts */
> + opts = qemu_opts_parse(&opts_list_03,
> + "size=1024,nodes=10,policy=bind",
> + false, NULL);
> + g_assert(opts);
> +
> + dict = qemu_opts_to_qdict(opts, NULL, QEMU_OPTS_REPEAT_POLICY_ERROR,
> + &error_abort);
> + g_assert_cmpstr(qdict_get_str(dict, "size"), ==, "1024");
> + g_assert_cmpstr(qdict_get_str(dict, "nodes"), ==, "10");
> + g_assert_cmpstr(qdict_get_str(dict, "policy"), ==, "bind");
g_assert(qdict_size(dict) == 3);
> +
> + QDECREF(dict);
> + qemu_opts_del(opts);
> + qemu_opts_reset(&opts_list_03);
> +}
> +
> int main(int argc, char *argv[])
> {
> register_opts();
> @@ -435,6 +559,14 @@ 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/repeat-last",
> + test_qemu_opts_to_qdict_repeat_last);
> + g_test_add_func("/qemu-opts/to_qdict/repeat-all",
> + test_qemu_opts_to_qdict_repeat_all);
> + g_test_add_func("/qemu-opts/to_qdict/repeat-err-fail",
> + test_qemu_opts_to_qdict_repeat_err_fail);
> + g_test_add_func("/qemu-opts/to_qdict/repeat-err-ok",
> + test_qemu_opts_to_qdict_repeat_err_ok);
> g_test_run();
> return 0;
> }
> diff --git a/tests/test-replication.c b/tests/test-replication.c
> index 0997bd8..e267f9a 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,
> + &error_abort);
> 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,
> + &error_abort);
> 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,
> + &error_abort);
> 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 418cbb6..0129ede 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -1057,23 +1057,73 @@ void qemu_opts_absorb_qdict(QemuOpts *opts, QDict
> *qdict, Error **errp)
> * The QDict values are of type QString.
> * TODO We'll want to use types appropriate for opt->desc->type, but
> * this is enough for now.
While there, we could document @obj and @dict properly. But see below
on @dict.
> + *
> + * If the @opts contains multiple occurrences of the same key,
> + * then the @repeatPolicy parameter determines how they are to
> + * be handled. Traditional behaviour was to only store the
> + * last occurrence, but if @repeatPolicy is set to
> + * QEMU_OPTS_REPEAT_POLICY_ALL, then a QList will be returned
> + * containing all values, for any key with multiple occurrences.
> + * The QEMU_OPTS_REPEAT_POLICY_ERROR value can be used to fail
> + * conversion with an error if multiple occurrences of a key
> + * are seen.
What the traditional behavior was is quite interesting in a commit
message. Less so in a comment. Suggest:
* be handled. With @repeatPolicy QEMU_OPTS_REPEAT_POLICY_LAST,
* all values but the last are silently ignored. With
* QEMU_OPTS_REPEAT_POLICY_ALL, a QList will be returned
* containing all values, for any key with multiple occurrences.
* With QEMU_OPTS_REPEAT_POLICY_ERROR, the conversion fails.
Let's spell out that the function can only fail with
QEMU_OPTS_REPEAT_POLICY_ERROR:
* This is the only way this function can fail.
> */
> -QDict *qemu_opts_to_qdict(const QemuOpts *opts, QDict *qdict)
> +QDict *qemu_opts_to_qdict(const QemuOpts *opts, QDict *qdict,
> + QemuOptsRepeatPolicy repeatPolicy,
> + Error **errp)
> {
> QemuOpt *opt;
> - QObject *val;
> + QObject *val, *prevval;
I'd prefer prev_val.
> + QList *list;
> + QDict *ret;
>
> - if (!qdict) {
> - qdict = qdict_new();
> + if (qdict) {
> + ret = qdict;
> + } else {
> + ret = qdict_new();
> }
You need this change to get the reference counting on failure right.
This makes me question the qdict parameter. I can see two non-null
arguments for it outside tests. The one in drive_new() is basically
stupid:
bs_opts = qdict_new();
qemu_opts_to_qdict(all_opts, bs_opts, QEMU_OPTS_REPEAT_POLICY_LAST,
&error_abort);
Easily replaced by
bs_opts = qemu_opts_to_qdict(all_opts, NULL, QEMU_OPTS_REPEAT_POLICY_LAST,
&error_abort);
The one in monitor_parse_arguments()
qemu_opts_to_qdict(opts, qdict, QEMU_OPTS_REPEAT_POLICY_LAST,
&error_abort);
could be replaced by
QDECREF(qdict);
qdict = qemu_opts_to_qdict(opts, NULL,
QEMU_OPTS_REPEAT_POLICY_LAST,
&error_abort);
at a negligible loss of efficiency. We could then drop the parameter,
simplifying the function's contract.
Let's insert a blank line here...
> if (opts->id) {
> - qdict_put(qdict, "id", qstring_from_str(opts->id));
> + qdict_put(ret, "id", qstring_from_str(opts->id));
> }
... and here.
> QTAILQ_FOREACH(opt, &opts->head, next) {
> val = QOBJECT(qstring_from_str(opt->str));
> - qdict_put_obj(qdict, opt->name, val);
> +
> + if (qdict_haskey(ret, opt->name)) {
> + switch (repeatPolicy) {
> + case QEMU_OPTS_REPEAT_POLICY_ERROR:
> + error_setg(errp, "Option '%s' appears more than once",
> + opt->name);
> + qobject_decref(val);
> + if (!qdict) {
> + QDECREF(ret);
> + }
> + return NULL;
> +
> + case QEMU_OPTS_REPEAT_POLICY_ALL:
> + prevval = qdict_get(ret, opt->name);
> + if (qobject_type(prevval) == QTYPE_QLIST) {
> + /* Already identified this key as a list */
> + list = qobject_to_qlist(prevval);
> + } else {
> + /* Replace current scalar with a list */
> + list = qlist_new();
> + qobject_incref(prevval);
Where is this reference decremented?
> + qlist_append_obj(list, prevval);
> + qdict_put_obj(ret, opt->name, QOBJECT(list));
> + }
> + qlist_append_obj(list, val);
> + break;
> +
> + case QEMU_OPTS_REPEAT_POLICY_LAST:
> + /* Just discard previously set value */
> + qdict_put_obj(ret, opt->name, val);
> + break;
> + }
> + } else {
> + qdict_put_obj(ret, opt->name, val);
> + }
A possible alternative:
val = QOBJECT(qstring_from_str(opt->str));
if (qdict_haskey(ret, opt->name)) {
switch (repeatPolicy) {
case QEMU_OPTS_REPEAT_POLICY_ERROR:
error_setg(errp, "Option '%s' appears more than once",
opt->name);
qobject_decref(val);
if (!qdict) {
QDECREF(ret);
}
return NULL;
case QEMU_OPTS_REPEAT_POLICY_ALL:
prevval = qdict_get(ret, opt->name);
if (qobject_type(prevval) == QTYPE_QLIST) {
/* Already identified this key as a list */
list = qobject_to_qlist(prevval);
} else {
/* Replace current scalar with a list */
list = qlist_new();
qobject_incref(prevval);
qlist_append_obj(list, prevval);
}
qlist_append_obj(list, val);
val = QOBJECT(list);
break;
case QEMU_OPTS_REPEAT_POLICY_LAST:
break;
}
}
qdict_put_obj(ret, opt->name, val);
This shows the common part of the behavior more clearly. Matter of
taste, you get to use your artistic license.
> }
> - return qdict;
> + return ret;
> }
>
> /* Validate parsed opts against descriptions where no