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',
> +]
> +
Since there's just one whitelist, all schemata share it, and that means
it's too permissive for each of them. Sloppy, but good enough.
If the sloppiness bothers us, here are two alternatives:
* Program takes the whitelist as argument, say
scripts/qapi-commands.py --legacy-returns qmp-legacy-returns ...
* Leave enforcing to C
If a command 'frobnicate' returns a non-dictionary, generate something
like
#ifndef FROBNICATE_LEGACY_RETURN_OK
#error Command 'frobnicate' should return a dictionary
#endif
Then manually define the macros necessary to keep the current use
working in a suitable header.
> 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
[...]
Reviewed-by: Markus Armbruster <arm...@redhat.com>
