[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v2 42/47] qapi-schema: Fix up misleading spe
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH RFC v2 42/47] qapi-schema: Fix up misleading specification of netdev_add |
Date: |
Thu, 23 Jul 2015 16:59:28 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 07/01/2015 02:22 PM, Markus Armbruster wrote:
> It doesn't take a 'props' argument, let alone one in the format
> "NAME=VALUE,..."
>
> The bogus arguments specification doesn't matter due to 'gen': false.
> Clean it up to be incomplete rather than wrong, and document the
> incompleteness.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> docs/qapi-code-gen.txt | 2 +-
> qapi-schema.json | 13 +++++++------
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> +++ b/qapi-schema.json
> @@ -2062,11 +2062,12 @@
> #
> # @id: the name of the new network backend
> #
> -# @props: #optional a list of properties to be passed to the backend in
> -# the format 'name=value', like 'ifname=tap0,script=no'
> +# Additional arguments depend on the type.
In other words, this would be a perfect candidate to write as taking a
flat union as the argument, where 'type' is the discriminator, and where
the additional arguments are branches of the union. Except that we
don't yet support passing a union as the direct "arguments":{...} of a
QMP call, so we still have some design work to do if we want to go
there. Later, of course.
> #
> -# Notes: The semantics of @props is not well defined. Future commands will
> be
> -# introduced that provide stronger typing for backend creation.
> +# TODO This command effectively bypasses QAPI completely due to its
> +# "additional arguments" business. It shouldn't have been added to
> +# the schema in this form. It should be qapified properly, or
> +# replaced by a properly qapified command.
I like that you are getting rid of the sentence about strong typing
(since flat unions ARE strongly typed) and instead couching it in terms
of an incomplete qapi description. I think we may even be able to come
up with a way to express it without needing a new command, and without
breaking back-compat, once we figure out how to pass unions as a command
argument.
In fact, let's suppose we do have a new union type, NetdevInfo, as well
as a way to flag that a given command will be called by the marshaler
using JUST a pointer to the struct, rather than as a list of parameters
to members of the struct.
{ 'enum': 'NetdevTypes', [ ... ] }
{ 'struct': 'NetdevInfoBase', 'data':
{ 'type': 'NetdevTypes', 'id': 'str' } }
{ 'union': 'NetdevInfo', 'base': 'NetdevInfoBase',
'discriminator': 'type', 'data': { ... } }
{ 'command': 'netdev_add', 'boxed': true,
'data': 'NetdevInfo' }
resulting in the marshaller calling
void qmp_netdev_add(NetdevInfo *data, Error **errp)
Here, 'boxed' is an optional key to each 'command', defaulting to false;
but when true, 'data' is passed in a box to the command called by the
marshaler, instead of broken out into fields. 'boxed' must be true for
unions, but may be true elsewhere.
And if we have that, then OTHER commands that have complained about
super-long parameter lists when their struct is exploded into a
parameter list could also use this mechanism, to write their handler in
terms of a single pointer to the struct.
Enough rambling about stuff for the future. This patch is fine as-is;
Reviewed-by: Eric Blake <address@hidden>
--
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 RFC v2 37/47] qapi: De-duplicate parameter list generation, (continued)
- [Qemu-devel] [PATCH RFC v2 38/47] qapi-commands: De-duplicate output marshaling functions, Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 28/47] qapi-commands: Convert to QAPISchemaVisitor, Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 43/47] qmp: Improve netdev_add usage example in the manual, Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 42/47] qapi-schema: Fix up misleading specification of netdev_add, Markus Armbruster, 2015/07/01
- Re: [Qemu-devel] [PATCH RFC v2 42/47] qapi-schema: Fix up misleading specification of netdev_add,
Eric Blake <=
- [Qemu-devel] [PATCH RFC v2 46/47] qapi-introspect: Map all integer types to 'int', Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 39/47] qapi: Improve built-in type documentation, Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 35/47] qapi-commands: Rearrange code, Markus Armbruster, 2015/07/01
- [Qemu-devel] [PATCH RFC v2 21/47] qapi: New QAPISchema intermediate reperesentation, Markus Armbruster, 2015/07/01