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

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 02/24/2017 03:06 PM, Eric Blake wrote:
>
>> On 02/24/2017 02:18 PM, Markus Armbruster wrote:
>>>> 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.
>>> 
>>> Let me rename the options:
>>> 
>>> * "no sugar -blockdev": both "FOO" and "noFOO" are rejected.
>>> 
>>> * "positive sugar blockdev": "FOO" is desugared to "FOO=on", "noFOO" to
>>>   "noFOO=on".
>>> 
>>> Which one do you prefer for 2.9?
>>
>> My current leanings:
>>
>> For 2.9: keyval_parse() should have no sugar.  Both "FOO" and "noFOO"
>> are rejected; an '=' must be present except for an implicit key -
>> although if -blockdev is the only client of keyval_parse(), then it's a
>> tossup whether we want to use it with an implicit key.

I agree on not doing implicit values for 2.9.  We have some thinking to
do on "noFOO", and having keyval_parse() support just "FOO" now could
backfire, as explained upthread.

Regarding implicit keys: I think we can just as well have them.  We're
not going to get rid of existing implicit keys.  Since keyval_parse()
has the code for supporting them already, we can just as well keep it.

>> Also for 2.9: turn on the deprecation warning for QemuOpts negative
>> sugar, but leave positive sugar unchanged (it seems like positive sugar
>> is probably more in use than negative sugar, other than chardev nowait).
>
> Capturing the gist of an IRC conversation:
>
> Right now, it appears that '-chardev nowait' is in heavy use ('git grep
> nowait' shows that qemu itself recommends it in documentation, nodelay
> being another one).  Right now, QemuOpts says that '-chardev
> wait,nowait' results in 'wait=off', and '-chardev nowait,wait' results
> in 'wait=on' (that is, last-one-wins semantics) - but in reality, there
> are probably few (if any) clients that rely on this particular semantics.
>
> If we get rid of 'noFOO' magic, we can still teach the chardev QemuOpts
> to recognize optional keys for BOTH 'wait' and 'nowait'; a user that
> specifies neither gets the default behavior as always; a user that
> specifies only 'wait' or 'nowait' gets the requested behavior (although
> now chardev has to check both key spellings rather than one), and a user
> that specifies 'wait,nowait' or 'nowait,wait' causes an error, not at
> QemuOpts parse time, but in the chardev initialization code.  That is,
> for any boolean option where 'noFOO' magic is currently handled by
> QemuOpts, we can still mostly keep back-compat by adding a new 'noFOO'
> key to the object, and teaching that object to check that 'FOO' and
> 'noFOO' are not both selected at once.

Another possibility is to start with deprecating "noFOO" only when we
don't have description for "FOO".

Hmm, could just as well also deprecate it when we do have one, but it
isn't QEMU_OPT_BOOL.

> But whether this change to QemuOpts is the right thing to do is not 2.9
> material, so let's take a breather and revisit it for 2.10 (or whenever
> we try to fix QemuOpts to layer on top of keyval_parse()).

Makes sense.

>> That way, we still have the option of enabling positive sugar blockdev
>> in a later version, if we decide its useful (and after dealing with any
>> QemuOpts fallout), but are not locked into having to use it.

Exactly.