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

[Markus Armbruster]

Eric Blake <address@hidden> 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 <address@hidden>
>> ---
>>   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 <address@hidden>
>
> 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!