[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 01/23] docs: update QMP documents for OOB com
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v8 01/23] docs: update QMP documents for OOB commands |
Date: |
Mon, 12 Mar 2018 11:32:46 +0800 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Fri, Mar 09, 2018 at 11:13:23AM -0600, Eric Blake wrote:
> On 03/09/2018 02:59 AM, Peter Xu wrote:
> > Update both the developer and spec for the new QMP OOB (Out-Of-Band)
> > command.
> >
> > Signed-off-by: Peter Xu <address@hidden>
> > ---
> > docs/devel/qapi-code-gen.txt | 65
> > ++++++++++++++++++++++++++++++++++++++++----
> > docs/interop/qmp-spec.txt | 30 +++++++++++++++++---
> > 2 files changed, 86 insertions(+), 9 deletions(-)
>
> If all goes well, I'm planning on taking this series through my QAPI tree,
> with a pull request either late today or early Monday in order to make the
> soft freeze deadline. We'll see what my review turns up, but hopefully at
> this point it's either clean or minor enough tweaks that I can polish it
> without a v9.
That'll be great. Thanks Eric!
[...]
> > @@ -102,10 +113,16 @@ The format for command execution is:
> > required. Each command documents what contents will be considered
> > valid when handling the json-argument
> > - The "id" member is a transaction identification associated with the
> > - command execution, it is optional and will be part of the response if
> > + command execution. It is required if OOB is enabled, and optional
> > + if not. The same "id" field will be part of the response if
>
> Ambiguous on whether this is the per-command 'run-oob', or whether this is
> the generic QMP capability requested during qmp_capabilities. I think the
> intent is that if you enabled the QMP capability up front, ALL commands must
> have an "id", even if they are run in-band without 'run-oob', because the
> 'id' in the response is what will distinguish whether a later OOB request
> overtook a pending in-band reply.
Yes, maybe I should use "OOB capability" to be explicit - exactly as
you explained.
>
> > 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 ("oob" as shortcut). The handling or
>
> A bit late to be introducing "oob" as shortcut, given that you've already
> used the abbreviation oob earlier in the document.
>
> > + 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 } }
> > 2.4 Commands Responses
> > ----------------------
> > @@ -113,6 +130,11 @@ The format for command execution is:
> > There are two possible responses which the Server will issue as the result
> > of a command execution: success or error.
> > +As long as the commands were issued with a proper "id" field, then the
> > +same "id" field will be attached in the corresponding response message
> > +so that requests and responses can match. Clients should drop all the
> > +responses that are with unknown "id" field.
>
> s/are with/have an/
>
> I've got quite a few wording tweaks, but the general concept is good. If you
> need to respin for other reasons, feel free to make those improvements, but
> I also don't mind making tweaks myself as part of queuing for a pull
> request, so:
>
> Reviewed-by: Eric Blake <address@hidden>
(I agree on all the rest of the review comments too)
Thanks!
--
Peter Xu
- [Qemu-devel] [PATCH v8 00/23] QMP: out-of-band (OOB) execution support, Peter Xu, 2018/03/09
- [Qemu-devel] [PATCH v8 02/23] qobject: introduce qstring_get_try_str(), Peter Xu, 2018/03/09
- [Qemu-devel] [PATCH v8 01/23] docs: update QMP documents for OOB commands, Peter Xu, 2018/03/09
- [Qemu-devel] [PATCH v8 03/23] qobject: introduce qobject_get_try_str(), Peter Xu, 2018/03/09
- [Qemu-devel] [PATCH v8 04/23] qobject: let object_property_get_str() use new API, Peter Xu, 2018/03/09
- [Qemu-devel] [PATCH v8 05/23] monitor: move skip_flush into monitor_data_init, Peter Xu, 2018/03/09
- [Qemu-devel] [PATCH v8 06/23] monitor: move the cur_mon hack deeper for QMP, Peter Xu, 2018/03/09
- [Qemu-devel] [PATCH v8 08/23] monitor: let mon_list be tail queue, Peter Xu, 2018/03/09
- [Qemu-devel] [PATCH v8 07/23] monitor: unify global init, Peter Xu, 2018/03/09
- [Qemu-devel] [PATCH v8 09/23] monitor: allow using IO thread for parsing, Peter Xu, 2018/03/09