qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH RFC v3 2/5] keyval: New keyval_pars


From: Eric Blake
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH RFC v3 2/5] keyval: New keyval_parse()
Date: Fri, 24 Feb 2017 13:25:53 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0

On 02/24/2017 01:06 PM, Markus Armbruster wrote:

> 
> The value of an implied key is not subject to key desugaring.

And that's a good thing.

> 
> Without an implied key, the "node" would desugar to "de=off".
> 
>> 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);
> 
> You mean "rather than 'nofoo'".

Yes.  Thanks for catching the intent.

> 
> "No magic at all" implies you also have to spell "foo=on" rather than
> "foo".

I'm a little more lenient for allowing 'foo' meaning 'foo=on' as a
shortcut (although making it force a QBool rather than a string "on" may
be nicer).  I'm also okay if -blockdev has no sugar at all in 2.9 (we
can add "foo" => "foo=on" sugar later if it makes QemuOpts conversion
easier in 2.10; that's much more palatable than adding "nofoo" =>
"foo=off" sugar).

> 
> I believe we need to have both or none in 2.9.  If we have only the "on"
> sugar, "no-flush" works, and we can't add the "off" sugar without
> breaking it.

I think the "off" sugar should die, if we can convince ourselves that no
one is using it that can't catch up to modern usage.

> 
>> -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).
> 
> Okay, I'll post QemuOpts patch to deprecate noKEY sugar.  Let's see
> whether anyone screams.

Summarizing to make sure we're on the same track:

2.9:
QemuOpts:
  "noFOO" => "FOO=off" sugar - deprecation warning
  "FOO" => "FOO=on" sugar - silently stays the same

-blockdev option 1:
  "FOO" => error, no '=' present (we haven't decided on sugaring this yet)
  "noFOO" => error, no '=' present (no off sugar, but also avoids
"noFOO" => "noFOO=on" sugar)

-blockdev option 2:
  "FOO" => "FOO=on" - enabled boolean sugar remains compact, if it is
not ambiguous, but anything we can do to limit it to just boolean values
rather than the string "on" might be nice
  "noFOO" => "noFOO=on" - different behavior than QemuOpts, but less magic

2.10:
QemuOpts will be implemented on keyval_parse(), behaving the same as
-blockdev. If we took option 1 in 2.9, we have:
  "noFOO" => "noFOO=on" (which probably triggers an error that "noFOO"
is not an option when strict parsing is in effect) (the deprecation
period is over, so we changed command-line behavior, but only for people
that didn't pay attention to the warnings in 2.9)
  "FOO" => "FOO=on" - keep this one to minimize back-compat breakage

If we took option 2 in 2.9, we can still turn on "FOO" => "FOO=on" sugar
to get back to option 1 in 2.10

or we can stick to option 2 forevermore:
  "FOO" => error, no '=' present. No longer possible to specify booleans
without the more verbose "FOO=on"

I have a preference for option 1 in the long run, but as it seems to be
upwards compatible from option 2 for -blockdev in 2.9, I'm leaning
towards option 2 for this release.

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

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]