diff mbox

[RFC,v4,30/32] qapi: New QMP command query-schema for QMP schema introspection

Message ID 1441290623-13631-31-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Sept. 3, 2015, 2:30 p.m. UTC
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 <armbru@redhat.com>
---
 .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

Comments

Eric Blake Sept. 3, 2015, 8:50 p.m. UTC | #1
On 09/03/2015 08:30 AM, Markus Armbruster wrote:
> qapi/introspect.json defines the introspection schema.  It's designed
> for QMP introspection, but should do for similar uses, such as QGA.

[review to follow in separate message; I'm using this message to focus
on one point for easier tracking of the sub-thread]

> 
> 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?

So, given the discussion on v3, I think we will have consensus if v5
does the following:

- Merge 30 and 31 into a single patch, and drop this TODO statement (I
think we have agreement that exposing QMP [that is, JSON number] wire
form, with no additional constraints, as a single 'int', rather than the
underlying generated C types, is the way to go), provided that...
- Document in the qapi side that the schema intentionally lacks details
on ranges, and that consulting the qapi and/or C code may be required to
see what actual values are valid anywhere introspection merely says 'int',
- leave 32 as a separate patch, as it is complex enough, and still may
have debate on whether the types '123' and 'int' should have
corresponding array types '[123]' and '[int]' rather than the v3 patch
munging of '456' and '789' (I haven't yet reviewed whether v4 changed that).

Any further arguments on whether exposing just 'int' in the
introspection for all integral types, and/or whether patches 30 and 31
should be merged, are best made in response to this mail.
Michael Roth Sept. 3, 2015, 11:59 p.m. UTC | #2
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 <armbru@redhat.com>
> ---
>  .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 <armbru@redhat.com>
> +#
> +# 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 <armbru@redhat.com>
> +#
> +# 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
Michael Roth Sept. 4, 2015, 12:04 a.m. UTC | #3
Quoting Eric Blake (2015-09-03 15:50:16)
> On 09/03/2015 08:30 AM, Markus Armbruster wrote:
> > qapi/introspect.json defines the introspection schema.  It's designed
> > for QMP introspection, but should do for similar uses, such as QGA.
> 
> [review to follow in separate message; I'm using this message to focus
> on one point for easier tracking of the sub-thread]
> 
> > 
> > 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?
> 
> So, given the discussion on v3, I think we will have consensus if v5
> does the following:
> 
> - Merge 30 and 31 into a single patch, and drop this TODO statement (I
> think we have agreement that exposing QMP [that is, JSON number] wire
> form, with no additional constraints, as a single 'int', rather than the
> underlying generated C types, is the way to go), provided that...
> - Document in the qapi side that the schema intentionally lacks details
> on ranges, and that consulting the qapi and/or C code may be required to
> see what actual values are valid anywhere introspection merely says 'int',
> - leave 32 as a separate patch, as it is complex enough, and still may
> have debate on whether the types '123' and 'int' should have
> corresponding array types '[123]' and '[int]' rather than the v3 patch
> munging of '456' and '789' (I haven't yet reviewed whether v4 changed that).

Agreed on all 3

> 
> Any further arguments on whether exposing just 'int' in the
> introspection for all integral types, and/or whether patches 30 and 31
> should be merged, are best made in response to this mail.

Given the above, I don't think I have any further objections.

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Eric Blake Sept. 4, 2015, 2:03 a.m. UTC | #4
On 09/03/2015 08:30 AM, Markus Armbruster wrote:
> qapi/introspect.json defines the introspection schema.  It's designed
> for QMP introspection, but should do for similar uses, such as QGA.

[review in this sub-thread; for comments on 'int' munging or other
followups, see other subthread]

There is at least one definite bug (see multiple references below to
[1]), and several ideas for cleanups, but in general, I think that the
remaining changes are going to be small enough that I'd probably be okay
if v5 started life with:
Reviewed-by: Eric Blake <eblake@redhat.com>
(Of course, you have every right to not add the R-b, especially if you
want me to do another close review :)

> 
> 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.
> 
> 
>   The empty object type is used when a command takes no arguments or
>   produces no results.

We had discussed whether to expose the empty type in a separate patch,
to reduce the size of this patch; but at this point in the game, I'm
happy to keep it as-is (the faster we get v5 out for review and into the
tree, the faster we can process the backlog of followup patches).

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---


> +++ b/docs/qapi-code-gen.txt
> @@ -494,13 +494,195 @@ Resulting in this JSON object:
>    "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
>  
>  
> +== Client JSON Protocol introspection ==
> +
> +Clients of a Client JSON Protocol commonly need to figure out what
> +exactly the server (QEMU) supports.
> +
> +For this purpose, QMP provides introspection via command query-schema.

Will need tweaking to the agreed-on name query-qmp-schema...

> +QGA currently doesn't support introspection.

... (and we'd obviously want qga to eventually gain a matching
counterpart, possibly 'guest-schema' or 'guest-get-schema', to match
prevailing naming in qga/qapi-schema.json).

> +
> +query-schema returns a JSON array of SchemaInfo objects.  These
> +objects together describe the wire ABI, as defined in the QAPI schema.

Maybe an additional sentence here, perhaps along the lines of:

Note that the wire format cannot express everything; it is not designed
to show semantic constraints (such as when exactly one of a pair of
mutually-exclusive optional members must be present, or when an integral
value has specific limitations on valid values).  Introspection allows
an application to know if a feature is present, but the application must
obey the qapi documentation to properly interact with that feature.

> +
> +Like any other command, query-schema is itself defined in the QAPI
> +schema, along with the SchemaInfo type.  This text attempts to give an
> +overview how things work.  For details you need to consult the QAPI
> +schema.
> +
> +SchemaInfo objects have common members "name" and "meta-type", and
> +additional variant members depending on the value of meta-type.
> +
> +Each SchemaInfo object describes a wire ABI entity of a certain
> +meta-type: a command, event or one of several kinds of type.
> +
> +SchemaInfo for entities defined in the QAPI schema have the same name
> +as in the schema.  This is the case for all commands and events, and
> +most types.

And hopefully patch 32 munges this text to match the simplification made
there (I haven't checked yet).

> +
> +Command and event names are part of the wire ABI, but type names are
> +not.  Therefore, looking up a type by its name in the QAPI schema is
> +wrong.  Look up the command or event, then follow references by name.
> +
> +QAPI schema definitions not reachable that way are omitted.
> +
> +The SchemaInfo for a command has meta-type "command", and variant
> +members "arg-type" and "ret-type".  The arguments member clients pass

I found it easier to read with:
s/arguments/arguments that/

> +with a command on the wire must conform to the type named by
> +"arg-type", which is always an object type.  The return value the

Likewise s/value/value that/

> +server passes in a success response conforms to the type named by
> +"ret-type".
> +

[not for this patch. Thinking aloud: I wonder if QGA's success-response
could be coded in by having 'ret-type':null as a way of saying the
command will have no response if it is successful; perhaps a bit nicer
than exposing an additional boolean flag member]

> +If the command takes no arguments, "arg-type" names an object type
> +without members.  Likewise, if the command returns nothing, "ret-type"
> +names an object type without members.
> +
> +Example: the SchemaInfo for command query-schema
> +
> +    { "name": "query-schema", "meta-type": "command",
> +      "arg-type": ":empty", "ret-type": "SchemaInfoList" }
> +
> +    Type ":empty" is an object type without members, and type
> +    "SchemaInfoList" is the array of SchemaInfo type.
> +
> +The SchemaInfo for an event has meta-type "event", and variant member
> +"arg-type".  The data member the server passes with an event conforms

again, reads a little better with s/member/member that/

> +to the type named by "arg-type".  It is always an object type.
> +
> +If the event carries no additional information, "arg-type" names an
> +object type without members.  The event may not have a data member on
> +the wire then.
> +
> +Each command or event defined with dictionary-valued 'data' in the
> +QAPI schema implicitly defines an object type called ":obj-NAME-arg",
> +where NAME is the command or event's name.
> +
> +Example: the SchemaInfo for EVENT_C from section Events
> +
> +    { "name": "EVENT_C", "meta-type": "event",
> +      "arg-type": ":obj-EVENT_C-arg" }
> +
> +    Type ":obj-EVENT_C-arg" is an implicitly defined object type with
> +    the two members from the event's definition.
> +
> +The SchemaInfo for struct and union types has meta-type "object".
> +
> +The SchemaInfo for a struct type has variant member "members".

Worth mentioning that "members" may be empty, for an empty struct?

> +
> +The SchemaInfo for a union type additionally has variant members "tag"
> +and "variants".

Worth mentioning that "members" will never be empty, because it contains
at least the member also referenced in "tag"?

> +
> +"members" is a JSON array describing the object's common members.
> +Each element is a JSON object with members "name" (the member's name),
> +"type" (the name of its type), and optionally "default".  The member
> +is optional if "default" is present.  Currently, "default" can only
> +have value null.  Other values are reserved for future extensions.
> +
> +Example: the SchemaInfo for MyType from section Struct types
> +
> +    { "name": "MyType", "meta-type": "object",
> +      "members": [
> +          { "name": "member1", "type": "str" },
> +          { "name": "member2", "type": "int" },
> +          { "name": "member3", "type": "str", "default": null } ] }
> +
> +"tag" is the name of the common member serving as type tag.
> +"variants" is a JSON array describing the object's variant members.
> +Each element is a JSON object with members "case" (the value of type
> +tag this element applies to) and "type" (the name of an object type
> +that provides the variant members for this type tag value).
> +
> +Example: the SchemaInfo for flat union BlockdevOptions from section
> +Union types

Hmm, the "Union types" section has two mentions of BlockdevOptions; and
this example matches the second. It may help readability if we rename
one of the two examples to be distinct (preferably the first, since it
doesn't match actual QMP).  I guess the phrase "flat union
BlockdevOptions" is sufficient to make it obvious that we are referring
to the second usage, but it is subtle.

> +
> +    { "name": "BlockdevOptions", "meta-type": "object",
> +      "members": [
> +          { "name": "driver", "type": "BlockdevDriver" },
> +          { "name": "readonly", "type": "bool"} ],
> +      "tag": "driver",
> +      "variants": [
> +          { "case": "file", "type": "FileOptions" },
> +          { "case": "qcow2", "type": "Qcow2Options" } ] }
> +
> +Note that base types are "flattened": its members are included in the
> +"members" array.
> +
> +A simple union implicitly defines an enumeration type for its implicit
> +discriminator (called "type" on the wire, see section Union types).
> +Such a type's name is made by appending "Kind" to the simple union's
> +name.
> +
> +A simple union implicitly defines an object type for each of its
> +variants.  The type's name is ":obj-NAME-wrapper", where NAME is the
> +name of the name of the variant's type.
> +
> +Example: the SchemaInfo for simple union BlockdevOptions from section
> +Union types

Ah, and here you refer to the other BlockdevOptions. So the point about
a judicious rename may still be warranted.

> +
> +    { "name": "BlockdevOptions", "meta-type": "object",
> +      "members": [
> +          { "name": "kind", "type": "BlockdevOptionsKind" } ],

[1] Ouch.  That should be "name": "type".  I pointed out the bug in v3,
but it looks like it still hasn't been fixed.  Or rather, that our
attempt to fix it wasn't correct.

> +      "tag": "type",
> +      "variants": [
> +          { "case": "file", "type": ":obj-FileOptions-wrapper" },
> +          { "case": "qcow2", "type": ":obj-Qcow2Options-wrapper" } ] }
> +
> +    Enumeration type "BlockdevOptionsKind" and the object types
> +    ":obj-FileOptions-wrapper", ":obj-Qcow2Options-wrapper" are
> +    implicitly defined.
> +
> +The SchemaInfo for an alternate type has meta-type "alternate", and
> +variant member "members".  "members" is a JSON array.  Each element is
> +a JSON object with member "type", which names a type.  Values of the
> +alternate type conform to exactly one of its member types.
> +
> +Example: the SchemaInfo for BlockRef from section Alternate types
> +
> +    { "name": "BlockRef", "meta-type": "alternate",
> +      "members": [
> +          { "type": "BlockdevOptions" },
> +          { "type": "str" } ] }
> +
> +The SchemaInfo for an array type has meta-type "array", and variant
> +member "element-type", which names the array's element type.  Array
> +types are implicitly defined.  An array type's name is made by
> +appending "List" to its element type's name.
> +
> +Example: the SchemaInfo for ['str']
> +
> +    { "name": "strList", "meta-type": "array",
> +      "element-type": "str" }
> +
> +The SchemaInfo for an enumeration type has meta-type "enum" and
> +variant member "values".
> +
> +Example: the SchemaInfo for MyEnum from section Enumeration types
> +
> +    { "name": "MyEnum", "meta-type": "enum",
> +      "values": [ "value1", "value2", "value3" ] }
> +
> +The SchemaInfo for a built-in type has the same name as the type in
> +the QAPI schema (see section Built-in Types).  It has variant member
> +"json-type" that shows how values of this type are encoded on the
> +wire.
> +
> +Example: the SchemaInfo for str
> +
> +    { "name": "str", "meta-type": "builtin", "json-type": "string" }
> +
> +As explained above, type names are not part of the wire ABI.  Not even
> +the names of built-in types.  Clients should examine member
> +"json-type" instead of hard-coding names of built-in types.

Good point (although we may still end up with clients that disobey this,
I will certainly make sure to do this in libvirt's interactions).

> +
> +
>  == Code generation ==
>  
> -Schemas are fed into 3 scripts to generate all the code/files that, paired
> -with the core QAPI libraries, comprise everything required to take JSON
> -commands read in by a Client JSON Protocol server, unmarshal the arguments into
> -the underlying C types, call into the corresponding C function, and map the
> -response back to a Client JSON Protocol response to be returned to the user.
> +Schemas are fed into four scripts to generate all the code/files that,
> +paired with the core QAPI libraries, comprise everything required to
> +take JSON commands read in by a Client JSON Protocol server, unmarshal
> +the arguments into the underlying C types, call into the corresponding
> +C function, and map the response back to a Client JSON Protocol
> +response to be returned to the user.

Doesn't mention introspection; but then again, it also doesn't mention
that it generates the C interface for emitting events.

>  
>  As an example, we'll use the following schema, which describes a single
>  complex user-defined type (which will produce a C struct, along with a list
> @@ -848,3 +1030,37 @@ Example:
>      extern const char *const example_QAPIEvent_lookup[];
>  
>      #endif
> +
> +=== scripts/qapi-introspect.py ===
> +
> +Used to generate the introspection C code for a schema. The following
> +files are created:
> +
> +$(prefix)qmp-introspect.c - Defines a string holding a JSON
> +                            description of the schema.
> +$(prefix)qmp-introspect.h - Declares the above string.
> +
> +Example:
> +
> +    $ python scripts/qapi-introspect.py --output-dir="qapi-generated"
> +    --prefix="example-" example-schema.json
> +    $ cat qapi-generated/example-qmp-introspect.c
> +[Uninteresting stuff omitted...]
> +
> +    const char example_qmp_schema_json[] = "["
> +        "{\"arg-type\": \":empty\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
> +        "{\"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": \"int\"}, "
> +        "{\"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": \"str\"}, "
> +        "{\"members\": [], \"meta-type\": \"object\", \"name\": \":empty\"}, "
> +        "{\"members\": [{\"name\": \"arg1\", \"type\": \"UserDefOne\"}], \"meta-type\": \"object\", \"name\": \":obj-my-command-arg\"}, "
> +        "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"UserDefOne\"}, "
> +        "{\"arg-type\": \":obj-my-command-arg\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"UserDefOne\"}]";
> +    $ cat qapi-generated/example-qmp-introspect.h
> +[Uninteresting stuff omitted...]
> +
> +    #ifndef EXAMPLE_QMP_INTROSPECT_H
> +    #define EXAMPLE_QMP_INTROSPECT_H
> +
> +    extern const char example_qmp_schema_json[];
> +
> +    #endif

Overall, a nice documentation addition, and solves one of my missing
checkboxes against v3 :)

> +++ b/qapi-schema.json
> @@ -14,6 +14,9 @@
>  # Tracing commands
>  { 'include': 'qapi/trace.json' }
>  
> +# QAPI introspection
> +{ 'include': 'qapi/introspect.json' }

When we add QGA introspection, we'll want qapi/introspect.json to
contain JUST types, and move the 'command' into qapi-schema.json proper
(that way, qga/qapi-schema.json can also include the same types, then
add its variation on the command).  As it won't matter until we actually
do get to QGA, I'm okay whether you do the hoisting of the 'command' now
or save it for later.

> +++ b/qapi/introspect.json
> @@ -0,0 +1,271 @@

> +
> +##
> +# @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.

Same as for the docs above: may be worth a paragraph explicitly
mentioning that the wire ABI cannot express everything, such as integer
ranges, or such as semantics where exactly one of two mutually-exclusive
optional members must be present.

...
> +##
> +# @SchemaInfoBase
> +#
> +# Members common to any @SchemaInfo.
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'SchemaInfoBase',
> +  'data': { 'name': 'str', 'meta-type': 'SchemaMetaType' } }
> +
> +##
> +# @SchemaInfo
> +#
> +# @name: the entity's name, inherited from @base.

Interesting way to document @name (I probably would have documented
@base as part of SchemaInfoBase); but it worked out well.  No need to
change it :)

> +# Entities defined in the QAPI schema have the name defined in the schema.
> +# Implicitly defined entities have generated names.  See
> +# docs/qapi-code-gen.txt section "Client JSON Protocol introspection"
> +# for details.
> +#
> +# All references to other SchemaInfo are by name.
> +#
> +# Command and event names are part of the wire ABI, but type names are
> +# not.  Therefore, looking up a type by "well-known" name is wrong.
> +# Look up the command or event, then follow the references.
> +#
> +# @meta-type: the entity's meta type, inherited from @base.
> +#
> +# Additional members depend on the value of @meta-type.
> +#
> +# Since: 2.5
> +##
> +{ 'union': 'SchemaInfo',
> +  'base': 'SchemaInfoBase',
> +  'discriminator': 'meta-type',
> +  'data': {
> +      'builtin': 'SchemaInfoBuiltin',
> +      'enum': 'SchemaInfoEnum',
> +      'array': 'SchemaInfoArray',
> +      'object': 'SchemaInfoObject',
> +      'alternate': 'SchemaInfoAlternate',
> +      'command': 'SchemaInfoCommand',
> +      'event': 'SchemaInfoEvent' } }
> +
> +##
> +# @SchemaInfoBuiltin
> +#
> +# Additional SchemaInfo members for meta-type 'builtin'.
> +#
> +# @json-type: the JSON type used for this type on the wire.

Might be worth repeating the caveat in the earlier docs that @name is
not guaranteed to be stable, so clients should check @json-type.

(especially true if we later decide to expose range information by
adding more builtins or user-defined subtypes of builtins to represent
those ranged integers).

...
> +
> +##
> +# @SchemaInfoEnum
> +#
> +# Additional SchemaInfo members for meta-type 'enum'.
> +#
> +# @values: the enumeration type's values.
> +#
> +# Values of this type are JSON string on the wire.

I'm assuming that if we add sorting of elements in a later patch, we can
better document that guarantee here.

> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'SchemaInfoEnum',
> +  'data': { 'values': ['str'] } }
> +
> +##
> +# @SchemaInfoArray
> +#
> +# Additional SchemaInfo members for meta-type 'array'.
> +#
> +# @element-type: the array type's element type.
> +#
> +# Values of this type are JSON array on the wire.

Is it worth adding either of these clarifications?

As required by RFC 7159, the order of individual elements with in the
array sent over the wire is assumed to be significant unless the
documented semantics in qapi state otherwise.

While RFC 7159 permits an array to have elements of different types, all
elements of a QMP array should have the same type.

> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'SchemaInfoArray',
> +  'data': { 'element-type': 'str' } }
> +
> +##
> +# @SchemaInfoObject
> +#
> +# Additional SchemaInfo members for meta-type 'object'.
> +#
> +# @members: the object type's (non-variant) members.
> +#
> +# @tag: #optional the name of the member serving as type tag.  An
> +# element of @members with this name must exist.
> +#
> +# @variants: #optional variant members, i.e. additional members that
> +# depend on the type tag's value.  Present exactly when @tag is
> +# present.
> +#
> +# Values of this type are JSON object on the wire.

Worth adding this clarification?

As documented by RFC 7159, the wire format allows members of the JSON
object to be sent in any order, and requires that members must not be
duplicated.

> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'SchemaInfoObject',
> +  'data': { 'members': [ 'SchemaInfoObjectMember' ],
> +            '*tag': 'str',
> +            '*variants': [ 'SchemaInfoObjectVariant' ] } }
> +
> +##
> +# @SchemaInfoObjectMember
> +#
> +# An object member.
> +#
> +# @name: the member's name, as defined in the QAPI schema.
> +#
> +# @type: the name of the member's type.
> +#
> +# @default: #optional default when used as command parameter.
> +#           If absent, the parameter is mandatory.
> +#           If present, the value must be null.  The parameter is

Maybe:

s/The parameter/On the wire, the parameter/

> +#           optional, and behavior when it's missing is not specified
> +#           here.
> +#           Future extension: if present and non-null, the parameter
> +#	    is optional, and defaults to this value.

TAB damage.

> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'SchemaInfoObjectMember',
> +  'data': { 'name': 'str', 'type': 'str', '*default': 'any' } }
> +# @default's type must be null or match @type

Is this line still worth keeping, given the more complete explanation
above?  But we do need to use 'any' here, as that is currently the only
way for qapi-generated code to allow for JSON null.

> +
> +##
> +# @SchemaInfoObjectVariant
> +#
> +# The variant members for a value of the type tag.
> +#
> +# @case: a value of the type tag.
> +#
> +# @type: the name of the object type that provides the variant members
> +# when the type tag has value @case.
> +#
> +# Since: 2.5
> +##
> +{ 'struct': 'SchemaInfoObjectVariant',
> +  'data': { 'case': 'str', 'type': 'str' } }

Do we want to document whether all case values of the tag are guaranteed
to be covered (happens to be true for all current qapi, but is not yet
enforced; although that's one of my planned followup patches is to start
enforcing it - so I guess I can document it at that time if we don't do
it here)

> +##
> +# @SchemaInfoCommand
> +#
> +# Additional SchemaInfo members for meta-type 'command'.
> +#
> +# @arg-type: the name of the object type that provides the command's
> +# parameters.
> +#
> +# @ret-type: the name of the command's result type.
> +#
> +# TODO @success-response (currently irrelevant, because it's QGA, not QMP)

and my idea of using 'ret-type':null may obviate the need for an actual
'success-response' parameter.

> +++ b/scripts/qapi-introspect.py
> @@ -0,0 +1,174 @@
> +#
> +# QAPI introspection generator
> +#
> +# Copyright (C) 2015 Red Hat, Inc.
> +#
> +# Authors:
> +#  Markus Armbruster <armbru@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.

No 'or later' clause?  Oh, because your hands are tied: scripts/qapi.py
is GPLv2-only.  Yuck. I wonder if we can fix that; although it may be
hard getting Anthony to explain his original choice.  Separate cleanup,
if at all. [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)

s/2.4/python 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))

This relies on 'key' never having any embedded ".  It happens to work
given our naming conventions of what we pass through this dumper, but
might be safer if you used key.replace('"', r'\"') to handle generic
dumping.

> +
> +    def _gen_member(self, member):
> +        ret = {'name': member.name, 'type': self._use_type(member.type)}
> +        if member.optional:
> +            ret['default'] = None
> +        return ret
> +
> +    def _gen_variants(self, tag_name, variants):
> +        return {'tag': tag_name or 'type',

[1] Ouch. Why are we still printing 'kind' as the name for simple
unions?  Oh, it's because tag_name is ALWAYS provided by the visitor, so
the "or 'type'" clause never fires.  I have a pending patch to change
the C code to generate 'type' as the C code name to match the wire ABI;
but until that patch is in, I think a hack solution might be to fix the
visitor to supply None instead of a tag_name for simple unions.  I'll
respond to the appropriate earlier patch.

> +h_comment = '''
> +/*
> + * QAPI/QMP schema introspection
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */

[2] It's interesting that you are following the pattern of
scripts/qapi-commands.py (commit c17d9908); even there, the script
itself was GPLv2-only, while the generated output was LGPLv2+.
Eric Blake Sept. 4, 2015, 3:12 a.m. UTC | #5
On 09/03/2015 08:03 PM, Eric Blake wrote:
> On 09/03/2015 08:30 AM, Markus Armbruster wrote:
>> qapi/introspect.json defines the introspection schema.  It's designed
>> for QMP introspection, but should do for similar uses, such as QGA.
> 
> [review in this sub-thread; for comments on 'int' munging or other
> followups, see other subthread]
> 
> There is at least one definite bug (see multiple references below to
> [1]), and several ideas for cleanups, but in general, I think that the
> remaining changes are going to be small enough that I'd probably be okay
> if v5 started life with:
> Reviewed-by: Eric Blake <eblake@redhat.com>
> (Of course, you have every right to not add the R-b, especially if you
> want me to do another close review :)
> 

> 
>> +
>> +    { "name": "BlockdevOptions", "meta-type": "object",
>> +      "members": [
>> +          { "name": "kind", "type": "BlockdevOptionsKind" } ],
> 
> [1] Ouch.  That should be "name": "type".  I pointed out the bug in v3,
> but it looks like it still hasn't been fixed.  Or rather, that our
> attempt to fix it wasn't correct.
> 

>> +    def _gen_variants(self, tag_name, variants):
>> +        return {'tag': tag_name or 'type',
> 
> [1] Ouch. Why are we still printing 'kind' as the name for simple
> unions?  Oh, it's because tag_name is ALWAYS provided by the visitor, so
> the "or 'type'" clause never fires.

Not quite the right comment.  'tag' is being output correctly (tag_name
was indeed None), what was wrong was the 'members':[] array (which was
assuming the member was named 'kind').

>   I have a pending patch to change
> the C code to generate 'type' as the C code name to match the wire ABI;
> but until that patch is in, I think a hack solution might be to fix the
> visitor to supply None instead of a tag_name for simple unions.  I'll
> respond to the appropriate earlier patch.

Rather, 3 earlier patches.  See my comments on 2, 10, and 11.
Markus Armbruster Sept. 4, 2015, 7:19 a.m. UTC | #6
Michael Roth <mdroth@linux.vnet.ibm.com> writes:

> Quoting Eric Blake (2015-09-03 15:50:16)
>> On 09/03/2015 08:30 AM, Markus Armbruster wrote:
>> > qapi/introspect.json defines the introspection schema.  It's designed
>> > for QMP introspection, but should do for similar uses, such as QGA.
>> 
>> [review to follow in separate message; I'm using this message to focus
>> on one point for easier tracking of the sub-thread]
>> 
>> > 
>> > 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?
>> 
>> So, given the discussion on v3, I think we will have consensus if v5
>> does the following:
>> 
>> - Merge 30 and 31 into a single patch, and drop this TODO statement (I
>> think we have agreement that exposing QMP [that is, JSON number] wire
>> form, with no additional constraints, as a single 'int', rather than the
>> underlying generated C types, is the way to go), provided that...
>> - Document in the qapi side that the schema intentionally lacks details
>> on ranges, and that consulting the qapi and/or C code may be required to
>> see what actual values are valid anywhere introspection merely says 'int',

Please review docs/qapi-code-gen.txt in this patch for this aspect.

>> - leave 32 as a separate patch, as it is complex enough, and still may
>> have debate on whether the types '123' and 'int' should have
>> corresponding array types '[123]' and '[int]' rather than the v3 patch
>> munging of '456' and '789' (I haven't yet reviewed whether v4 changed that).
>
> Agreed on all 3

Excellent.

>> Any further arguments on whether exposing just 'int' in the
>> introspection for all integral types, and/or whether patches 30 and 31
>> should be merged, are best made in response to this mail.
>
> Given the above, I don't think I have any further objections.

Thanks!
Markus Armbruster Sept. 4, 2015, 8:54 a.m. UTC | #7
Michael Roth <mdroth@linux.vnet.ibm.com> writes:

> 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 <armbru@redhat.com>
>> ---
>>  .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 <armbru@redhat.com>
>> +#
>> +# 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."

Hmm, my paragraph tries to explain something else, namely that the QAPI
schema defines purely internal interfaces in addition to QMP interfaces.
The latter consist of the QMP wire interface and the internal QMP
interface.  Generated code translates between the two.

Furthermore, "additional restrictions" apply not only to "values of
individual fields", but also to structure.  For instance,
SchemaInfoObject members tag and variants must both be present or both
be absent.

What about having this somewhere:

    The SchemaInfo can't reflect all the rules and restrictions that
    apply to QMP.  It's interface introspection (figuring out what's
    there), not interface specification.  The specification is in the
    QAPI schema, and users should peruse it to understand how QMP is to
    be used.

Then continue with an example showing a restriction that's spelled out
in the QAPI schema, but can't be expressed in SchemaInfo.

> <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.

qmp-commands.hx needs to die.  

Until it's dead, some information needs to be duplicated there.  It
already points to docs/qapi-code-gen.txt, which in turn points to the
QAPI schema.  I can add a direct pointer if you think it'll help.

>> +
>> +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 <armbru@redhat.com>
>> +#
>> +# 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.

The obvious Python JSON library is "json — JSON encoder and decoder"[*],
which provides pretty-printing, but it's 2.6, and we're stuck in 2.4.

I tried to structure the code in a way that makes dropping in the
library relatively painless, see function to_json() above.

>> +        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

[*] https://docs.python.org/2.7/library/json.html
Markus Armbruster Sept. 4, 2015, 9:55 a.m. UTC | #8
Eric Blake <eblake@redhat.com> writes:

> On 09/03/2015 08:30 AM, Markus Armbruster wrote:
>> qapi/introspect.json defines the introspection schema.  It's designed
>> for QMP introspection, but should do for similar uses, such as QGA.
>
> [review in this sub-thread; for comments on 'int' munging or other
> followups, see other subthread]
>
> There is at least one definite bug (see multiple references below to
> [1]), and several ideas for cleanups, but in general, I think that the
> remaining changes are going to be small enough that I'd probably be okay
> if v5 started life with:
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

> (Of course, you have every right to not add the R-b, especially if you
> want me to do another close review :)

Judgement call, as always :)

>> 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.
>> 
>> 
>>   The empty object type is used when a command takes no arguments or
>>   produces no results.
>
> We had discussed whether to expose the empty type in a separate patch,
> to reduce the size of this patch;

I decided against it, because I feel the churn between revisions of the
series would outweigh the relatively small reduction in this patch's
size, and...

>                                   but at this point in the game, I'm
> happy to keep it as-is (the faster we get v5 out for review and into the
> tree, the faster we can process the backlog of followup patches).

... I also feel we should get this series wrapped as soon as possible.

>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>
>> +++ b/docs/qapi-code-gen.txt
>> @@ -494,13 +494,195 @@ Resulting in this JSON object:
>>    "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
>>  
>>  
>> +== Client JSON Protocol introspection ==
>> +
>> +Clients of a Client JSON Protocol commonly need to figure out what
>> +exactly the server (QEMU) supports.
>> +
>> +For this purpose, QMP provides introspection via command query-schema.
>
> Will need tweaking to the agreed-on name query-qmp-schema...

Yup.

>> +QGA currently doesn't support introspection.
>
> ... (and we'd obviously want qga to eventually gain a matching
> counterpart, possibly 'guest-schema' or 'guest-get-schema', to match
> prevailing naming in qga/qapi-schema.json).

Think so, but I don't wish to tell the QGA maintainers what they need,
so I chose neutral language.

>> +
>> +query-schema returns a JSON array of SchemaInfo objects.  These
>> +objects together describe the wire ABI, as defined in the QAPI schema.
>
> Maybe an additional sentence here, perhaps along the lines of:
>
> Note that the wire format cannot express everything; it is not designed
> to show semantic constraints (such as when exactly one of a pair of
> mutually-exclusive optional members must be present, or when an integral
> value has specific limitations on valid values).  Introspection allows
> an application to know if a feature is present, but the application must
> obey the qapi documentation to properly interact with that feature.

Michael raised the same point, and I drafted a paragraph in my reply to
him:

    The SchemaInfo can't reflect all the rules and restrictions that
    apply to QMP.  It's interface introspection (figuring out what's
    there), not interface specification.  The specification is in the
    QAPI schema, and users should peruse it to understand how QMP is to
    be used.

Then continue with an example showing a restriction that's spelled out
in the QAPI schema, but can't be expressed in SchemaInfo.

Use mine as is?  Merge our two proposals somehow?

>> +
>> +Like any other command, query-schema is itself defined in the QAPI
>> +schema, along with the SchemaInfo type.  This text attempts to give an
>> +overview how things work.  For details you need to consult the QAPI
>> +schema.
>> +
>> +SchemaInfo objects have common members "name" and "meta-type", and
>> +additional variant members depending on the value of meta-type.
>> +
>> +Each SchemaInfo object describes a wire ABI entity of a certain
>> +meta-type: a command, event or one of several kinds of type.
>> +
>> +SchemaInfo for entities defined in the QAPI schema have the same name
>> +as in the schema.  This is the case for all commands and events, and
>> +most types.
>
> And hopefully patch 32 munges this text to match the simplification made
> there (I haven't checked yet).

It does :)

>> +Command and event names are part of the wire ABI, but type names are
>> +not.  Therefore, looking up a type by its name in the QAPI schema is
>> +wrong.  Look up the command or event, then follow references by name.
>> +
>> +QAPI schema definitions not reachable that way are omitted.
>> +
>> +The SchemaInfo for a command has meta-type "command", and variant
>> +members "arg-type" and "ret-type".  The arguments member clients pass
>
> I found it easier to read with:
> s/arguments/arguments that/

Easier to read, but no longer emphasizes that the specific "arguments"
member of the { "execute" ... } form needs to conform.  I'll use yours
anyway, unless you have a better idea.

>> +with a command on the wire must conform to the type named by
>> +"arg-type", which is always an object type.  The return value the
>
> Likewise s/value/value that/

Likewise.

>> +server passes in a success response conforms to the type named by
>> +"ret-type".
>> +
>
> [not for this patch. Thinking aloud: I wonder if QGA's success-response
> could be coded in by having 'ret-type':null as a way of saying the
> command will have no response if it is successful; perhaps a bit nicer
> than exposing an additional boolean flag member]

The QAPI schema could also do it this way.  What do you think?

>> +If the command takes no arguments, "arg-type" names an object type
>> +without members.  Likewise, if the command returns nothing, "ret-type"
>> +names an object type without members.
>> +
>> +Example: the SchemaInfo for command query-schema
>> +
>> +    { "name": "query-schema", "meta-type": "command",
>> +      "arg-type": ":empty", "ret-type": "SchemaInfoList" }
>> +
>> +    Type ":empty" is an object type without members, and type
>> +    "SchemaInfoList" is the array of SchemaInfo type.
>> +
>> +The SchemaInfo for an event has meta-type "event", and variant member
>> +"arg-type".  The data member the server passes with an event conforms
>
> again, reads a little better with s/member/member that/

My remark on command arguments applies.

>> +to the type named by "arg-type".  It is always an object type.
>> +
>> +If the event carries no additional information, "arg-type" names an
>> +object type without members.  The event may not have a data member on
>> +the wire then.
>> +
>> +Each command or event defined with dictionary-valued 'data' in the
>> +QAPI schema implicitly defines an object type called ":obj-NAME-arg",
>> +where NAME is the command or event's name.
>> +
>> +Example: the SchemaInfo for EVENT_C from section Events
>> +
>> +    { "name": "EVENT_C", "meta-type": "event",
>> +      "arg-type": ":obj-EVENT_C-arg" }
>> +
>> +    Type ":obj-EVENT_C-arg" is an implicitly defined object type with
>> +    the two members from the event's definition.
>> +
>> +The SchemaInfo for struct and union types has meta-type "object".
>> +
>> +The SchemaInfo for a struct type has variant member "members".
>
> Worth mentioning that "members" may be empty, for an empty struct?

Its value gets explained below.  I feel discussing the special case
"empty" here would be premature.

>> +
>> +The SchemaInfo for a union type additionally has variant members "tag"
>> +and "variants".
>
> Worth mentioning that "members" will never be empty, because it contains
> at least the member also referenced in "tag"?

Likewise.

>> +
>> +"members" is a JSON array describing the object's common members.

Could say "the object's common members, if any."  What do you think?

>> +Each element is a JSON object with members "name" (the member's name),
>> +"type" (the name of its type), and optionally "default".  The member
>> +is optional if "default" is present.  Currently, "default" can only
>> +have value null.  Other values are reserved for future extensions.
>> +
>> +Example: the SchemaInfo for MyType from section Struct types
>> +
>> +    { "name": "MyType", "meta-type": "object",
>> +      "members": [
>> +          { "name": "member1", "type": "str" },
>> +          { "name": "member2", "type": "int" },
>> +          { "name": "member3", "type": "str", "default": null } ] }
>> +
>> +"tag" is the name of the common member serving as type tag.

This implies that this member must exist in "members".  Want me to
repeat that explicitly?

>> +"variants" is a JSON array describing the object's variant members.
>> +Each element is a JSON object with members "case" (the value of type
>> +tag this element applies to) and "type" (the name of an object type
>> +that provides the variant members for this type tag value).
>> +
>> +Example: the SchemaInfo for flat union BlockdevOptions from section
>> +Union types
>
> Hmm, the "Union types" section has two mentions of BlockdevOptions; and
> this example matches the second. It may help readability if we rename
> one of the two examples to be distinct (preferably the first, since it
> doesn't match actual QMP).  I guess the phrase "flat union
> BlockdevOptions" is sufficient to make it obvious that we are referring
> to the second usage, but it is subtle.

Follow-up patch?

>> +
>> +    { "name": "BlockdevOptions", "meta-type": "object",
>> +      "members": [
>> +          { "name": "driver", "type": "BlockdevDriver" },
>> +          { "name": "readonly", "type": "bool"} ],
>> +      "tag": "driver",
>> +      "variants": [
>> +          { "case": "file", "type": "FileOptions" },
>> +          { "case": "qcow2", "type": "Qcow2Options" } ] }
>> +
>> +Note that base types are "flattened": its members are included in the
>> +"members" array.
>> +
>> +A simple union implicitly defines an enumeration type for its implicit
>> +discriminator (called "type" on the wire, see section Union types).
>> +Such a type's name is made by appending "Kind" to the simple union's
>> +name.
>> +
>> +A simple union implicitly defines an object type for each of its
>> +variants.  The type's name is ":obj-NAME-wrapper", where NAME is the
>> +name of the name of the variant's type.
>> +
>> +Example: the SchemaInfo for simple union BlockdevOptions from section
>> +Union types
>
> Ah, and here you refer to the other BlockdevOptions. So the point about
> a judicious rename may still be warranted.
>
>> +
>> +    { "name": "BlockdevOptions", "meta-type": "object",
>> +      "members": [
>> +          { "name": "kind", "type": "BlockdevOptionsKind" } ],
>
> [1] Ouch.  That should be "name": "type".  I pointed out the bug in v3,
> but it looks like it still hasn't been fixed.  Or rather, that our
> attempt to fix it wasn't correct.

I'll double-check.

>> +      "tag": "type",
>> +      "variants": [
>> +          { "case": "file", "type": ":obj-FileOptions-wrapper" },
>> +          { "case": "qcow2", "type": ":obj-Qcow2Options-wrapper" } ] }
>> +
>> +    Enumeration type "BlockdevOptionsKind" and the object types
>> +    ":obj-FileOptions-wrapper", ":obj-Qcow2Options-wrapper" are
>> +    implicitly defined.
>> +
>> +The SchemaInfo for an alternate type has meta-type "alternate", and
>> +variant member "members".  "members" is a JSON array.  Each element is
>> +a JSON object with member "type", which names a type.  Values of the
>> +alternate type conform to exactly one of its member types.
>> +
>> +Example: the SchemaInfo for BlockRef from section Alternate types
>> +
>> +    { "name": "BlockRef", "meta-type": "alternate",
>> +      "members": [
>> +          { "type": "BlockdevOptions" },
>> +          { "type": "str" } ] }
>> +
>> +The SchemaInfo for an array type has meta-type "array", and variant
>> +member "element-type", which names the array's element type.  Array
>> +types are implicitly defined.  An array type's name is made by
>> +appending "List" to its element type's name.
>> +
>> +Example: the SchemaInfo for ['str']
>> +
>> +    { "name": "strList", "meta-type": "array",
>> +      "element-type": "str" }
>> +
>> +The SchemaInfo for an enumeration type has meta-type "enum" and
>> +variant member "values".
>> +
>> +Example: the SchemaInfo for MyEnum from section Enumeration types
>> +
>> +    { "name": "MyEnum", "meta-type": "enum",
>> +      "values": [ "value1", "value2", "value3" ] }
>> +
>> +The SchemaInfo for a built-in type has the same name as the type in
>> +the QAPI schema (see section Built-in Types).  It has variant member
>> +"json-type" that shows how values of this type are encoded on the
>> +wire.
>> +
>> +Example: the SchemaInfo for str
>> +
>> +    { "name": "str", "meta-type": "builtin", "json-type": "string" }
>> +
>> +As explained above, type names are not part of the wire ABI.  Not even
>> +the names of built-in types.  Clients should examine member
>> +"json-type" instead of hard-coding names of built-in types.
>
> Good point (although we may still end up with clients that disobey this,
> I will certainly make sure to do this in libvirt's interactions).

We can't fully prevent misuse of our interfaces.  Where we can't, we can
still try to make correct use as easy and obvious as we can.

PATCH 32 prevents misuse of most type names by hiding them.  RFC v2 hid
all type names.  In review, we decided not to hide the names of built-in
types.  Tradeoff: makes the introspection value easier to read for
humans (good), but enables misuse by machines (bad).

Either way works for me, hide non-built-in type names (and have docs
tell users not to rely on them), or hide them all.  Got a preference?

>> +
>> +
>>  == Code generation ==
>>  
>> -Schemas are fed into 3 scripts to generate all the code/files that, paired
>> -with the core QAPI libraries, comprise everything required to take JSON
>> -commands read in by a Client JSON Protocol server, unmarshal the arguments into
>> -the underlying C types, call into the corresponding C function, and map the
>> -response back to a Client JSON Protocol response to be returned to the user.
>> +Schemas are fed into four scripts to generate all the code/files that,
>> +paired with the core QAPI libraries, comprise everything required to
>> +take JSON commands read in by a Client JSON Protocol server, unmarshal
>> +the arguments into the underlying C types, call into the corresponding
>> +C function, and map the response back to a Client JSON Protocol
>> +response to be returned to the user.
>
> Doesn't mention introspection; but then again, it also doesn't mention
> that it generates the C interface for emitting events.

Good enough for me now, but I'm of course happy to consider proposed
improvements.

>>  
>>  As an example, we'll use the following schema, which describes a single
>>  complex user-defined type (which will produce a C struct, along with a list
>> @@ -848,3 +1030,37 @@ Example:
>>      extern const char *const example_QAPIEvent_lookup[];
>>  
>>      #endif
>> +
>> +=== scripts/qapi-introspect.py ===
>> +
>> +Used to generate the introspection C code for a schema. The following
>> +files are created:
>> +
>> +$(prefix)qmp-introspect.c - Defines a string holding a JSON
>> +                            description of the schema.
>> +$(prefix)qmp-introspect.h - Declares the above string.
>> +
>> +Example:
>> +
>> +    $ python scripts/qapi-introspect.py --output-dir="qapi-generated"
>> +    --prefix="example-" example-schema.json
>> +    $ cat qapi-generated/example-qmp-introspect.c
>> +[Uninteresting stuff omitted...]
>> +
>> +    const char example_qmp_schema_json[] = "["
>> + "{\"arg-type\": \":empty\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
>> + "{\"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": \"int\"}, "
>> + "{\"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": \"str\"}, "
>> + "{\"members\": [], \"meta-type\": \"object\", \"name\": \":empty\"}, "
>> + "{\"members\": [{\"name\": \"arg1\", \"type\": \"UserDefOne\"}], \"meta-type\": \"object\", \"name\": \":obj-my-command-arg\"}, "
>> + "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"UserDefOne\"}, "
>> + "{\"arg-type\": \":obj-my-command-arg\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"UserDefOne\"}]";
>> +    $ cat qapi-generated/example-qmp-introspect.h
>> +[Uninteresting stuff omitted...]
>> +
>> +    #ifndef EXAMPLE_QMP_INTROSPECT_H
>> +    #define EXAMPLE_QMP_INTROSPECT_H
>> +
>> +    extern const char example_qmp_schema_json[];
>> +
>> +    #endif
>
> Overall, a nice documentation addition, and solves one of my missing
> checkboxes against v3 :)

Thanks!

>> +++ b/qapi-schema.json
>> @@ -14,6 +14,9 @@
>>  # Tracing commands
>>  { 'include': 'qapi/trace.json' }
>>  
>> +# QAPI introspection
>> +{ 'include': 'qapi/introspect.json' }
>
> When we add QGA introspection, we'll want qapi/introspect.json to
> contain JUST types, and move the 'command' into qapi-schema.json proper
> (that way, qga/qapi-schema.json can also include the same types, then
> add its variation on the command).  As it won't matter until we actually
> do get to QGA, I'm okay whether you do the hoisting of the 'command' now
> or save it for later.

Yes, let's keep things simple now, and complicate them only as needed.

>> +++ b/qapi/introspect.json
>> @@ -0,0 +1,271 @@
>
>> +
>> +##
>> +# @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.
>
> Same as for the docs above: may be worth a paragraph explicitly
> mentioning that the wire ABI cannot express everything, such as integer
> ranges, or such as semantics where exactly one of two mutually-exclusive
> optional members must be present.

Here's where Michael commented.

> ...
>> +##
>> +# @SchemaInfoBase
>> +#
>> +# Members common to any @SchemaInfo.
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'SchemaInfoBase',
>> +  'data': { 'name': 'str', 'meta-type': 'SchemaMetaType' } }
>> +
>> +##
>> +# @SchemaInfo
>> +#
>> +# @name: the entity's name, inherited from @base.
>
> Interesting way to document @name (I probably would have documented
> @base as part of SchemaInfoBase); but it worked out well.  No need to
> change it :)

Okay, thanks :)

>> +# Entities defined in the QAPI schema have the name defined in the schema.
>> +# Implicitly defined entities have generated names.  See
>> +# docs/qapi-code-gen.txt section "Client JSON Protocol introspection"
>> +# for details.
>> +#
>> +# All references to other SchemaInfo are by name.
>> +#
>> +# Command and event names are part of the wire ABI, but type names are
>> +# not.  Therefore, looking up a type by "well-known" name is wrong.
>> +# Look up the command or event, then follow the references.
>> +#
>> +# @meta-type: the entity's meta type, inherited from @base.
>> +#
>> +# Additional members depend on the value of @meta-type.
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'union': 'SchemaInfo',
>> +  'base': 'SchemaInfoBase',
>> +  'discriminator': 'meta-type',
>> +  'data': {
>> +      'builtin': 'SchemaInfoBuiltin',
>> +      'enum': 'SchemaInfoEnum',
>> +      'array': 'SchemaInfoArray',
>> +      'object': 'SchemaInfoObject',
>> +      'alternate': 'SchemaInfoAlternate',
>> +      'command': 'SchemaInfoCommand',
>> +      'event': 'SchemaInfoEvent' } }
>> +
>> +##
>> +# @SchemaInfoBuiltin
>> +#
>> +# Additional SchemaInfo members for meta-type 'builtin'.
>> +#
>> +# @json-type: the JSON type used for this type on the wire.
>
> Might be worth repeating the caveat in the earlier docs that @name is
> not guaranteed to be stable, so clients should check @json-type.

Yes, the QAPI schema should contain all the important information.
Terse is okay.

"Earlier docs" = docs/qapi-code-gen.txt.  That one should be less terse,
but doesn't have to cover all the details.

> (especially true if we later decide to expose range information by
> adding more builtins or user-defined subtypes of builtins to represent
> those ranged integers).
>
> ...
>> +
>> +##
>> +# @SchemaInfoEnum
>> +#
>> +# Additional SchemaInfo members for meta-type 'enum'.
>> +#
>> +# @values: the enumeration type's values.
>> +#
>> +# Values of this type are JSON string on the wire.
>
> I'm assuming that if we add sorting of elements in a later patch, we can
> better document that guarantee here.

Yes.

>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'SchemaInfoEnum',
>> +  'data': { 'values': ['str'] } }
>> +
>> +##
>> +# @SchemaInfoArray
>> +#
>> +# Additional SchemaInfo members for meta-type 'array'.
>> +#
>> +# @element-type: the array type's element type.
>> +#
>> +# Values of this type are JSON array on the wire.
>
> Is it worth adding either of these clarifications?
>
> As required by RFC 7159, the order of individual elements with in the
> array sent over the wire is assumed to be significant unless the
> documented semantics in qapi state otherwise.

Got an example where insignificance of order actually matters?

Need to check for overlap with docs/qmp/qmp-spec.txt.

> While RFC 7159 permits an array to have elements of different types, all
> elements of a QMP array should have the same type.

Implied by having an element-type, but making it explicit won't hurt.
The new 'any' type lets us do inhomogeneous arrays, though: ['any'].  If
I can think of a concise way to clarify all that, I'll add it.  If not,
we can add it later.

>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'SchemaInfoArray',
>> +  'data': { 'element-type': 'str' } }
>> +
>> +##
>> +# @SchemaInfoObject
>> +#
>> +# Additional SchemaInfo members for meta-type 'object'.
>> +#
>> +# @members: the object type's (non-variant) members.
>> +#
>> +# @tag: #optional the name of the member serving as type tag.  An
>> +# element of @members with this name must exist.
>> +#
>> +# @variants: #optional variant members, i.e. additional members that
>> +# depend on the type tag's value.  Present exactly when @tag is
>> +# present.
>> +#
>> +# Values of this type are JSON object on the wire.
>
> Worth adding this clarification?
>
> As documented by RFC 7159, the wire format allows members of the JSON
> object to be sent in any order, and requires that members must not be
> duplicated.

Need to check for overlap with docs/qmp/qmp-spec.txt.

>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'SchemaInfoObject',
>> +  'data': { 'members': [ 'SchemaInfoObjectMember' ],
>> +            '*tag': 'str',
>> +            '*variants': [ 'SchemaInfoObjectVariant' ] } }
>> +
>> +##
>> +# @SchemaInfoObjectMember
>> +#
>> +# An object member.
>> +#
>> +# @name: the member's name, as defined in the QAPI schema.
>> +#
>> +# @type: the name of the member's type.
>> +#
>> +# @default: #optional default when used as command parameter.
>> +#           If absent, the parameter is mandatory.
>> +#           If present, the value must be null.  The parameter is
>
> Maybe:
>
> s/The parameter/On the wire, the parameter/

Okay.

>> +#           optional, and behavior when it's missing is not specified
>> +#           here.
>> +#           Future extension: if present and non-null, the parameter
>> +#	    is optional, and defaults to this value.
>
> TAB damage.

Will fix, and search for more.

>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'SchemaInfoObjectMember',
>> +  'data': { 'name': 'str', 'type': 'str', '*default': 'any' } }
>> +# @default's type must be null or match @type
>
> Is this line still worth keeping, given the more complete explanation
> above?  But we do need to use 'any' here, as that is currently the only
> way for qapi-generated code to allow for JSON null.

I'll drop it.  We'll add it back with support for non-null values
(assuming we get around to that).

>> +
>> +##
>> +# @SchemaInfoObjectVariant
>> +#
>> +# The variant members for a value of the type tag.
>> +#
>> +# @case: a value of the type tag.
>> +#
>> +# @type: the name of the object type that provides the variant members
>> +# when the type tag has value @case.
>> +#
>> +# Since: 2.5
>> +##
>> +{ 'struct': 'SchemaInfoObjectVariant',
>> +  'data': { 'case': 'str', 'type': 'str' } }
>
> Do we want to document whether all case values of the tag are guaranteed
> to be covered (happens to be true for all current qapi, but is not yet
> enforced; although that's one of my planned followup patches is to start
> enforcing it - so I guess I can document it at that time if we don't do
> it here)

Requiring all cases to be covered explicitly may have value in the QAPI
schema (more verbose, but you get a reminder to update the union after
you extended the tag enum), but it's pretty useless in introspection,
isn't it?

>> +##
>> +# @SchemaInfoCommand
>> +#
>> +# Additional SchemaInfo members for meta-type 'command'.
>> +#
>> +# @arg-type: the name of the object type that provides the command's
>> +# parameters.
>> +#
>> +# @ret-type: the name of the command's result type.
>> +#
>> +# TODO @success-response (currently irrelevant, because it's QGA, not QMP)
>
> and my idea of using 'ret-type':null may obviate the need for an actual
> 'success-response' parameter.

So let's read the TODO as "need to cover @success-response somehow for
QGA" :)

>> +++ b/scripts/qapi-introspect.py
>> @@ -0,0 +1,174 @@
>> +#
>> +# QAPI introspection generator
>> +#
>> +# Copyright (C) 2015 Red Hat, Inc.
>> +#
>> +# Authors:
>> +#  Markus Armbruster <armbru@redhat.com>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2.
>
> No 'or later' clause?  Oh, because your hands are tied: scripts/qapi.py
> is GPLv2-only.  Yuck.

Since I wrote this file from scratch, and even the qapi.py interface it
uses is pretty much mine, I guess I could use "or later" here.

>                       I wonder if we can fix that; although it may be
> hard getting Anthony to explain his original choice.  Separate cleanup,
> if at all. [2]

I truly wish we'd simply told all contributors from the start "this is
our license, take it or leave it".

>> +# 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)
>
> s/2.4/python 2.4/

Okay.

>> +# 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))
>
> This relies on 'key' never having any embedded ".  It happens to work
> given our naming conventions of what we pass through this dumper, but
> might be safer if you used key.replace('"', r'\"') to handle generic
> dumping.

Either that, or assert.

>> +
>> +    def _gen_member(self, member):
>> +        ret = {'name': member.name, 'type': self._use_type(member.type)}
>> +        if member.optional:
>> +            ret['default'] = None
>> +        return ret
>> +
>> +    def _gen_variants(self, tag_name, variants):
>> +        return {'tag': tag_name or 'type',
>
> [1] Ouch. Why are we still printing 'kind' as the name for simple
> unions?  Oh, it's because tag_name is ALWAYS provided by the visitor, so
> the "or 'type'" clause never fires.  I have a pending patch to change
> the C code to generate 'type' as the C code name to match the wire ABI;
> but until that patch is in, I think a hack solution might be to fix the
> visitor to supply None instead of a tag_name for simple unions.  I'll
> respond to the appropriate earlier patch.

You followed up with patches, and I'll peruse them after lunch.

>> +h_comment = '''
>> +/*
>> + * QAPI/QMP schema introspection
>> + *
>> + * Copyright (C) 2015 Red Hat, Inc.
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version
> 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>
> [2] It's interesting that you are following the pattern of
> scripts/qapi-commands.py (commit c17d9908); even there, the script
> itself was GPLv2-only, while the generated output was LGPLv2+.
Eric Blake Sept. 4, 2015, 1:58 p.m. UTC | #9
On 09/04/2015 03:55 AM, Markus Armbruster wrote:

>>> +
>>> +query-schema returns a JSON array of SchemaInfo objects.  These
>>> +objects together describe the wire ABI, as defined in the QAPI schema.
>>
>> Maybe an additional sentence here, perhaps along the lines of:
>>
>> Note that the wire format cannot express everything; it is not designed
>> to show semantic constraints (such as when exactly one of a pair of
>> mutually-exclusive optional members must be present, or when an integral
>> value has specific limitations on valid values).  Introspection allows
>> an application to know if a feature is present, but the application must
>> obey the qapi documentation to properly interact with that feature.
> 
> Michael raised the same point, and I drafted a paragraph in my reply to
> him:
> 
>     The SchemaInfo can't reflect all the rules and restrictions that
>     apply to QMP.  It's interface introspection (figuring out what's
>     there), not interface specification.  The specification is in the
>     QAPI schema, and users should peruse it to understand how QMP is to
>     be used.
> 
> Then continue with an example showing a restriction that's spelled out
> in the QAPI schema, but can't be expressed in SchemaInfo.
> 
> Use mine as is?  Merge our two proposals somehow?

Your wording works okay for me, but if you want to merge any elements of
my wording in, go for it.


>>> +The SchemaInfo for a command has meta-type "command", and variant
>>> +members "arg-type" and "ret-type".  The arguments member clients pass
>>
>> I found it easier to read with:
>> s/arguments/arguments that/
> 
> Easier to read, but no longer emphasizes that the specific "arguments"
> member of the { "execute" ... } form needs to conform.  I'll use yours
> anyway, unless you have a better idea.
> 
>>> +with a command on the wire must conform to the type named by
>>> +"arg-type", which is always an object type.  The return value the
>>
>> Likewise s/value/value that/
> 
> Likewise.
> 
>>> +server passes in a success response conforms to the type named by
>>> +"ret-type".

Maybe:

On the wire, the "arguments" key of a client's "execute" command must
conform to the type named by "arg-type", which is always an object type.
The "return" key that the server passes in a success response will
always conform to the type named by "ret-type".

Up to you; we'll see how it looks in v5, and at that point, any further
word-smithing can be done in followup patches rather than delaying this
series further.

>> [not for this patch. Thinking aloud: I wonder if QGA's success-response
>> could be coded in by having 'ret-type':null as a way of saying the
>> command will have no response if it is successful; perhaps a bit nicer
>> than exposing an additional boolean flag member]
> 
> The QAPI schema could also do it this way.  What do you think?

Sounds like I have another trivial followup patch added to my work queue :)


>>> +The SchemaInfo for an event has meta-type "event", and variant member
>>> +"arg-type".  The data member the server passes with an event conforms
>>
>> again, reads a little better with s/member/member that/
> 
> My remark on command arguments applies.
> 
>>> +to the type named by "arg-type".  It is always an object type.

Maybe:

On the wire, the "data" member included in any event passed by the
server will conform to the type named by "arg-type".

I'm not sure if we need to document whether it is always an object type;
command arguments are user-supplied, while event data is
server-supplied.  And there's the question of any possible duplication
with the QMP spec file.  I guess I'm fine as long as we aren't stating
conflicting things between the two places.


>>> +
>>> +"members" is a JSON array describing the object's common members.
> 
> Could say "the object's common members, if any."  What do you think?

Yes, adding the 'if any' helps.


>>
>> Hmm, the "Union types" section has two mentions of BlockdevOptions; and
>> this example matches the second. It may help readability if we rename
>> one of the two examples to be distinct (preferably the first, since it
>> doesn't match actual QMP).  I guess the phrase "flat union
>> BlockdevOptions" is sufficient to make it obvious that we are referring
>> to the second usage, but it is subtle.
> 
> Follow-up patch?

Yes.

>>
>> [1] Ouch.  That should be "name": "type".  I pointed out the bug in v3,
>> but it looks like it still hasn't been fixed.  Or rather, that our
>> attempt to fix it wasn't correct.
> 
> I'll double-check.

As you noted later, I've already done that legwork.


>>> +
>>> +As explained above, type names are not part of the wire ABI.  Not even
>>> +the names of built-in types.  Clients should examine member
>>> +"json-type" instead of hard-coding names of built-in types.
>>
>> Good point (although we may still end up with clients that disobey this,
>> I will certainly make sure to do this in libvirt's interactions).
> 
> We can't fully prevent misuse of our interfaces.  Where we can't, we can
> still try to make correct use as easy and obvious as we can.
> 
> PATCH 32 prevents misuse of most type names by hiding them.  RFC v2 hid
> all type names.  In review, we decided not to hide the names of built-in
> types.  Tradeoff: makes the introspection value easier to read for
> humans (good), but enables misuse by machines (bad).
> 
> Either way works for me, hide non-built-in type names (and have docs
> tell users not to rely on them), or hide them all.  Got a preference?
> 

I'm still leaning towards exposing built-in type names directly, and
furthermore exposing arrays via '[123]' naming. As you say, correct
clients shouldn't be abusing this, but it IS easier to see, and I'm
having a hard time seeing how it could paint us into corners in the
future for having exposed too much information up front (while we DO
rename object types and don't want that to affect ABI, I don't see us
trying to rename builtin types - at most, we might add subtypes with
range or other restrictions, but those would be new types and still
leave the basic 'int' and 'str' unchanged).  So leaving it as just the
documentation on what proper clients should do is sufficient; don't go
back to the v2 munging of builtin names.

>>> +++ b/qapi/introspect.json
>>> @@ -0,0 +1,271 @@
>>
>>> +
>>> +##
>>> +# @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.
>>
>> Same as for the docs above: may be worth a paragraph explicitly
>> mentioning that the wire ABI cannot express everything, such as integer
>> ranges, or such as semantics where exactly one of two mutually-exclusive
>> optional members must be present.
> 
> Here's where Michael commented.

Actually, he mentioned the .hx file, which I didn't. I don't know how
many duplicate places need to refer to the same information, or just
abbreviate by pointing to other places. But I'll be happy as long as the
information is present at least somewhere, and that somewhere is easy to
find (perhaps by good references rather than duplications in the other
places).


>>
>> As required by RFC 7159, the order of individual elements with in the
>> array sent over the wire is assumed to be significant unless the
>> documented semantics in qapi state otherwise.
> 
> Got an example where insignificance of order actually matters?

migrate-set-capabilities does not care what order the 'capabilities'
array is in, for example.


>>> +{ 'struct': 'SchemaInfoObjectVariant',
>>> +  'data': { 'case': 'str', 'type': 'str' } }
>>
>> Do we want to document whether all case values of the tag are guaranteed
>> to be covered (happens to be true for all current qapi, but is not yet
>> enforced; although that's one of my planned followup patches is to start
>> enforcing it - so I guess I can document it at that time if we don't do
>> it here)
> 
> Requiring all cases to be covered explicitly may have value in the QAPI
> schema (more verbose, but you get a reminder to update the union after
> you extended the tag enum), but it's pretty useless in introspection,
> isn't it?

At first glance, I wouldn't call it useless.  But thinking about it
more: Right now, if not all members of the enum are listed, the
generated code abort()s when passing the uncovered enum value.  But
ideally we want semantics where an omitted case in the qapi (or maybe
explicit 'case':null or 'case':{} notation) results in adding no further
variant members to the overall object.  In that case, knowing that the
tag enum value is valid and maps to ':empty' on the wire is worth
advertising, even if we choose to allow omitting a case in the qapi
.json as the syntax for that action.

So we can touch up the wording at the same time we fix code to not abort().


>>> +
>>> +# Caveman's json.dumps() replacement (we're stuck at 2.4)
>>
>> s/2.4/python 2.4/
> 
> Okay.

Of course, if Dan's query proves we can bump our minimum to python 2.6
anyways, this may go away soon.
Markus Armbruster Sept. 4, 2015, 3:08 p.m. UTC | #10
Eric Blake <eblake@redhat.com> writes:

> On 09/04/2015 03:55 AM, Markus Armbruster wrote:
>
>>>> +
>>>> +query-schema returns a JSON array of SchemaInfo objects.  These
>>>> +objects together describe the wire ABI, as defined in the QAPI schema.
>>>
>>> Maybe an additional sentence here, perhaps along the lines of:
>>>
>>> Note that the wire format cannot express everything; it is not designed
>>> to show semantic constraints (such as when exactly one of a pair of
>>> mutually-exclusive optional members must be present, or when an integral
>>> value has specific limitations on valid values).  Introspection allows
>>> an application to know if a feature is present, but the application must
>>> obey the qapi documentation to properly interact with that feature.
>> 
>> Michael raised the same point, and I drafted a paragraph in my reply to
>> him:
>> 
>>     The SchemaInfo can't reflect all the rules and restrictions that
>>     apply to QMP.  It's interface introspection (figuring out what's
>>     there), not interface specification.  The specification is in the
>>     QAPI schema, and users should peruse it to understand how QMP is to
>>     be used.
>> 
>> Then continue with an example showing a restriction that's spelled out
>> in the QAPI schema, but can't be expressed in SchemaInfo.
>> 
>> Use mine as is?  Merge our two proposals somehow?
>
> Your wording works okay for me, but if you want to merge any elements of
> my wording in, go for it.

I'll probably use mine, but I'll look at it once more when I respin this
patch.

>>>> +The SchemaInfo for a command has meta-type "command", and variant
>>>> +members "arg-type" and "ret-type".  The arguments member clients pass
>>>
>>> I found it easier to read with:
>>> s/arguments/arguments that/
>> 
>> Easier to read, but no longer emphasizes that the specific "arguments"
>> member of the { "execute" ... } form needs to conform.  I'll use yours
>> anyway, unless you have a better idea.
>> 
>>>> +with a command on the wire must conform to the type named by
>>>> +"arg-type", which is always an object type.  The return value the
>>>
>>> Likewise s/value/value that/
>> 
>> Likewise.
>> 
>>>> +server passes in a success response conforms to the type named by
>>>> +"ret-type".
>
> Maybe:
>
> On the wire, the "arguments" key of a client's "execute" command must
> conform to the type named by "arg-type", which is always an object type.
> The "return" key that the server passes in a success response will
> always conform to the type named by "ret-type".
>
> Up to you; we'll see how it looks in v5, and at that point, any further
> word-smithing can be done in followup patches rather than delaying this
> series further.

Sounds nice, I'll probably adopt it.

>>> [not for this patch. Thinking aloud: I wonder if QGA's success-response
>>> could be coded in by having 'ret-type':null as a way of saying the
>>> command will have no response if it is successful; perhaps a bit nicer
>>> than exposing an additional boolean flag member]
>> 
>> The QAPI schema could also do it this way.  What do you think?
>
> Sounds like I have another trivial followup patch added to my work queue :)
>
>
>>>> +The SchemaInfo for an event has meta-type "event", and variant member
>>>> +"arg-type".  The data member the server passes with an event conforms
>>>
>>> again, reads a little better with s/member/member that/
>> 
>> My remark on command arguments applies.
>> 
>>>> +to the type named by "arg-type".  It is always an object type.
>
> Maybe:
>
> On the wire, the "data" member included in any event passed by the
> server will conform to the type named by "arg-type".
>
> I'm not sure if we need to document whether it is always an object type;
> command arguments are user-supplied, while event data is
> server-supplied.  And there's the question of any possible duplication
> with the QMP spec file.  I guess I'm fine as long as we aren't stating
> conflicting things between the two places.

I feel duplication is okay when it makes qapi-code-gen.txt easier to
understand.

>>>> +
>>>> +"members" is a JSON array describing the object's common members.
>> 
>> Could say "the object's common members, if any."  What do you think?
>
> Yes, adding the 'if any' helps.

Sold.

>>>
>>> Hmm, the "Union types" section has two mentions of BlockdevOptions; and
>>> this example matches the second. It may help readability if we rename
>>> one of the two examples to be distinct (preferably the first, since it
>>> doesn't match actual QMP).  I guess the phrase "flat union
>>> BlockdevOptions" is sufficient to make it obvious that we are referring
>>> to the second usage, but it is subtle.
>> 
>> Follow-up patch?
>
> Yes.
>
>>>
>>> [1] Ouch.  That should be "name": "type".  I pointed out the bug in v3,
>>> but it looks like it still hasn't been fixed.  Or rather, that our
>>> attempt to fix it wasn't correct.
>> 
>> I'll double-check.
>
> As you noted later, I've already done that legwork.

Thanks a lot for that!

>>>> +
>>>> +As explained above, type names are not part of the wire ABI.  Not even
>>>> +the names of built-in types.  Clients should examine member
>>>> +"json-type" instead of hard-coding names of built-in types.
>>>
>>> Good point (although we may still end up with clients that disobey this,
>>> I will certainly make sure to do this in libvirt's interactions).
>> 
>> We can't fully prevent misuse of our interfaces.  Where we can't, we can
>> still try to make correct use as easy and obvious as we can.
>> 
>> PATCH 32 prevents misuse of most type names by hiding them.  RFC v2 hid
>> all type names.  In review, we decided not to hide the names of built-in
>> types.  Tradeoff: makes the introspection value easier to read for
>> humans (good), but enables misuse by machines (bad).
>> 
>> Either way works for me, hide non-built-in type names (and have docs
>> tell users not to rely on them), or hide them all.  Got a preference?
>> 
>
> I'm still leaning towards exposing built-in type names directly, and
> furthermore exposing arrays via '[123]' naming. As you say, correct
> clients shouldn't be abusing this, but it IS easier to see, and I'm
> having a hard time seeing how it could paint us into corners in the
> future for having exposed too much information up front (while we DO
> rename object types and don't want that to affect ABI, I don't see us
> trying to rename builtin types - at most, we might add subtypes with
> range or other restrictions, but those would be new types and still
> leave the basic 'int' and 'str' unchanged).  So leaving it as just the
> documentation on what proper clients should do is sufficient; don't go
> back to the v2 munging of builtin names.

Okay, no change then.  We can explore arrays in follow-up patches.

>>>> +++ b/qapi/introspect.json
>>>> @@ -0,0 +1,271 @@
>>>
>>>> +
>>>> +##
>>>> +# @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.
>>>
>>> Same as for the docs above: may be worth a paragraph explicitly
>>> mentioning that the wire ABI cannot express everything, such as integer
>>> ranges, or such as semantics where exactly one of two mutually-exclusive
>>> optional members must be present.
>> 
>> Here's where Michael commented.
>
> Actually, he mentioned the .hx file, which I didn't. I don't know how
> many duplicate places need to refer to the same information, or just
> abbreviate by pointing to other places. But I'll be happy as long as the
> information is present at least somewhere, and that somewhere is easy to
> find (perhaps by good references rather than duplications in the other
> places).

Judging from how I look for information, the QAPI schema should have the
complete reference information, while pointers suffice for
qmp-commands.hx.  docs/qapi-code-gen.txt is introduction / big picture,
so it should *explain* things, but it doesn't have to be exhaustively
complete.

>>>
>>> As required by RFC 7159, the order of individual elements with in the
>>> array sent over the wire is assumed to be significant unless the
>>> documented semantics in qapi state otherwise.
>> 
>> Got an example where insignificance of order actually matters?
>
> migrate-set-capabilities does not care what order the 'capabilities'
> array is in, for example.

Correct, but I'm not sure it matters enough to justify repeating basic
JSON semantics here once more, so let's leave things as they are.  Could
be just me getting worn out by this series (I really, really want it
finished).  Perhaps I'll be more amenable to a follow-up patch than to
changing it now.

>>>> +{ 'struct': 'SchemaInfoObjectVariant',
>>>> +  'data': { 'case': 'str', 'type': 'str' } }
>>>
>>> Do we want to document whether all case values of the tag are guaranteed
>>> to be covered (happens to be true for all current qapi, but is not yet
>>> enforced; although that's one of my planned followup patches is to start
>>> enforcing it - so I guess I can document it at that time if we don't do
>>> it here)
>> 
>> Requiring all cases to be covered explicitly may have value in the QAPI
>> schema (more verbose, but you get a reminder to update the union after
>> you extended the tag enum), but it's pretty useless in introspection,
>> isn't it?
>
> At first glance, I wouldn't call it useless.  But thinking about it
> more: Right now, if not all members of the enum are listed, the
> generated code abort()s when passing the uncovered enum value.  But
> ideally we want semantics where an omitted case in the qapi (or maybe
> explicit 'case':null or 'case':{} notation) results in adding no further
> variant members to the overall object.  In that case, knowing that the
> tag enum value is valid and maps to ':empty' on the wire is worth
> advertising, even if we choose to allow omitting a case in the qapi
> .json as the syntax for that action.
>
> So we can touch up the wording at the same time we fix code to not abort().

SchemaInfoObject needs to show the complete set of accepted type tag
values, and for each value, the variant members.

Currently, SchemaInfoObject.tag always names a member of enumeration
type (I guess I should add that to the docs).  That type is the set of
accepted type tag values.

SchemaInfoObject.variants neither adds nor removes from that set.  All
it does is show variant members.

If the object has none for a certain type tag value, then a natural and
compact way to encode that is not to have it in .variants.

Another way is to have a rather pointless .variants member that
explicitly states "no members", basically {"case": "FOO", "type":
":empty"}.

Only if we permitted the type tags type to have more values than object
actually accepts as type tags, the presence of a type tag value in
.variants becomes meaningful even when it adds no variant members.

Why would we ever want that?

>>>> +
>>>> +# Caveman's json.dumps() replacement (we're stuck at 2.4)
>>>
>>> s/2.4/python 2.4/
>> 
>> Okay.
>
> Of course, if Dan's query proves we can bump our minimum to python 2.6
> anyways, this may go away soon.

I'll start a thread.
diff mbox

Patch

diff --git a/.gitignore b/.gitignore
index cb4b8ec..ce02e3c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -34,6 +34,7 @@ 
 /qapi-visit.[ch]
 /qapi-event.[ch]
 /qmp-commands.h
+/qmp-introspect.[ch]
 /qmp-marshal.c
 /qemu-doc.html
 /qemu-tech.html
diff --git a/Makefile b/Makefile
index 9ce3972..b053195 100644
--- a/Makefile
+++ b/Makefile
@@ -52,6 +52,8 @@  endif
 GENERATED_HEADERS = config-host.h qemu-options.def
 GENERATED_HEADERS += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
 GENERATED_SOURCES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
+GENERATED_HEADERS += qmp-introspect.h
+GENERATED_SOURCES += qmp-introspect.c
 
 GENERATED_HEADERS += trace/generated-events.h
 GENERATED_SOURCES += trace/generated-events.c
@@ -264,7 +266,7 @@  $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 
 qapi-modules = $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/qapi/common.json \
                $(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \
-               $(SRC_PATH)/qapi/event.json
+               $(SRC_PATH)/qapi/event.json $(SRC_PATH)/qapi/introspect.json
 
 qapi-types.c qapi-types.h :\
 $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
@@ -286,6 +288,11 @@  $(qapi-modules) $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
 		$(gen-out-type) -o "." -m $<, \
 		"  GEN   $@")
+qmp-introspect.h qmp-introspect.c :\
+$(qapi-modules) $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py)
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-introspect.py \
+		$(gen-out-type) -o "." $<, \
+		"  GEN   $@")
 
 QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
diff --git a/Makefile.objs b/Makefile.objs
index f094eff..b5a9e99 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,7 +1,8 @@ 
 #######################################################################
 # Common libraries for tools and emulators
 stub-obj-y = stubs/
-util-obj-y = util/ qobject/ qapi/ qapi-types.o qapi-visit.o qapi-event.o
+util-obj-y = util/ qobject/ qapi/
+util-obj-y += qmp-introspect.o qapi-types.o qapi-visit.o qapi-event.o
 util-obj-y += crypto/
 
 #######################################################################
@@ -83,6 +84,7 @@  common-obj-$(CONFIG_FDT) += device_tree.o
 # qapi
 
 common-obj-y += qmp-marshal.o
+common-obj-y += qmp-introspect.o
 common-obj-y += qmp.o hmp.o
 endif
 
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index ce32d74..1f8f891 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -494,13 +494,195 @@  Resulting in this JSON object:
   "timestamp": { "seconds": 1267020223, "microseconds": 435656 } }
 
 
+== Client JSON Protocol introspection ==
+
+Clients of a Client JSON Protocol commonly need to figure out what
+exactly the server (QEMU) supports.
+
+For this purpose, QMP provides introspection via command query-schema.
+QGA currently doesn't support introspection.
+
+query-schema returns a JSON array of SchemaInfo objects.  These
+objects together describe the wire ABI, as defined in the QAPI schema.
+
+Like any other command, query-schema is itself defined in the QAPI
+schema, along with the SchemaInfo type.  This text attempts to give an
+overview how things work.  For details you need to consult the QAPI
+schema.
+
+SchemaInfo objects have common members "name" and "meta-type", and
+additional variant members depending on the value of meta-type.
+
+Each SchemaInfo object describes a wire ABI entity of a certain
+meta-type: a command, event or one of several kinds of type.
+
+SchemaInfo for entities defined in the QAPI schema have the same name
+as in the schema.  This is the case for all commands and events, and
+most types.
+
+Command and event names are part of the wire ABI, but type names are
+not.  Therefore, looking up a type by its name in the QAPI schema is
+wrong.  Look up the command or event, then follow references by name.
+
+QAPI schema definitions not reachable that way are omitted.
+
+The SchemaInfo for a command has meta-type "command", and variant
+members "arg-type" and "ret-type".  The arguments member clients pass
+with a command on the wire must conform to the type named by
+"arg-type", which is always an object type.  The return value the
+server passes in a success response conforms to the type named by
+"ret-type".
+
+If the command takes no arguments, "arg-type" names an object type
+without members.  Likewise, if the command returns nothing, "ret-type"
+names an object type without members.
+
+Example: the SchemaInfo for command query-schema
+
+    { "name": "query-schema", "meta-type": "command",
+      "arg-type": ":empty", "ret-type": "SchemaInfoList" }
+
+    Type ":empty" is an object type without members, and type
+    "SchemaInfoList" is the array of SchemaInfo type.
+
+The SchemaInfo for an event has meta-type "event", and variant member
+"arg-type".  The data member the server passes with an event conforms
+to the type named by "arg-type".  It is always an object type.
+
+If the event carries no additional information, "arg-type" names an
+object type without members.  The event may not have a data member on
+the wire then.
+
+Each command or event defined with dictionary-valued 'data' in the
+QAPI schema implicitly defines an object type called ":obj-NAME-arg",
+where NAME is the command or event's name.
+
+Example: the SchemaInfo for EVENT_C from section Events
+
+    { "name": "EVENT_C", "meta-type": "event",
+      "arg-type": ":obj-EVENT_C-arg" }
+
+    Type ":obj-EVENT_C-arg" is an implicitly defined object type with
+    the two members from the event's definition.
+
+The SchemaInfo for struct and union types has meta-type "object".
+
+The SchemaInfo for a struct type has variant member "members".
+
+The SchemaInfo for a union type additionally has variant members "tag"
+and "variants".
+
+"members" is a JSON array describing the object's common members.
+Each element is a JSON object with members "name" (the member's name),
+"type" (the name of its type), and optionally "default".  The member
+is optional if "default" is present.  Currently, "default" can only
+have value null.  Other values are reserved for future extensions.
+
+Example: the SchemaInfo for MyType from section Struct types
+
+    { "name": "MyType", "meta-type": "object",
+      "members": [
+          { "name": "member1", "type": "str" },
+          { "name": "member2", "type": "int" },
+          { "name": "member3", "type": "str", "default": null } ] }
+
+"tag" is the name of the common member serving as type tag.
+"variants" is a JSON array describing the object's variant members.
+Each element is a JSON object with members "case" (the value of type
+tag this element applies to) and "type" (the name of an object type
+that provides the variant members for this type tag value).
+
+Example: the SchemaInfo for flat union BlockdevOptions from section
+Union types
+
+    { "name": "BlockdevOptions", "meta-type": "object",
+      "members": [
+          { "name": "driver", "type": "BlockdevDriver" },
+          { "name": "readonly", "type": "bool"} ],
+      "tag": "driver",
+      "variants": [
+          { "case": "file", "type": "FileOptions" },
+          { "case": "qcow2", "type": "Qcow2Options" } ] }
+
+Note that base types are "flattened": its members are included in the
+"members" array.
+
+A simple union implicitly defines an enumeration type for its implicit
+discriminator (called "type" on the wire, see section Union types).
+Such a type's name is made by appending "Kind" to the simple union's
+name.
+
+A simple union implicitly defines an object type for each of its
+variants.  The type's name is ":obj-NAME-wrapper", where NAME is the
+name of the name of the variant's type.
+
+Example: the SchemaInfo for simple union BlockdevOptions from section
+Union types
+
+    { "name": "BlockdevOptions", "meta-type": "object",
+      "members": [
+          { "name": "kind", "type": "BlockdevOptionsKind" } ],
+      "tag": "type",
+      "variants": [
+          { "case": "file", "type": ":obj-FileOptions-wrapper" },
+          { "case": "qcow2", "type": ":obj-Qcow2Options-wrapper" } ] }
+
+    Enumeration type "BlockdevOptionsKind" and the object types
+    ":obj-FileOptions-wrapper", ":obj-Qcow2Options-wrapper" are
+    implicitly defined.
+
+The SchemaInfo for an alternate type has meta-type "alternate", and
+variant member "members".  "members" is a JSON array.  Each element is
+a JSON object with member "type", which names a type.  Values of the
+alternate type conform to exactly one of its member types.
+
+Example: the SchemaInfo for BlockRef from section Alternate types
+
+    { "name": "BlockRef", "meta-type": "alternate",
+      "members": [
+          { "type": "BlockdevOptions" },
+          { "type": "str" } ] }
+
+The SchemaInfo for an array type has meta-type "array", and variant
+member "element-type", which names the array's element type.  Array
+types are implicitly defined.  An array type's name is made by
+appending "List" to its element type's name.
+
+Example: the SchemaInfo for ['str']
+
+    { "name": "strList", "meta-type": "array",
+      "element-type": "str" }
+
+The SchemaInfo for an enumeration type has meta-type "enum" and
+variant member "values".
+
+Example: the SchemaInfo for MyEnum from section Enumeration types
+
+    { "name": "MyEnum", "meta-type": "enum",
+      "values": [ "value1", "value2", "value3" ] }
+
+The SchemaInfo for a built-in type has the same name as the type in
+the QAPI schema (see section Built-in Types).  It has variant member
+"json-type" that shows how values of this type are encoded on the
+wire.
+
+Example: the SchemaInfo for str
+
+    { "name": "str", "meta-type": "builtin", "json-type": "string" }
+
+As explained above, type names are not part of the wire ABI.  Not even
+the names of built-in types.  Clients should examine member
+"json-type" instead of hard-coding names of built-in types.
+
+
 == Code generation ==
 
-Schemas are fed into 3 scripts to generate all the code/files that, paired
-with the core QAPI libraries, comprise everything required to take JSON
-commands read in by a Client JSON Protocol server, unmarshal the arguments into
-the underlying C types, call into the corresponding C function, and map the
-response back to a Client JSON Protocol response to be returned to the user.
+Schemas are fed into four scripts to generate all the code/files that,
+paired with the core QAPI libraries, comprise everything required to
+take JSON commands read in by a Client JSON Protocol server, unmarshal
+the arguments into the underlying C types, call into the corresponding
+C function, and map the response back to a Client JSON Protocol
+response to be returned to the user.
 
 As an example, we'll use the following schema, which describes a single
 complex user-defined type (which will produce a C struct, along with a list
@@ -848,3 +1030,37 @@  Example:
     extern const char *const example_QAPIEvent_lookup[];
 
     #endif
+
+=== scripts/qapi-introspect.py ===
+
+Used to generate the introspection C code for a schema. The following
+files are created:
+
+$(prefix)qmp-introspect.c - Defines a string holding a JSON
+                            description of the schema.
+$(prefix)qmp-introspect.h - Declares the above string.
+
+Example:
+
+    $ python scripts/qapi-introspect.py --output-dir="qapi-generated"
+    --prefix="example-" example-schema.json
+    $ cat qapi-generated/example-qmp-introspect.c
+[Uninteresting stuff omitted...]
+
+    const char example_qmp_schema_json[] = "["
+        "{\"arg-type\": \":empty\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
+        "{\"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": \"int\"}, "
+        "{\"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": \"str\"}, "
+        "{\"members\": [], \"meta-type\": \"object\", \"name\": \":empty\"}, "
+        "{\"members\": [{\"name\": \"arg1\", \"type\": \"UserDefOne\"}], \"meta-type\": \"object\", \"name\": \":obj-my-command-arg\"}, "
+        "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"UserDefOne\"}, "
+        "{\"arg-type\": \":obj-my-command-arg\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"UserDefOne\"}]";
+    $ cat qapi-generated/example-qmp-introspect.h
+[Uninteresting stuff omitted...]
+
+    #ifndef EXAMPLE_QMP_INTROSPECT_H
+    #define EXAMPLE_QMP_INTROSPECT_H
+
+    extern const char example_qmp_schema_json[];
+
+    #endif
diff --git a/monitor.c b/monitor.c
index f654607..47494c2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -74,6 +74,7 @@ 
 #include "block/qapi.h"
 #include "qapi/qmp-event.h"
 #include "qapi-event.h"
+#include "qmp-introspect.h"
 #include "sysemu/block-backend.h"
 
 /* for hmp_info_irq/pic */
@@ -924,6 +925,20 @@  EventInfoList *qmp_query_events(Error **errp)
     return ev_list;
 }
 
+/*
+ * Minor hack: generated marshalling suppressed for this command
+ * ('gen': false in the schema) so we can parse the JSON string
+ * directly into QObject instead of first parsing it with
+ * visit_type_SchemaInfoList() into a SchemaInfoList, then marshal it
+ * to QObject with generated output marshallers, every time.  Instead,
+ * we do it in test-qmp-input-visitor.c, just to make sure
+ * qapi-introspect.py's output actually conforms to the schema.
+ */
+static void qmp_query_schema(QDict *qdict, QObject **ret_data, Error **errp)
+{
+    *ret_data = qobject_from_json(qmp_schema_json);
+}
+
 /* set the current CPU defined by the user */
 int monitor_set_cpu(int cpu_index)
 {
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 <armbru@redhat.com>
+#
+# 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.
+#
+# Since: 2.5
+##
+{ 'command': 'query-schema',
+  'returns': [ 'SchemaInfo' ],
+  'gen': false }                # just to simplify qmp_query_json()
+
+##
+# @SchemaMetaType
+#
+# This is a @SchemaInfo's meta type, i.e. the kind of entity it
+# describes.
+#
+# @builtin: a predefined type such as 'int' or 'bool'.
+#
+# @enum: an enumeration type
+#
+# @array: an array type
+#
+# @object: an object type (struct or union)
+#
+# @alternate: an alternate type
+#
+# @command: a QMP command
+#
+# @event: a QMP event
+#
+# Since: 2.5
+##
+{ 'enum': 'SchemaMetaType',
+  'data': [ 'builtin', 'enum', 'array', 'object', 'alternate',
+            'command', 'event' ] }
+
+##
+# @SchemaInfoBase
+#
+# Members common to any @SchemaInfo.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoBase',
+  'data': { 'name': 'str', 'meta-type': 'SchemaMetaType' } }
+
+##
+# @SchemaInfo
+#
+# @name: the entity's name, inherited from @base.
+# Entities defined in the QAPI schema have the name defined in the schema.
+# Implicitly defined entities have generated names.  See
+# docs/qapi-code-gen.txt section "Client JSON Protocol introspection"
+# for details.
+#
+# All references to other SchemaInfo are by name.
+#
+# Command and event names are part of the wire ABI, but type names are
+# not.  Therefore, looking up a type by "well-known" name is wrong.
+# Look up the command or event, then follow the references.
+#
+# @meta-type: the entity's meta type, inherited from @base.
+#
+# Additional members depend on the value of @meta-type.
+#
+# Since: 2.5
+##
+{ 'union': 'SchemaInfo',
+  'base': 'SchemaInfoBase',
+  'discriminator': 'meta-type',
+  'data': {
+      'builtin': 'SchemaInfoBuiltin',
+      'enum': 'SchemaInfoEnum',
+      'array': 'SchemaInfoArray',
+      'object': 'SchemaInfoObject',
+      'alternate': 'SchemaInfoAlternate',
+      'command': 'SchemaInfoCommand',
+      'event': 'SchemaInfoEvent' } }
+
+##
+# @SchemaInfoBuiltin
+#
+# Additional SchemaInfo members for meta-type 'builtin'.
+#
+# @json-type: the JSON type used for this type on the wire.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoBuiltin',
+  'data': { 'json-type': 'JSONType' } }
+
+##
+# @JSONType
+#
+# The four primitive and two structured types according to RFC 7159
+# section 1, plus 'int' (split off 'number'), plus the obvious top
+# type 'value'.
+#
+# Since: 2.5
+##
+{ 'enum': 'JSONType',
+  'data': [ 'string', 'number', 'int', 'boolean', 'null',
+            'object', 'array', 'value' ] }
+
+##
+# @SchemaInfoEnum
+#
+# Additional SchemaInfo members for meta-type 'enum'.
+#
+# @values: the enumeration type's values.
+#
+# Values of this type are JSON string on the wire.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoEnum',
+  'data': { 'values': ['str'] } }
+
+##
+# @SchemaInfoArray
+#
+# Additional SchemaInfo members for meta-type 'array'.
+#
+# @element-type: the array type's element type.
+#
+# Values of this type are JSON array on the wire.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoArray',
+  'data': { 'element-type': 'str' } }
+
+##
+# @SchemaInfoObject
+#
+# Additional SchemaInfo members for meta-type 'object'.
+#
+# @members: the object type's (non-variant) members.
+#
+# @tag: #optional the name of the member serving as type tag.  An
+# element of @members with this name must exist.
+#
+# @variants: #optional variant members, i.e. additional members that
+# depend on the type tag's value.  Present exactly when @tag is
+# present.
+#
+# Values of this type are JSON object on the wire.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoObject',
+  'data': { 'members': [ 'SchemaInfoObjectMember' ],
+            '*tag': 'str',
+            '*variants': [ 'SchemaInfoObjectVariant' ] } }
+
+##
+# @SchemaInfoObjectMember
+#
+# An object member.
+#
+# @name: the member's name, as defined in the QAPI schema.
+#
+# @type: the name of the member's type.
+#
+# @default: #optional default when used as command parameter.
+#           If absent, the parameter is mandatory.
+#           If present, the value must be null.  The parameter is
+#           optional, and behavior when it's missing is not specified
+#           here.
+#           Future extension: if present and non-null, the parameter
+#	    is optional, and defaults to this value.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoObjectMember',
+  'data': { 'name': 'str', 'type': 'str', '*default': 'any' } }
+# @default's type must be null or match @type
+
+##
+# @SchemaInfoObjectVariant
+#
+# The variant members for a value of the type tag.
+#
+# @case: a value of the type tag.
+#
+# @type: the name of the object type that provides the variant members
+# when the type tag has value @case.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoObjectVariant',
+  'data': { 'case': 'str', 'type': 'str' } }
+
+##
+# @SchemaInfoAlternate
+#
+# Additional SchemaInfo members for meta-type 'alternate'.
+#
+# @members: the alternate type's members.  The members wire encoding
+# is distinct, see docs/qapi-code-gen.txt section Alternate types.
+#
+# On the wire, this can be any of the members.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoAlternate',
+  'data': { 'members': [ 'SchemaInfoAlternateMember' ] } }
+
+##
+# @SchemaInfoAlternateMember
+#
+# An alternate member.
+#
+# @type: the name of the member's type.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoAlternateMember',
+  'data': { 'type': 'str' } }
+
+##
+# @SchemaInfoCommand
+#
+# Additional SchemaInfo members for meta-type 'command'.
+#
+# @arg-type: the name of the object type that provides the command's
+# parameters.
+#
+# @ret-type: the name of the command's result type.
+#
+# TODO @success-response (currently irrelevant, because it's QGA, not QMP)
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoCommand',
+  'data': { 'arg-type': 'str', 'ret-type': 'str' } }
+
+##
+# @SchemaInfoEvent
+#
+# Additional SchemaInfo members for meta-type 'event'.
+#
+# @arg-type: the name of the object type that provides the event's
+# parameters.
+#
+# Since: 2.5
+##
+{ 'struct': 'SchemaInfoEvent',
+  'data': { 'arg-type': 'str' } }
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.
+
+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 <armbru@redhat.com>
+#
+# 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
+        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
+
+    def _use_type(self, typ):
+        if typ not in self.used_types:
+            self.used_types.append(typ)
+        return typ.name
+
+    def _gen_json(self, name, mtype, obj):
+        obj['name'] = name
+        obj['meta-type'] = mtype
+        self.jsons.append(obj)
+
+    def _gen_member(self, member):
+        ret = {'name': member.name, 'type': self._use_type(member.type)}
+        if member.optional:
+            ret['default'] = None
+        return ret
+
+    def _gen_variants(self, tag_name, variants):
+        return {'tag': tag_name or 'type',
+                'variants': [self._gen_variant(v) for v in variants]}
+
+    def _gen_variant(self, variant):
+        return {'case': variant.name, 'type': self._use_type(variant.type)}
+
+    def visit_builtin_type(self, name, info, json_type):
+        self._gen_json(name, 'builtin', {'json-type': json_type})
+
+    def visit_enum_type(self, name, info, values):
+        self._gen_json(name, 'enum', {'values': values})
+
+    def visit_array_type(self, name, info, element_type):
+        self._gen_json(name, 'array',
+                       {'element-type': self._use_type(element_type)})
+
+    def visit_object_type_flat(self, name, info, members, variants):
+        obj = {'members': [self._gen_member(m) for m in members]}
+        if variants:
+            obj.update(self._gen_variants(variants.tag_name,
+                                          variants.variants))
+        self._gen_json(name, 'object', obj)
+
+    def visit_alternate_type(self, name, info, variants):
+        self._gen_json(name, 'alternate',
+                       {'members': [{'type': self._use_type(m.type)}
+                                    for m in variants.variants]})
+
+    def visit_command(self, name, info, arg_type, ret_type,
+                      gen, success_response):
+        arg_type = arg_type or self.schema.the_empty_object_type
+        ret_type = ret_type or self.schema.the_empty_object_type
+        self._gen_json(name, 'command',
+                       {'arg-type': self._use_type(arg_type),
+                        'ret-type': self._use_type(ret_type)})
+
+    def visit_event(self, name, info, arg_type):
+        arg_type = arg_type or self.schema.the_empty_object_type
+        self._gen_json(name, 'event', {'arg-type': self._use_type(arg_type)})
+
+(input_file, output_dir, do_c, do_h, prefix, dummy) = parse_command_line()
+
+c_comment = '''
+/*
+ * QAPI/QMP schema introspection
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+'''
+h_comment = '''
+/*
+ * QAPI/QMP schema introspection
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+'''
+
+(fdef, fdecl) = open_output(output_dir, do_c, do_h, prefix,
+                            'qmp-introspect.c', 'qmp-introspect.h',
+                            c_comment, h_comment)
+
+fdef.write(mcgen('''
+#include "%(prefix)sqmp-introspect.h"
+
+''',
+                 prefix=prefix))
+
+schema = QAPISchema(input_file)
+gen = QAPISchemaGenIntrospectVisitor()
+schema.visit(gen)
+fdef.write(gen.defn)
+fdecl.write(gen.decl)
+
+close_output(fdef, fdecl)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7fe9f30..8c651f9 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -773,6 +773,8 @@  class QAPISchemaVisitor(object):
         pass
     def visit_object_type(self, name, info, base, members, variants):
         pass
+    def visit_object_type_flat(self, name, info, members, variants):
+        pass
     def visit_alternate_type(self, name, info, variants):
         pass
     def visit_command(self, name, info, arg_type, ret_type,
@@ -896,6 +898,8 @@  class QAPISchemaObjectType(QAPISchemaType):
     def visit(self, visitor):
         visitor.visit_object_type(self.name, self.info,
                                   self.base, self.local_members, self.variants)
+        visitor.visit_object_type_flat(self.name, self.info,
+                                       self.members, self.variants)
 
 class QAPISchemaObjectTypeMember(object):
     def __init__(self, name, typ, optional):
@@ -1047,6 +1051,9 @@  class QAPISchema(object):
                   ('bool',   'boolean', 'bool',     'false'),
                   ('any',    'value',   'QObject' + pointer_suffix , 'NULL')]:
             self._def_builtin_type(*t)
+        self.the_empty_object_type = QAPISchemaObjectType(':empty', None, None,
+                                                          [], None)
+        self._def_entity(self.the_empty_object_type)
 
     def _make_implicit_enum_type(self, name, values):
         name = name + 'Kind'
@@ -1191,9 +1198,10 @@  class QAPISchema(object):
             ent.check(self)
 
     def visit(self, visitor):
-        visitor.visit_begin(self)
+        ignore = visitor.visit_begin(self)
         for name in sorted(self.entity_dict.keys()):
-            self.entity_dict[name].visit(visitor)
+            if not ignore or not isinstance(self.entity_dict[name], ignore):
+                self.entity_dict[name].visit(visitor)
         visitor.visit_end()
 
 #
diff --git a/tests/.gitignore b/tests/.gitignore
index ccc92e4..e476d52 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -28,6 +28,7 @@  test-qmp-commands.h
 test-qmp-event
 test-qmp-input-strict
 test-qmp-input-visitor
+test-qmp-introspect.[ch]
 test-qmp-marshal.c
 test-qmp-output-visitor
 test-rcu-list
diff --git a/tests/Makefile b/tests/Makefile
index 68adad4..92d682f 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -263,7 +263,8 @@  check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	struct-base-clash.json struct-base-clash-deep.json )
 
 GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h \
-		     tests/test-qmp-commands.h tests/test-qapi-event.h
+	tests/test-qmp-commands.h tests/test-qapi-event.h \
+	tests/test-qmp-introspect.h
 
 test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 	tests/check-qlist.o tests/check-qfloat.o tests/check-qjson.o \
@@ -276,7 +277,7 @@  test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 	tests/rcutorture.o tests/test-rcu-list.o
 
 test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
-		  tests/test-qapi-event.o
+		  tests/test-qapi-event.o tests/test-qmp-introspect.o
 
 $(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
@@ -337,6 +338,11 @@  $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-eve
 	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-event.py \
 		$(gen-out-type) -o tests -p "test-" $<, \
 		"  GEN   $@")
+tests/test-qmp-introspect.c tests/test-qmp-introspect.h :\
+$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-introspect.py $(qapi-py)
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-introspect.py \
+		$(gen-out-type) -o tests -p "test-" $<, \
+		"  GEN   $@")
 
 tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
diff --git a/tests/qapi-schema/alternate-good.out b/tests/qapi-schema/alternate-good.out
index 3d765ff..65af727 100644
--- a/tests/qapi-schema/alternate-good.out
+++ b/tests/qapi-schema/alternate-good.out
@@ -1,3 +1,4 @@ 
+object :empty
 alternate Alt
     case value: int
     case string: Enum
diff --git a/tests/qapi-schema/args-member-array.out b/tests/qapi-schema/args-member-array.out
index b67384c..b3b92df 100644
--- a/tests/qapi-schema/args-member-array.out
+++ b/tests/qapi-schema/args-member-array.out
@@ -1,3 +1,4 @@ 
+object :empty
 object :obj-okay-arg
     member member1: intList optional=False
     member member2: defList optional=False
diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index 6161b90..9e2c656 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1 +1,2 @@ 
+object :empty
 enum Status ['good', 'bad', 'ugly']
diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
index e69de29..272b161 100644
--- a/tests/qapi-schema/empty.out
+++ b/tests/qapi-schema/empty.out
@@ -0,0 +1 @@ 
+object :empty
diff --git a/tests/qapi-schema/enum-empty.out b/tests/qapi-schema/enum-empty.out
index e09b00f..a449d45 100644
--- a/tests/qapi-schema/enum-empty.out
+++ b/tests/qapi-schema/enum-empty.out
@@ -1 +1,2 @@ 
+object :empty
 enum MyEnum []
diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out
index b5ae4c2..cdfd264 100644
--- a/tests/qapi-schema/event-case.out
+++ b/tests/qapi-schema/event-case.out
@@ -1 +1,2 @@ 
+object :empty
 event oops None
diff --git a/tests/qapi-schema/flat-union-reverse-define.out b/tests/qapi-schema/flat-union-reverse-define.out
index 477fb31..a5a9134 100644
--- a/tests/qapi-schema/flat-union-reverse-define.out
+++ b/tests/qapi-schema/flat-union-reverse-define.out
@@ -1,3 +1,4 @@ 
+object :empty
 object TestBase
     member enum1: TestEnum optional=False
 enum TestEnum ['value1', 'value2']
diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out
index 9577d1b..f4542b1 100644
--- a/tests/qapi-schema/ident-with-escape.out
+++ b/tests/qapi-schema/ident-with-escape.out
@@ -1,3 +1,4 @@ 
+object :empty
 object :obj-fooA-arg
     member bar1: str optional=False
 command fooA :obj-fooA-arg -> None
diff --git a/tests/qapi-schema/include-relpath.out b/tests/qapi-schema/include-relpath.out
index 6161b90..9e2c656 100644
--- a/tests/qapi-schema/include-relpath.out
+++ b/tests/qapi-schema/include-relpath.out
@@ -1 +1,2 @@ 
+object :empty
 enum Status ['good', 'bad', 'ugly']
diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out
index 6161b90..9e2c656 100644
--- a/tests/qapi-schema/include-repetition.out
+++ b/tests/qapi-schema/include-repetition.out
@@ -1 +1,2 @@ 
+object :empty
 enum Status ['good', 'bad', 'ugly']
diff --git a/tests/qapi-schema/include-simple.out b/tests/qapi-schema/include-simple.out
index 6161b90..9e2c656 100644
--- a/tests/qapi-schema/include-simple.out
+++ b/tests/qapi-schema/include-simple.out
@@ -1 +1,2 @@ 
+object :empty
 enum Status ['good', 'bad', 'ugly']
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index c5af55a..226d300 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -1,3 +1,4 @@ 
+object :empty
 command eins None -> None
    gen=True success_response=True
 command zwei None -> None
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index fbb590f..a9c87a0 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -1,3 +1,4 @@ 
+object :empty
 object :obj-EVENT_C-arg
     member a: int optional=True
     member b: UserDefOne optional=True
diff --git a/tests/qapi-schema/returns-int.out b/tests/qapi-schema/returns-int.out
index 1ac3e1e..a2da259 100644
--- a/tests/qapi-schema/returns-int.out
+++ b/tests/qapi-schema/returns-int.out
@@ -1,2 +1,3 @@ 
+object :empty
 command guest-get-time None -> int
    gen=True success_response=True
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index a2ae786..53a7693 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -19,6 +19,9 @@ 
 #include "test-qapi-types.h"
 #include "test-qapi-visit.h"
 #include "qapi/qmp/types.h"
+#include "test-qmp-introspect.h"
+#include "qmp-introspect.h"
+#include "qapi-visit.h"
 
 typedef struct TestInputVisitorData {
     QObject *obj;
@@ -62,6 +65,30 @@  Visitor *validate_test_init(TestInputVisitorData *data,
     return v;
 }
 
+/* similar to validate_test_init(), but does not expect a string
+ * literal/format json_string argument and so can be used for
+ * programatically generated strings (and we can't pass in programatically
+ * generated strings via %s format parameters since qobject_from_jsonv()
+ * will wrap those in double-quotes and treat the entire object as a
+ * string)
+ */
+static Visitor *validate_test_init_raw(TestInputVisitorData *data,
+                                       const char *json_string)
+{
+    Visitor *v;
+
+    data->obj = qobject_from_json(json_string);
+    g_assert(data->obj != NULL);
+
+    data->qiv = qmp_input_visitor_new_strict(data->obj);
+    g_assert(data->qiv != NULL);
+
+    v = qmp_input_get_visitor(data->qiv);
+    g_assert(v != NULL);
+
+    return v;
+}
+
 typedef struct TestStruct
 {
     int64_t integer;
@@ -293,6 +320,32 @@  static void test_validate_fail_alternate(TestInputVisitorData *data,
     qapi_free_UserDefAlternate(tmp);
 }
 
+static void do_test_validate_qmp_introspect(TestInputVisitorData *data,
+                                            const char *schema_json)
+{
+    SchemaInfoList *schema = NULL;
+    Error *err = NULL;
+    Visitor *v;
+
+    v = validate_test_init_raw(data, schema_json);
+
+    visit_type_SchemaInfoList(v, &schema, NULL, &err);
+    if (err) {
+        fprintf(stderr, "%s", error_get_pretty(err));
+    }
+    g_assert(!err);
+    g_assert(schema);
+
+    qapi_free_SchemaInfoList(schema);
+}
+
+static void test_validate_qmp_introspect(TestInputVisitorData *data,
+                                           const void *unused)
+{
+    do_test_validate_qmp_introspect(data, test_qmp_schema_json);
+    do_test_validate_qmp_introspect(data, qmp_schema_json);
+}
+
 static void validate_test_add(const char *testpath,
                                TestInputVisitorData *data,
                                void (*test_func)(TestInputVisitorData *data, const void *user_data))
@@ -333,6 +386,8 @@  int main(int argc, char **argv)
                       &testdata, test_validate_fail_alternate);
     validate_test_add("/visitor/input-strict/fail/union-native-list",
                       &testdata, test_validate_fail_union_native_list);
+    validate_test_add("/visitor/input-strict/pass/qmp-introspect",
+                      &testdata, test_validate_qmp_introspect);
 
     g_test_run();