qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 1/2] docs/interop: Convert qmp-spec.txt to rST


From: Eric Blake
Subject: Re: [PATCH 1/2] docs/interop: Convert qmp-spec.txt to rST
Date: Thu, 20 Apr 2023 13:42:18 -0500
User-agent: NeoMutt/20230407

On Thu, Apr 20, 2023 at 04:03:51PM +0100, Peter Maydell wrote:
> Convert the qmp-spec.txt document to restructuredText.
> Notable points about the conversion:
>  * numbers at the start of section headings are removed, to match
>    the style of the rest of the manual
>  * cross-references to other sections or documents are hyperlinked
>  * various formatting tweaks (notably the examples, which need the
>    -> and <- prefixed so the QMP code-block lexer will accept them)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---

> +Server Greeting
> +---------------
> +
>  Right when connected the Server will issue a greeting message, which signals
>  that the connection has been successfully established and that the Server is
>  ready for capabilities negotiation (for more information refer to section
> -'4. Capabilities Negotiation').
> +`Capabilities Negotiation`_).
>  
>  The greeting message format is:
>  
> -{ "QMP": { "version": json-object, "capabilities": json-array } }
> +.. code-block::
>  
> - Where,
> +  { "QMP": { "version": json-object, "capabilities": json-array } }
>  
> -- The "version" member contains the Server's version information (the format
> +Where:
> +
> +- The ``version`` member contains the Server's version information (the 
> format
>    is the same of the query-version command)

As long as we're touching this, I think s/the same of the/the same as
for the/ sounds better.
> +Issuing Commands
> +----------------

> +Where:
> +
> +- The ``execute`` or ``exec-oob`` member identifies the command to be
>    executed by the server.  The latter requests out-of-band execution.

Ends in '.'

> -- The "arguments" member is used to pass any arguments required for the
> +- The ``arguments`` member is used to pass any arguments required for the
>    execution of the command, it is optional when no arguments are
>    required. Each command documents what contents will be considered
>    valid when handling the json-argument

does not end in '.' (pre-existing)

> -- The "id" member is a transaction identification associated with the
> +- The ``id`` member is a transaction identification associated with the
>    command execution, it is optional and will be part of the response
> -  if provided.  The "id" member can be any json-value.  A json-number
> +  if provided.  The ``id`` member can be any json-value.  A json-number
>    incremented for each successive command works fine.

ends in '.'.  While here, it might be nice to consistently end in '.'

> +Asynchronous events
> +-------------------
>  
>  As a result of state changes, the Server may send messages unilaterally
>  to the Client at any time, when not in the middle of any other
> @@ -198,44 +208,45 @@ response. They are called "asynchronous events".
>  
>  The format of asynchronous events is:
>  
> -{ "event": json-string, "data": json-object,
> -  "timestamp": { "seconds": json-number, "microseconds": json-number } }
> +.. code-block::
>  
> - Where,
> +  { "event": json-string, "data": json-object,
> +    "timestamp": { "seconds": json-number, "microseconds": json-number } }
>  
> -- 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
> +Where:
> +
> +- 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

Pre-patch this was one sentence, not ending in '.'. Post-patch it is
two sentences, but the second still lacks '.';

> +- 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.

while this one ends in '.'. Another place where consistency would be nice.

> +Forcing the JSON parser into known-good state
> +---------------------------------------------
>  
>  Incomplete or invalid input can leave the server's JSON parser in a
>  state where it can't parse additional commands.  To get it back into
>  known-good state, the client should provoke a lexical error.
>  
>  The cleanest way to do that is sending an ASCII control character
> -other than '\t' (horizontal tab), '\r' (carriage return), or '\n' (new
> -line).
> +other than ``\t`` (horizontal tab), ``\r`` (carriage return), or
> +``\n`` (new line).
>  
>  Sadly, older versions of QEMU can fail to flag this as an error.  If a
>  client needs to deal with them, it should send a 0xFF byte.

Do we still care about these older versions of QEMU, or has it been
long enough that we can start trimming this text?  (But that would be
a separate patch).

Overall, this is a nice patch; it looked fairly mechanical (and
produced a hard-to-read diff, but that's not your fault).  Separating
mechanics from content change is good, so I'm fine with:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




reply via email to

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