Re: [Qemu-devel] [PATCH] Deprecate QMP `cpu-add`

[Kashyap Chamarthy]

On Thu, Sep 20, 2018 at 02:27:02PM +0200, Markus Armbruster wrote:
> Kashyap Chamarthy <address@hidden> writes:

[...]

> When I try this, JSON gets formatted slightly differently:
> 
>     {
>         "execute": "query-hotpluggable-cpus", 
>         "arguments": {}
>     }
>     {
>         "return": [
>             {
>                 "type": "IvyBridge-IBRS-x86_64-cpu", 
>                 "vcpus-count": 1, 
>                 "props": {
>                     "socket-id": 1, 
>                     "core-id": 0, 
>                     "thread-id": 0
>                 }
>             }, 
>             {
>                 "qom-path": "/machine/unattached/device[0]", 
>                 "type": "IvyBridge-IBRS-x86_64-cpu", 
>                 "vcpus-count": 1, 
>                 "props": {
>                     "socket-id": 0, 
>                     "core-id": 0, 
>                     "thread-id": 0
>                 }
>             }
>         ]
>     }
> 
> How come?

You caught me :-).  I actually get the same as you well.  But I fudged
the output by running it through:
https://jsonformatter.curiousconcept.com/ with "2 Space Tab", but it
doesn't make any difference.  

So I'll actually retain the original raw output from 'qmp-shell'.

> > (4) We can see that socket 1 is free, so use `device_add` to hotplug
> >     "IvyBridge-IBRS-x86_64-cpu":
> >
> >     (QEMU) device_add id=cpu-2 driver=IvyBridge-IBRS-x86_64-cpu socket-id=1 
> > core-id=0 thread-id=0
> >     {
> >         "execute": "device_add",
> >         "arguments": {
> >             "socket-id": 1,
> >             "driver": "IvyBridge-IBRS-x86_64-cpu",
> >             "id": "cpu-2",
> >             "core-id": 0,
> >             "thread-id": 0
> >         }
> >     }
> >     {
> >         "return": {}
> >     }
> >     (QEMU)
> >
> > (5) Optionally, run QMP `query-cpus-fast` for some details about the
> >     vCPUs.
> 
> Publishing this in a commit message is much better than not publishing
> it.  Still, can we find a better home somewhere under docs/?

Good point.  I'll make an 'cpu-hotplug.rst' in docs/ and submit it
as a patch in v2.  Hope that's okay with you.

[...]

> > ---
> > Also shouldn't we update tests/cpu-plug-test.c to make it use
> > `device_add`.  Can take a stab at it, if you'd like me to.
> 
> Yes, please.

Igor was saying on IRC that we can fix the test when we _actually_ do
the deprecation.  So I'll make a TODO (and won't forget) to make it as a
separate patch after we deprecate this.

[...]

> >  # @id: ID of CPU to be created, valid values [0..max_cpus)
> >  #
> 
> Eduardo pointed out that query-hotpluggable-cpus's documentation is
> lacking.  Add a suitable TODO there?

Yes, will do.

> > diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> > index 1b9c007f12..9c6d70d43a 100644
> > --- a/qemu-deprecated.texi
> > +++ b/qemu-deprecated.texi
> > @@ -155,6 +155,11 @@ The ``query-cpus'' command is replaced by the 
> > ``query-cpus-fast'' command.
> >  The ``arch'' output member of the ``query-cpus-fast'' command is
> >  replaced by the ``target'' output member.
> >  
> > address@hidden cpu-add (since 3.1)
> > +
> > +The intended functionality of ``cpu-add'' command, which is the ability
> > +to hot-plug vCPUs, can now be achieved by the ``device_add'' command.
> > +
> 
> For me, "now" suggests device_add can achieve this since 3.1.  Scratch
> "now"?

Yes, will do.

Thanks for the quick review!

-- 
/kashyap