Re: [Qemu-devel] [PATCH RFC v4 30/32] qapi: New QMP command query-schema for QMP schema introspection

[Michael Roth]

Quoting Markus Armbruster (2015-09-03 09:30:21)
> qapi/introspect.json defines the introspection schema.  It's designed
> for QMP introspection, but should do for similar uses, such as QGA.
> 
> The introspection schema does not reflect all the rules and
> restrictions that apply to QAPI schemata.  A valid QAPI schema has an
> introspection value conforming to the introspection schema, but the
> converse is not true.
> 
> Introspection lowers away a number of schema details, and makes
> implicit things explicit:
> 
> * The built-in types are declared with their JSON type.
> 
>   TODO Should we map all the integer types to just int?
> 
> * Implicit type definitions are made explicit, and given
>   auto-generated names:
> 
>   - Array types, named by appending "List" to the name of their
>     element type, like in generated C.
> 
>   - The enumeration types implicitly defined by simple union types,
>     named by appending "Kind" to the name of their simple union type,
>     like in generated C.
> 
>   - Types that don't occur in generated C.  Their names start with ':'
>     so they don't clash with the user's names.
> 
> * All type references are by name.
> 
> * The struct and union types are generalized into an object type.
> 
> * Base types are flattened.
> 
> * Commands take a single argument and return a single result.
> 
>   Dictionary argument or list result is an implicit type definition.
> 
>   The empty object type is used when a command takes no arguments or
>   produces no results.
> 
>   The argument is always of object type, but the introspection schema
>   doesn't reflect that.
> 
>   The 'gen': false directive is omitted as implementation detail.
> 
>   The 'success-response' directive is omitted as well for now, even
>   though it's not an implementation detail, because it's not used by
>   QMP.
> 
> * Events carry a single data value.
> 
>   Implicit type definition and empty object type use, just like for
>   commands.
> 
>   The value is of object type, but the introspection schema doesn't
>   reflect that.
> 
> * Types not used by commands or events are omitted.
> 
>   Indirect use counts as use.
> 
> * Optional members have a default, which can only be null right now
> 
>   Instead of a mandatory "optional" flag, we have an optional default.
>   No default means mandatory, default null means optional without
>   default value.  Non-null is available for optional with default
>   (possible future extension).
> 
> * Clients should *not* look up types by name, because type names are
>   not ABI.  Look up the command or event you're interested in, then
>   follow the references.
> 
>   TODO Should we hide the type names to eliminate the temptation?
> 
> New generator scripts/qapi-introspect.py computes an introspection
> value for its input, and generates a C variable holding it.
> 
> It can generate awfully long lines.  Marked TODO.
> 
> A new test-qmp-input-visitor test case feeds its result for both
> tests/qapi-schema/qapi-schema-test.json and qapi-schema.json to a
> QmpInputVisitor to verify it actually conforms to the schema.
> 
> New QMP command query-schema takes its return value from that
> variable.  Its reply is some 85KiBytes for me right now.
> 
> If this turns out to be too much, we have a couple of options:
> 
> * We can use shorter names in the JSON.  Not the QMP style.
> 
> * Optionally return the sub-schema for commands and events given as
>   arguments.
> 
>   Right now qmp_query_schema() sends the string literal computed by
>   qmp-introspect.py.  To compute sub-schema at run time, we'd have to
>   duplicate parts of qapi-introspect.py in C.  Unattractive.
> 
> * Let clients cache the output of query-schema.
> 
>   It changes only on QEMU upgrades, i.e. rarely.  Provide a command
>   query-schema-hash.  Clients can have a cache indexed by hash, and
>   re-query the schema only when they don't have it cached.  Even
>   simpler: put the hash in the QMP greeting.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  .gitignore                                      |   1 +
>  Makefile                                        |   9 +-
>  Makefile.objs                                   |   4 +-
>  docs/qapi-code-gen.txt                          | 226 +++++++++++++++++++-
>  monitor.c                                       |  15 ++
>  qapi-schema.json                                |   3 +
>  qapi/introspect.json                            | 271 
> ++++++++++++++++++++++++
>  qmp-commands.hx                                 |  17 ++
>  scripts/qapi-introspect.py                      | 174 +++++++++++++++
>  scripts/qapi.py                                 |  12 +-
>  tests/.gitignore                                |   1 +
>  tests/Makefile                                  |  10 +-
>  tests/qapi-schema/alternate-good.out            |   1 +
>  tests/qapi-schema/args-member-array.out         |   1 +
>  tests/qapi-schema/comments.out                  |   1 +
>  tests/qapi-schema/empty.out                     |   1 +
>  tests/qapi-schema/enum-empty.out                |   1 +
>  tests/qapi-schema/event-case.out                |   1 +
>  tests/qapi-schema/flat-union-reverse-define.out |   1 +
>  tests/qapi-schema/ident-with-escape.out         |   1 +
>  tests/qapi-schema/include-relpath.out           |   1 +
>  tests/qapi-schema/include-repetition.out        |   1 +
>  tests/qapi-schema/include-simple.out            |   1 +
>  tests/qapi-schema/indented-expr.out             |   1 +
>  tests/qapi-schema/qapi-schema-test.out          |   1 +
>  tests/qapi-schema/returns-int.out               |   1 +
>  tests/test-qmp-input-strict.c                   |  55 +++++
>  27 files changed, 801 insertions(+), 11 deletions(-)
>  create mode 100644 qapi/introspect.json
>  create mode 100644 scripts/qapi-introspect.py
> 

<snip>

> diff --git a/qapi-schema.json b/qapi-schema.json
> index e91e3b9..af19810 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -14,6 +14,9 @@
>  # Tracing commands
>  { 'include': 'qapi/trace.json' }
> 
> +# QAPI introspection
> +{ 'include': 'qapi/introspect.json' }
> +
>  ##
>  # @LostTickPolicy:
>  #
> diff --git a/qapi/introspect.json b/qapi/introspect.json
> new file mode 100644
> index 0000000..171baba
> --- /dev/null
> +++ b/qapi/introspect.json
> @@ -0,0 +1,271 @@
> +# -*- Mode: Python -*-
> +#
> +# QAPI/QMP introspection
> +#
> +# Copyright (C) 2015 Red Hat, Inc.
> +#
> +# Authors:
> +#  Markus Armbruster <address@hidden>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +
> +##
> +# @query-schema
> +#
> +# Command query-schema exposes the QMP wire ABI as an array of
> +# SchemaInfo.  This lets QMP clients figure out what commands and
> +# events are available in this QEMU, and their parameters and results.
> +#
> +# Returns: array of @SchemaInfo, where each element describes an
> +# entity in the ABI: command, event, type, ...
> +#
> +# Note: the QAPI schema is also used to help define *internal*
> +# interfaces, by defining QAPI types.  These are not part of the QMP
> +# wire ABI, and therefore not returned by this command.

Maybe add something like:

"These internal interfaces may place additional restrictions on the
values of individual fields, so users should reference the QAPI schema
to avoid unexpected behavior resulting from invalid field values."

<snip>

> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index b06d74c..6232ca0 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2168,6 +2168,23 @@ EQMP
>      },
> 
>  SQMP
> +query-schema
> +------------
> +
> +Return the QMP wire schema.  The returned value is a json-array of
> +named schema entities.  Entities are commands, events and various
> +types.  See docs/qapi-code-gen.txt for information on their structure
> +and intended use.

We should probably duplicate any notes from QAPI schema description here
as well, or at least direct users to reference them.

> +
> +EQMP
> +
> +    {
> +        .name       = "query-schema",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_query_schema,
> +    },
> +
> +SQMP
>  query-chardev
>  -------------
> 
> diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
> new file mode 100644
> index 0000000..4bcffc4
> --- /dev/null
> +++ b/scripts/qapi-introspect.py
> @@ -0,0 +1,174 @@
> +#
> +# QAPI introspection generator
> +#
> +# Copyright (C) 2015 Red Hat, Inc.
> +#
> +# Authors:
> +#  Markus Armbruster <address@hidden>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.
> +# See the COPYING file in the top-level directory.
> +
> +from qapi import *
> +import string
> +
> +# Caveman's json.dumps() replacement (we're stuck at 2.4)
> +# TODO try to use json.dumps() once we get unstuck
> +def to_json(obj, level=0):
> +    if obj == None:
> +        ret = 'null'
> +    elif isinstance(obj, str):
> +        ret = '"' + obj.replace('"', r'\"') + '"'
> +    elif isinstance(obj, list):
> +        elts = [to_json(elt, level + 1)
> +                for elt in obj]
> +        ret = '[' + ', '.join(elts) + ']'
> +    elif isinstance(obj, dict):
> +        elts = ['"%s": %s' % (key, to_json(obj[key], level + 1))
> +                for key in sorted(obj.keys())]
> +        ret = '{' + ', '.join(elts) + '}'
> +    else:
> +        assert False                # not implemented
> +    if level == 1:
> +        ret = '\n' + ret
> +    return ret
> +
> +def to_c_string(string):
> +    return '"' + string.replace('\\', r'\\').replace('"', r'\"') + '"'
> +
> +class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
> +    def __init__(self):
> +        self.defn = None
> +        self.decl = None
> +        self.schema = None
> +        self.jsons = None
> +        self.used_types = None
> +
> +    def visit_begin(self, schema):
> +        self.schema = schema
> +        self.jsons = []
> +        self.used_types = []
> +        return QAPISchemaType   # don't visit types for now
> +
> +    def visit_end(self):
> +        # visit the types that are actually used
> +        for typ in self.used_types:
> +            typ.visit(self)
> +        self.jsons.sort()
> +        # generate C
> +        # TODO can generate awfully long lines

Not sure if this is planned for this series, but a multi-line
representation in the generated files would make things a bit
easier to review/check. If we went a little further and made
them pretty-printed it might make some of the docs referencing
the output a bit more clear as well. Not a huge deal, but don't
think it would take much code. Would suggest a JSON library or
something but that would probably mangle the ordering, which
wouldn't help with readability.

> +        name = prefix + 'qmp_schema_json'
> +        self.decl = mcgen('''
> +extern const char %(c_name)s[];
> +''',
> +                          c_name=c_name(name))
> +        lines = to_json(self.jsons).split('\n')
> +        c_string = '\n    '.join([to_c_string(line) for line in lines])
> +        self.defn = mcgen('''
> +const char %(c_name)s[] = %(c_string)s;
> +''',
> +                          c_name=c_name(name),
> +                          c_string=c_string)
> +        self.schema = None
> +        self.jsons = None
> +        self.used_types = None