diff mbox

[RFC,v3,2/2] spapr: implement query-hotpluggable-cpus QMP command

Message ID 1458048248-4605-3-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 returns a list of present/possible to hotplug CPU
objects with a list of properties to use with
device_add.

in spapr case returned list would looks like:
-> { "execute": "query-hotpluggable-cpus" }
<- {"return": [
     { "props": { "core": 1 }, "type": "spapr-cpu-core",
       "vcpus-count": 2 },
     { "props": { "core": 0 }, "type": "spapr-cpu-core",
       "vcpus-count": 2,
       "qom-path": "/machine/unattached/device[0]"}
   ]}'

TODO:
  add 'node' property for core <-> numa node mapping

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
it's only compile tested
---
 hw/ppc/spapr.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Bharata B Rao March 16, 2016, 5:19 a.m. UTC | #1
On Tue, Mar 15, 2016 at 02:24:08PM +0100, Igor Mammedov wrote:
> it returns a list of present/possible to hotplug CPU
> objects with a list of properties to use with
> device_add.
> 
> in spapr case returned list would looks like:
> -> { "execute": "query-hotpluggable-cpus" }
> <- {"return": [
>      { "props": { "core": 1 }, "type": "spapr-cpu-core",
>        "vcpus-count": 2 },
>      { "props": { "core": 0 }, "type": "spapr-cpu-core",
>        "vcpus-count": 2,
>        "qom-path": "/machine/unattached/device[0]"}
>    ]}'
> 
> TODO:
>   add 'node' property for core <-> numa node mapping
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> it's only compile tested
> ---
>  hw/ppc/spapr.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b1e9ba2..e1ce983 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -65,6 +65,7 @@
>  #include "hw/compat.h"
>  #include "qemu-common.h"
>  #include "hw/ppc/spapr_cpu_core.h"
> +#include "qmp-commands.h"
> 
>  #include <libfdt.h>
> 
> @@ -2399,6 +2400,37 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
>      return cpu_index / smp_threads / smp_cores;
>  }
> 
> +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> +{
> +    int i;
> +    HotpluggableCPUList *head = NULL;
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +    int spapr_max_cores = max_cpus / smp_threads;
> +
> +    for (i = 0; i < spapr_max_cores; i++) {
> +        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> +        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> +        CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
> +
> +        cpu_item->type = g_strdup(TYPE_SPAPR_CPU_CORE);
> +        cpu_item->vcpus_count = smp_threads;

Shouldn't this be fetched from "threads" property of the core device
instead of directly using smp_threads ? But again, what that would mean
for not-yet-plugged in cores and how to get that for them is a question.

Regards,
Bharata.
David Gibson March 16, 2016, 5:41 a.m. UTC | #2
On Wed, 16 Mar 2016 10:49:41 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> On Tue, Mar 15, 2016 at 02:24:08PM +0100, Igor Mammedov wrote:
> > it returns a list of present/possible to hotplug CPU
> > objects with a list of properties to use with
> > device_add.
> > 
> > in spapr case returned list would looks like:  
> > -> { "execute": "query-hotpluggable-cpus" }  
> > <- {"return": [
> >      { "props": { "core": 1 }, "type": "spapr-cpu-core",
> >        "vcpus-count": 2 },
> >      { "props": { "core": 0 }, "type": "spapr-cpu-core",
> >        "vcpus-count": 2,
> >        "qom-path": "/machine/unattached/device[0]"}
> >    ]}'
> > 
> > TODO:
> >   add 'node' property for core <-> numa node mapping
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > it's only compile tested
> > ---
> >  hw/ppc/spapr.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index b1e9ba2..e1ce983 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -65,6 +65,7 @@
> >  #include "hw/compat.h"
> >  #include "qemu-common.h"
> >  #include "hw/ppc/spapr_cpu_core.h"
> > +#include "qmp-commands.h"
> > 
> >  #include <libfdt.h>
> > 
> > @@ -2399,6 +2400,37 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
> >      return cpu_index / smp_threads / smp_cores;
> >  }
> > 
> > +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> > +{
> > +    int i;
> > +    HotpluggableCPUList *head = NULL;
> > +    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> > +    int spapr_max_cores = max_cpus / smp_threads;
> > +
> > +    for (i = 0; i < spapr_max_cores; i++) {
> > +        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> > +        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> > +        CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
> > +
> > +        cpu_item->type = g_strdup(TYPE_SPAPR_CPU_CORE);
> > +        cpu_item->vcpus_count = smp_threads;  
> 
> Shouldn't this be fetched from "threads" property of the core device
> instead of directly using smp_threads ? But again, what that would mean
> for not-yet-plugged in cores and how to get that for them is a question.

Yeah, I think Igor's patch is correct here.  The information flow goes
the other direction: the machine type code advertises smp_threads here,
which management then passes back to the device_add spapr-core in the
threads property.
Igor Mammedov March 16, 2016, 3:55 p.m. UTC | #3
On Wed, 16 Mar 2016 10:49:41 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> On Tue, Mar 15, 2016 at 02:24:08PM +0100, Igor Mammedov wrote:
> > it returns a list of present/possible to hotplug CPU
> > objects with a list of properties to use with
> > device_add.
> > 
> > in spapr case returned list would looks like:  
> > -> { "execute": "query-hotpluggable-cpus" }  
> > <- {"return": [
> >      { "props": { "core": 1 }, "type": "spapr-cpu-core",
> >        "vcpus-count": 2 },
> >      { "props": { "core": 0 }, "type": "spapr-cpu-core",
> >        "vcpus-count": 2,
> >        "qom-path": "/machine/unattached/device[0]"}
> >    ]}'
> > 
> > TODO:
> >   add 'node' property for core <-> numa node mapping
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > it's only compile tested
> > ---
> >  hw/ppc/spapr.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index b1e9ba2..e1ce983 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -65,6 +65,7 @@
> >  #include "hw/compat.h"
> >  #include "qemu-common.h"
> >  #include "hw/ppc/spapr_cpu_core.h"
> > +#include "qmp-commands.h"
> > 
> >  #include <libfdt.h>
> > 
> > @@ -2399,6 +2400,37 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
> >      return cpu_index / smp_threads / smp_cores;
> >  }
> > 
> > +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> > +{
> > +    int i;
> > +    HotpluggableCPUList *head = NULL;
> > +    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> > +    int spapr_max_cores = max_cpus / smp_threads;
> > +
> > +    for (i = 0; i < spapr_max_cores; i++) {
> > +        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
> > +        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
> > +        CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
> > +
> > +        cpu_item->type = g_strdup(TYPE_SPAPR_CPU_CORE);
> > +        cpu_item->vcpus_count = smp_threads;  
> 
> Shouldn't this be fetched from "threads" property of the core device
> instead of directly using smp_threads ? But again, what that would mean
> for not-yet-plugged in cores and how to get that for them is a question.
my understanding is that libvirt wants to know that info for unplugged
CPUs as well and machine model + core(cpu_model) combo would fix it
at machine startup time for current use case.

> 
> Regards,
> Bharata.
>
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b1e9ba2..e1ce983 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -65,6 +65,7 @@ 
 #include "hw/compat.h"
 #include "qemu-common.h"
 #include "hw/ppc/spapr_cpu_core.h"
+#include "qmp-commands.h"
 
 #include <libfdt.h>
 
@@ -2399,6 +2400,37 @@  static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
     return cpu_index / smp_threads / smp_cores;
 }
 
+HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
+{
+    int i;
+    HotpluggableCPUList *head = NULL;
+    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+    int spapr_max_cores = max_cpus / smp_threads;
+
+    for (i = 0; i < spapr_max_cores; i++) {
+        HotpluggableCPUList *list_item = g_new0(typeof(*list_item), 1);
+        HotpluggableCPU *cpu_item = g_new0(typeof(*cpu_item), 1);
+        CpuInstanceProperties *cpu_props = g_new0(typeof(*cpu_props), 1);
+
+        cpu_item->type = g_strdup(TYPE_SPAPR_CPU_CORE);
+        cpu_item->vcpus_count = smp_threads;
+        cpu_props->has_core = true;
+        cpu_props->core = i;
+        /* TODO: add 'has_node/node' here to describe
+           to which node core belongs */
+
+        cpu_item->props = cpu_props;
+        if (spapr->cores[i]) {
+            cpu_item->has_qom_path = true;
+            cpu_item->qom_path = object_get_canonical_path(spapr->cores[i]);
+        }
+        list_item->value = cpu_item;
+        list_item->next = head;
+        head = list_item;
+    }
+    return head;
+}
+
 static void spapr_machine_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);