mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

qemu-option: clean up id vs. list->merge_lists

Browse Source 
Looking at all merge-lists QemuOptsList, here is how they access their
QemuOpts:

reopen_opts in qemu-io-cmds.c ("qemu-img reopen -o")
	qemu_opts_find(&reopen_opts, NULL)

empty_opts in qemu-io.c ("qemu-io open -o")
	qemu_opts_find(&empty_opts, NULL)

qemu_rtc_opts ("-rtc")
	qemu_find_opts_singleton("rtc")

qemu_machine_opts ("-M")
	qemu_find_opts_singleton("machine")

qemu_action_opts ("-name")
	qemu_opts_foreach->process_runstate_actions

qemu_boot_opts ("-boot")
	in hw/nvram/fw_cfg.c and hw/s390x/ipl.c:
	  QTAILQ_FIRST(&qemu_find_opts("bootopts")->head)
	in softmmu/vl.c:
	  qemu_opts_find(qemu_find_opts("boot-opts"), NULL)

qemu_name_opts ("-name")
	qemu_opts_foreach->parse_name
	parse_name does not use id

qemu_mem_opts ("-m")
	qemu_find_opts_singleton("memory")

qemu_icount_opts ("-icount")
	qemu_opts_foreach->do_configure_icount
	do_configure_icount->icount_configure
	icount_configure does not use id

qemu_smp_opts ("-smp")
	qemu_opts_find(qemu_find_opts("smp-opts"), NULL)

qemu_spice_opts ("-spice")
	QTAILQ_FIRST(&qemu_spice_opts.head)

i.e. they don't need an id.  Sometimes its presence is ignored
(e.g. when using qemu_opts_foreach), sometimes all the options
with the id are skipped, sometimes only the first option on the
command line is considered.  -boot does two different things
depending on who's looking at the options.

With this patch we just forbid id on merge-lists QemuOptsLists; if the
command line still works, it has the same semantics as before.

qemu_opts_create's fail_if_exists parameter is now unnecessary:

- it is unused if id is NULL

- opts_parse only passes false if reached from qemu_opts_set_defaults,
in which case this patch enforces that id must be NULL

- other callers that can pass a non-NULL id always set it to true

Assert that it is true in the only case where "fail_if_exists" matters,
i.e. "id && !lists->merge_lists".  This means that if an id is present,
duplicates are always forbidden, which was already the status quo.

Discounting the case that aborts as it's not user-controlled (it's
"just" a matter of inspecting qemu_opts_create callers), the paths
through qemu_opts_create can be summarized as:

- merge_lists = true: singleton opts with NULL id; non-NULL id fails

- merge_lists = false: always return new opts; non-NULL id fails if dup

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This commit is contained in:
Paolo Bonzini 2020-11-09 04:58:24 -05:00
parent 653c974735
commit 63758d1073
1 changed files with 14 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
util/qemu-option.c
View File

@ -619,7 +619,17 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
{
QemuOpts *opts = NULL;
if (id) {
if (list->merge_lists) {
if (id) {
error_setg(errp, QERR_INVALID_PARAMETER, "id");
return NULL;
}
opts = qemu_opts_find(list, NULL);
if (opts) {
return opts;
}
} else if (id) {
assert(fail_if_exists);
if (!id_wellformed(id)) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id",
"an identifier");
@ -629,17 +639,8 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
}
opts = qemu_opts_find(list, id);
if (opts != NULL) {
if (fail_if_exists && !list->merge_lists) {
error_setg(errp, "Duplicate ID '%s' for %s", id, list->name);
return NULL;
} else {
return opts;
}
}
} else if (list->merge_lists) {
opts = qemu_opts_find(list, NULL);
if (opts) {
return opts;
error_setg(errp, "Duplicate ID '%s' for %s", id, list->name);
return NULL;
}
}
opts = g_malloc0(sizeof(*opts));
@ -893,7 +894,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
* (if unlikely) future misuse:
*/
assert(!defaults || list->merge_lists);
opts = qemu_opts_create(list, id, !defaults, errp);
opts = qemu_opts_create(list, id, !list->merge_lists, errp);
g_free(id);
if (opts == NULL) {
return NULL;