qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, imammedo@redhat.com, armbru@redhat.com
Subject: [PATCH 01/25] qemu-option: clean up id vs. list->merge_lists
Date: Mon, 18 Jan 2021 11:30:49 -0500	[thread overview]
Message-ID: <20210118163113.780171-2-pbonzini@redhat.com> (raw)
In-Reply-To: <20210118163113.780171-1-pbonzini@redhat.com>

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_boot_opts ("-boot")
	QTAILQ_FIRST(&qemu_find_opts("bootopts")->head)

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_configuree_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.  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 *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;
-- 
2.26.2




  reply	other threads:[~2021-01-18 16:37 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-18 16:30 [PATCH 00/25] qemu-option, keyval, vl: switch -object/-M/-accel to keyval parsing Paolo Bonzini
2021-01-18 16:30 ` Paolo Bonzini [this message]
2021-01-19 12:33   ` [PATCH 01/25] qemu-option: clean up id vs. list->merge_lists Kevin Wolf
2021-01-19 13:58   ` Markus Armbruster
2021-01-19 14:20     ` Paolo Bonzini
2021-01-20  8:03       ` Markus Armbruster
2021-01-20 12:37         ` Paolo Bonzini
2021-01-20 12:50           ` Markus Armbruster
2021-01-18 16:30 ` [PATCH 02/25] qemu-option: move help handling to get_opt_name_value Paolo Bonzini
2021-01-19 15:10   ` Markus Armbruster
2021-01-18 16:30 ` [PATCH 03/25] qemu-option: warn for short-form boolean options Paolo Bonzini
2021-01-19 15:56   ` Markus Armbruster
2021-01-19 17:04     ` Paolo Bonzini
2021-01-20  8:42       ` Markus Armbruster
2021-01-20 12:40         ` Paolo Bonzini
2021-01-20 12:59           ` Markus Armbruster
2021-01-20 14:05             ` Paolo Bonzini
2021-01-18 16:30 ` [PATCH 04/25] keyval: accept escaped commas in implied option Paolo Bonzini
2021-01-21 12:58   ` Markus Armbruster
2021-01-22  8:39   ` Markus Armbruster
2021-01-18 16:30 ` [PATCH 05/25] keyval: simplify keyval_parse_one Paolo Bonzini
2021-01-22 13:48   ` Markus Armbruster
2021-01-22 15:00     ` Paolo Bonzini
2021-01-22 15:44       ` Markus Armbruster
2021-01-18 16:30 ` [PATCH 06/25] tests: convert check-qom-proplist to keyval Paolo Bonzini
2021-01-22 14:14   ` Markus Armbruster
2021-01-22 14:38     ` Paolo Bonzini
2021-01-22 14:48     ` Paolo Bonzini
2021-01-18 16:30 ` [PATCH 07/25] keyval: introduce keyval_parse_into Paolo Bonzini
2021-01-22 14:22   ` Markus Armbruster
2021-01-22 14:30     ` Paolo Bonzini
2021-01-18 16:30 ` [PATCH 08/25] hmp: replace "O" parser with keyval Paolo Bonzini
2021-01-25  9:00   ` Markus Armbruster
2021-02-26 11:25     ` Paolo Bonzini
2021-03-01 10:14       ` Markus Armbruster
2021-03-01 10:23         ` Paolo Bonzini
2021-03-01 13:35           ` Markus Armbruster
2021-03-01 10:43     ` Markus Armbruster
2021-03-01 11:54       ` Paolo Bonzini
2021-01-18 16:30 ` [PATCH 09/25] qom: use qemu_printf to print help for user-creatable objects Paolo Bonzini
2021-01-25 12:47   ` Markus Armbruster
2021-01-18 16:30 ` [PATCH 10/25] hmp: special case help options for object_add Paolo Bonzini
2021-01-25 12:48   ` Markus Armbruster
2021-01-25 12:49     ` Paolo Bonzini
2021-01-25 14:02       ` Markus Armbruster
2021-01-18 16:30 ` [PATCH 11/25] remove -writeconfig Paolo Bonzini
2021-01-25 12:53   ` Markus Armbruster
2021-01-25 14:01     ` Paolo Bonzini
2021-01-25 14:12       ` Daniel P. Berrangé
2021-01-18 16:31 ` [PATCH 12/25] qemu-config: add error propagation to qemu_config_parse Paolo Bonzini
2021-01-25 13:54   ` Markus Armbruster
2021-01-18 16:31 ` [PATCH 13/25] qemu-option: support accept-any QemuOptsList in qemu_opts_absorb_qdict Paolo Bonzini
2021-01-18 16:31 ` [PATCH 14/25] qemu-config: parse configuration files to a QDict Paolo Bonzini
2021-01-18 16:31 ` [PATCH 15/25] vl: plumb keyval-based options into -set and -readconfig Paolo Bonzini
2021-01-25 11:48   ` Markus Armbruster
2021-01-25 13:59     ` Paolo Bonzini
2021-01-18 16:31 ` [PATCH 16/25] qom: do not modify QDict argument in user_creatable_add_dict Paolo Bonzini
2021-01-18 16:31 ` [PATCH 17/25] qemu-io: use keyval for -object parsing Paolo Bonzini
2021-01-18 16:31 ` [PATCH 18/25] qemu-nbd: " Paolo Bonzini
2021-01-18 16:31 ` [PATCH 19/25] qemu-img: " Paolo Bonzini
2021-01-18 16:31 ` [PATCH 20/25] qemu: " Paolo Bonzini
2021-01-18 16:31 ` [PATCH 21/25] storage-daemon: do not register the "object" group with QemuOpts Paolo Bonzini
2021-01-18 16:31 ` [PATCH 22/25] qom: export more functions for use with non-UserCreatable objects Paolo Bonzini
2021-01-18 16:31 ` [PATCH 23/25] vl: switch -M parsing to keyval Paolo Bonzini
2021-01-18 16:31 ` [PATCH 24/25] qemu-option: remove now-dead code Paolo Bonzini
2021-01-18 16:31 ` [PATCH 25/25] vl: switch -accel parsing to keyval Paolo Bonzini
2021-01-18 17:18 ` [PATCH 00/25] qemu-option, keyval, vl: switch -object/-M/-accel to keyval parsing no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210118163113.780171-2-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=armbru@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).