Re: [Qemu-devel] [PATCH] qapi/misc.json: Clarify about usage of QMP 'cpu-add'

[Eduardo Habkost]

On Wed, Sep 19, 2018 at 12:22:19PM +0200, Kashyap Chamarthy wrote:
> On Tue, Sep 18, 2018 at 11:19:38AM -0300, Eduardo Habkost wrote:
> > On Tue, Sep 18, 2018 at 03:23:03PM +0200, Kashyap Chamarthy wrote:
> > > Eduardo Habkost mentioned on IRC that the intended functionality of QMP
> > > 'cpu-add' is replaced with a combination of 'query-hotpluggable-cpus'
> > > and 'device_add'.  And 'cpu-add' is likely to be deprecated in the
> > > future.
> > > 
> > > Add a note in the QAPI schema to reflect that.
> > > 
> > > Suggested-by: Eduardo Habkost <address@hidden
> > > Signed-off-by: Kashyap Chamarthy <address@hidden>
> > > ---
> > >  qapi/misc.json | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/qapi/misc.json b/qapi/misc.json
> > > index d450cfef21..178a94b904 100644
> > > --- a/qapi/misc.json
> > > +++ b/qapi/misc.json
> > > @@ -1104,7 +1104,13 @@
> > >  ##
> > >  # @cpu-add:
> > >  #
> > > -# Adds CPU with specified ID
> > > +# Adds CPU with specified ID.
> > > +#
> > > +# Notes: This command is likely to be deprecated in the future.  The
> > 
> > I suggest we deprecate the command instead of documenting it as
> > likely to be deprecated.
> 
> Noted; shall I send a reworded patch to say: "This command will be
> deprecated in the near future"?

I'd prefer a patch (against this file and qemu-deprecated.texi)
saying "this command is deprecated".

I don't see a reason to not deprecate the command immediately
(instead of just promising that it will be deprecated).


> > > +#        way to achieve the intended functionality of 'cpu-add', which
> > > +#        is to allow CPU hot-plug, is possible with the combination of
> > > +#        QMP 'query-hotpluggable-cpus' and 'device_add'.  (And
> > > +#        hot-unplug via 'device_del'.)
> > 
> > I was going to suggest just writing "This command is deprecated
> > and was replaced by device_add.  See query-hotpluggable-cpus for
> > details", but it looks like the query-hotpluggable-cpus
> > documentation doesn't explain how exactly we can use its results.
> 
> Yeah, to be clear, would you like me to amend the text in a different
> phrasing?  Or is the current phrasing alright?  Maybe needs an
> additional sentence about: "Refer the documentation of
> 'query-hotpluggable-cpus'"?

I'd prefer a more succinct phrasing.  e.g.:

  "This command is deprecated.  The `device_add` command should
  be used instead.  See the `query-hotpluggable-cpus` command for
  details."

The main problem is that the details I expected to see on the
documentation of `query-hotpluggable-cpus` aren't there.

But before writing `query-hotpluggable-cpus` documentation from
scratch, I would like to find the answer to this:

> 
> > Igor, is the QMP documentation for query-hotpluggable-cpus the
> > only documentation we have on CPU hotplug?
> > 
> > 


-- 
Eduardo