Re: [PATCH v2 4/6] qemu-option: clean up id vs. list->merge_lists

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 09/11/20 17:56, Markus Armbruster wrote:
>> Just like any other QemuOptsList, "machine" may have any number of
>> QemuOpts.  The ones with non-null ID happen to be ignored silently.
>> Known[*] trap for the unwary.
>> 
>> Are these all singletons?
>
> They are never qemu_opts_find'd with non-NULL id, so I'd say they are.

We also need to check qemu_opts_foreach().

>> If lists>merge_lists, you no longer check id_wellformed().  Easy enough
>> to fix: lift the check before this conditional.
>
> Intentional: we always error with INVALID_PARAMETER, so it's pointless 
> to check if the id is well-formed.

My head was about to explode, and then it farted instead.  Sorry fore
the noise!

>> After the patch:
>> 
>>     id        fail_if_exists  merge_lists  |  return
>>     non-null  don't care      true         |  fail
>>     null      don't care      true         |  existing or else new opts
>>     non-null  false           false        |  abort
>>     non-null  true            false        |  new opts / fail if exist
>>     null      don't care      false        |  new opts
>> 
>> Still too many 
>
> Discounting the case that aborts as it's not user-controlled (it's 
> "just" a matter of inspecting qemu_opts_create callers), the rest can be 
> summarized as:
>
> - merge_lists = false: singleton opts with NULL id; non-NULL id fails

Do you mean merge_lists = true here, and ...

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

... = false here?

>> [*] Known to the QemuOpts cognoscenti, whose number could be
>> embarrasingly close to one.
>
> Maybe not one, but a single hand certainly has a surplus of fingers.
>
> Paolo