[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
Re: [Qemu-block] [Qemu-devel] [PATCH v14 01/21] option: make parse_option_bool/number non-static
Fri, 21 Oct 2016 12:12:19 -0500
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
> 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
Description: OpenPGP digital signature