Re: [Qemu-devel] [PATCH 0/1] Get the list of arguments from a QMP command

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 11.03.2015 um 20:22 hat Markus Armbruster geschrieben:
>> Alberto Garcia <address@hidden> writes:
>> 
>> > On Wed, Mar 11, 2015 at 11:21:43AM +0100, Markus Armbruster wrote:
>> >
>> >> > I can actually try to implement full introspection support, but I
>> >> > would like to know what API you would like. Something like this?
>> >> >
>> >> >    'query-commands'
>> >> >    'query-enums'
>> >> >    'query-events'
>> >> >    'query-types'
>> >> >    'query-unions'
>> >> 
>> >> You propose a separate query-FOO for each kind of thing in the
>> >> schema: command, event, the various types.  Not fundamentally
>> >> different to a single query-schema.  We can discuss which of the two
>> >> approaches is easier to use.
>> >
>> > I don't think it makes much difference in terms of complexity
>> > from the QEMU side. I proposed those because query-commands and
>> > query-events already exist, so adding the missing ones seemed the most
>> > straightforward solution to me.
>> 
>> They only tell you what commands and events exist, but nothing about
>> their 'data' or 'returns'.
>
> True, but luckily they were defined to return not just a list of
> strings, but of CommandInfo dicts. So extending that seems to be the
> natural option in order to get a coherent interface in the end.
>
>> > But if query-schema is more convenient I don't have any problem with
>> > that.
>> 
>> The real work is processing the full schema into the externally visible
>> sub-schema.  Whether to split it into several parts for presentation is
>> the least of my worries.
>> 
>> I'm fine with with building up introspection step by step.  But I want
>> us to have a reasonable idea of the end result.  Without that,
>> incremental development together with our ABI promise is prone to
>> produce a tangled mess.  We already have enough of those :)
>
> Okay. Let's try to define just enough to get a reasonable idea of the
> end result.
>
> Let's start with the commands that we will have, without looking at
> the structure of their arguments and return values yet:
>
> * query-commands, which returns a list of CommandInfos. This contains at
>   least the name of each command. (Mandated by backwards compatibility
>   requirements)
>
> * query-events, which returns a list of EventInfos.  This contains at
>   least the name of each event. (Mandated by backwards compatibility
>   requirements)
>
> * Something to query the full description of commands, including the
>   type of their return value and name, type and optionality of their
>   arguments.
>
>   I propose combining this with query-commands by extending CommandInfo.

Detail.  I want us to figure out what information we want to provide
first, how to encode it second, and how to fit it into the existing
introspection interfaces third.

That said, extending CommandInfo isn't unreasonable.  I'm happy to get
back to the idea when we're ready to discuss "third".

Let me make an attempt at "first".

QMP introspection answers reasonable questions about the QMP interface.

Proposition: introspection of a suitable subset of the QAPI schema
provides QMP introspection.

Rationale:

    The QAPI schema is (meant to be) the definition of the QMP interface
    (plus a internal interfaces, but we can ignore that).

    Therefore, introspection answers can be found in the QAPI schema.
    If the schema can't answer a reasonable question, it's a defect in
    the schema or the interface.

Example:

    "Does this version of QEMU support frobnicating over QMP?" is a
    reasonable question.

    If we provide a QMP command 'frobnicate', the schema can answer the
    question.

    Likewise if we add a 'frobnicate' parameter to an existing QMP
    command.

    But if we extend the set of acceptable values of an existing
    parameter, it can't.  You can view this as a defect in the schema
    language: it can't express acceptable values.  Or you can view it as
    a defect in the interface: new features shouldn't be shoehorned into
    existing commands that way.

What we need to provide is a suitable subset of the information in the
QAPI schema.

What information is in the QAPI schema?  This is a surprisingly hard
question, because the QAPI schema is poorly defined[1].

The general form of a QMP command is

    { "execute": COMMAND-NAME, "arguments": ARGS, "id": ID }

where COMMAND-NAME and ID are JSON strings, and ARGS is a JSON object.

The general form of a success response is

    { "return": VALUE, "id": ID }

where VALUE can be almost any JSON value (more below), and ID is the
command's ID.

Error responses aren't interesting here, because they're shared by all
commands.

The schema has the following information on commands:

* Name

  This declares an accepted COMMAND-NAME.

* Parameters

  Declare what ARGS are accepted with COMMAND-NAME, one of:

  - A set of names with properties

    The names specify the object's acceptable member names.

    A name's properties are type and whether the member is optional.

    With Eric's series applied, a name's type property can only be a
    type name, not an anonymous complex type specification.

    Since you started at the interesting rather than the basic end of
    the schema, we haven't discussed types, yet, but if we had, you'd
    see that this is exactly a complex type declaration.

    Aside: generalization to arbitrary rather than just complex type
    would be possible.  Arbitrary type is natural when you view commands
    as taking a single (usually complex) argument and produce a single
    result.  Flat union types could be handy.

    Currently, the schema syntax doesn't permit properties beyond type
    and optionalness, but Eric's series prepares the ground for
    extensibility.  We already discussed adding a default value
    property.

  - A name of a complex type

    Same as above, except the type is named rather than anonymous.

  - "Anything"

    Any ARGS are accepted.

* Return value

  Declare what VALUE can be produced, one of:

  - JSON object: a set of names with properties or a name of a complex
    type

    Declares possible objects just like we for parameters anove.

    Can't say offhand whether union types are already possible here.

  - JSON array of a single type: a type name

  - JSON number, string or boolean: a built-in type, or the name of an
    enum type

Now we can discuss what subset to provide at first.  My opening bid is
"all of it", because it's really not all that much.

> * Something to query QAPI types, including:
>
>   - The fields of a complex type, including their name, type and
>     optionality.
>
>     I propose that this information be returned in a structure analogous
>     to how arguments are described in query-commands, whatever that may
>     be.
>
>   - The possible values of an enum type
>
>   - Union types are complex (sorry, my bad), we probably don't want to
>     expose them the way they are in the schema. This needs more thought.

Another thing to think through is base types.  Do we want to expose
them, or do we want to flatten them away?

>   The details of how each type is described, as well as the exact
>   command to query it, aren't of importance for adding command argument
>   querying and shouldn't affect it, so I'd leave that for later.

If we chicken out of introspecting types, we need to expand type names
into their definitions.  Remember, a command's parameters can be defined
as a type name, and its return value, too.

I'm not sure this would reduce complexity much short term.  I suspect
it'll increase it for the complete solution.

>   What I do propose is that in order to maintain consistency with the
>   already existing query-{commands,events}, we introduct a separate
>   command for these. This may be a single 'query-types', but it could
>   also be 'query-complex-types'/'query-enums'/'query-unions'.

Again, I'm not ready to discuss commands just yet.

>   Unless you think it makes a difference for defining what we need now,
>   I think we can leave this question open.
>
> * Possibly something to query the fields of events, if we ever need
>   that (I doubt it). If we do, in consistency with the above I propose
>   extending EventInfo and exposing the information the same way as for
>   command arguments and complex type fields.
>
> Do you agree so far?
>
> Anything else you want to get answered before you think you have a
> reasonable idea of the end result on the high level?

Yes: I want an analysis of types and events similar to my above analysis
of commands.  I'll do it myself.  Just not today, I'm running rather
late already.

> If not, the next step would be to define the exact structure of each
> command. I for one don't think that there is a need to define it for all
> of the commands now, but you may think otherwise?
>
> If so, putting together the schema for the query commands (without
> implementing them yet) shouldn't be that hard and I'd be happy to do
> that. Perhaps with the exception of union types, but if you think it's
> important, I can propose something for them, too.
>
>> I feel a necessary early step is to actually define our schema language,
>> and separate syntactic sugar from its core.  Introspection wants the
>> core, but for schema development, a modest amount of sugar is
>> convenient.  Some of the current schema syntax needs to be redefined as
>> sugar for a suitable desugared form.  We've discussed this for "asterisk
>> means optional" already.  Eric's series brings us closer, it just needs
>> to be finished.
>
> Our schema language is an implementation detail, we shouldn't worry
> about it too much in this discussion. As long as we agree that arguments
> do have a name, a type and a boolean that describes whether they are
> optional (and I don't think anyone would deny that), we can use that in
> a future-proof external API.
>
>> If we can't crack the whole problem in the next development cycle,
>> perhaps we can identify a minimally useful sub-schema that is unlikely
>> to be affected by the above work.  We could then expose that early, with
>> the idea of extending it later.
>
> We've done that for several years and we haven't cracked it. We need to
> start doing something. Now.
>
> Look at blockdev. We only started moving because we stopped discussing
> and actually started hacking on it. And blockdev is much more complex
> and we still haven't figured out all the details. In comparison with
> blockdev, schema introspection doesn't look that hard any more.

I don't think you can compare blockdev-add and introspection at this
time.

The history of our introspection efforts isn't "we've done that for
several years" without success.  What we've done is dicking around with
various half-hearted work-arounds.  The only real attempt at the problem
so far has been Amos's, and we let his effort stall.  I've been able to
spare enough time for introspection to shoot holes into proposals, but
not for real analysis.

The history of our blockdev effort isn't "we only started moving because
we stopped discussing and actually started hacking on it".  I had been
hacking on it on and off (mostly off) for a long time, but produced only
solutions to preliminary problems (and a few unfinished, hacked-to-death
branches in my private repo).  Useful preliminaries alright, but not
enough.  We "started moving" when you were able to invest a really big
chunk of time, and approached the problem from a different angle.

I readily accept the idea that some problems defy analysis without
experiments (read: actual coding).  With a bit of luck, the code can
even grow into a solution.  But that's no reason to dismiss analysis
without even giving it a try.

Let's give it a try.


[1] It's whatever scripts/qapi*.py accept, except when their output is
nonsense.  A product of "doing something, now" incremental development.
Eric's stalled series improves the situation.  If you want to peek:
http://repo.or.cz/w/qemu/armbru.git/shortlog/refs/heads/review-eblake-qapi