qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH RFC v4 30/32] qapi: New QMP command query-schema for QMP schema introspection
Date: Fri, 04 Sep 2015 10:54:34 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Michael Roth <address@hidden> 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 <address@hidden>
>> ---
>>  .gitignore                                      |   1 +
>>  Makefile                                        |   9 +-
>>  Makefile.objs                                   |   4 +-
>>  docs/qapi-code-gen.txt                          | 226 +++++++++++++++++++-
>>  monitor.c                                       |  15 ++
>>  qapi-schema.json                                |   3 +
>>  qapi/introspect.json                            | 271 
>> ++++++++++++++++++++++++
>>  qmp-commands.hx                                 |  17 ++
>>  scripts/qapi-introspect.py                      | 174 +++++++++++++++
>>  scripts/qapi.py                                 |  12 +-
>>  tests/.gitignore                                |   1 +
>>  tests/Makefile                                  |  10 +-
>>  tests/qapi-schema/alternate-good.out            |   1 +
>>  tests/qapi-schema/args-member-array.out         |   1 +
>>  tests/qapi-schema/comments.out                  |   1 +
>>  tests/qapi-schema/empty.out                     |   1 +
>>  tests/qapi-schema/enum-empty.out                |   1 +
>>  tests/qapi-schema/event-case.out                |   1 +
>>  tests/qapi-schema/flat-union-reverse-define.out |   1 +
>>  tests/qapi-schema/ident-with-escape.out         |   1 +
>>  tests/qapi-schema/include-relpath.out           |   1 +
>>  tests/qapi-schema/include-repetition.out        |   1 +
>>  tests/qapi-schema/include-simple.out            |   1 +
>>  tests/qapi-schema/indented-expr.out             |   1 +
>>  tests/qapi-schema/qapi-schema-test.out          |   1 +
>>  tests/qapi-schema/returns-int.out               |   1 +
>>  tests/test-qmp-input-strict.c                   |  55 +++++
>>  27 files changed, 801 insertions(+), 11 deletions(-)
>>  create mode 100644 qapi/introspect.json
>>  create mode 100644 scripts/qapi-introspect.py
>> 
>
> <snip>
>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index e91e3b9..af19810 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -14,6 +14,9 @@
>>  # Tracing commands
>>  { 'include': 'qapi/trace.json' }
>> 
>> +# QAPI introspection
>> +{ 'include': 'qapi/introspect.json' }
>> +
>>  ##
>>  # @LostTickPolicy:
>>  #
>> diff --git a/qapi/introspect.json b/qapi/introspect.json
>> new file mode 100644
>> index 0000000..171baba
>> --- /dev/null
>> +++ b/qapi/introspect.json
>> @@ -0,0 +1,271 @@
>> +# -*- Mode: Python -*-
>> +#
>> +# QAPI/QMP introspection
>> +#
>> +# Copyright (C) 2015 Red Hat, Inc.
>> +#
>> +# Authors:
>> +#  Markus Armbruster <address@hidden>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
>> +# See the COPYING file in the top-level directory.
>> +
>> +##
>> +# @query-schema
>> +#
>> +# Command query-schema exposes the QMP wire ABI as an array of
>> +# SchemaInfo.  This lets QMP clients figure out what commands and
>> +# events are available in this QEMU, and their parameters and results.
>> +#
>> +# Returns: array of @SchemaInfo, where each element describes an
>> +# entity in the ABI: command, event, type, ...
>> +#
>> +# Note: the QAPI schema is also used to help define *internal*
>> +# interfaces, by defining QAPI types.  These are not part of the QMP
>> +# wire ABI, and therefore not returned by this command.
>
> Maybe add something like:
>
> "These internal interfaces may place additional restrictions on the
> values of individual fields, so users should reference the QAPI schema
> to avoid unexpected behavior resulting from invalid field values."

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 <address@hidden>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2.
>> +# See the COPYING file in the top-level directory.
>> +
>> +from qapi import *
>> +import string
>> +
>> +# Caveman's json.dumps() replacement (we're stuck at 2.4)
>> +# TODO try to use json.dumps() once we get unstuck
>> +def to_json(obj, level=0):
>> +    if obj == None:
>> +        ret = 'null'
>> +    elif isinstance(obj, str):
>> +        ret = '"' + obj.replace('"', r'\"') + '"'
>> +    elif isinstance(obj, list):
>> +        elts = [to_json(elt, level + 1)
>> +                for elt in obj]
>> +        ret = '[' + ', '.join(elts) + ']'
>> +    elif isinstance(obj, dict):
>> +        elts = ['"%s": %s' % (key, to_json(obj[key], level + 1))
>> +                for key in sorted(obj.keys())]
>> +        ret = '{' + ', '.join(elts) + '}'
>> +    else:
>> +        assert False                # not implemented
>> +    if level == 1:
>> +        ret = '\n' + ret
>> +    return ret
>> +
>> +def to_c_string(string):
>> +    return '"' + string.replace('\\', r'\\').replace('"', r'\"') + '"'
>> +
>> +class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
>> +    def __init__(self):
>> +        self.defn = None
>> +        self.decl = None
>> +        self.schema = None
>> +        self.jsons = None
>> +        self.used_types = None
>> +
>> +    def visit_begin(self, schema):
>> +        self.schema = schema
>> +        self.jsons = []
>> +        self.used_types = []
>> +        return QAPISchemaType   # don't visit types for now
>> +
>> +    def visit_end(self):
>> +        # visit the types that are actually used
>> +        for typ in self.used_types:
>> +            typ.visit(self)
>> +        self.jsons.sort()
>> +        # generate C
>> +        # TODO can generate awfully long lines
>
> Not sure if this is planned for this series, but a multi-line
> representation in the generated files would make things a bit
> easier to review/check. If we went a little further and made
> them pretty-printed it might make some of the docs referencing
> the output a bit more clear as well. Not a huge deal, but don't
> think it would take much code. Would suggest a JSON library or
> something but that would probably mangle the ordering, which
> wouldn't help with readability.

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]