On Thu, Jan 11, 2018 at 05:07:11PM -0600, Eric Blake wrote: > On 12/19/2017 02:45 AM, Peter Xu wrote: > > There was no QMP capabilities defined. Define the first "oob" as > > s/was/were/
Fixed. Just to confirm: is "There was no QMP capability" also correct? > > > capability to allow out-of-band messages. > > > > Also, touch up qmp-test.c to test the new bits. > > > > Signed-off-by: Peter Xu <pet...@redhat.com> > > --- > > 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. Yes, I put the whole document at the end of the series. I can put it as the first patch too. > > > +++ 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). Yes, client can't if without further patches. > > > > > +## > > +# @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. I'll take your advise. > > > +# > > +# @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...) Sorry for the inconvenience. Please refer to the last doc patch for details. I thought the doc patch would explain itself but obviously I should be more careful on the ordering next time to ease reviewers. I'll move the doc patch to front when repost, and I'll note here to refer to qmp-spec.txt for more information. Thanks, -- Peter Xu