Re: [Qemu-devel] [PATCH v5 01/28] qapi: Document type-safety considerations

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> Go into more details about the various types of valid expressions
> in a qapi schema, including tweaks to document fixes being done
> later in the current patch series.  Also fix some stale and missing
> documentation in the QMP specification.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
>
> I'm not sure if it is okay to assert GPLv2+ licensing without an
> explicit Copyright, but as I am not the original author, I don't
> know who to attribute any original Copyright to.  Advice?  Should
> I split the license addition to a separate patch?
> ---
>  docs/qapi-code-gen.txt | 338 
> ++++++++++++++++++++++++++++++++++++++++---------
>  docs/qmp/qmp-spec.txt  |  92 +++++++++++---
>  2 files changed, 350 insertions(+), 80 deletions(-)
>
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 8313ba6..ce9c4b9 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -1,17 +1,18 @@
> +This document is licensed under the GPLv2 (or later).
> +
>  = How to use the QAPI code generator =
>
>  QAPI is a native C API within QEMU which provides management-level
> -functionality to internal/external users. For external
> +functionality to internal and external users. For external
>  users/processes, this interface is made available by a JSON-based
>  QEMU Monitor protocol that is provided by the QMP server.
>
> -To map QMP-defined interfaces to the native C QAPI implementations,
> -a JSON-based schema is used to define types and function
> -signatures, and a set of scripts is used to generate types/signatures,
> -and marshaling/dispatch code. The QEMU Guest Agent also uses these
> -scripts, paired with a separate schema, to generate
> -marshaling/dispatch code for the guest agent server running in the
> -guest.
> +To map QMP-defined interfaces to the native C QAPI implementations, a
> +JSON-based schema is used to define types and function signatures, and
> +a set of scripts is used to generate types, signatures, and
> +marshaling/dispatch code. The QEMU Guest Agent also uses these
> +scripts, paired with a separate schema, to generate marshaling and
> +dispatch code for the guest agent server running in the guest.
>
>  This document will describe how the schemas, scripts, and resulting
>  code are used.
> @@ -22,40 +23,150 @@ code are used.
>  This file defines the types, commands, and events used by QMP.  It should
>  fully describe the interface used by QMP.
>
> -This file is designed to be loosely based on JSON although it's technically
> -executable Python.  While dictionaries are used, they are parsed as
> -OrderedDicts so that ordering is preserved.
> +A QAPI file is designed to be loosely based on JSON, which is then
> +parsed by a python code generation program.  A valid QAPI schema
> +consists of a list of top-level expressions, with no commas between
> +them.  Where dictionaries are used, they are parsed as OrderedDicts so
> +that ordering is preserved (for predictable layout of generated C
> +structs and parameter lists).  Ordering doesn't matter for top-level
> +expressions, but does matter within 'data' members of a single
> +expression.  QAPI input is written using 'single quotes' instead of
> +JSON's "double quotes" (in contrast, QMP is strict JSON and only uses
> +"double quotes").  As in JSON, trailing commas are not permitted in
> +arrays or dictionaries.  Input must be ASCII (although QMP supports
> +full Unicode strings, the QAPI parser does not).
>
> -There are two basic syntaxes used, type definitions and command definitions.
> +Comments are allowed; anything between an unquoted # and the following
> +newline is ignored.  Although there is not yet a documentation
> +generator, a form of stylized comments has developed for consistently
> +documenting details about an expression and when it was added to the
> +schema.  The documentation is delimited between two lines of ##, then
> +the first line names the expression, an optional overview is provided,
> +then individual documentation about each member of 'data' is provided,
> +and finally, a 'Since: x.y.z' tag lists the release that introduced
> +the expression.  Optional fields are tagged with the phrase
> +'#optional', often with their default value; and extensions added
> +after the expression was first released are also given a '(since
> +x.y.z)' comment.  For example:
>
> -The first syntax defines a type and is represented by a dictionary.  There 
> are
> -three kinds of user-defined types that are supported: complex types,
> -enumeration types and union types.
> +    ##
> +    # @BlockStats:
> +    #
> +    # Statistics of a virtual block device or a block backing device.
> +    #
> +    # @device: #optional If the stats are for a virtual block device, the 
> name
> +    #          corresponding to the virtual block device.
> +    #
> +    # @stats:  A @BlockDeviceStats for the device.
> +    #
> +    # @parent: #optional This describes the file block device if it has one.
> +    #
> +    # @backing: #optional This describes the backing block device if it has 
> one.
> +    #           (Since 2.0)
> +    #
> +    # Since: 0.14.0
> +    ##
> +    { 'type': 'BlockStats',
> +      'data': {'*device': 'str', 'stats': 'BlockDeviceStats',
> +               '*parent': 'BlockStats',
> +               '*backing': 'BlockStats'} }
>
> -Generally speaking, types definitions should always use CamelCase for the 
> type
> -names. Command names should be all lower case with words separated by a 
> hyphen.
> +The schema sets up a series of types, as well as commands and events
> +that will use those types.  Forward references are allowed: the parser
> +scans in two passes, where the first pass learns all type names, and
> +the second validates the schema and generates the code.  This allows
> +the definition of complex structs that can have mutually recursive
> +types, and allows for indefinite nesting of QMP that satisfies the
> +schema.  A type name should not be defined more than once.
>
> +There are six top-level expressions recognized by the parser:
> +'include', 'command', 'type', 'enum', 'union', and 'event'.  There are
> +several built-in types, such as 'int' and 'str'; additionally, the
> +top-level expressions can define complex types, enumeration types, and
> +several flavors of union types.  The 'command' expression can refer to
> +existing types by name, or list an anonymous type as a dictionary.

'event' can do that, too.

> +
> +Listing a type name inside an array refers to a single-dimension array
> +of that type; multi-dimension arrays are not directly supported
> +(although an array of a complex struct that contains an array member
> +is possible).
> +
> +Types, commands, and events share a common namespace.  Therefore,
> +generally speaking, type definitions should always use CamelCase for
> +user-defined type names, while built-in types are lowercase. Type
> +definitions should not end in 'Kind', as this namespace is used for
> +creating implicit C enums for visiting union types.  Command names,
> +and field names within a type, should be all lower case with words
> +separated by a hyphen.  However, some existing older commands and
> +complex types use underscore; when extending such expressions,
> +consistency is preferred over blindly avoiding underscore.  Event
> +names should be ALL_CAPS with words separated by underscore.  The
> +special string '**' appears for some commands that manually perform
> +their own type checking rather than relying on the type-safe code
> +produced by the qapi code generators.

The generator generates C identifiers pretty thoughtlessly, and is
therefore prone to generate clashes.  Fixable, but we've got bigger fish
to fry, and this series is hefty enough as it is.  Documenting the mess
instead makes sense.

> +
> +Any command, type, or field name beginning with "x-" is marked
> +experimental, and may be withdrawn or changed incompatibly in a future
> +release.  Downstream vendors may add extensions; such extensions
> +should begin with a prefix matching "__RFQDN_" (for the
> +reverse-fully-qualified-domain-name of the vendor), even if the rest
> +of the command or field name uses dash (example:
> +__com.redhat_drive-mirror).  Other than the dots used in RFQDN of a
> +downstream extension, all command, type, and field names should begin
> +with a letter, and contain only ASCII letters, numbers, dash, and
> +underscore.  It is okay to reuse names that match C keywords; the
> +generator will rename a field named "default" in the QAPI to
> +"q_default" in the generated C code.
> +
> +
> +=== Built-in Types ===
> +
> +The following types are built-in to the parser:
> +  'str' - arbitrary UTF-8 string
> +  'int' - 64-bit signed integer (although the C code may place further
> +          restrictions on acceptable range)
> +  'number' - floating point number
> +  'bool' - JSON value of true or false
> +  'int8', 'int16', 'int32', 'int64' - like 'int', but enforce maximum
> +                                      bit size
> +  'uint8', 'uint16', 'uint32', 'uint64' - unsigned counterparts

Apropos uint64_t: QInt wraps int64_t, and I wouldn't be willing to bet
on uint64_t values it can't represent to actually work.  But if they
don't, it's a bug that is outside the scope of this series.

> +  'size' - like 'uint64', but allows scaled suffix from command line
> +           visitor
>
>  === Includes ===
>
> +Usage: { 'include': 'str' }
> +
>  The QAPI schema definitions can be modularized using the 'include' directive:
>
>   { 'include': 'path/to/file.json'}

If you need to respin for some reason, insert a space before the closing
brace.

>
>  The directive is evaluated recursively, and include paths are relative to the
> -file using the directive. Multiple includes of the same file are safe.
> +file using the directive. Multiple includes of the same file are
> +safe.  No other keys should appear in the expression, and the include
> +value should be a string.
> +
> +As a matter of style, it is a good idea to have all files be
> +self-contained, but at the moment, nothing prevents an included file
> +from making a forward reference to a type that is only introduced by
> +an outer file.  The parser may be made stricter in the future to
> +prevent incomplete include files.
>
>
>  === Complex types ===
>
> -A complex type is a dictionary containing a single key whose value is a
> -dictionary.  This corresponds to a struct in C or an Object in JSON.  An
> -example of a complex type is:
> +Usage: { 'type': 'str', 'data': 'dict', '*base': 'complex-type-name' }

Here, the reader needs to get that 'type' and 'data' and 'base' are
literals, but 'str', 'dict' and 'complex-type-name' are placeholders.  I
like setting apart placeholders with typograhic conventions.  If we want
to do that, let's do it on top.

'*base' is interesting.  It's a tacit use of the schema language's
"'*name' means member 'name' is optional" rule on meta-schema-level.  We
can take care of that when/if we clarify placeholders.

> +
> +A complex type is a dictionary containing a single 'data' key whose
> +value is a dictionary.  This corresponds to a struct in C or an Object
> +in JSON. Each value of the 'data' dictionary must be the name of a
> +complex, enum, union, or built-in type,

Are there any others?  If not, we can simplify to

    Each value of the 'data' dictionary must be a type name

>                                          or a one-element array
> +containing a type name.  An example of a complex type is:
>
>   { 'type': 'MyType',
>     'data': { 'member1': 'str', 'member2': 'int', '*member3': 'str' } }
>
> -The use of '*' as a prefix to the name means the member is optional.
> +The use of '*' as a prefix to the name means the member is optional in
> +the corresponding QMP usage.

Not just in QMP, also QGA and (internal) C interfaces.  Since explaining
that is tiresome, I'd be tempted to stick to the original sentence here.

Preexisting: we sometimes say "QMP wire format", and sometimes simply
"JSON".  Again, we got bigger fish to fry.

>
>  The default initialization value of an optional argument should not be 
> changed
>  between versions of QEMU unless the new default maintains backward
> @@ -100,22 +211,52 @@ both fields like this:
>   { "file": "/some/place/my-image",
>     "backing": "/some/place/my-backing-file" }
>
> +
>  === Enumeration types ===
>
> -An enumeration type is a dictionary containing a single key whose value is a
> -list of strings.  An example enumeration is:
> +Usage: { 'enum': 'str', 'data': [ 'str' ] }
> +
> +An enumeration type is a dictionary containing a single 'data' key
> +whose value is a list of strings.  An example enumeration is:
>
>   { 'enum': 'MyEnum', 'data': [ 'value1', 'value2', 'value3' ] }
>
> +Nothing prevents an empty enumeration, although it is probably not
> +useful.  The list of strings should be lower case; if an enum name
> +represents multiple words, use '-' between words.  The string 'max' is
> +not allowed as an enum value, and values should not be repeated.
> +
> +The enumeration values are passed as strings over the QMP protocol,
> +but are encoded as C enum integral values in generated code.  While
> +the C code starts numbering at 0, it is better to use explicit
> +comparisons to enum values than implicit comparisons to 0; the C code
> +will also include a generated enum member ending in _MAX for tracking
> +the size of the enum, useful when using common functions for
> +converting between strings and enum values.  Since the wire format
> +always passes by name, it is acceptable to reorder or add new
> +enumeration members in any location without breaking QMP clients;
> +however, removing enum values would break compatibility.  For any
> +complex type that has a field that will only contain a finite set of
> +string values, using an enum type for that field is better than
> +open-coding the field to be type 'str'.
> +
> +
>  === Union types ===
>
> -Union types are used to let the user choose between several different data
> -types.  A union type is defined using a dictionary as explained in the
> -following paragraphs.
> +Usage: { 'union': 'str', 'data': 'dict' }
> +or:    { 'union': 'str', 'data': 'dict', 'base': 'complex-type-name',
> +         'discriminator': 'enum-member-of-base' }
> +or:    { 'union': 'str', 'data': 'dict', 'discriminator': {} }
>
> +Union types are used to let the user choose between several different
> +data types.  There are three flavors: simple (no discriminator), flat
> +(a base type is mandatory, and discriminator is the name of an enum
> +field within that base type), and anonymous (discriminator is an
> +empty dictionary).  A union type is defined using a data dictionary as
> +explained in the following paragraphs.
>
> -A simple union type defines a mapping from discriminator values to data types
> -like in this example:
> +A simple union type defines a mapping from automatic discriminator
> +values to data types like in this example:
>
>   { 'type': 'FileOptions', 'data': { 'filename': 'str' } }
>   { 'type': 'Qcow2Options',
> @@ -132,10 +273,17 @@ specified data type corresponding to the discriminator 
> value:
>   { "type": "qcow2", "data" : { "backing-file": "/some/place/my-image",
>                                 "lazy-refcounts": true } }
>
> +Additionally, an implicit C enum NameKind is created, corresponding to
> +the union Name, for accessing the various branches of the union.  No
> +branch of the union can be named 'max', as this would collide with the
> +implicit enum.

Aside: I guess this implicit enum is just like a normal QAPI enum,
except it doesn't have a name in the schema.

>
> -A union definition can specify a complex type as its base. In this case, the
> -fields of the complex type are included as top-level fields of the union
> -dictionary in the QMP wire format. An example definition is:
> +
> +A flat union definition specifies a complex type as its base, and
> +avoids nesting on the wire.  In this case, the fields of the complex
> +type are included as top-level fields of the union dictionary in the
> +QMP wire format, and the 'discriminator' field must be the name of an
> +enum-typed member of the base type. An example definition is:
>
>   { 'type': 'BlockdevCommonOptions', 'data': { 'readonly': 'bool' } }
>   { 'union': 'BlockdevOptions',

Worth mentioning explicitly that the base type's members are common to
all the union's cases?

> @@ -150,11 +298,11 @@ And it looks like this on the wire:
>     "data" : { "backing-file": "/some/place/my-image",
>                "lazy-refcounts": true } }
>
> -
> -Flat union types avoid the nesting on the wire. They are used whenever a
> -specific field of the base type is declared as the discriminator ('type' is
> -then no longer generated). The discriminator must be of enumeration type.
> -The above example can then be modified as follows:
> +Notice that in a flat union, a 'type' field is no longer generated,
> +and the keys of the 'data' dictionary must match the valid values for
> +the discriminator (although not necessarily in the same order). The
> +above example for simple unions can be modified to a flat union as
> +follows:
>
>   { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
>   { 'type': 'BlockdevCommonOptions',
> @@ -173,13 +321,15 @@ Resulting in this JSON object:
>     "lazy-refcounts": true }
>
>
> -A special type of unions are anonymous unions. They don't form a dictionary 
> in
> -the wire format but allow the direct use of different types in their place. 
> As
> -they aren't structured, they don't have any explicit discriminator but use
> -the (QObject) data type of their value as an implicit discriminator. This 
> means
> -that they are restricted to using only one discriminator value per QObject
> -type. For example, you cannot have two different complex types in an 
> anonymous
> -union, or two different integer types.
> +The final flavor of unions is an anonymous union. While the other two
> +union types are always passed as a dictionary in the wire format, an
> +anonymous union instead allows the direct use of different types in
> +its place. As they aren't structured, they don't have any explicit
> +discriminator but use the (QObject) data type of their value as an
> +implicit discriminator. This means that they are restricted to using
> +only one discriminator value per QObject type. For example, you cannot
> +have two different complex types in an anonymous union, or two
> +different integer types.

This is the first mention of "QObject type".  How it's related to JSON
is left to the reader's deductive skills :)

The QObject types are QTYPE_NONE, QTYPE_QINT, QTYPE_QSTRING,
QTYPE_QDICT, QTYPE_QLIST, QTYPE_QFLOAT, QTYPE_QBOOL, QTYPE_QERROR.

The connections JSON string - QTYPE_QSTRING, JSON object - QTYPE_QDICT,
JSON array - QTYPE_QLIST and JSON boolean - QTYPE_QBOOL are obvious
enough.

If I remember correctly, our JSON parser chokes on the JSON keyword
null.

That leaves just JSON numbers - QTYPE_QINT or QTYPE_QFLOAT.  Can an
anonymous union have a separate case for each of the two?

For completeness: on the QTYPE_ side, it leaves QTYPE_QERROR and
QTYPE_NONE.  QTYPE_QERROR is internal only, and will hopefully be gone
soon.  I can't see QTYPE_NONE objects being created anywhere.

Should we explain this in terms of JSON types instead of QObject types?

>
>  Anonymous unions are declared using an empty dictionary as their 
> discriminator.
>  The discriminator values never appear on the wire, they are only used in the
> @@ -200,23 +350,93 @@ This example allows using both of the following example 
> objects:
>
>  === Commands ===
>
> -Commands are defined by using a list containing three members.  The first
> -member is the command name, the second member is a dictionary containing
> -arguments, and the third member is the return type.
> +Usage: { 'command': 'str', '*data': 'dict-or-complex-type-name',
> +         '*returns': 'type',

Shoudn't this be '*returns': 'dict-or-type-name'?

> +         '*gen': false, '*success-response': false }
>
> -An example command is:
> +Commands are defined by using a dictionary containing several members,
> +where three members are most common.  The 'command' member is a
> +mandatory string, and determines the "execute" value passed in a QMP
> +command exchange.
> +
> +The 'data' member is optional; if absent, the command accepts an
> +optional empty dictionary.

Suggest: The 'data' member is optional, and defaults to {}.

> +                            If present, it must be the string name of
> +a complex type, a one-element array containing the name of a complex
> +type, or a dictionary that declares an anonymous type with the same
> +semantics as a 'type' expression, with one exception noted below when
> +'gen' is used.  The 'data' argument maps to the "arguments" dictionary
> +passed in as part of a QMP command.

Suggest to move the last sentence to the beginning of the paragaph.

> +
> +The 'returns' member describes what will appear in the "return" field
> +of a QMP reply on successful completion of a command.  The member is
> +optional from the command declaration; if absent, the "return" field
> +will be an empty dictionary.  If 'returns' is present, it must be the
> +string name of a complex or built-in type, a one-element array
> +containing the name of a complex or built-in type, or a dictionary
> +that declares an anonymous type with the same semantics as a 'type'
> +expression, with one exception noted below when 'gen' is used.

Oh, 'gen' affect 'returns', too?  Live and learn...

> +Although it is permitted to have the 'returns' member name a built-in
> +type or an array of built-in types, any command that does this cannot
> +be extended to return additional information in the future; thus, new
> +commands should strongly consider returning a dictionary-based type or
> +an array of dictionaries, even if the dictionary only contains one
> +field at the present.
> +
> +All commands use a dictionary to report failure, with no way to
> +specify that in QAPI.  Where the error return is different than the
> +usual GenericError class in order to help the client react differently
> +to certain error conditions, it is worth documenting this in the
> +comments before the command declaration.
> +
> +Some example commands:
> +
> + { 'command': 'my-first-command',
> +   'data': { 'arg1': 'str', '*arg2': 'str' } }
> + { 'type': 'MyType', 'data': { '*value': 'str' } }
> + { 'command': 'my-second-command',
> +   'returns': [ 'MyType' ] }
> +
> +which would validate this QMP transaction:
> +
> + => { "execute": "my-first-command",
> +      "arguments": { "arg1": "hello" } }
> + <= { "return": { } }
> + => { "execute": "my-second-command" }
> + <= { "return": [ { "value": "one" }, { } ] }
> +
> +In rare cases, QAPI cannot express a type-safe representation of a
> +corresponding QMP command.  In these cases, if the command expression
> +includes the key 'gen' with boolean value false, then the 'data' or
> +'returns' member that intends to bypass generated type-safety and do
> +its own manual validation should use '**' rather than a valid type
> +name.  Please try to avoid adding new commands that rely on this, and
> +instead use type-safe unions.  For an example of bypass usage:
> +
> + { 'command': 'netdev_add',
> +   'data': {'type': 'str', 'id': 'str', '*props': '**'},
> +   'gen': false }

We use 'gen': 'no' until PATCH 18.

> +
> +Normally, the QAPI schema is used to describe synchronous exchanges,
> +where a response is expected.  But in some cases, the action of a
> +command is expected to change state in a way that a successful
> +response is not possible (the command still returns a normal
> +dictionary error on failure).  When a successful reply is not
> +possible, the command expression should include the optional key
> +'success-response' with boolean value false.  So far, only the
> +qemu-guest-agent makes use of this field.

I had no idea :)

>
> - { 'command': 'my-command',
> -   'data': { 'arg1': 'str', '*arg2': 'str' },
> -   'returns': 'str' }
>
>  === Events ===
>
> -Events are defined with the keyword 'event'.  When 'data' is also specified,
> -additional info will be included in the event.  Finally there will be C API
> -generated in qapi-event.h; when called by QEMU code, a message with timestamp
> -will be emitted on the wire.  If timestamp is -1, it means failure to 
> retrieve
> -host time.
> +Usage: { 'event': 'str', '*data': 'dict-or-complex-type-name' }
> +
> +Events are defined with the keyword 'event'.  It is not allowed to
> +name an event 'MAX', since the generator also produces a C enumeration
> +of all event names with a generated _MAX value at the end.  When
> +'data' is also specified, additional info will be included in the
> +event, with similar semantics to a 'type' expression.  Finally there
> +will be C API generated in qapi-event.h; when called by QEMU code, a
> +message with timestamp will be emitted on the wire.
>
>  An example event is:
>
> @@ -311,7 +531,7 @@ Example:
>      #ifndef EXAMPLE_QAPI_TYPES_H
>      #define EXAMPLE_QAPI_TYPES_H
>
> -[Builtin types omitted...]
> +[Built-in types omitted...]
>
>      typedef struct UserDefOne UserDefOne;
>
> @@ -324,7 +544,7 @@ Example:
>          struct UserDefOneList *next;
>      } UserDefOneList;
>
> -[Functions on builtin types omitted...]
> +[Functions on built-in types omitted...]
>
>      struct UserDefOne
>      {
> @@ -423,7 +643,7 @@ Example:
>      #ifndef EXAMPLE_QAPI_VISIT_H
>      #define EXAMPLE_QAPI_VISIT_H
>
> -[Visitors for builtin types omitted...]
> +[Visitors for built-in types omitted...]
>
>      void visit_type_UserDefOne(Visitor *m, UserDefOne **obj, const char 
> *name, Error **errp);
>      void visit_type_UserDefOneList(Visitor *m, UserDefOneList **obj, const 
> char *name, Error **errp);
> diff --git a/docs/qmp/qmp-spec.txt b/docs/qmp/qmp-spec.txt
> index 22568c6..1fb642b 100644
> --- a/docs/qmp/qmp-spec.txt
> +++ b/docs/qmp/qmp-spec.txt
> @@ -1,10 +1,20 @@
>                        QEMU Machine Protocol Specification
>
> +0. About This Document
> +======================
> +
> +This document is licensed under the GPLv2 (or later).
> +
> +Last revised in March 2015.
> +
>  1. Introduction
>  ===============
>
> -This document specifies the QEMU Machine Protocol (QMP), a JSON-based 
> protocol
> -which is available for applications to operate QEMU at the machine-level.
> +This document specifies the QEMU Machine Protocol (QMP), a JSON-based
> +protocol which is available for applications to operate QEMU at the
> +machine-level.  It is also in use by the QEMU Guest Agent (QGA), which
> +is available for host applications to interact with the guest
> +operating system.
>
>  2. Protocol Specification
>  =========================
> @@ -23,9 +33,11 @@ the JSON standard:
>
>  http://www.ietf.org/rfc/rfc4627.txt

Upgrade to RFC 7159?

>
> -For convenience, json-object members and json-array elements mentioned in
> -this document will be in a certain order. However, in real protocol usage
> -they can be in ANY order, thus no particular order should be assumed.
> +For convenience, json-object members mentioned in this document will
> +be in a certain order. However, in real protocol usage they can be in
> +ANY order, thus no particular order should be assumed. On the other
> +hand, use of json-array elements presumes that preserving order is
> +important unless specifically documented otherwise.
>
>  2.1 General Definitions
>  -----------------------
> @@ -52,7 +64,19 @@ The greeting message format is:
>  - The "version" member contains the Server's version information (the format
>    is the same of the query-version command)
>  - The "capabilities" member specify the availability of features beyond the
> -  baseline specification
> +  baseline specification; the order of elements in this array has no
> +  particular significance, so a client must search the entire array
> +  when looking for a particular capability
> +
> +When first connecting to the server, the connection is in a capability
> +exchange mode, further documented below.
> +
> +2.2.1 Capabilities
> +------------------
> +
> +As of the date this document was last revised, no server or client
> +capability strings have been defined.
> +
>
>  2.3 Issuing Commands
>  --------------------
> @@ -81,13 +105,15 @@ of a command execution: success or error.
>
>  The format of a success response is:
>
> -{ "return": json-object, "id": json-value }
> +{ "return": json-entity, "id": json-value }

Unlike the other json-FOOs we use, "entity" isn't defined in RFC4627.
"value" is, and we already use json-value.  What's the difference
between the two?

>
>   Where,
>
> -- The "return" member contains the command returned data, which is defined
> -  in a per-command basis or an empty json-object if the command does not
> -  return data
> +- The "return" member contains the data returned by the command, which
> +  is defined on a per-command basis (usually a json-object or
> +  json-array of json-objects, but sometimes a json-value, json-string,
> +  or json-array of json-strings); it is an empty json-object if the
> +  command does not return data

Err, aren't json-object, -string, -array all json-value?  At least
that's how the JSON RFC uses "value".

>  - The "id" member contains the transaction identification associated
>    with the command execution if issued by the Client
>
> @@ -114,7 +140,8 @@ if provided by the client.
>  -----------------------
>
>  As a result of state changes, the Server may send messages unilaterally
> -to the Client at any time. They are called "asynchronous events".
> +to the Client at any time, when not in the middle of any other
> +response. They are called "asynchronous events".
>
>  The format of asynchronous events is:
>
> @@ -126,13 +153,27 @@ The format of asynchronous events is:
>  - The "event" member contains the event's name
>  - The "data" member contains event specific data, which is defined in a
>    per-event basis, it is optional
> -- The "timestamp" member contains the exact time of when the event occurred
> -  in the Server. It is a fixed json-object with time in seconds and
> -  microseconds
> +- The "timestamp" member contains the exact time of when the event
> +  occurred in the Server. It is a fixed json-object with time in
> +  seconds and microseconds relative to the Unix Epoch (1 Jan 1970); if
> +  there is a failure to retrieve host time, both members of the
> +  timestamp will be set to -1.
>
>  For a listing of supported asynchronous events, please, refer to the
>  qmp-events.txt file.
>
> +2.5 QGA Synchronization
> +-----------------------
> +
> +When using QGA, an additional synchronization feature is built into
> +the protocol.  If the Client sends a raw 0xFF sentinel byte (not valid
> +JSON), then the Server will reset its state and discard all pending
> +data prior to the sentinel.  Conversely, if the Client makes use of
> +the 'guest-sync-delimited' command, the Server will send a raw 0xFF
> +sentinel byte prior to its response, to aid the Client in discarding
> +any data prior to the sentinel.
> +
> +
>  3. QMP Examples
>  ===============
>
> @@ -145,32 +186,37 @@ This section provides some examples of real QMP usage, 
> in all of them
>  S: { "QMP": { "version": { "qemu": { "micro": 50, "minor": 6, "major": 1 },
>       "package": ""}, "capabilities": []}}
>
> -3.2 Simple 'stop' execution
> +3.2 Client QMP negotiation
> +--------------------------
> +C: { "execute": "qmp_capabilities" }
> +S: { "return": {}}
> +
> +3.3 Simple 'stop' execution
>  ---------------------------
>
>  C: { "execute": "stop" }
>  S: { "return": {} }
>
> -3.3 KVM information
> +3.4 KVM information
>  -------------------
>
>  C: { "execute": "query-kvm", "id": "example" }
>  S: { "return": { "enabled": true, "present": true }, "id": "example"}
>
> -3.4 Parsing error
> +3.5 Parsing error
>  ------------------
>
>  C: { "execute": }
>  S: { "error": { "class": "GenericError", "desc": "Invalid JSON syntax" } }
>
> -3.5 Powerdown event
> +3.6 Powerdown event
>  -------------------
>
>  S: { "timestamp": { "seconds": 1258551470, "microseconds": 802384 },
>      "event": "POWERDOWN" }
>
>  4. Capabilities Negotiation
> -----------------------------
> +===========================
>
>  When a Client successfully establishes a connection, the Server is in
>  Capabilities Negotiation mode.
> @@ -189,7 +235,7 @@ effect, all commands (except qmp_capabilities) are 
> allowed and asynchronous
>  messages are delivered.
>
>  5 Compatibility Considerations
> -------------------------------
> +==============================
>
>  All protocol changes or new features which modify the protocol format in an
>  incompatible way are disabled by default and will be advertised by the
> @@ -213,12 +259,16 @@ However, Clients must not assume any particular:
>  - Amount of errors generated by a command, that is, new errors can be added
>    to any existing command in newer versions of the Server
>
> +Any command or field name beginning with "x-" is deemed experimental,
> +and may be withdrawn or changed in an incompatible manner in a future
> +release.
> +
>  Of course, the Server does guarantee to send valid JSON.  But apart from
>  this, a Client should be "conservative in what they send, and liberal in
>  what they accept".
>
>  6. Downstream extension of QMP
> -------------------------------
> +==============================
>
>  We recommend that downstream consumers of QEMU do *not* modify QMP.
>  Management tools should be able to support both upstream and downstream

Massive improvement.  We can always improve some more on top, thus:

Reviewed-by: Markus Armbruster <address@hidden>