diff options
author | Paolo Bonzini <pbonzini@redhat.com> | 2020-11-09 04:46:30 -0500 |
---|---|---|
committer | Paolo Bonzini <pbonzini@redhat.com> | 2020-12-10 12:15:12 -0500 |
commit | 32c02fdda49b8ace1517f1b95bfc215e0b92a154 (patch) | |
tree | 53084bf9c1b912cb2eb7d8f379e7e2be4c029540 /util | |
parent | 4d2c17b0ef760881485a3d31f941117d9fc71bd8 (diff) |
qemu-option: restrict qemu_opts_set to merge-lists QemuOpts
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>
Diffstat (limited to 'util')
-rw-r--r-- | util/qemu-option.c | 9 |
1 files changed, 3 insertions, 6 deletions
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); } |