[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 22/28] qapi: Whitelist commands that don't re
Re: [Qemu-devel] [PATCH v5 22/28] qapi: Whitelist commands that don't return dictionary
Fri, 27 Mar 2015 14:29:10 -0600
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0
On 03/27/2015 10:19 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>> ...or an array of dictionaries. Although we have to cater to
>> existing commands, returning a non-dictionary means the command
>> is not extensible (no new name/value pairs can be added if more
>> information must be returned in parallel). By making the
>> whitelist explicit, any new command that falls foul of this
>> practice will have to be self-documenting, which will encourage
>> developers to either justify the action or rework the design to
>> use a dictionary after all.
>> Signed-off-by: Eric Blake <address@hidden>
> Thinking on introspection, I started to wonder whether there's actually
> a command returning a union, yet. So I applied the appended stupid
> patch on top, and found the following commands returning (list of)
> non-struct type:
> * qapi-schema.json:
> 'ringbuf-read' built-in type 'str'
> 'human-monitor-command' built-in type 'str'
> 'query-migrate-cache-size' built-in type 'int'
> 'query-tpm-models' enum type 'TpmModel'
More precisely, "array of enum type 'TpmModel'" (or "list", depending on
whether we go with JSON array/object terminology, or QObject dict/list).
I wonder if it is worth trying to tweak the error message to more
precisely track when I strip away the  earlier in check_type to still
report sane messages about 'array of ...' if a later check fails.
> 'query-tpm-types' enum type 'TpmType'
> 'query-memory-devices' union type 'MemoryDeviceInfo'
> * qga/qapi-schema.json:
> 'guest-sync-delimited' built-in type 'int'
> 'guest-sync' built-in type 'int'
> 'guest-get-time' built-in type 'int'
> 'guest-file-open' built-in type 'int'
> 'guest-fsfreeze-status' enum type 'GuestFsfreezeStatus'
> 'guest-fsfreeze-freeze' built-in type 'int'
> 'guest-fsfreeze-freeze-list' built-in type 'int'
> 'guest-fsfreeze-thaw' built-in type 'int'
> 'guest-set-vcpus' built-in type 'int'
Good - your patch found all of my whitelists, plus...
> The sole example for union is 'MemoryDeviceInfo'. It has one case %-}
Yeah, MemoryDeviceInfo as a union currently has only one type, but it
was done that way in case we add other memory devices. So it was
actually quite forward-thinking.
...one additional thing. But returning (an array of) a union should be
okay (it is a dictionary, and therefore extensible); this patch was only
about flagging non-dictionaries.
[side note: again, my idea of renaming 'type' into 'struct' in the .json
files would make it easier to talk about "complex types" as the set of
"struct" and "union" types, rather than the current confusion of
deciding if "type" means all meta-types or just struct meta-types.]
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v5 26/28] qapi: Drop inline nested type in query-version, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 22/28] qapi: Whitelist commands that don't return dictionary, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 23/28] qapi: More rigorous checking for type safety bypass, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 19/28] qapi: Add some type check tests, Eric Blake, 2015/03/24
[Qemu-devel] [PATCH v5 16/28] qapi: Better error messages for duplicated expressions, Eric Blake, 2015/03/24
- Re: [Qemu-devel] [PATCH v5 06/28] qapi: Add some union tests, (continued)