[Top][All Lists]

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

Re: [Qemu-block] [Qemu-devel] [PATCH v14 01/21] option: make parse_optio

From: Eric Blake
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v14 01/21] option: make parse_option_bool/number non-static
Date: Fri, 21 Oct 2016 12:12:19 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

On 10/21/2016 11:55 AM, Markus Armbruster wrote:

> On first glance, this patch makes the boolean values recognized on the
> command line consistent regardless of which part of the code is used to
> recognize them, by extending the QemuOpts parser to accept the option
> visitor's additional convenience values.
> Once we've done that in a release, we can't go back.  No problem if it
> actually achieved consistency.  But it doesn't: there are other parsers
> that recognize only "on" and "off".  A quick grep finds
> select_display(), bdrv_open_inherit(), netfilter_set_status().

Would it be wrong to convert these to use the same function, to get us
to the point where they are consistent instead of self-limited?  Yes, it
means that newer qemu will accept spellings that older doesn't, and that
we can't later prune the set of spellings back down, but trying to
remain backwards-compatible to every different set of inputs on a
command-by-command basis is harder than just making all commands accept
all spellings.

> bdrv_open_inherit() is particularly instructive: -drive gets parsed by
> QemuOpts (evidence: your patch makes read-only=y work), but by the time
> the options arrive here, they're a QDict.  Curiously,
> bdrv_open_inherit() expects the value of key "read-only" to be a
> *string*, and it checks for "on".  Fortunately, something on the way to
> bdrv_open_inherit() replaces the "y" by "on", so this works.  My point
> is: this patch is trickier than it looks on first glance.
> There's also HMP, which continues to accept only "on" and "off".

Particularly for HMP, where we DON'T have backwards-compatibility
constraints, I'd argue that ease-of-use is the driving factor and that
HMP should allow ALL spellings of boolean arguments.

> We might want to treat the option visitor's additional boolean values
> like its other syntax extensions: keep for compatibility, but confine
> to existing uses.

As in - new interfaces ONLY get to pass "true" or "false", and only
existing interfaces get to also pass "y" or "on"?

> I think we should decide that when we merge the rest of your option
> visitor replacement work, hopefully in 2.9.

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]