All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-2.7 v6 0/2] ispapr: QMP: add query-hotpluggable-cpus
@ 2016-04-08 11:29 Igor Mammedov
  2016-04-08 11:29 ` [Qemu-devel] [PATCH for-2.7 v6 1/2] " Igor Mammedov
  2016-04-08 11:29 ` [Qemu-devel] [PATCH for-2.7 v6 2/2] spapr: implement query-hotpluggable-cpus callback Igor Mammedov
  0 siblings, 2 replies; 12+ messages in thread
From: Igor Mammedov @ 2016-04-08 11:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: mjrosato, thuth, pkrempa, ehabkost, aik, armbru, agraf,
	borntraeger, qemu-ppc, bharata, pbonzini, dgibson, mdroth,
	afaerber

Changes since v4:
 - fix style issues in qapi-schema and qmp-commands,
   Eric Blake <eblake@redhat.com>
 - rebase on top current master (fix query-gic-capabilities conflict)
 - fix s390 build failure:
    undefined reference to `qmp_query_hotpluggable_cpus'
Changes since v3:
 - replace qmp_query_hotpluggable_cpus with MachineClass->query_hotpluggable_cpus
   callback. (Eduardo Habkost <ehabkost@redhat.com>)
 - fix cover letter to explain why new command is needed.
   (Markus Armbruster <armbru@redhat.com>)
Changes since v2:
 - rebase on top of hte lates spapr cpu hotpug series
 - 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
 - drop pre_plug spapr impl out of series as not related to QMP command
 - drop generic pre hotplug callback as not related to QMP command

Changes since RFC:
 - drop arch_id
 - move CPU properties into separate structure
 - target implements its own qmp callback version
 - rebased on top of [RFC PATCH v1 00/10] Core based CPU hotplug for PowerPC sPAPR
                      https://www.mail-archive.com/qemu-devel@nongnu.org/msg357567.html
    - convert slot name to core id hack
    - drop links
    - add generic pre hotplug callback
    - implement query-hotpluggable-cpus

Series adds query-hotpluggable-cpus QMP command to allow mgmt
query board specific configuration of possible CPU objects
with their properties, which is target specific and also
depends on CLI options (like -smp/-cpu/-numa).
Returned information includes QOM type of CPU objects and
a set of properties that are necessary for hotplugging them.
Information will be used with device_add/-device when
hotplugging a CPU and migrating QEMU instance with hotplugged CPUs.

PS:
The first patch (QMP API) in this series could go in first
allowing individual targets to post their hotplug
implementation independently on top of it.

PS2:
Summary on QMP vs QOM interface discussion:

a QMP command is
 1. a single / atomic command 
 2. well documented
 3. fixed at compile time/staic so it's easy for mgmt to discover it.

while a considered alternative QOM interface via qom-get(path) is
 1. not atomic, i.e. requires a lot of qom-get requests over the wire
    to traverse QOM tree and fetch information
 2. not documented
 3. dynamically generated and would require more complicated coding
    even to create a simplistic interface similar to proposed QMP command
    (for example: possible_cpu QOM objects with a related properties)



Igor Mammedov (2):
  QMP: add query-hotpluggable-cpus
  spapr: implement query-hotpluggable-cpus callback

 hw/ppc/spapr.c      | 33 +++++++++++++++++++++++++++++++++
 include/hw/boards.h |  5 +++++
 monitor.c           | 13 +++++++++++++
 qapi-schema.json    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx     | 41 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 138 insertions(+)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH for-2.7 v6 1/2] QMP: add query-hotpluggable-cpus
  2016-04-08 11:29 [Qemu-devel] [PATCH for-2.7 v6 0/2] ispapr: QMP: add query-hotpluggable-cpus Igor Mammedov
@ 2016-04-08 11:29 ` Igor Mammedov
  2016-04-11  4:20   ` David Gibson
  2016-04-11 16:14   ` Michael Roth
  2016-04-08 11:29 ` [Qemu-devel] [PATCH for-2.7 v6 2/2] spapr: implement query-hotpluggable-cpus callback Igor Mammedov
  1 sibling, 2 replies; 12+ messages in thread
From: Igor Mammedov @ 2016-04-08 11:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: mjrosato, thuth, pkrempa, ehabkost, aik, armbru, agraf,
	borntraeger, qemu-ppc, bharata, pbonzini, dgibson, mdroth,
	afaerber

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 and set
 MachineClass.query_hotpluggable_cpus
callback, 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>
---
v6:
 - fix style issues in qapi-schema and qmp-commands,
   Eric Blake <eblake@redhat.com>
 - rebase on top current master (query-gic-capabilities conflict)
v5:
 - fix s390 build failure:
    undefined reference to `qmp_query_hotpluggable_cpus'
v4:
 - add MachineClass method to get CPU object list
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

query_fixup
---
 include/hw/boards.h |  5 +++++
 monitor.c           | 13 +++++++++++++
 qapi-schema.json    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 qmp-commands.hx     | 41 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 105 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index aad5f2a..c122a70 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -81,6 +81,10 @@ typedef struct {
  *    Returns an array of @CPUArchId architecture-dependent CPU IDs
  *    which includes CPU IDs for present and possible to hotplug CPUs.
  *    Caller is responsible for freeing returned list.
+ * @query_hotpluggable_cpus:
+ *    Returns a @HotpluggableCPUList, which describes CPUs objects which
+ *    could be added with -device/device_add.
+ *    Caller is responsible for freeing returned list.
  */
 struct MachineClass {
     /*< private >*/
@@ -123,6 +127,7 @@ struct MachineClass {
                                            DeviceState *dev);
     unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
     CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
+    HotpluggableCPUList *(*query_hotpluggable_cpus)(MachineState *machine);
 };
 
 /**
diff --git a/monitor.c b/monitor.c
index d1c1930..b469225 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4267,3 +4267,16 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
     return NULL;
 }
 #endif
+
+HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+
+    if (!mc->query_hotpluggable_cpus) {
+        error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
+        return NULL;
+    }
+
+    return mc->query_hotpluggable_cpus(ms);
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 54634c4..4d1d71d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4178,3 +4178,49 @@
 # Since: 2.6
 ##
 { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
+
+##
+# CpuInstanceProperties
+#
+# @node: #optional NUMA node ID the CPU belongs to
+# @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
+#
+# Returns: a list of HotpluggableCPU objects.
+#
+# Since: 2.7
+##
+{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index de896a5..96f4454 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4880,3 +4880,44 @@ Example:
                 { "version": 3, "emulated": false, "kernel": true } ] }
 
 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]"}
+   ]}'
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH for-2.7 v6 2/2] spapr: implement query-hotpluggable-cpus callback
  2016-04-08 11:29 [Qemu-devel] [PATCH for-2.7 v6 0/2] ispapr: QMP: add query-hotpluggable-cpus Igor Mammedov
  2016-04-08 11:29 ` [Qemu-devel] [PATCH for-2.7 v6 1/2] " Igor Mammedov
@ 2016-04-08 11:29 ` Igor Mammedov
  2016-04-11  4:54   ` David Gibson
  1 sibling, 1 reply; 12+ messages in thread
From: Igor Mammedov @ 2016-04-08 11:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: mjrosato, thuth, pkrempa, ehabkost, aik, armbru, agraf,
	borntraeger, qemu-ppc, bharata, pbonzini, dgibson, mdroth,
	afaerber

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
v2:
 - s/qmp_query_hotpluggable_cpus/MachineClass->query_hotpluggable_cpus/ callback
---
 hw/ppc/spapr.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 760a42f..c38995e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -66,6 +66,7 @@
 #include "hw/compat.h"
 #include "qemu/cutils.h"
 #include "hw/ppc/spapr_cpu_core.h"
+#include "qmp-commands.h"
 
 #include <libfdt.h>
 
@@ -2382,6 +2383,37 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
     return cpu_index / smp_threads / smp_cores;
 }
 
+static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
+{
+    int i;
+    HotpluggableCPUList *head = NULL;
+    sPAPRMachineState *spapr = SPAPR_MACHINE(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);
@@ -2412,6 +2444,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
     hc->plug = spapr_machine_device_plug;
     hc->unplug = spapr_machine_device_unplug;
     mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
+    mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
 
     smc->dr_lmb_enabled = true;
     smc->dr_cpu_enabled = true;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v6 1/2] QMP: add query-hotpluggable-cpus
  2016-04-08 11:29 ` [Qemu-devel] [PATCH for-2.7 v6 1/2] " Igor Mammedov
@ 2016-04-11  4:20   ` David Gibson
  2016-04-11  9:35     ` Igor Mammedov
  2016-04-11 16:14   ` Michael Roth
  1 sibling, 1 reply; 12+ messages in thread
From: David Gibson @ 2016-04-11  4:20 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: qemu-devel, mjrosato, thuth, pkrempa, ehabkost, aik, armbru,
	agraf, borntraeger, qemu-ppc, bharata, pbonzini, mdroth,
	afaerber

[-- Attachment #1: Type: text/plain, Size: 7631 bytes --]

On Fri,  8 Apr 2016 13:29:55 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> 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 and set
>  MachineClass.query_hotpluggable_cpus
> callback, 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>
> ---
> v6:
>  - fix style issues in qapi-schema and qmp-commands,
>    Eric Blake <eblake@redhat.com>
>  - rebase on top current master (query-gic-capabilities conflict)
> v5:
>  - fix s390 build failure:
>     undefined reference to `qmp_query_hotpluggable_cpus'
> v4:
>  - add MachineClass method to get CPU object list
> 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
> 
> query_fixup
> ---
>  include/hw/boards.h |  5 +++++
>  monitor.c           | 13 +++++++++++++
>  qapi-schema.json    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx     | 41 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 105 insertions(+)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index aad5f2a..c122a70 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -81,6 +81,10 @@ typedef struct {
>   *    Returns an array of @CPUArchId architecture-dependent CPU IDs
>   *    which includes CPU IDs for present and possible to hotplug CPUs.
>   *    Caller is responsible for freeing returned list.
> + * @query_hotpluggable_cpus:
> + *    Returns a @HotpluggableCPUList, which describes CPUs objects which
> + *    could be added with -device/device_add.
> + *    Caller is responsible for freeing returned list.
>   */
>  struct MachineClass {
>      /*< private >*/
> @@ -123,6 +127,7 @@ struct MachineClass {
>                                             DeviceState *dev);
>      unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
>      CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> +    HotpluggableCPUList *(*query_hotpluggable_cpus)(MachineState *machine);
>  };
>  
>  /**
> diff --git a/monitor.c b/monitor.c
> index d1c1930..b469225 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4267,3 +4267,16 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
>      return NULL;
>  }
>  #endif
> +
> +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
> +    if (!mc->query_hotpluggable_cpus) {
> +        error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> +        return NULL;
> +    }
> +
> +    return mc->query_hotpluggable_cpus(ms);
> +}
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 54634c4..4d1d71d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4178,3 +4178,49 @@
>  # Since: 2.6
>  ##
>  { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
> +
> +##
> +# CpuInstanceProperties
> +#
> +# @node: #optional NUMA node ID the CPU belongs to
> +# @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'
> +  }
> +}

Is there somewhere we should document the fact that these particular
properties are common ones, but there could be more.  The point is that
management should not assume these are the only fields here, but should
be prepared to accept anything, and echo it back to the device_add.


> +
> +##
> +# @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
> +#
> +# Returns: a list of HotpluggableCPU objects.
> +#
> +# Since: 2.7
> +##
> +{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index de896a5..96f4454 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4880,3 +4880,44 @@ Example:
>                  { "version": 3, "emulated": false, "kernel": true } ] }
>  
>  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:

Might be best to say "pseries" here, since that's the exposed machine
type name, even though we use sPAPR a lot internally.

> +
> +-> { "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]"}
> +   ]}'
> -- 
> 1.8.3.1
> 


-- 
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v6 2/2] spapr: implement query-hotpluggable-cpus callback
  2016-04-08 11:29 ` [Qemu-devel] [PATCH for-2.7 v6 2/2] spapr: implement query-hotpluggable-cpus callback Igor Mammedov
@ 2016-04-11  4:54   ` David Gibson
  2016-04-11  9:23     ` Igor Mammedov
  0 siblings, 1 reply; 12+ messages in thread
From: David Gibson @ 2016-04-11  4:54 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: qemu-devel, mjrosato, thuth, pkrempa, ehabkost, aik, armbru,
	agraf, borntraeger, qemu-ppc, bharata, pbonzini, mdroth,
	afaerber

[-- Attachment #1: Type: text/plain, Size: 3412 bytes --]

On Fri,  8 Apr 2016 13:29:56 +0200
Igor Mammedov <imammedo@redhat.com> 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
> v2:
>  - s/qmp_query_hotpluggable_cpus/MachineClass->query_hotpluggable_cpus/ callback
> ---
>  hw/ppc/spapr.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 760a42f..c38995e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -66,6 +66,7 @@
>  #include "hw/compat.h"
>  #include "qemu/cutils.h"
>  #include "hw/ppc/spapr_cpu_core.h"
> +#include "qmp-commands.h"
>  
>  #include <libfdt.h>
>  
> @@ -2382,6 +2383,37 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
>      return cpu_index / smp_threads / smp_cores;
>  }
>  
> +static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> +{
> +    int i;
> +    HotpluggableCPUList *head = NULL;
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(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);

So, Bharata's latest core stuff is (as suggested by you and others)
moving to using different derived types, rather than a cpu_model
property, so this will need to change a bit.

> +        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);
> @@ -2412,6 +2444,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      hc->plug = spapr_machine_device_plug;
>      hc->unplug = spapr_machine_device_unplug;
>      mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
> +    mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
>  
>      smc->dr_lmb_enabled = true;
>      smc->dr_cpu_enabled = true;
> -- 
> 1.8.3.1
> 


-- 
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v6 2/2] spapr: implement query-hotpluggable-cpus callback
  2016-04-11  4:54   ` David Gibson
@ 2016-04-11  9:23     ` Igor Mammedov
  2016-04-11  9:38       ` Bharata B Rao
  0 siblings, 1 reply; 12+ messages in thread
From: Igor Mammedov @ 2016-04-11  9:23 UTC (permalink / raw)
  To: David Gibson, bharata
  Cc: qemu-devel, mjrosato, thuth, pkrempa, ehabkost, aik, armbru,
	agraf, borntraeger, qemu-ppc, pbonzini, mdroth, afaerber

On Mon, 11 Apr 2016 14:54:24 +1000
David Gibson <dgibson@redhat.com> wrote:

> On Fri,  8 Apr 2016 13:29:56 +0200
> Igor Mammedov <imammedo@redhat.com> 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
> > v2:
> >  - s/qmp_query_hotpluggable_cpus/MachineClass->query_hotpluggable_cpus/ callback
> > ---
> >  hw/ppc/spapr.c | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 760a42f..c38995e 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -66,6 +66,7 @@
> >  #include "hw/compat.h"
> >  #include "qemu/cutils.h"
> >  #include "hw/ppc/spapr_cpu_core.h"
> > +#include "qmp-commands.h"
> >  
> >  #include <libfdt.h>
> >  
> > @@ -2382,6 +2383,37 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
> >      return cpu_index / smp_threads / smp_cores;
> >  }
> >  
> > +static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> > +{
> > +    int i;
> > +    HotpluggableCPUList *head = NULL;
> > +    sPAPRMachineState *spapr = SPAPR_MACHINE(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);  
> 
> So, Bharata's latest core stuff is (as suggested by you and others)
> moving to using different derived types, rather than a cpu_model
> property, so this will need to change a bit.
Yep, I guess so.
It would be even better if Bharata took merged this patches
with his series and took care to make necessary changes to
this call back.

> 
> > +        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);
> > @@ -2412,6 +2444,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
> >      hc->plug = spapr_machine_device_plug;
> >      hc->unplug = spapr_machine_device_unplug;
> >      mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
> > +    mc->query_hotpluggable_cpus = spapr_query_hotpluggable_cpus;
> >  
> >      smc->dr_lmb_enabled = true;
> >      smc->dr_cpu_enabled = true;
> > -- 
> > 1.8.3.1
> >   
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v6 1/2] QMP: add query-hotpluggable-cpus
  2016-04-11  4:20   ` David Gibson
@ 2016-04-11  9:35     ` Igor Mammedov
  2016-04-12  1:40       ` David Gibson
  0 siblings, 1 reply; 12+ messages in thread
From: Igor Mammedov @ 2016-04-11  9:35 UTC (permalink / raw)
  To: David Gibson
  Cc: qemu-devel, mjrosato, thuth, pkrempa, ehabkost, aik, armbru,
	agraf, borntraeger, qemu-ppc, bharata, pbonzini, mdroth,
	afaerber

On Mon, 11 Apr 2016 14:20:27 +1000
David Gibson <dgibson@redhat.com> wrote:

> On Fri,  8 Apr 2016 13:29:55 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > 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 and set
> >  MachineClass.query_hotpluggable_cpus
> > callback, 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>
> > ---
> > v6:
> >  - fix style issues in qapi-schema and qmp-commands,
> >    Eric Blake <eblake@redhat.com>
> >  - rebase on top current master (query-gic-capabilities conflict)
> > v5:
> >  - fix s390 build failure:
> >     undefined reference to `qmp_query_hotpluggable_cpus'
> > v4:
> >  - add MachineClass method to get CPU object list
> > 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
> > 
> > query_fixup
> > ---
> >  include/hw/boards.h |  5 +++++
> >  monitor.c           | 13 +++++++++++++
> >  qapi-schema.json    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> >  qmp-commands.hx     | 41 +++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 105 insertions(+)
> > 
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index aad5f2a..c122a70 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -81,6 +81,10 @@ typedef struct {
> >   *    Returns an array of @CPUArchId architecture-dependent CPU IDs
> >   *    which includes CPU IDs for present and possible to hotplug CPUs.
> >   *    Caller is responsible for freeing returned list.
> > + * @query_hotpluggable_cpus:
> > + *    Returns a @HotpluggableCPUList, which describes CPUs objects which
> > + *    could be added with -device/device_add.
> > + *    Caller is responsible for freeing returned list.
> >   */
> >  struct MachineClass {
> >      /*< private >*/
> > @@ -123,6 +127,7 @@ struct MachineClass {
> >                                             DeviceState *dev);
> >      unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
> >      CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> > +    HotpluggableCPUList *(*query_hotpluggable_cpus)(MachineState *machine);
> >  };
> >  
> >  /**
> > diff --git a/monitor.c b/monitor.c
> > index d1c1930..b469225 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -4267,3 +4267,16 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> >      return NULL;
> >  }
> >  #endif
> > +
> > +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> > +{
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> > +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> > +
> > +    if (!mc->query_hotpluggable_cpus) {
> > +        error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> > +        return NULL;
> > +    }
> > +
> > +    return mc->query_hotpluggable_cpus(ms);
> > +}
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 54634c4..4d1d71d 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4178,3 +4178,49 @@
> >  # Since: 2.6
> >  ##
> >  { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
> > +
> > +##
> > +# CpuInstanceProperties
> > +#
> > +# @node: #optional NUMA node ID the CPU belongs to
> > +# @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'
> > +  }
> > +}  
> 
> Is there somewhere we should document the fact that these particular
> properties are common ones, but there could be more.  The point is that
> management should not assume these are the only fields here, but should
> be prepared to accept anything, and echo it back to the device_add.

Something along these lines?

+##
+# CpuInstanceProperties
+#
+# List of properties to be used for hotplugging a CPU instance,
+# it should be passed by management with device_add command when
+# a CPU is being hotplugged.
+#
+# Note: currently there are 4 properties that could be present
+# but management should be prepared to pass through other
+# properties with device_add command to allow for future
+# interface extension.
 
> > +
> > +##
> > +# @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
> > +#
> > +# Returns: a list of HotpluggableCPU objects.
> > +#
> > +# Since: 2.7
> > +##
> > +{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index de896a5..96f4454 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -4880,3 +4880,44 @@ Example:
> >                  { "version": 3, "emulated": false, "kernel": true } ] }
> >  
> >  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:  
> 
> Might be best to say "pseries" here, since that's the exposed machine
> type name, even though we use sPAPR a lot internally.
Di you mean something like this?

+Example for 'pseries' 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]"}
> > +   ]}'
> > -- 
> > 1.8.3.1
> >   
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v6 2/2] spapr: implement query-hotpluggable-cpus callback
  2016-04-11  9:23     ` Igor Mammedov
@ 2016-04-11  9:38       ` Bharata B Rao
  2016-04-11  9:47         ` Igor Mammedov
  0 siblings, 1 reply; 12+ messages in thread
From: Bharata B Rao @ 2016-04-11  9:38 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: David Gibson, qemu-devel, mjrosato, thuth, pkrempa, ehabkost,
	aik, armbru, agraf, borntraeger, qemu-ppc, pbonzini, mdroth,
	afaerber

On Mon, Apr 11, 2016 at 11:23:57AM +0200, Igor Mammedov wrote:
> On Mon, 11 Apr 2016 14:54:24 +1000
> David Gibson <dgibson@redhat.com> wrote:
> 
> > On Fri,  8 Apr 2016 13:29:56 +0200
> > Igor Mammedov <imammedo@redhat.com> 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
> > > v2:
> > >  - s/qmp_query_hotpluggable_cpus/MachineClass->query_hotpluggable_cpus/ callback
> > > ---
> > >  hw/ppc/spapr.c | 33 +++++++++++++++++++++++++++++++++
> > >  1 file changed, 33 insertions(+)
> > > 
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 760a42f..c38995e 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -66,6 +66,7 @@
> > >  #include "hw/compat.h"
> > >  #include "qemu/cutils.h"
> > >  #include "hw/ppc/spapr_cpu_core.h"
> > > +#include "qmp-commands.h"
> > >  
> > >  #include <libfdt.h>
> > >  
> > > @@ -2382,6 +2383,37 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
> > >      return cpu_index / smp_threads / smp_cores;
> > >  }
> > >  
> > > +static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> > > +{
> > > +    int i;
> > > +    HotpluggableCPUList *head = NULL;
> > > +    sPAPRMachineState *spapr = SPAPR_MACHINE(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);  
> > 
> > So, Bharata's latest core stuff is (as suggested by you and others)
> > moving to using different derived types, rather than a cpu_model
> > property, so this will need to change a bit.
> Yep, I guess so.
> It would be even better if Bharata took merged this patches
> with his series and took care to make necessary changes to
> this call back.

I will take these patches into my series and make the necessary changes.

Regards,
Bharata.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v6 2/2] spapr: implement query-hotpluggable-cpus callback
  2016-04-11  9:38       ` Bharata B Rao
@ 2016-04-11  9:47         ` Igor Mammedov
  0 siblings, 0 replies; 12+ messages in thread
From: Igor Mammedov @ 2016-04-11  9:47 UTC (permalink / raw)
  To: Bharata B Rao
  Cc: David Gibson, qemu-devel, mjrosato, thuth, pkrempa, ehabkost,
	aik, armbru, agraf, borntraeger, qemu-ppc, pbonzini, mdroth,
	afaerber

On Mon, 11 Apr 2016 15:08:34 +0530
Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:

> On Mon, Apr 11, 2016 at 11:23:57AM +0200, Igor Mammedov wrote:
> > On Mon, 11 Apr 2016 14:54:24 +1000
> > David Gibson <dgibson@redhat.com> wrote:
> >   
> > > On Fri,  8 Apr 2016 13:29:56 +0200
> > > Igor Mammedov <imammedo@redhat.com> 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
> > > > v2:
> > > >  - s/qmp_query_hotpluggable_cpus/MachineClass->query_hotpluggable_cpus/ callback
> > > > ---
> > > >  hw/ppc/spapr.c | 33 +++++++++++++++++++++++++++++++++
> > > >  1 file changed, 33 insertions(+)
> > > > 
> > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > > index 760a42f..c38995e 100644
> > > > --- a/hw/ppc/spapr.c
> > > > +++ b/hw/ppc/spapr.c
> > > > @@ -66,6 +66,7 @@
> > > >  #include "hw/compat.h"
> > > >  #include "qemu/cutils.h"
> > > >  #include "hw/ppc/spapr_cpu_core.h"
> > > > +#include "qmp-commands.h"
> > > >  
> > > >  #include <libfdt.h>
> > > >  
> > > > @@ -2382,6 +2383,37 @@ static unsigned spapr_cpu_index_to_socket_id(unsigned cpu_index)
> > > >      return cpu_index / smp_threads / smp_cores;
> > > >  }
> > > >  
> > > > +static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)
> > > > +{
> > > > +    int i;
> > > > +    HotpluggableCPUList *head = NULL;
> > > > +    sPAPRMachineState *spapr = SPAPR_MACHINE(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);    
> > > 
> > > So, Bharata's latest core stuff is (as suggested by you and others)
> > > moving to using different derived types, rather than a cpu_model
> > > property, so this will need to change a bit.  
> > Yep, I guess so.
> > It would be even better if Bharata took merged this patches
> > with his series and took care to make necessary changes to
> > this call back.  
> 
> I will take these patches into my series and make the necessary changes.
Thanks!

> 
> Regards,
> Bharata.
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v6 1/2] QMP: add query-hotpluggable-cpus
  2016-04-08 11:29 ` [Qemu-devel] [PATCH for-2.7 v6 1/2] " Igor Mammedov
  2016-04-11  4:20   ` David Gibson
@ 2016-04-11 16:14   ` Michael Roth
  2016-04-12 12:31     ` Igor Mammedov
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Roth @ 2016-04-11 16:14 UTC (permalink / raw)
  To: Igor Mammedov, qemu-devel
  Cc: mjrosato, thuth, pkrempa, ehabkost, aik, armbru, agraf,
	borntraeger, qemu-ppc, bharata, pbonzini, dgibson, afaerber

Quoting Igor Mammedov (2016-04-08 06:29:55)
> 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 and set
>  MachineClass.query_hotpluggable_cpus
> callback, 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>
> ---
> v6:
>  - fix style issues in qapi-schema and qmp-commands,
>    Eric Blake <eblake@redhat.com>
>  - rebase on top current master (query-gic-capabilities conflict)
> v5:
>  - fix s390 build failure:
>     undefined reference to `qmp_query_hotpluggable_cpus'
> v4:
>  - add MachineClass method to get CPU object list
> 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
> 
> query_fixup
> ---
>  include/hw/boards.h |  5 +++++
>  monitor.c           | 13 +++++++++++++
>  qapi-schema.json    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  qmp-commands.hx     | 41 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 105 insertions(+)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index aad5f2a..c122a70 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -81,6 +81,10 @@ typedef struct {
>   *    Returns an array of @CPUArchId architecture-dependent CPU IDs
>   *    which includes CPU IDs for present and possible to hotplug CPUs.
>   *    Caller is responsible for freeing returned list.
> + * @query_hotpluggable_cpus:
> + *    Returns a @HotpluggableCPUList, which describes CPUs objects which
> + *    could be added with -device/device_add.
> + *    Caller is responsible for freeing returned list.
>   */
>  struct MachineClass {
>      /*< private >*/
> @@ -123,6 +127,7 @@ struct MachineClass {
>                                             DeviceState *dev);
>      unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
>      CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> +    HotpluggableCPUList *(*query_hotpluggable_cpus)(MachineState *machine);
>  };
> 
>  /**
> diff --git a/monitor.c b/monitor.c
> index d1c1930..b469225 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4267,3 +4267,16 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
>      return NULL;
>  }
>  #endif
> +
> +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
> +    if (!mc->query_hotpluggable_cpus) {
> +        error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> +        return NULL;
> +    }
> +
> +    return mc->query_hotpluggable_cpus(ms);
> +}
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 54634c4..4d1d71d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4178,3 +4178,49 @@
>  # Since: 2.6
>  ##
>  { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
> +
> +##
> +# CpuInstanceProperties
> +#
> +# @node: #optional NUMA node ID the CPU belongs to
> +# @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
> +#
> +# Returns: a list of HotpluggableCPU objects.
> +#
> +# Since: 2.7
> +##
> +{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index de896a5..96f4454 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -4880,3 +4880,44 @@ Example:
>                  { "version": 3, "emulated": false, "kernel": true } ] }
> 
>  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]"}
> +   ]}'

These examples kind of imply the granularity of "props" field has some
relation to the granularity afforded by -smp. But IIUC what really
matters is the granularity of the object type specified by "type".

If so, it's also confusing in the sense that the output above likely
would look different if x86 implements device_add functionality in
the form of socket-level objects, which AFAIK is stil the plan
there?

Perhaps we should stick with pseries as the one concrete example,
then for other examples provide 'theoretical' examples that touch
on the granularity of the hotpluggable object and how that factors
into what management would expect in the returned output. Sort of
as a reference for future implementers.

Then we can add a concrete x86 example when that's in.

> +
> +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]"}
> +   ]}'
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v6 1/2] QMP: add query-hotpluggable-cpus
  2016-04-11  9:35     ` Igor Mammedov
@ 2016-04-12  1:40       ` David Gibson
  0 siblings, 0 replies; 12+ messages in thread
From: David Gibson @ 2016-04-12  1:40 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: qemu-devel, mjrosato, thuth, pkrempa, ehabkost, aik, armbru,
	agraf, borntraeger, qemu-ppc, bharata, pbonzini, mdroth,
	afaerber

[-- Attachment #1: Type: text/plain, Size: 9415 bytes --]

On Mon, 11 Apr 2016 11:35:12 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Mon, 11 Apr 2016 14:20:27 +1000
> David Gibson <dgibson@redhat.com> wrote:
> 
> > On Fri,  8 Apr 2016 13:29:55 +0200
> > Igor Mammedov <imammedo@redhat.com> wrote:
> >   
> > > 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 and set
> > >  MachineClass.query_hotpluggable_cpus
> > > callback, 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>
> > > ---
> > > v6:
> > >  - fix style issues in qapi-schema and qmp-commands,
> > >    Eric Blake <eblake@redhat.com>
> > >  - rebase on top current master (query-gic-capabilities conflict)
> > > v5:
> > >  - fix s390 build failure:
> > >     undefined reference to `qmp_query_hotpluggable_cpus'
> > > v4:
> > >  - add MachineClass method to get CPU object list
> > > 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
> > > 
> > > query_fixup
> > > ---
> > >  include/hw/boards.h |  5 +++++
> > >  monitor.c           | 13 +++++++++++++
> > >  qapi-schema.json    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  qmp-commands.hx     | 41 +++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 105 insertions(+)
> > > 
> > > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > > index aad5f2a..c122a70 100644
> > > --- a/include/hw/boards.h
> > > +++ b/include/hw/boards.h
> > > @@ -81,6 +81,10 @@ typedef struct {
> > >   *    Returns an array of @CPUArchId architecture-dependent CPU IDs
> > >   *    which includes CPU IDs for present and possible to hotplug CPUs.
> > >   *    Caller is responsible for freeing returned list.
> > > + * @query_hotpluggable_cpus:
> > > + *    Returns a @HotpluggableCPUList, which describes CPUs objects which
> > > + *    could be added with -device/device_add.
> > > + *    Caller is responsible for freeing returned list.
> > >   */
> > >  struct MachineClass {
> > >      /*< private >*/
> > > @@ -123,6 +127,7 @@ struct MachineClass {
> > >                                             DeviceState *dev);
> > >      unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
> > >      CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> > > +    HotpluggableCPUList *(*query_hotpluggable_cpus)(MachineState *machine);
> > >  };
> > >  
> > >  /**
> > > diff --git a/monitor.c b/monitor.c
> > > index d1c1930..b469225 100644
> > > --- a/monitor.c
> > > +++ b/monitor.c
> > > @@ -4267,3 +4267,16 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> > >      return NULL;
> > >  }
> > >  #endif
> > > +
> > > +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> > > +{
> > > +    MachineState *ms = MACHINE(qdev_get_machine());
> > > +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> > > +
> > > +    if (!mc->query_hotpluggable_cpus) {
> > > +        error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> > > +        return NULL;
> > > +    }
> > > +
> > > +    return mc->query_hotpluggable_cpus(ms);
> > > +}
> > > diff --git a/qapi-schema.json b/qapi-schema.json
> > > index 54634c4..4d1d71d 100644
> > > --- a/qapi-schema.json
> > > +++ b/qapi-schema.json
> > > @@ -4178,3 +4178,49 @@
> > >  # Since: 2.6
> > >  ##
> > >  { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
> > > +
> > > +##
> > > +# CpuInstanceProperties
> > > +#
> > > +# @node: #optional NUMA node ID the CPU belongs to
> > > +# @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'
> > > +  }
> > > +}    
> > 
> > Is there somewhere we should document the fact that these particular
> > properties are common ones, but there could be more.  The point is that
> > management should not assume these are the only fields here, but should
> > be prepared to accept anything, and echo it back to the device_add.  
> 
> Something along these lines?

Something like that, yes.

> +##
> +# CpuInstanceProperties
> +#
> +# List of properties to be used for hotplugging a CPU instance,
> +# it should be passed by management with device_add command when
> +# a CPU is being hotplugged.
> +#
> +# Note: currently there are 4 properties that could be present
> +# but management should be prepared to pass through other
> +# properties with device_add command to allow for future
> +# interface extension.
>  
> > > +
> > > +##
> > > +# @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
> > > +#
> > > +# Returns: a list of HotpluggableCPU objects.
> > > +#
> > > +# Since: 2.7
> > > +##
> > > +{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> > > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > > index de896a5..96f4454 100644
> > > --- a/qmp-commands.hx
> > > +++ b/qmp-commands.hx
> > > @@ -4880,3 +4880,44 @@ Example:
> > >                  { "version": 3, "emulated": false, "kernel": true } ] }
> > >  
> > >  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:    
> > 
> > Might be best to say "pseries" here, since that's the exposed machine
> > type name, even though we use sPAPR a lot internally.  
> Di you mean something like this?
> 
> +Example for 'pseries' target started with -smp 2,cores=2,maxcpus=4: 

Perhaps "'pseries' machine type" explicitly, since "target" tends to
refer to ISA rather than board in qemu.

> 
> >   
> > > +
> > > +-> { "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]"}
> > > +   ]}'
> > > -- 
> > > 1.8.3.1
> > >     
> > 
> >   
> 


-- 
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.7 v6 1/2] QMP: add query-hotpluggable-cpus
  2016-04-11 16:14   ` Michael Roth
@ 2016-04-12 12:31     ` Igor Mammedov
  0 siblings, 0 replies; 12+ messages in thread
From: Igor Mammedov @ 2016-04-12 12:31 UTC (permalink / raw)
  To: Michael Roth
  Cc: qemu-devel, mjrosato, thuth, pkrempa, ehabkost, aik, armbru,
	agraf, borntraeger, qemu-ppc, bharata, pbonzini, dgibson,
	afaerber

On Mon, 11 Apr 2016 11:14:27 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> Quoting Igor Mammedov (2016-04-08 06:29:55)
> > 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 and set
> >  MachineClass.query_hotpluggable_cpus
> > callback, 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>
> > ---
> > v6:
> >  - fix style issues in qapi-schema and qmp-commands,
> >    Eric Blake <eblake@redhat.com>
> >  - rebase on top current master (query-gic-capabilities conflict)
> > v5:
> >  - fix s390 build failure:
> >     undefined reference to `qmp_query_hotpluggable_cpus'
> > v4:
> >  - add MachineClass method to get CPU object list
> > 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
> > 
> > query_fixup
> > ---
> >  include/hw/boards.h |  5 +++++
> >  monitor.c           | 13 +++++++++++++
> >  qapi-schema.json    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> >  qmp-commands.hx     | 41 +++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 105 insertions(+)
> > 
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index aad5f2a..c122a70 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -81,6 +81,10 @@ typedef struct {
> >   *    Returns an array of @CPUArchId architecture-dependent CPU IDs
> >   *    which includes CPU IDs for present and possible to hotplug CPUs.
> >   *    Caller is responsible for freeing returned list.
> > + * @query_hotpluggable_cpus:
> > + *    Returns a @HotpluggableCPUList, which describes CPUs objects which
> > + *    could be added with -device/device_add.
> > + *    Caller is responsible for freeing returned list.
> >   */
> >  struct MachineClass {
> >      /*< private >*/
> > @@ -123,6 +127,7 @@ struct MachineClass {
> >                                             DeviceState *dev);
> >      unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
> >      CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> > +    HotpluggableCPUList *(*query_hotpluggable_cpus)(MachineState *machine);
> >  };
> > 
> >  /**
> > diff --git a/monitor.c b/monitor.c
> > index d1c1930..b469225 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -4267,3 +4267,16 @@ GICCapabilityList *qmp_query_gic_capabilities(Error **errp)
> >      return NULL;
> >  }
> >  #endif
> > +
> > +HotpluggableCPUList *qmp_query_hotpluggable_cpus(Error **errp)
> > +{
> > +    MachineState *ms = MACHINE(qdev_get_machine());
> > +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> > +
> > +    if (!mc->query_hotpluggable_cpus) {
> > +        error_setg(errp, QERR_FEATURE_DISABLED, "query-hotpluggable-cpus");
> > +        return NULL;
> > +    }
> > +
> > +    return mc->query_hotpluggable_cpus(ms);
> > +}
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 54634c4..4d1d71d 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -4178,3 +4178,49 @@
> >  # Since: 2.6
> >  ##
> >  { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
> > +
> > +##
> > +# CpuInstanceProperties
> > +#
> > +# @node: #optional NUMA node ID the CPU belongs to
> > +# @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
> > +#
> > +# Returns: a list of HotpluggableCPU objects.
> > +#
> > +# Since: 2.7
> > +##
> > +{ 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index de896a5..96f4454 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -4880,3 +4880,44 @@ Example:
> >                  { "version": 3, "emulated": false, "kernel": true } ] }
> > 
> >  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]"}
> > +   ]}'  
> 
> These examples kind of imply the granularity of "props" field has some
> relation to the granularity afforded by -smp. But IIUC what really
> matters is the granularity of the object type specified by "type".
props doesn't imply granularity, it's just a list properties for
instantiating a particular instance of CPU object.
However mgmt could be taught to understand some props elements
for pining purposes.
 
> If so, it's also confusing in the sense that the output above likely
> would look different if x86 implements device_add functionality in
> the form of socket-level objects, which AFAIK is stil the plan
> there?
For x86 I'm planning to submit patches with thread level cpu hotplug as
it matches the old one and allows to replace old hotplug with a new
without breaking anything migration wise, we won't even have to keep
compat code to make it work.
 
> Perhaps we should stick with pseries as the one concrete example,
> then for other examples provide 'theoretical' examples that touch
> on the granularity of the hotpluggable object and how that factors
> into what management would expect in the returned output. Sort of
> as a reference for future implementers.
> 
> Then we can add a concrete x86 example when that's in.
Ok, I'll drop x86 example from this patch and add it when x86
patches are posted.
 
> > +
> > +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]"}
> > +   ]}'
> > -- 
> > 1.8.3.1
> >   
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-04-12 12:31 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-08 11:29 [Qemu-devel] [PATCH for-2.7 v6 0/2] ispapr: QMP: add query-hotpluggable-cpus Igor Mammedov
2016-04-08 11:29 ` [Qemu-devel] [PATCH for-2.7 v6 1/2] " Igor Mammedov
2016-04-11  4:20   ` David Gibson
2016-04-11  9:35     ` Igor Mammedov
2016-04-12  1:40       ` David Gibson
2016-04-11 16:14   ` Michael Roth
2016-04-12 12:31     ` Igor Mammedov
2016-04-08 11:29 ` [Qemu-devel] [PATCH for-2.7 v6 2/2] spapr: implement query-hotpluggable-cpus callback Igor Mammedov
2016-04-11  4:54   ` David Gibson
2016-04-11  9:23     ` Igor Mammedov
2016-04-11  9:38       ` Bharata B Rao
2016-04-11  9:47         ` Igor Mammedov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.