[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 061/113] qemu-option: restrict qemu_opts_set to merge-lists QemuOp
From: |
Paolo Bonzini |
Subject: |
[PULL 061/113] qemu-option: restrict qemu_opts_set to merge-lists QemuOpts |
Date: |
Wed, 2 Dec 2020 03:07:57 -0500 |
qemu_opts_set is used to create default network backends and to
parse sugar options -kernel, -initrd, -append, -bios and -dtb.
These are very different uses:
I would *expect* a function named qemu_opts_set to set an option in a
merge-lists QemuOptsList, such as -kernel, and possibly to set an option
in a non-merge-lists QemuOptsList with non-NULL id, similar to -set.
However, it wouldn't *work* to use qemu_opts_set for the latter
because qemu_opts_set uses fail_if_exists==1. So, for non-merge-lists
QemuOptsList and non-NULL id, the semantics of qemu_opts_set (fail if the
(QemuOptsList, id) pair already exists) are debatable.
On the other hand, I would not expect qemu_opts_set to create a
non-merge-lists QemuOpts with a single option; which it does, though.
For this case of non-merge-lists QemuOptsList and NULL id, qemu_opts_set
hardly adds value over qemu_opts_parse. It does skip some parsing and
unescaping, but that's not needed when creating default network
backends.
So qemu_opts_set has warty behavior for non-merge-lists QemuOptsList
if id is non-NULL, and it's mostly pointless if id is NULL. My
solution to keeping the API as simple as possible is to limit
qemu_opts_set to merge-lists QemuOptsList. For them, it's useful (we
don't want comma-unescaping for -kernel) *and* has sane semantics.
Network backend creation is switched to qemu_opts_parse.
qemu_opts_set is now only used on merge-lists QemuOptsList... except
in the testcase, which is changed to use a merge-list QemuOptsList.
With this change we can also remove the id parameter. With the
parameter always NULL, we know that qemu_opts_create cannot fail
and can pass &error_abort to it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/option.h | 3 +--
softmmu/vl.c | 19 +++++++------------
tests/test-qemu-opts.c | 20 +++++++++++++++++---
util/qemu-option.c | 9 +++------
4 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index ac69352e0e..f73e0dc7d9 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -119,8 +119,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char
*id,
int fail_if_exists, Error **errp);
void qemu_opts_reset(QemuOptsList *list);
void qemu_opts_loc_restore(QemuOpts *opts);
-bool qemu_opts_set(QemuOptsList *list, const char *id,
- const char *name, const char *value, Error **errp);
+bool qemu_opts_set(QemuOptsList *list, const char *name, const char *value,
Error **errp);
const char *qemu_opts_id(QemuOpts *opts);
void qemu_opts_set_id(QemuOpts *opts, char *id);
void qemu_opts_del(QemuOpts *opts);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 85b23d51be..5571f9ff75 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3388,20 +3388,16 @@ void qemu_init(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_kernel:
- qemu_opts_set(qemu_find_opts("machine"), NULL, "kernel",
optarg,
- &error_abort);
+ qemu_opts_set(qemu_find_opts("machine"), "kernel", optarg,
&error_abort);
break;
case QEMU_OPTION_initrd:
- qemu_opts_set(qemu_find_opts("machine"), NULL, "initrd",
optarg,
- &error_abort);
+ qemu_opts_set(qemu_find_opts("machine"), "initrd", optarg,
&error_abort);
break;
case QEMU_OPTION_append:
- qemu_opts_set(qemu_find_opts("machine"), NULL, "append",
optarg,
- &error_abort);
+ qemu_opts_set(qemu_find_opts("machine"), "append", optarg,
&error_abort);
break;
case QEMU_OPTION_dtb:
- qemu_opts_set(qemu_find_opts("machine"), NULL, "dtb", optarg,
- &error_abort);
+ qemu_opts_set(qemu_find_opts("machine"), "dtb", optarg,
&error_abort);
break;
case QEMU_OPTION_cdrom:
drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
@@ -3511,8 +3507,7 @@ void qemu_init(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_bios:
- qemu_opts_set(qemu_find_opts("machine"), NULL, "firmware",
optarg,
- &error_abort);
+ qemu_opts_set(qemu_find_opts("machine"), "firmware", optarg,
&error_abort);
break;
case QEMU_OPTION_singlestep:
singlestep = 1;
@@ -4395,9 +4390,9 @@ void qemu_init(int argc, char **argv, char **envp)
if (default_net) {
QemuOptsList *net = qemu_find_opts("net");
- qemu_opts_set(net, NULL, "type", "nic", &error_abort);
+ qemu_opts_parse(net, "nic", true, &error_abort);
#ifdef CONFIG_SLIRP
- qemu_opts_set(net, NULL, "type", "user", &error_abort);
+ qemu_opts_parse(net, "user", true, &error_abort);
#endif
}
diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
index 297ffe79dd..2aab831d10 100644
--- a/tests/test-qemu-opts.c
+++ b/tests/test-qemu-opts.c
@@ -84,11 +84,25 @@ static QemuOptsList opts_list_03 = {
},
};
+static QemuOptsList opts_list_04 = {
+ .name = "opts_list_04",
+ .head = QTAILQ_HEAD_INITIALIZER(opts_list_04.head),
+ .merge_lists = true,
+ .desc = {
+ {
+ .name = "str3",
+ .type = QEMU_OPT_STRING,
+ },
+ { /* end of list */ }
+ },
+};
+
static void register_opts(void)
{
qemu_add_opts(&opts_list_01);
qemu_add_opts(&opts_list_02);
qemu_add_opts(&opts_list_03);
+ qemu_add_opts(&opts_list_04);
}
static void test_find_unknown_opts(void)
@@ -402,17 +416,17 @@ static void test_qemu_opts_set(void)
QemuOpts *opts;
const char *opt;
- list = qemu_find_opts("opts_list_01");
+ list = qemu_find_opts("opts_list_04");
g_assert(list != NULL);
g_assert(QTAILQ_EMPTY(&list->head));
- g_assert_cmpstr(list->name, ==, "opts_list_01");
+ g_assert_cmpstr(list->name, ==, "opts_list_04");
/* should not find anything at this point */
opts = qemu_opts_find(list, NULL);
g_assert(opts == NULL);
/* implicitly create opts and set str3 value */
- qemu_opts_set(list, NULL, "str3", "value", &error_abort);
+ qemu_opts_set(list, "str3", "value", &error_abort);
g_assert(!QTAILQ_EMPTY(&list->head));
/* get the just created opts */
diff --git a/util/qemu-option.c b/util/qemu-option.c
index acefbc23fa..25792159ba 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -670,15 +670,12 @@ void qemu_opts_loc_restore(QemuOpts *opts)
loc_restore(&opts->loc);
}
-bool qemu_opts_set(QemuOptsList *list, const char *id,
- const char *name, const char *value, Error **errp)
+bool qemu_opts_set(QemuOptsList *list, const char *name, const char *value,
Error **errp)
{
QemuOpts *opts;
- opts = qemu_opts_create(list, id, 1, errp);
- if (!opts) {
- return false;
- }
+ assert(list->merge_lists);
+ opts = qemu_opts_create(list, NULL, 0, &error_abort);
return qemu_opt_set(opts, name, value, errp);
}
--
2.26.2
- [PULL 074/113] vl: start VM via qmp_cont, (continued)
- [PULL 074/113] vl: start VM via qmp_cont, Paolo Bonzini, 2020/12/02
- [PULL 084/113] kernel-doc: add support for ____cacheline_aligned_in_smp attribute, Paolo Bonzini, 2020/12/02
- [PULL 086/113] scripts: kernel-doc: proper handle @foo->bar(), Paolo Bonzini, 2020/12/02
- [PULL 088/113] scripts: kernel-doc: accept blank lines on parameter description, Paolo Bonzini, 2020/12/02
- [PULL 090/113] scripts/kernel-doc: parse __ETHTOOL_DECLARE_LINK_MODE_MASK, Paolo Bonzini, 2020/12/02
- [PULL 087/113] scripts: kernel-doc: accept negation like !@var, Paolo Bonzini, 2020/12/02
- [PULL 069/113] vl: separate qemu_apply_machine_options, Paolo Bonzini, 2020/12/02
- [PULL 051/113] treewide: do not use short-form boolean options, Paolo Bonzini, 2020/12/02
- [PULL 056/113] vl: extract qemu_init_subsystems, Paolo Bonzini, 2020/12/02
- [PULL 060/113] vl: extract various command line desugaring snippets to a new function, Paolo Bonzini, 2020/12/02
- [PULL 061/113] qemu-option: restrict qemu_opts_set to merge-lists QemuOpts,
Paolo Bonzini <=
- [PULL 067/113] vl: separate qemu_create_late_backends, Paolo Bonzini, 2020/12/02
- [PULL 072/113] vl: move -global check earlier, Paolo Bonzini, 2020/12/02
- [PULL 073/113] migration, vl: start migration via qmp_migrate_incoming, Paolo Bonzini, 2020/12/02
- [PULL 068/113] vl: separate qemu_create_machine, Paolo Bonzini, 2020/12/02
- [PULL 079/113] vl: remove serial_max_hds, Paolo Bonzini, 2020/12/02
- [PULL 075/113] hmp: introduce cmd_available, Paolo Bonzini, 2020/12/02
- [PULL 077/113] vl: extract machine done notifiers, Paolo Bonzini, 2020/12/02
- [PULL 094/113] kernel-doc: add support for ____cacheline_aligned attribute, Paolo Bonzini, 2020/12/02
- [PULL 096/113] Revert "scripts/kerneldoc: For Sphinx 3 use c:macro for macros with arguments", Paolo Bonzini, 2020/12/02
- [PULL 083/113] kernel-doc: fix processing nested structs with attributes, Paolo Bonzini, 2020/12/02