Re: [PATCH] block: Error out on image creation with conflicting size options

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 11/26/19 10:26 AM, Kevin Wolf wrote:
>
>>> Actually, your patch fails to diagnose:
>>>
>>> $ qemu-img create -o size=1m,size=2m -f qcow2 x.qcow2
>>> Formatting 'x.qcow2', fmt=qcow2 size=2097152 cluster_size=65536
>>> lazy_refcounts=off refcount_bits=16
>>>
>>> so you may want to enhance this patch to also catch the case of -o size used
>>> more than once.

Please don't.  Let me explain.

>> Hm... Isn't this something that QemuOpts should already catch? Or do we
>> have callers that actually expect the same option specified multiple
>> times?
>
> QemuOpts is horrible.  It allows duplication, and leaves it up to the
> client what to do about it.  Some clients use the duplication to
> collect multiple arguments (such as specifying more than one cpu),
> some callers treat duplication by honoring only the FIRST option
> specified (and ignoring later uses - I find this ugly), and yet other
> callers treat duplication by honoring only the LAST option specified.

QemuOpts has always permitted multiple keys, last one wins.
key=1,key=2,key=3 means key=3.  Permits things like overriding settings
read from a configuration file on the command line.

The straightforward way to use QemuOpts lets the last one win:
qemu_opt_set() & friends insert at the end of the tail queue,
qemu_opt_find() searches backwards for a match.

Its storing of duplicates may have been just an implementation artifact
initially, but then some "clever" uses were invented.  These work by
iterating over all QemuOpts parameters with qemu_opt_foreach(), or over
all parameters of a certain name with qemu_opt_iter_init() and
qemu_opt_iter_next().

>>
>> Somehow I'm almost sure that Markus will know an example...
>
> Probably of all three (ab)uses of QemuOpt duplication.

What the iterating ones all do is anybody's guess.

A common use is collecting all values.  This presses key repetition into
list service: key=1,key=2,key=3 is interpreted like key: [1, 2, 3].

Abusing iteration to honor the first one instead of the last one would
be disgusting.  I'm not aware of such a user, but I'm also not betting
my own money on absence of such abuse.

Needless to say, the peculiarities of QemuOpts are all ABI by now.
Doesn't mean we cannot change them, only that changes need to be
deliberate and careful.

>> But anyway, I figure the same problem exists for almost all options.

It's a feature, not a problem :)