[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 13/15] net: Complete qapi-fication of netdev_
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v7 13/15] net: Complete qapi-fication of netdev_add |
Date: |
Sat, 2 Jul 2016 16:58:20 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 06/16/2016 07:40 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> We finally have all the required pieces for doing a type-safe
>> representation of netdev_add as a flat union, where the
>> discriminator 'type' now selects which additional members may
>> appear in the "arguments" JSON object sent over QMP, while
>> making no changes to the set of previously-valid QMP commands
>> that would work, and without breaking command line parsing.
>
> Isn't it amazing that you pulled this off without a compatibility break?
No command line compatibility break, but in testing, I _did_ notice a
potential QMP break [it's hard to argue whether it is a break, given
that it was previously undocumented - I don't know if any QMP clients
were actually relying on loose behavior]:
Pre-patch:
{'execute':'netdev_add',
'arguments':{'id':'net1', 'type':'hubport', 'hubid':1}}
{"return": {}}
{'execute':'netdev_add',
'arguments':{'id':'net2', 'type':'hubport', 'hubid':"2"}}
{"return": {}}
Post-patch:
{'execute':'netdev_add',
'arguments':{'id':'net1', 'type':'hubport', 'hubid':1}}
{"return": {}}
{'execute':'netdev_add',
'arguments':{'id':'net2', 'type':'hubport', 'hubid':"2"}}
{"error": {"class": "GenericError", "desc": "Invalid parameter type
for 'hubid', expected: integer"}}
I'm half-tempted to claim that we should update the QMP spec to say that
our parser is ALWAYS loose (anywhere a built-in scalar type is listed in
introspection, whether bool or integer, the parser will always accept an
equivalent string on input - but output will always be the named type),
and then relax qmp-input-visitor accordingly. In fact, danpb has
already proposed patches that allow "parse-string-as-int" as intentional
behavior, although under the guise of a new visitor rather than tweaking
qmp-input-visitor - so it just becomes a question of do we do it in
limited situations, or always. "Be liberal in what you accept" comes to
mind.
And as a followon thought: if we DO update the QMP spec to state that we
always accept a string in place of an integer, then we also have the
luxury of stating that accepting a string "inf" for a QAPI 'number' is
valid (even though strict JSON will not let us pass a bare-word inf) -
and that hits back on my proposal of whether we want to accept bare-word
inf on input as an extension, and whether outputting a string "inf" when
we specified a QAPI type of 'number' would be acceptable (since we would
be canonicalizing input "2" into output 2, going the other direction and
canonicalizing input inf into output "inf" is a bit easier to justify).
But given that it is soft freeze time, I guess we need to be
conservative at what changes we want to support at this phase of
development.
--
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 v7 13/15] net: Complete qapi-fication of netdev_add,
Eric Blake <=