[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 01/25] qemu-option: clean up id vs. list->merge_lists

From: Paolo Bonzini
Subject: [PATCH 01/25] qemu-option: clean up id vs. list->merge_lists
Date: Mon, 18 Jan 2021 11:30:49 -0500

Looking at all merge-lists QemuOptsList, here is how they access their

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_machine_opts ("-M")

qemu_boot_opts ("-boot")

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

qemu_mem_opts ("-m")

qemu_icount_opts ("-icount")
        icount_configure does not use id

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

qemu_spice_opts ("-spice")

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.  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

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
 util/qemu-option.c | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/util/qemu-option.c b/util/qemu-option.c
index c88e159f18..91f4120ce1 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -619,7 +619,17 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char 
     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 
         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 
      * (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);
     if (opts == NULL) {
         return NULL;

reply via email to

[Prev in Thread] Current Thread [Next in Thread]