Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com>
> ---
> scripts/qapi.py | 30 ++++++++++++++++++++++++++++--
> tests/qapi-schema/returns-alternate.err | 1 +
> tests/qapi-schema/returns-alternate.exit | 2 +-
> tests/qapi-schema/returns-alternate.json | 2 +-
> tests/qapi-schema/returns-alternate.out | 4 ----
> tests/qapi-schema/returns-int.json | 3 ++-
> tests/qapi-schema/returns-int.out | 2 +-
> tests/qapi-schema/returns-whitelist.err | 1 +
> tests/qapi-schema/returns-whitelist.exit | 2 +-
> tests/qapi-schema/returns-whitelist.json | 2 +-
> tests/qapi-schema/returns-whitelist.out | 7 -------
> 11 files changed, 37 insertions(+), 19 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index ed5385a..9421431 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -33,6 +33,30 @@ builtin_types = {
> 'size': 'QTYPE_QINT',
> }
>
> +# Whitelist of commands allowed to return a non-dictionary
> +returns_whitelist = [
> + # From QMP:
> + 'human-monitor-command',
> + 'query-migrate-cache-size',
> + 'query-tpm-models',
> + 'query-tpm-types',
> + 'ringbuf-read',
> +
> + # From QGA:
> + 'guest-file-open',
> + 'guest-fsfreeze-freeze',
> + 'guest-fsfreeze-freeze-list',
> + 'guest-fsfreeze-status',
> + 'guest-fsfreeze-thaw',
> + 'guest-get-time',
> + 'guest-set-vcpus',
> + 'guest-sync',
> + 'guest-sync-delimited',
> +
> + # From qapi-schema-test:
> + 'user_def_cmd3',
> +]
> +
> enum_types = []
> struct_types = []
> union_types = []
> @@ -350,10 +374,12 @@ def check_command(expr, expr_info):
> check_type(expr_info, "'data' for command '%s'" % name,
> expr.get('data'),
> allowed_metas=['union', 'struct'], allow_optional=True)
> + returns_meta = ['union', 'struct']
> + if name in returns_whitelist:
> + returns_meta += ['built-in', 'alternate', 'enum']
> check_type(expr_info, "'returns' for command '%s'" % name,
> expr.get('returns'), allow_array=True,
> - allowed_metas=['built-in', 'union', 'alternate', 'struct',
> - 'enum'], allow_optional=True)
> + allowed_metas=returns_meta, allow_optional=True)
>
> def check_event(expr, expr_info):
> global events
[...]
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'
'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'
The sole example for union is 'MemoryDeviceInfo'. It has one case %-}
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8e5b4ad..c4c6a6e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -353,9 +353,12 @@ def check_type(expr_info, source, data, allow_array =
False,
"%s uses unknown type '%s'"
% (source, data))
if not all_names[data] in allowed_metas:
- raise QAPIExprError(expr_info,
- "%s cannot use %s type '%s'"
- % (source, all_names[data], data))
+# raise QAPIExprError(expr_info,
+# "%s cannot use %s type '%s'"
+# % (source, all_names[data], data))
+ print >>sys.stderr, QAPIExprError(expr_info,
+ "%s cannot use %s type '%s'"
+ % (source, all_names[data], data))
return
# data is a dictionary, check that each member is okay
@@ -387,6 +390,7 @@ def check_command(expr, expr_info):
returns_meta = ['union', 'struct']
if name in returns_whitelist:
returns_meta += ['built-in', 'alternate', 'enum']
+ returns_meta = ['struct']
check_type(expr_info, "'returns' for command '%s'" % name,
expr.get('returns'), allow_array=True,
allowed_metas=returns_meta, allow_optional=True,
