[Qemu-devel] [PATCH 03/32] docs/interop/qmp: Improve OOB documentation

Markus Armbruster posted 32 patches 7 years, 4 months ago
[Qemu-devel] [PATCH 03/32] docs/interop/qmp: Improve OOB documentation
Posted by Markus Armbruster 7 years, 4 months ago
OOB documentation is spread over qmp-spec.txt sections 2.2.1
Capabilities and 2.3 Issuing Commands.  The amount of detail is a bit
distracting there.  Move the meat of the matter to new section 2.3.1
Out of band execution.

Throw in a few other improvements while there:

* 2.2 Server Greetung: Drop advice to search entire capabilities
  array; should be obvious.

* 3. QMP Examples

  - 3.1 Server Greeting: Update greeting to current one.  Now shows
    capability "oob".  Update qmp-intro.txt likewise.

  - 3.2 Capabilities negotiation: Show client accepting capability
    "oob".

  - 3.7 Out-of-band execution: New.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/interop/qmp-intro.txt | 13 +++----
 docs/interop/qmp-spec.txt  | 74 +++++++++++++++++++++++++-------------
 2 files changed, 56 insertions(+), 31 deletions(-)

diff --git a/docs/interop/qmp-intro.txt b/docs/interop/qmp-intro.txt
index 900d69d612..3dafa35ea6 100644
--- a/docs/interop/qmp-intro.txt
+++ b/docs/interop/qmp-intro.txt
@@ -52,13 +52,14 @@ Escape character is '^]'.
     "QMP": {
         "version": {
             "qemu": {
-                "micro": 50, 
-                "minor": 6, 
-                "major": 1
-            }, 
-            "package": ""
-        }, 
+                "micro": 50,
+                "minor": 12,
+                "major": 2
+            },
+            "package": "v2.12.0-1763-g4e022f5ccd"
+        },
         "capabilities": [
+            "oob"
         ]
     }
 }
diff --git a/docs/interop/qmp-spec.txt b/docs/interop/qmp-spec.txt
index 2bb492d1ea..e5f8116c54 100644
--- a/docs/interop/qmp-spec.txt
+++ b/docs/interop/qmp-spec.txt
@@ -77,8 +77,7 @@ The greeting message format is:
   is the same of the query-version command)
 - The "capabilities" member specify the availability of features beyond the
   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
+  particular significance.
 
 2.2.1 Capabilities
 ------------------
@@ -86,16 +85,7 @@ The greeting message format is:
 Currently supported capabilities are:
 
 - "oob": the QMP server supports "out-of-band" (OOB) command
-  execution.  For more details, please see the "run-oob" parameter in
-  the "Issuing Commands" section below.  Not all commands allow this
-  "oob" execution.  The "query-qmp-schema" command can be used to
-  inspect which commands support "oob" execution.
-
-QMP clients can get a list of supported QMP capabilities of the QMP
-server in the greeting message mentioned above.  By default, all the
-capabilities are off.  To enable any QMP capabilities, the QMP client
-needs to send the "qmp_capabilities" command with an extra parameter
-for the requested capabilities.
+  execution, as described in section "2.3.1 Out-of-band execution".
 
 2.3 Issuing Commands
 --------------------
@@ -115,14 +105,38 @@ The format for command execution is:
 - The "id" member is a transaction identification associated with the
   command execution.  It is required for all commands if the OOB -
   capability was enabled at startup, and optional otherwise.  The same
-  "id" field will be part of the response if provided. The "id" member
-  can be any json-value, although most clients merely use a
-  json-number incremented for each successive command
-- The "control" member is optional, and currently only used for
-  out-of-band execution. The handling or response of an "oob" command
-  can overtake prior in-band commands.  To enable "oob" handling of a
-  particular command, just provide a control field with: { "control":
-  { "run-oob": true } }
+  "id" field will be part of the response if provided.  The "id"
+  member can be any json-value.  A json-number incremented for each
+  successive command works fine.
+- The optional "control" member further specifies how the command is
+  to be executed.  Currently, its only member is optional "run-oob".
+  See section "2.3.1 Out-of-band execution" for details.
+
+
+2.3.1 Out-of-band execution
+---------------------------
+
+The server normally reads, executes and responds to one command after
+the other.  The client therefore receives command respones in issue
+order.
+
+With out-of-band execution enabled via capability negotiation (section
+4.), the server reads and queues commands as they arrive.  It executes
+commands from the queue one after the other.  Commands executed
+out-of-band jump the queue: they command get executed right away,
+possibly overtaking prior in-band commands.  The client may therefore
+receive such a command's response before responses from prior in-band
+commands.
+
+To execute a command out-of-band, the client puts "run-oob": true into
+execute's member "control".
+
+If the client sends in-band commands faster than the server can
+execute them, the server will eventually drop commands to limit the
+queue length.  The sever sends event COMMAND_DROPPED then.
+
+Only a few commands support out-of-band execution.  The ones that do
+have "allow-oob": true in output of query-qmp-schema.
 
 2.4 Commands Responses
 ----------------------
@@ -223,12 +237,13 @@ This section provides some examples of real QMP usage, in all of them
 3.1 Server greeting
 -------------------
 
-S: { "QMP": { "version": { "qemu": { "micro": 50, "minor": 6, "major": 1 },
-     "package": ""}, "capabilities": []}}
+S: { "QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2},
+     "package": "v2.12.0-1763-g4e022f5ccd"}, "capabilities": ["oob"] } }
 
-3.2 Client QMP negotiation
---------------------------
-C: { "execute": "qmp_capabilities" }
+3.2 Capabilities negotiation
+----------------------------
+
+C: { "execute": "qmp_capabilities", "arguments": { "enable": ["oob"] } }
 S: { "return": {}}
 
 3.3 Simple 'stop' execution
@@ -255,6 +270,15 @@ S: { "error": { "class": "GenericError", "desc": "Invalid JSON syntax" } }
 S: { "timestamp": { "seconds": 1258551470, "microseconds": 802384 },
     "event": "POWERDOWN" }
 
+3.7 Out-of-band execution
+-------------------------
+
+C: { "execute": "migrate-pause", "id": 42, "control": { "run-oob": true } }
+S: { "id": 42,
+     "error": { "class": "GenericError",
+      "desc": "migrate-pause is currently only supported during postcopy-active state" } }
+
+
 4. Capabilities Negotiation
 ===========================
 
-- 
2.17.1


Re: [Qemu-devel] [PATCH 03/32] docs/interop/qmp: Improve OOB documentation
Posted by Eric Blake 7 years, 4 months ago
On 07/02/2018 11:21 AM, Markus Armbruster wrote:
> OOB documentation is spread over qmp-spec.txt sections 2.2.1
> Capabilities and 2.3 Issuing Commands.  The amount of detail is a bit
> distracting there.  Move the meat of the matter to new section 2.3.1
> Out of band execution.
> 
> Throw in a few other improvements while there:
> 
> * 2.2 Server Greetung: Drop advice to search entire capabilities

s/Greetung/Greeting/

>    array; should be obvious.
> 
> * 3. QMP Examples
> 
>    - 3.1 Server Greeting: Update greeting to current one.  Now shows
>      capability "oob".  Update qmp-intro.txt likewise.
> 
>    - 3.2 Capabilities negotiation: Show client accepting capability
>      "oob".
> 
>    - 3.7 Out-of-band execution: New.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   docs/interop/qmp-intro.txt | 13 +++----
>   docs/interop/qmp-spec.txt  | 74 +++++++++++++++++++++++++-------------
>   2 files changed, 56 insertions(+), 31 deletions(-)
> 
> diff --git a/docs/interop/qmp-intro.txt b/docs/interop/qmp-intro.txt
> index 900d69d612..3dafa35ea6 100644
> --- a/docs/interop/qmp-intro.txt
> +++ b/docs/interop/qmp-intro.txt
> @@ -52,13 +52,14 @@ Escape character is '^]'.
>       "QMP": {
>           "version": {
>               "qemu": {
> -                "micro": 50,
> -                "minor": 6,
> -                "major": 1
> -            },
> -            "package": ""
> -        },
> +                "micro": 50,
> +                "minor": 12,
> +                "major": 2
> +            },
> +            "package": "v2.12.0-1763-g4e022f5ccd"

Obviously tested during development rather than on a released version, 
but that's reasonable.


> +2.3.1 Out-of-band execution
> +---------------------------
> +
> +The server normally reads, executes and responds to one command after
> +the other.  The client therefore receives command respones in issue

s/respones/responses/

> +order.
> +
> +With out-of-band execution enabled via capability negotiation (section
> +4.), the server reads and queues commands as they arrive.  It executes
> +commands from the queue one after the other.  Commands executed
> +out-of-band jump the queue: they command get executed right away,

s/they command get/the command gets/

> +possibly overtaking prior in-band commands.  The client may therefore
> +receive such a command's response before responses from prior in-band
> +commands.
> +
> +To execute a command out-of-band, the client puts "run-oob": true into
> +execute's member "control".

Are we still hoping for a later patch to ditch "control" and add 
"exec-oob"?  But if we do that, building it on top of this patch (with 
appropriate doc tweaks at that time) makes sense.

> +3.7 Out-of-band execution
> +-------------------------
> +
> +C: { "execute": "migrate-pause", "id": 42, "control": { "run-oob": true } }
> +S: { "id": 42,
> +     "error": { "class": "GenericError",
> +      "desc": "migrate-pause is currently only supported during postcopy-active state" } }

Different from a successful command, but works for me.

With the typo fixes,
Reviewed-by: Eric Blake <eblake@redhat.com>

I don't know if this patch will make it in time for 3.0 soft freeze, but 
resolving missing documentation feels like enough of a bug fix to be 
included during freeze as needed.

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

Re: [Qemu-devel] [PATCH 03/32] docs/interop/qmp: Improve OOB documentation
Posted by Markus Armbruster 7 years, 4 months ago
Eric Blake <eblake@redhat.com> writes:

> On 07/02/2018 11:21 AM, Markus Armbruster wrote:
>> OOB documentation is spread over qmp-spec.txt sections 2.2.1
>> Capabilities and 2.3 Issuing Commands.  The amount of detail is a bit
>> distracting there.  Move the meat of the matter to new section 2.3.1
>> Out of band execution.
>>
>> Throw in a few other improvements while there:
>>
>> * 2.2 Server Greetung: Drop advice to search entire capabilities
>
> s/Greetung/Greeting/
>
>>    array; should be obvious.
>>
>> * 3. QMP Examples
>>
>>    - 3.1 Server Greeting: Update greeting to current one.  Now shows
>>      capability "oob".  Update qmp-intro.txt likewise.
>>
>>    - 3.2 Capabilities negotiation: Show client accepting capability
>>      "oob".
>>
>>    - 3.7 Out-of-band execution: New.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   docs/interop/qmp-intro.txt | 13 +++----
>>   docs/interop/qmp-spec.txt  | 74 +++++++++++++++++++++++++-------------
>>   2 files changed, 56 insertions(+), 31 deletions(-)
>>
>> diff --git a/docs/interop/qmp-intro.txt b/docs/interop/qmp-intro.txt
>> index 900d69d612..3dafa35ea6 100644
>> --- a/docs/interop/qmp-intro.txt
>> +++ b/docs/interop/qmp-intro.txt
>> @@ -52,13 +52,14 @@ Escape character is '^]'.
>>       "QMP": {
>>           "version": {
>>               "qemu": {
>> -                "micro": 50,
>> -                "minor": 6,
>> -                "major": 1
>> -            },
>> -            "package": ""
>> -        },
>> +                "micro": 50,
>> +                "minor": 12,
>> +                "major": 2
>> +            },
>> +            "package": "v2.12.0-1763-g4e022f5ccd"
>
> Obviously tested during development rather than on a released version,
> but that's reasonable.

I could change it to whatever we expect for 3.0.  Tidier, I guess.

>> +2.3.1 Out-of-band execution
>> +---------------------------
>> +
>> +The server normally reads, executes and responds to one command after
>> +the other.  The client therefore receives command respones in issue
>
> s/respones/responses/
>
>> +order.
>> +
>> +With out-of-band execution enabled via capability negotiation (section
>> +4.), the server reads and queues commands as they arrive.  It executes
>> +commands from the queue one after the other.  Commands executed
>> +out-of-band jump the queue: they command get executed right away,
>
> s/they command get/the command gets/

My typing deteriorates as the soft freeze closes in...

>> +possibly overtaking prior in-band commands.  The client may therefore
>> +receive such a command's response before responses from prior in-band
>> +commands.
>> +
>> +To execute a command out-of-band, the client puts "run-oob": true into
>> +execute's member "control".
>
> Are we still hoping for a later patch to ditch "control" and add
> "exec-oob"?  But if we do that, building it on top of this patch (with
> appropriate doc tweaks at that time) makes sense.

[PATCH 12/32] qmp: Redo how the client requests out-of-band execution

>> +3.7 Out-of-band execution
>> +-------------------------
>> +
>> +C: { "execute": "migrate-pause", "id": 42, "control": { "run-oob": true } }
>> +S: { "id": 42,
>> +     "error": { "class": "GenericError",
>> +      "desc": "migrate-pause is currently only supported during postcopy-active state" } }
>
> Different from a successful command, but works for me.
>
> With the typo fixes,
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
> I don't know if this patch will make it in time for 3.0 soft freeze,
> but resolving missing documentation feels like enough of a bug fix to
> be included during freeze as needed.

The OOB feature has been merged already.  We had to downgrade it to
"experimental" when we ran into regressions.  Lucky us; we found more
issues since then.  This series works towards getting it ready.  I think
that's something we can do even after soft feature freeze, as long as we
deem the patches sufficiently unintrusive.  Judgement call, soliciting
yours.

Thanks!