Re: [Qemu-devel] [PATCH v9 16/17] qapi: Tweak QmpInputVisitor to optionally do string conversion

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]