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

[Markus Armbruster]

Eduardo Habkost <address@hidden> writes:

> 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).

Agreed.

>
>> > > +#        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.

Decouple improving documentation of query-hotpluggable-cpus from this
patch by adding a TODO comment to the documentation?

> 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?

Good question.