Re: [Qemu-devel] [PATCH RFC v3 2/5] keyval: New keyval_parse()

[Eric Blake]

On 02/24/2017 01:58 AM, Markus Armbruster wrote:

>> I hate the 'no$key' sugar, and would love to get rid of it.  It makes
>> things ambiguous (thus confusing) for precious little gain: 'novocaine'
>> can mean 'novocaine=on' or 'vocaine=off'.  QemuOpts picks the latter,
>> even when a QemuOpt named 'novocaine' has been declared.
>>
>> keyval_parse() is meant to behave like QemuOpts, so I'm reproducing this
>> bit of bad sugar, reluctantly.
> 
> Implied value alternatives / variations:
> 
> (1) Don't implement the "no" sugar
> 
>     I think we'd have to deprecate it in QemuOpts now, to give users
>     some time to adapt before keyval_parse() replaces QemuOpts.
> 

> 
> If we're happy with bad sugar like "can't abbreviate novocaine=on to
> novocaine" (confusing), "filename without a value gets you the file
> 'on'", we reimplement the bad sugar and move on.
> 
> If not, we have the choice between less magic or more magic.
> 
> Less magic: (1) and/or (2).
> 
> More magic: (3) or something like it.
> 
> If we don't want to decide right now, we can do
> 
> (4) Don't implement any implied value sugar for now
> 
>     You have to spell out =on and =off.  Big fat comment to remind us
>     about the QemuOpts incompatibility.
> 
> I think this would be acceptable for -blockdev in 2.9.
> 
> Opinions?

Do any existing blockdev sub-parameters begin with 'no'?  Ouch:
BlockdevOptions has '*cache':'BlockdevCacheOptions' which in turn has
'*no-flush':'bool'.  Even worse, I think you could abuse 'nono-flush' to
enable flushing - my head hurts.

And of course, there's the obvious BlockDevOptions '*node-name':'str'.
No one in their right mind would expect 'node-name' without parameters
to result in 'de-name=no'.

Also, I found that at least libvirt has existing command line usage
-numa node,nodeid=0, where qemu_numa_opts uses .implied_opt_name="type",
.desc = {{0}} to be visited by OptsVisitor.  Because of the implied opt,
we get the desired "type=node" rather than the accidental "de=no", but
the magic of implied keys can certainly make it weird to think about.

How about:

for 2.9: -blockdev has no magic at all (you HAVE to spell 'foo=off'
rather than 'foo' for any boolean parameters that you want disabled in
-blockdev); and in QemuOpts, we issue deprecation warnings but keep
existing behavior any time someone uses 'noFOO' that we turn into
'FOO=no'.  Then, for 2.10, we can decide to remove the deprecation
warnings if they pointed out real (ab)use of the sugar in the wild, or
(hopefully) to kill the sugar entirely (as part of converting QemuOpts
over to keyval_parse).

In short, getting rid of the 'no' prefix magic, after suitable
deprecation warnings, seems like a good plan to me; and having -blockdev
be slightly tighter than the rest of command line options for 2.9 only
is no real loss (no one uses -blockdev yet, so they can be written to
avoid the use of 'no' magic to begin with).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature