Re: [PATCH 02/11] keyval: introduce keyval_merge

[Paolo Bonzini]

On 16/06/21 10:38, Markus Armbruster wrote:
> ... and both values are the same other QType (QTYPE_QNULL, QTYPE_QNUM,
> QTYPE_QSTRING, QTYPE_QBOOL): overwrite.
>
> Why is overwrite restricted to same QType?  Is there no need for
> overwriting say a string with a number?  Hmm, I guess it's okay, because
> keyval_parse() only ever produces QTYPE_QSTRING scalars.  May be worth a
> comment, preferably in a function contract.

Good point, I can add an assert that the only scalars are QTYPE_QSTRING.

> However, some users look at*all*  pairs.  This provides "repeated keys
> build up a list" semantics.
>
> Example 4: repeated option key builds a list
>
>      $ qemu-system-x86_64 -S -spice 
> tls-port=12345,tls-channel=main,tls-channel=display
>
> This fails for me because I don't have Spice set up (and know basically
> nothing about it), but a working variation of it should configure*two*
> channels, not one: the second tls-channel= does*not*  overwrite the
> first one, it configures another channel.
>
> Since -spice is a merge_lists option, this should be the case for
> multiple -spice, too.  Merging the two options with keyval_merge() would
> not preserve that behavior, I'm afraid.
>
> QemuOpts is... complicated.

Right, QemuOpts is complicated and keyval a bit less so, which makes 
keyval not applicable to every case: see -object where we had to resort 
to the OptsVisitor.  For those cases where repeated keys build up a 
list, it will not be possible to switch them to keyval.

The other remarks are mostly cosmetic, so there's not much to say on them.

Paolo