diff mbox

[v3,1/2] QMP: add query-hotpluggable-cpus

Message ID 1458048248-4605-2-git-send-email-imammedo@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Igor Mammedov March 15, 2016, 1:24 p.m. UTC 
it will allow mgmt to query present and hotpluggable
CPU objects, it is required from a target platform that
wish to support command to implement
 qmp_query_hotpluggable_cpus()
function, which will return a list of possible CPU objects
with options that would be needed for hotplugging possible
CPU objects.

There are:
'type': 'str' - QOM CPU object type for usage with device_add
'vcpus-count': 'int' - number of logical VCPU threads per
                        CPU object (mgmt needs to know)

and a set of optional fields that are to used for hotplugging
a CPU objects and would allows mgmt tools to know what/where
it could be hotplugged;
[node],[socket],[core],[thread]

For present CPUs there is a 'qom-path' field which
would allow mgmt to inspect whatever object/abstraction
the target platform considers as CPU object.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
 - add 'vcpus-count' field, pkrempa@redhat.com
 - s/CpuInstanceProps/CpuInstanceProperties/
 - use '#optional' marker
 - make "props" as always present even if it's empty
 - fix JSON examples
 - fix minor typos
---
 qapi-schema.json                    | 41 +++++++++++++++++++++++++++++++++++
 qmp-commands.hx                     | 43 +++++++++++++++++++++++++++++++++++++
 stubs/Makefile.objs                 |  1 +
 stubs/qmp_query_hotpluggable_cpus.c |  9 ++++++++
 4 files changed, 94 insertions(+)
 create mode 100644 stubs/qmp_query_hotpluggable_cpus.c

Comments

Eduardo Habkost March 18, 2016, 7:26 p.m. UTC | #1 
On Tue, Mar 15, 2016 at 02:24:07PM +0100, Igor Mammedov wrote:
[...]
> diff --git a/stubs/qmp_query_hotpluggable_cpus.c b/stubs/qmp_query_hotpluggable_cpus.c
> new file mode 100644
> index 0000000..21a75a3
> --- /dev/null
> +++ b/stubs/qmp_query_hotpluggable_cpus.c
> @@ -0,0 +1,9 @@
> +#include "qemu/osdep.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qmp-commands.h"
> +
> +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> +{
> +    error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> +    return NULL;
> +}

Sorry if this was discussed in previous threads that I haven't
read, but: isn't this supposed to be a MachineClass method?  I
remember David saying once that we have the habit of assuming
that a single QEMU binary can run only one family of machines
that are very similar (like x86), but that's not always true.
Igor Mammedov March 21, 2016, 10:53 a.m. UTC | #2 
On Fri, 18 Mar 2016 16:26:28 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Tue, Mar 15, 2016 at 02:24:07PM +0100, Igor Mammedov wrote:
> [...]
> > diff --git a/stubs/qmp_query_hotpluggable_cpus.c b/stubs/qmp_query_hotpluggable_cpus.c
> > new file mode 100644
> > index 0000000..21a75a3
> > --- /dev/null
> > +++ b/stubs/qmp_query_hotpluggable_cpus.c
> > @@ -0,0 +1,9 @@
> > +#include "qemu/osdep.h"
> > +#include "qapi/qmp/qerror.h"
> > +#include "qmp-commands.h"
> > +
> > +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> > +{
> > +    error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> > +    return NULL;
> > +}  
> 
> Sorry if this was discussed in previous threads that I haven't
> read, but: isn't this supposed to be a MachineClass method?  I
> remember David saying once that we have the habit of assuming
> that a single QEMU binary can run only one family of machines
> that are very similar (like x86), but that's not always true.
Stub approach works for current qemu with one target per binary
but it won't for multi-target binary.

I've been trying to not clutter MachineClass with hooks
that not must have right now but I don't have a strong opinion
on this so if MachineClass method is preferred way,
I can rewrite it this patch to use it on respin.
David Gibson March 21, 2016, 11:39 p.m. UTC | #3 
On Mon, 21 Mar 2016 11:53:23 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

> On Fri, 18 Mar 2016 16:26:28 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Tue, Mar 15, 2016 at 02:24:07PM +0100, Igor Mammedov wrote:
> > [...]  
> > > diff --git a/stubs/qmp_query_hotpluggable_cpus.c b/stubs/qmp_query_hotpluggable_cpus.c
> > > new file mode 100644
> > > index 0000000..21a75a3
> > > --- /dev/null
> > > +++ b/stubs/qmp_query_hotpluggable_cpus.c
> > > @@ -0,0 +1,9 @@
> > > +#include "qemu/osdep.h"
> > > +#include "qapi/qmp/qerror.h"
> > > +#include "qmp-commands.h"
> > > +
> > > +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> > > +{
> > > +    error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> > > +    return NULL;
> > > +}    
> > 
> > Sorry if this was discussed in previous threads that I haven't
> > read, but: isn't this supposed to be a MachineClass method?  I
> > remember David saying once that we have the habit of assuming
> > that a single QEMU binary can run only one family of machines
> > that are very similar (like x86), but that's not always true.  
> Stub approach works for current qemu with one target per binary
> but it won't for multi-target binary.

This approach won't work even now.  We have draft implementations of
the hook for spapr, but those are absolutely wrong for mac99 or the
many other ppc machine classes.

> I've been trying to not clutter MachineClass with hooks
> that not must have right now but I don't have a strong opinion
> on this so if MachineClass method is preferred way,
> I can rewrite it this patch to use it on respin.
> 
>
Igor Mammedov March 22, 2016, 9:19 a.m. UTC | #4 
On Tue, 22 Mar 2016 10:39:28 +1100
David Gibson <dgibson@redhat.com> wrote:

> On Mon, 21 Mar 2016 11:53:23 +0100
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Fri, 18 Mar 2016 16:26:28 -0300
> > Eduardo Habkost <ehabkost@redhat.com> wrote:
> >   
> > > On Tue, Mar 15, 2016 at 02:24:07PM +0100, Igor Mammedov wrote:
> > > [...]    
> > > > diff --git a/stubs/qmp_query_hotpluggable_cpus.c b/stubs/qmp_query_hotpluggable_cpus.c
> > > > new file mode 100644
> > > > index 0000000..21a75a3
> > > > --- /dev/null
> > > > +++ b/stubs/qmp_query_hotpluggable_cpus.c
> > > > @@ -0,0 +1,9 @@
> > > > +#include "qemu/osdep.h"
> > > > +#include "qapi/qmp/qerror.h"
> > > > +#include "qmp-commands.h"
> > > > +
> > > > +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> > > > +{
> > > > +    error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> > > > +    return NULL;
> > > > +}      
> > > 
> > > Sorry if this was discussed in previous threads that I haven't
> > > read, but: isn't this supposed to be a MachineClass method?  I
> > > remember David saying once that we have the habit of assuming
> > > that a single QEMU binary can run only one family of machines
> > > that are very similar (like x86), but that's not always true.    
> > Stub approach works for current qemu with one target per binary
> > but it won't for multi-target binary.  
> 
> This approach won't work even now.  We have draft implementations of
> the hook for spapr, but those are absolutely wrong for mac99 or the
> many other ppc machine classes.
Ok, I'll respin it as MachineClass method.

> 
> > I've been trying to not clutter MachineClass with hooks
> > that not must have right now but I don't have a strong opinion
> > on this so if MachineClass method is preferred way,
> > I can rewrite it this patch to use it on respin.
> > 
> >   
> 
>
diff mbox

Patch

diff --git a/qapi-schema.json b/qapi-schema.json
index 362c9d8..105511d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4122,3 +4122,44 @@ 
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# CpuInstanceProperties
+#
+# @node: NUMA node ID the CPU belongs to, optional
+# @socket: #optional socket number within node/board the CPU belongs to
+# @core: #optional core number within socket the CPU belongs to
+# @thread: #optional thread number within core the CPU belongs to
+#
+# Since: 2.7
+{ 'struct': 'CpuInstanceProperties',
+  'data': { '*node': 'int',
+            '*socket': 'int',
+            '*core': 'int',
+            '*thread': 'int'
+  }
+}
+
+##
+# @HotpluggableCPU
+#
+# @type: CPU object type for usage with device_add command
+# @props: list of properties to be used for hotplugging CPU
+# @vcpus-count: number of logical VCPU threads @HotpluggableCPU provides
+# @qom-path: #optional link to existing CPU object if CPU is present or
+#            omitted if CPU is not present.
+#
+# Since: 2.7
+{ 'struct': 'HotpluggableCPU',
+  'data': { 'type': 'str',
+            'vcpus-count': 'int',
+            'props': 'CpuInstanceProperties',
+            '*qom-path': 'str'
+          }
+}
+
+##
+# @query-hotpluggable-cpus
+#
+# Since: 2.7
+{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index b629673..41a00b3 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4853,3 +4853,46 @@  Example:
                  {"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
                   "pop-vlan": 1, "id": 251658240}
    ]}
+
+EQMP
+
+    {
+        .name       = "query-hotpluggable-cpus",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_query_hotpluggable_cpus,
+    },
+
+SQMP
+Show  existing/possible CPUs
+-------------------------------
+
+Arguments: None.
+
+Example for x86 target started with -smp 2,sockets=2,cores=1,threads=3,maxcpus=6:
+
+-> { "execute": "query-hotpluggable-cpus" }
+<- {"return": [
+     { "props": { "core": 0, "socket": 1, "thread": 2},
+       "type": "qemu64-x86_64-cpu", "vcpus-count": 1 },
+     { "props": { "core": 0, "socket": 1, "thread": 1},
+       "type": "qemu64-x86_64-cpu", "vcpus-count": 1 },
+     { "props": { "core": 0, "socket": 1, "thread": 0},
+       "type": "qemu64-x86_64-cpu", "vcpus-count": 1 },
+     { "props": { "core": 0, "socket": 0, "thread": 2},
+       "type": "qemu64-x86_64-cpu", "vcpus-count": 1 },
+     { "props": { "core": 0, "socket": 0, "thread": 1},
+       "type": "qemu64-x86_64-cpu", "vcpus-count": 1,
+       "qom-path": "/machine/unattached/device[3]"},
+     { "props": { "core": 0, "socket": 0, "thread": 0},
+       "type": "qemu64-x86_64-cpu", "vcpus-count": 1,
+       "qom-path": "/machine/unattached/device[0]"}
+   ]}'
+
+Example for SPAPR target started with -smp 2,cores=2,maxcpus=4:
+
+-> { "execute": "query-hotpluggable-cpus" }
+<- {"return": [
+     { "props": { "core": 1 }, "type": "spapr-cpu-core", "vcpus-count": 1 },
+     { "props": { "core": 0 }, "type": "spapr-cpu-core", "vcpus-count": 1,
+       "qom-path": "/machine/unattached/device[0]"}
+   ]}'
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index e922de9..0011173 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -39,3 +39,4 @@  stub-obj-y += qmp_pc_dimm_device_list.o
 stub-obj-y += target-monitor-defs.o
 stub-obj-y += target-get-monitor-def.o
 stub-obj-y += vhost.o
+stub-obj-y += qmp_query_hotpluggable_cpus.o
diff --git a/stubs/qmp_query_hotpluggable_cpus.c b/stubs/qmp_query_hotpluggable_cpus.c
new file mode 100644
index 0000000..21a75a3
--- /dev/null
+++ b/stubs/qmp_query_hotpluggable_cpus.c
@@ -0,0 +1,9 @@ 
+#include "qemu/osdep.h"
+#include "qapi/qmp/qerror.h"
+#include "qmp-commands.h"
+
+HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
+{
+    error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
+    return NULL;
+}