Re: [Qemu-devel] [RFC v6 11/27] qmp: introduce QMPCapability

[Eric Blake]

On 12/19/2017 02:45 AM, Peter Xu wrote:
> There was no QMP capabilities defined.  Define the first "oob" as

s/was/were/

> capability to allow out-of-band messages.
> 
> Also, touch up qmp-test.c to test the new bits.
> 
> Signed-off-by: Peter Xu <address@hidden>
> ---
>  monitor.c        | 10 ++++++++--
>  qapi-schema.json | 13 +++++++++++++
>  tests/qmp-test.c | 10 +++++++++-
>  3 files changed, 30 insertions(+), 3 deletions(-)

I'm assuming later patches will document this?

I'm somewhat a fan of documentation alongside or before implementation,
as getting the general overview right and then checking that the code
matches is a bit nicer than random coding then documenting what we ended
up with.  But I don't know if reordering patches in your series is
necessary, as long as the end product is properly documented.

> +++ b/qapi-schema.json
> @@ -118,6 +118,19 @@
>  ##
>  { 'command': 'qmp_capabilities' }

The client can't request a particular feature alongside the command?  Or
is that in later patches?  With just this patch, the enum QMPCapability
is not introspected, because it is not referenced by any command
(although introspection is a bit moot, since the client will learn what
the host advertises from the initial handshake before the client can
even request introspection).

>  
> +##
> +# @QMPCapability:
> +#
> +# QMP supported capabilities to be broadcasted to the clients.

'broadcast' is one of those weird verbs that doesn't change spelling
when constructing its past tense (there is no 'broadcasted').  However,
I think this description is a bit nicer (and avoids the problematic word
altogether):

Enumeration of capabilities to be advertised during initial client
connection, used for agreeing on particular QMP extension behaviors.

> +#
> +# @oob:   QMP ability to support Out-Of-Band requests.

Rather terse (it doesn't say what Out-Of-Band requests are); even a
pointer to the QMP spec (where OOB is more fully documented) might be
nice (of course, that means we need a patch to docs/interop/qmp-spec.txt
somewhere in the series, especially since this patch renders 2.2.1 in
that document obsolete...)

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

signature.asc
Description: OpenPGP digital signature