[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 16/17] qapi: Tweak QmpInputVisitor to optiona
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v9 16/17] qapi: Tweak QmpInputVisitor to optionally do string conversion |
Date: |
Thu, 14 Jul 2016 06:43:16 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 07/14/2016 02:04 AM, Daniel P. Berrange wrote:
> On Wed, Jul 13, 2016 at 09:50:27PM -0600, Eric Blake wrote:
>> Currently the QmpInputVisitor assumes that all scalar
>> values are directly represented as their final types.
>> ie it assumes an 'int' is using QInt, and a 'bool' is
>> using QBool.
>>
>> This adds an alternative mode where a QString can also
>> be parsed in place of the native type, by adding a parameter
>> and updating all callers. For now, only the testsuite sets
>> the new autocast parameter.
>>
>> This makes it possible to use QmpInputVisitor with a QDict
>> produced from QemuOpts, where everything is in string format.
>> It also makes it possible for the next patch to support
>> back-compat in legacy commands like 'netdev_add' that no
>> longer use QemuOpts, but must parse the same set of QMP
>> constructs that QemuOpts would historically allow.
>
> I'm not really a huge fan of the approach there. IMHO the
> input visitor should strictly operate in "all native types"
> or "all string types" mode. The way you've done it here
> means that users can actually mix & match strings vs native
> types for each individual parameter :-(
>
> IMHO, I'd suggest you try to parse netdev_add args with
> it in "all native types" mode, if that fails, then re-parse
> in "all string types" mode.
Sadly, this idea won't work. Without this series, the existing QMP
command 'netdev_add' takes mixed integers and strings in a single call,
by virtue of converting QObject into QemuOpts and back into QObject.
Forcing the parser to allow only integers or only strings would reject
current uses of netdev_add, and we don't yet have a good idea whether
any existing clients were depending on that behavior.
Also, see patch 17/17 - the easiest way to implement a relaxed QMP parse
is by setting a single flag which controls which visitor constructor is
used. Your idea of parsing once with integer-only, then falling back to
parse a second time with string-only, would complicate the generator to
have to create two different visitors, rather than passing a single
boolean parameter through to the single visitor constructor.
>
> For that you'd not have to modify my patch at all.
>
Also not quite true - your patch no longer applies to master, so it
needed rebasing anyway.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v9 09/17] qapi: Implement boxed types for commands/events, (continued)
- [Qemu-devel] [PATCH v9 13/17] net: Use correct type for bool flag, Eric Blake, 2016/07/13
- [Qemu-devel] [PATCH v9 11/17] block: Simplify drive-mirror, Eric Blake, 2016/07/13
- [Qemu-devel] [PATCH v9 15/17] option: make parse_option_bool/number non-static, Eric Blake, 2016/07/13
- [Qemu-devel] [PATCH v9 10/17] block: Simplify block_set_io_throttle, Eric Blake, 2016/07/13
- [Qemu-devel] [PATCH v9 14/17] net: Complete qapi-fication of netdev_add, Eric Blake, 2016/07/13
- [Qemu-devel] [PATCH v9 16/17] qapi: Tweak QmpInputVisitor to optionally do string conversion, Eric Blake, 2016/07/13
[Qemu-devel] [PATCH v9 17/17] qapi: Restore autocast behavior in 'netdev_add', Eric Blake, 2016/07/13
[Qemu-devel] [PATCH v9 12/17] qapi: Change Netdev into a flat union, Eric Blake, 2016/07/13
Re: [Qemu-devel] [PATCH for-2.7 v9 00/17] qapi netdev_add introspection (post-introspection cleanups subset F), Markus Armbruster, 2016/07/14