Re: Making QMP self-documenting (was: [Qemu-devel] [PATCH 11/11] Change the monitor to use the new do_info_qtree.)

[Luiz Capitulino]

On Tue, 12 Jan 2010 19:57:29 +0100
Markus Armbruster <address@hidden> wrote:

> Here's my stab at self-documenting commands.  We need to describe the
> request, the reply, and possible errors.  First the request part.  Its
> format according to qemu-spec.txt is:
> 
> { "execute": json-string, "arguments": json-object, "id": json-value }
> 
> The bits to document are:
> 
> * Name.  This is the value of member "execute" in request objects.
> 
>   Aside: qmp-spec.txt permits an arbitrary string there.  I think we
>   better restrict ourselves to something more tasteful.

 For example?

> * Description (arbitrary text).
> 
>   This is for human readers.

 Would be good to to use the command's help or the manual's description
from qemu-monitor.hx, so that the help command (and even the monitor's
documentation) could be generated from that data.

 The only problem are commands like balloon, which may behave
differently.

> * Request arguments.  The value of member "arguments" in request
>   objects.  It's an object, so we just document the members.  For each
>   member:
> 
>   - Name
> 
>   - Description
> 
>   - Type (more on that below)
> 
>   - Whether it is optional or required
> 
>   If we need more expressiveness than that, we might be making things
>   too complicated.
> 
> JSON Schema note: a natural way to describe all the possible request
> objects is as a union type of the individual request object types.  To
> document a request, you create a schema for its object type.
> 
> Example:
> 
>     {
>         "title": "A balloon request",
>         "description": "Ask the guest to change its memory allocation."
>         "type": "object",
>         "properties": {
>             "execute": {
>                 "type": "string",
>                 "enum": [ "balloon" ]
>             },
>             "arguments": {
>                 "type": "object",
>                 "properties": {
>                     "value": {
>                         "type": "integer",
>                         "description": "How much memory to use, in bytes."
>                     }
>                 }
>             },
>             "id": {
>                 "type": "object"
>             }
>         }
>     }

 Looks good to me.

 Something for the future and not completely related to this, is that
today we use the args_type to do input validation (in both the user
and protocol Monitor).

 It would be a good step forward if we could move it to use this instead,
the only problem is how to translate some types.

> Now, that looks like a workable way to describe the balloon request to a
> client, but it's too much boilerplate to be suitable as source format
> for request documentation.  Even if we omit unneeded schema attributes
> like "type": "object".  I'd rather write the documentation in a more
> concise form, then encode it as JSON schema by substituting it into a
> template.
> 
> Say we put it in the source, right next to the handler function:
> 
> mon_cmd_doc balloon_doc = {
>     .name = "balloon",
>     .description = "Ask the guest to change its memory allocation."
>     .arguments = { // this is an array
>         {
>             .name = "value",
>             .type = "integer",
>                   // ^^^ this is a JSON schema type definition
>             .description = "How much memory to use, in bytes."
>         }
>     }
> };
> 
> Or put it into qemu-monitor.hx.  I prefer next to the code, because that
> maximizes the chance that it gets updated when the code changes.

 What's the advantage of having it as C code (besides being
next to the code)?

 And what about generating user docs from that, for both user Monitor
and the protocol?

 My initial idea was to have it in pure JSON format somewhere, say
qemu-monitor.json.

 This way this file can be read by the Monitor (through the parser's API)
and also by an external script to generate the user docs.

 The disadvantages are:

1. Won't be next to the code

2. We may want to add more text to the user docs, like usage
examples

3. We'll have to write documentation in json format (not too bad,
as today it's a mix of C and some other format in qemu-monitor.hx)

> We could also get fancy and invent some text markup, which we then
> process into C code with a script, but I doubt it's worth it.

 I also don't think it's needed.

> On to the successful response part.  Its format according to
> qemu-spec.txt is:
> 
> { "return": json-object, "id": json-value }
> 
> Actually, we also return arrays of objects, so 'json-object' is a bug in
> the specification.
> 
> To keep this growing memo under control, let's ignore returning arrays
> for now.
> 
> The part to document is the return object(s).  This is similar to
> documenting the request's arguments object.  However, while many
> requests yield a single kind of response object, possibly with some
> optional parts, some requests yield one of several kinds of responses.
> 
> Example: query-migrate has three kinds of responses: completed,
> active/not-block, active/block.  Here's its current documentation:
> 
>   - "status": migration status
>   - "ram": only present if "status" is "active", it is a QDict with the
>     following RAM information (in bytes):
>            - "transferred": amount transferred
>            - "remaining": amount remaining
>            - "total": total
>   - "disk": only present if "status" is "active" and it is a block migration,
>     it is a QDict with the following disk information (in bytes):
>            - "transferred": amount transferred
>            - "remaining": amount remaining
>            - "total": total
> 
> The current documentation uses a "only present if DISCRIMINATOR is
> VALUE" conditional.  It's orthogonal to optional: both "ram" and "disk"
> are only present if "status" is "active", but "ram" is required then,
> while "disk" is optional.
> 
> Another, more general way to describe such things are union types: you
> just enumerate all possible replies.  Two problems.

 Optional and required seems simpler to me.

> Now errors.  Different commands can throw the same error, so it makes
> sense to specify errors separate from commands, and have commands
> reference them by name.  The separate error documentation contains a
> generic description of the error.  We might need a way to extend or
> override it with a command-specific description, to explain what the
> error means for this particular command.
> 
> Format of an error response according to qemu-spec.txt is:
> 
> { "error": { "class": json-string, "data": json-object, "desc": json-string },
>   "id": json-value }
> 
> Bits to document:
> 
> * Name.  This is the value of member "class".
> 
> * Description.  Not to be confused with member "desc".
> 
> * Data.  Document just like response object's return member.
> 
> 
> Besides commands, we need to cover capabilities and asynchronous events,
> but I believe they're just more of the same.

 Agreed.