| Message ID | 1261862362-2530-12-git-send-email-nathan@parenthephobia.org.uk |
|---|---|
| State | New |
| Headers | show |
On Sat, 26 Dec 2009 21:19:22 +0000 Nathan Baum <nathan@parenthephobia.org.uk> wrote: > Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk> > --- > hw/qdev.c | 9 ++++++++- > hw/qdev.h | 3 ++- > monitor.c | 3 ++- > 3 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/hw/qdev.c b/hw/qdev.c > index f5d68c6..d9d3778 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -727,6 +727,12 @@ static void do_info_qbus(Monitor *mon, BusState *bus, QObject **ret_data) > *ret_data = (QObject *) qdict; > } > > +void do_info_qtree(Monitor *mon, QObject **ret_data) > +{ > + if (main_system_bus) > + do_info_qbus(mon, main_system_bus, ret_data); > +} > + What I'm missing here is a high-level documentation of the data type you're building.
On Tue, 2009-12-29 at 15:15 -0200, Luiz Capitulino wrote: > On Sat, 26 Dec 2009 21:19:22 +0000 > Nathan Baum <nathan@parenthephobia.org.uk> wrote: > > > Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk> > > --- > > hw/qdev.c | 9 ++++++++- > > hw/qdev.h | 3 ++- > > monitor.c | 3 ++- > > 3 files changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/hw/qdev.c b/hw/qdev.c > > index f5d68c6..d9d3778 100644 > > --- a/hw/qdev.c > > +++ b/hw/qdev.c > > @@ -727,6 +727,12 @@ static void do_info_qbus(Monitor *mon, BusState *bus, QObject **ret_data) > > *ret_data = (QObject *) qdict; > > } > > > > +void do_info_qtree(Monitor *mon, QObject **ret_data) > > +{ > > + if (main_system_bus) > > + do_info_qbus(mon, main_system_bus, ret_data); > > +} > > + > > What I'm missing here is a high-level documentation of the > data type you're building. Oh yes. I didn't think about that! I'm not sure if there's a policy on how complicated QMP responses will be documented? One possibility that seems quite nice is to use something like JSON Schema[1], which describes JSON objects using JSON, since that means QMP clients can ask Qemu itself to describe its own protocol, and the result could be automatically processed (with some hypothetical "qmpdoc" tool) to produce human-readable documentation. In the mean time, I'm quite happy to write up something in the spirit of the the current informal specification language from qmp-spec. (Like: qtree-info = [ qbus-info* ] qbus-info = { "bus": json-string, "type": json-string, "allow_hotplug": json-bool, "children": [ qdev-info* ] } etc... ?) This should give people who don't want to have to wade through the code an idea of what they can expect out of it! :-) [1] http://json-schema.org/
On Tue, 29 Dec 2009 19:25:24 +0000 Nathan Baum <nathan@parenthephobia.org.uk> wrote: > On Tue, 2009-12-29 at 15:15 -0200, Luiz Capitulino wrote: > > On Sat, 26 Dec 2009 21:19:22 +0000 > > Nathan Baum <nathan@parenthephobia.org.uk> wrote: > > > > > Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk> > > > --- > > > hw/qdev.c | 9 ++++++++- > > > hw/qdev.h | 3 ++- > > > monitor.c | 3 ++- > > > 3 files changed, 12 insertions(+), 3 deletions(-) > > > > > > diff --git a/hw/qdev.c b/hw/qdev.c > > > index f5d68c6..d9d3778 100644 > > > --- a/hw/qdev.c > > > +++ b/hw/qdev.c > > > @@ -727,6 +727,12 @@ static void do_info_qbus(Monitor *mon, BusState *bus, QObject **ret_data) > > > *ret_data = (QObject *) qdict; > > > } > > > > > > +void do_info_qtree(Monitor *mon, QObject **ret_data) > > > +{ > > > + if (main_system_bus) > > > + do_info_qbus(mon, main_system_bus, ret_data); > > > +} > > > + > > > > What I'm missing here is a high-level documentation of the > > data type you're building. > > Oh yes. I didn't think about that! > > I'm not sure if there's a policy on how complicated QMP responses will > be documented? No. Markus and I have just started talking about a documentation format for QMP. Markus, it's time to dump your ideas on the list. > One possibility that seems quite nice is to use something like JSON > Schema[1], which describes JSON objects using JSON, since that means QMP > clients can ask Qemu itself to describe its own protocol, and the result > could be automatically processed (with some hypothetical "qmpdoc" tool) > to produce human-readable documentation. This looks very good! We were considering something along these lines but we didn't consider having the description as part of the dict, for example. This solves some issues we were trying to address. The only problem I can see is that the documentation and code will be in different places, which makes it easier to get outdated. > In the mean time, I'm quite happy to write up something in the spirit of > the the current informal specification language from qmp-spec. (Like: > > qtree-info = [ qbus-info* ] > > qbus-info = { "bus": json-string, > "type": json-string, > "allow_hotplug": json-bool, > "children": [ qdev-info* ] } > > etc... > > ?) > > This should give people who don't want to have to wade through the code > an idea of what they can expect out of it! :-) Well, you would have to change it later so it's better to wait. > > [1] http://json-schema.org/ >
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Tue, 29 Dec 2009 19:25:24 +0000 > Nathan Baum <nathan@parenthephobia.org.uk> wrote: > >> On Tue, 2009-12-29 at 15:15 -0200, Luiz Capitulino wrote: >> > On Sat, 26 Dec 2009 21:19:22 +0000 >> > Nathan Baum <nathan@parenthephobia.org.uk> wrote: >> > >> > > Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk> >> > > --- >> > > hw/qdev.c | 9 ++++++++- >> > > hw/qdev.h | 3 ++- >> > > monitor.c | 3 ++- >> > > 3 files changed, 12 insertions(+), 3 deletions(-) >> > > >> > > diff --git a/hw/qdev.c b/hw/qdev.c >> > > index f5d68c6..d9d3778 100644 >> > > --- a/hw/qdev.c >> > > +++ b/hw/qdev.c >> > > @@ -727,6 +727,12 @@ static void do_info_qbus(Monitor *mon, BusState *bus, QObject **ret_data) >> > > *ret_data = (QObject *) qdict; >> > > } >> > > >> > > +void do_info_qtree(Monitor *mon, QObject **ret_data) >> > > +{ >> > > + if (main_system_bus) >> > > + do_info_qbus(mon, main_system_bus, ret_data); >> > > +} >> > > + >> > >> > What I'm missing here is a high-level documentation of the >> > data type you're building. >> >> Oh yes. I didn't think about that! >> >> I'm not sure if there's a policy on how complicated QMP responses will >> be documented? > > No. Markus and I have just started talking about a documentation format > for QMP. That was just before Christmas, and that's why I haven't gotten around to posting it here properly. > Markus, it's time to dump your ideas on the list. Indeed. I'm working on it, and hope get it posted today. >> One possibility that seems quite nice is to use something like JSON >> Schema[1], which describes JSON objects using JSON, since that means QMP >> clients can ask Qemu itself to describe its own protocol, and the result >> could be automatically processed (with some hypothetical "qmpdoc" tool) >> to produce human-readable documentation. > > This looks very good! It does. Thanks for the pointer! > We were considering something along these lines > but we didn't consider having the description as part of the dict, > for example. We did, actually :) > This solves some issues we were trying to address. > > The only problem I can see is that the documentation and code will > be in different places, which makes it easier to get outdated. Not necessarily. Documentation will be in whatever place we put it. Some places require tools to extract it. [...]
On Tue, 12 Jan 2010 09:25:05 +0100 Markus Armbruster <armbru@redhat.com> wrote: > > We were considering something along these lines > > but we didn't consider having the description as part of the dict, > > for example. > > We did, actually :) That's good then! > > This solves some issues we were trying to address. > > > > The only problem I can see is that the documentation and code will > > be in different places, which makes it easier to get outdated. > > Not necessarily. Documentation will be in whatever place we put it. > Some places require tools to extract it. I was thinking in having them on separate file just to make the work as easy as possible, having a tool to extract them from the function's documentation is perfect, but I'm afraid you'll have to come up with your own tags to distinguish among responses, events, commands etc.
I'd like to make QMP "self-documenting", i.e. make documentation
available within QMP, in structured form, so that clients can discover
available capabilities, commands, their arguments, possible responses
and errors, and so forth.
The core protocol is outside the scope of self-documentation. By "core
protocol" I mean the stuff covered by QMP/qmp-spec.txt section 2.
I want self-documentation to be sufficiently formal to let clients know
what messages they can send and expect to receive. Formality means more
work, but on the flip side the extra cost could help us keep things
simple. It's all too easy to write code sending and receiving messy
messages, but specifying such messes formally is hard.
I'd like to do self-documentation in a way that makes it also available
server side, so that we can check actual behavior against the
documentation. At compile time would be ideal, but run-time is better
than nothing and probably far easier.
We need to figure out what documentation data we want, and how to encode
it.
Nathan suggested to use JSON schema[1] for describing QMP responses.
Since this is such a natural fit for QMP self-documentation, I'm going
to add fine print to my analysis to connect it to JSON schema. But we
need to be careful not to let the encoding (which is really just an
implementation detail) unduly interfere with our analysis of what data
we want.
Since I'm new to JSON schema, technical mistakes are quite possible in
the schema fine print.
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.
* Description (arbitrary text).
This is for human readers.
* 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"
}
}
}
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.
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.
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.
One, how to tell the types apart. Easy if we restrict ourselves to
discriminated union types, i.e. there is one member common to all types,
and it has a distinct set of values for each one.
Two, such union types often have a common part, and I don't fancy
repeating the specification of a common part for each member of the
union.
JSON schema note: we have union types, and we can specify members with a
single possible value, so we can do discriminated unions. The "extends"
mechanism could help with common parts (but now we're getting a bit
fancy for my taste). I don't think we can do the conditional.
Same boilerplate problem as with requests.
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.
[1] http://json-schema.org/
On Tue, 12 Jan 2010 19:57:29 +0100 Markus Armbruster <armbru@redhat.com> 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.
diff --git a/hw/qdev.c b/hw/qdev.c index f5d68c6..d9d3778 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -727,6 +727,12 @@ static void do_info_qbus(Monitor *mon, BusState *bus, QObject **ret_data) *ret_data = (QObject *) qdict; } +void do_info_qtree(Monitor *mon, QObject **ret_data) +{ + if (main_system_bus) + do_info_qbus(mon, main_system_bus, ret_data); +} + #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## __VA_ARGS__) static void qbus_print(Monitor *mon, BusState *bus, int indent); @@ -780,8 +786,9 @@ static void qbus_print(Monitor *mon, BusState *bus, int indent) } #undef qdev_printf -void do_info_qtree(Monitor *mon) +void do_info_qtree_print(Monitor *mon, const QObject *data) { + // TODO: Display qtree from the data! if (main_system_bus) qbus_print(mon, main_system_bus, 0); } diff --git a/hw/qdev.h b/hw/qdev.h index 93467a5..aad4f75 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -168,7 +168,8 @@ void qbus_free(BusState *bus); /*** monitor commands ***/ -void do_info_qtree(Monitor *mon); +void do_info_qtree(Monitor *mon, QObject **ret_data); +void do_info_qtree_print(Monitor *mon, const QObject *data); void do_info_qdm(Monitor *mon); void do_device_add(Monitor *mon, const QDict *qdict); void do_device_del(Monitor *mon, const QDict *qdict); diff --git a/monitor.c b/monitor.c index e6b5424..8d5b650 100644 --- a/monitor.c +++ b/monitor.c @@ -2607,7 +2607,8 @@ static const mon_cmd_t info_cmds[] = { .args_type = "", .params = "", .help = "show device tree", - .mhandler.info = do_info_qtree, + .user_print = do_info_qtree_print, + .mhandler.info_new = do_info_qtree, }, { .name = "qdm",
Signed-off-by: Nathan Baum <nathan@parenthephobia.org.uk> --- hw/qdev.c | 9 ++++++++- hw/qdev.h | 3 ++- monitor.c | 3 ++- 3 files changed, 12 insertions(+), 3 deletions(-)