Too late but I will point it out in the commit that cleans up the iteration.

Paolo

Il lun 25 gen 2021, 08:42 Markus Armbruster <armbru@redhat.com> ha scritto:

> Paolo Bonzini <pbonzini@redhat.com> writes:
>
> > 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")
>
> Pasto: it's "-action".
>
> >       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
>
> Let's insert
>
>     (when using qemu_find_opts_singleton() or qemu_opts_find(list, NULL))
>
> right after skipped, and
>
> > command line is considered.  -boot does two different things
>
>     (when using QTAILQ_FIRST)
>
> right after considered.
>
> > 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>
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
>