[Qemu-devel] [PATCH v2 0/2] qapi: Fix up cpu hotplug property names and add witness for cpu hotplug support

[Qemu-devel] [PATCH v2 0/2] qapi: Fix up cpu hotplug property names and add witness for cpu hotplug support

[Peter Krempa]

Version 2:
- fix typos/incompetence/drowsiness based language errors in commit message
- select version 1 as prefered way
- add -id suffix to all members of CpuInstanceProperties
- note in qapi-schema the need to keep members in sync
- fix output text field names in HMP impl

Peter Krempa (2):
  qapi: Report support for -device cpu hotplug in query-machines
  qapi: keep names in 'CpuInstanceProperties' in sync with struct
    CPUCore

 hmp.c                 | 16 ++++++++--------
 hw/ppc/spapr.c        |  4 ++--
 include/hw/cpu/core.h |  3 +++
 qapi-schema.json      | 24 ++++++++++++++----------
 vl.c                  |  1 +
 5 files changed, 28 insertions(+), 20 deletions(-)

-- 
2.8.3

[Qemu-devel] [PATCH v2 1/2] qapi: Report support for -device cpu hotplug in query-machines

[Peter Krempa]

For management apps it's very useful to know whether the selected
machine type supports cpu hotplug via the new -device approach. Using
the presence of 'query-hotpluggable-cpus' alone is not enough as a
witness.

Add a property to 'MachineInfo' called 'hotpluggable-cpus' that will
report the presence of this feature.

Example of output:
    {
        "hotpluggable-cpus": false,
        "name": "mac99",
        "cpu-max": 1
    },
    {
        "hotpluggable-cpus": true,
        "name": "pseries-2.7",
        "is-default": true,
        "cpu-max": 255,
        "alias": "pseries"
    },

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 qapi-schema.json | 5 ++++-
 vl.c             | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 0964eec..24ede28 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2986,11 +2986,14 @@
 # @cpu-max: maximum number of CPUs supported by the machine type
 #           (since 1.5.0)
 #
+# @hotpluggable-cpus: cpu hotplug via -device is supported (since 2.7.0)
+#
 # Since: 1.2.0
 ##
 { 'struct': 'MachineInfo',
   'data': { 'name': 'str', '*alias': 'str',
-            '*is-default': 'bool', 'cpu-max': 'int' } }
+            '*is-default': 'bool', 'cpu-max': 'int',
+            'hotpluggable-cpus': 'bool'} }

 ##
 # @query-machines:
diff --git a/vl.c b/vl.c
index c85833a..4c1f9ae 100644
--- a/vl.c
+++ b/vl.c
@@ -1524,6 +1524,7 @@ MachineInfoList *qmp_query_machines(Error **errp)

         info->name = g_strdup(mc->name);
         info->cpu_max = !mc->max_cpus ? 1 : mc->max_cpus;
+        info->hotpluggable_cpus = !!mc->query_hotpluggable_cpus;

         entry = g_malloc0(sizeof(*entry));
         entry->value = info;
-- 
2.8.3

Re: [Qemu-devel] [PATCH v2 1/2] qapi: Report support for -device cpu hotplug in query-machines

[Igor Mammedov]

On Thu, 23 Jun 2016 23:23:33 +0200
Peter Krempa <pkrempa@redhat.com> wrote:

> For management apps it's very useful to know whether the selected
> machine type supports cpu hotplug via the new -device approach. Using
> the presence of 'query-hotpluggable-cpus' alone is not enough as a
> witness.
> 
> Add a property to 'MachineInfo' called 'hotpluggable-cpus' that will
> report the presence of this feature.
> 
> Example of output:
>     {
>         "hotpluggable-cpus": false,
>         "name": "mac99",
>         "cpu-max": 1
>     },
>     {
>         "hotpluggable-cpus": true,
>         "name": "pseries-2.7",
>         "is-default": true,
>         "cpu-max": 255,
>         "alias": "pseries"
>     },
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  qapi-schema.json | 5 ++++-
>  vl.c             | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 0964eec..24ede28 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2986,11 +2986,14 @@
>  # @cpu-max: maximum number of CPUs supported by the machine type
>  #           (since 1.5.0)
>  #
> +# @hotpluggable-cpus: cpu hotplug via -device is supported (since
> 2.7.0) +#
>  # Since: 1.2.0
>  ##
>  { 'struct': 'MachineInfo',
>    'data': { 'name': 'str', '*alias': 'str',
> -            '*is-default': 'bool', 'cpu-max': 'int' } }
> +            '*is-default': 'bool', 'cpu-max': 'int',
> +            'hotpluggable-cpus': 'bool'} }
> 
>  ##
>  # @query-machines:
> diff --git a/vl.c b/vl.c
> index c85833a..4c1f9ae 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1524,6 +1524,7 @@ MachineInfoList *qmp_query_machines(Error
> **errp)
> 
>          info->name = g_strdup(mc->name);
>          info->cpu_max = !mc->max_cpus ? 1 : mc->max_cpus;
> +        info->hotpluggable_cpus = !!mc->query_hotpluggable_cpus;
> 
>          entry = g_malloc0(sizeof(*entry));
>          entry->value = info;

[Qemu-devel] [PATCH v2 2/2] qapi: keep names in 'CpuInstanceProperties' in sync with struct CPUCore

[Peter Krempa]

struct CPUCore uses 'id' suffix in the property name. As docs for
query-hotpluggable-cpus state that the cpu core properties should be
passed back to device_add by management in case new members are added
and thus the names for the fields should be kept in sync.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 hmp.c                 | 16 ++++++++--------
 hw/ppc/spapr.c        |  4 ++--
 include/hw/cpu/core.h |  3 +++
 qapi-schema.json      | 19 ++++++++++---------
 4 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/hmp.c b/hmp.c
index 997a768..925601a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2457,17 +2457,17 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)

         c = l->value->props;
         monitor_printf(mon, "  CPUInstance Properties:\n");
-        if (c->has_node) {
-            monitor_printf(mon, "    node: \"%" PRIu64 "\"\n", c->node);
+        if (c->has_node_id) {
+            monitor_printf(mon, "    node-id: \"%" PRIu64 "\"\n", c->node_id);
         }
-        if (c->has_socket) {
-            monitor_printf(mon, "    socket: \"%" PRIu64 "\"\n", c->socket);
+        if (c->has_socket_id) {
+            monitor_printf(mon, "    socket-id: \"%" PRIu64 "\"\n", c->socket_id);
         }
-        if (c->has_core) {
-            monitor_printf(mon, "    core: \"%" PRIu64 "\"\n", c->core);
+        if (c->has_core_id) {
+            monitor_printf(mon, "    core-id: \"%" PRIu64 "\"\n", c->core_id);
         }
-        if (c->has_thread) {
-            monitor_printf(mon, "    thread: \"%" PRIu64 "\"\n", c->thread);
+        if (c->has_thread_id) {
+            monitor_printf(mon, "    thread-id: \"%" PRIu64 "\"\n", c->thread_id);
         }

         l = l->next;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 778fa25..0b6bb9c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2367,8 +2367,8 @@ static HotpluggableCPUList *spapr_query_hotpluggable_cpus(MachineState *machine)

         cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model);
         cpu_item->vcpus_count = smp_threads;
-        cpu_props->has_core = true;
-        cpu_props->core = i * smt;
+        cpu_props->has_core_id = true;
+        cpu_props->core_id = i * smt;
         /* TODO: add 'has_node/node' here to describe
            to which node core belongs */

diff --git a/include/hw/cpu/core.h b/include/hw/cpu/core.h
index 4540a7d..79ac79c 100644
--- a/include/hw/cpu/core.h
+++ b/include/hw/cpu/core.h
@@ -26,6 +26,9 @@ typedef struct CPUCore {
     int nr_threads;
 } CPUCore;

+/* Note: topology field names need to be kept in sync with
+ * 'CpuInstanceProperties' */
+
 #define CPU_CORE_PROP_CORE_ID "core-id"

 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index 24ede28..d0c4be1 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4267,20 +4267,21 @@
 # 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.
+# interface extension. This also requires the filed names to be kept in sync
+# sync with the properties passed to -device/device_add.
 #
-# @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
+# @node-id: #optional NUMA node ID the CPU belongs to
+# @socket-id: #optional socket number within node/board the CPU belongs to
+# @core-id: #optional core number within socket the CPU belongs to
+# @thread-id: #optional thread number within core the CPU belongs to
 #
 # Since: 2.7
 ##
 { 'struct': 'CpuInstanceProperties',
-  'data': { '*node': 'int',
-            '*socket': 'int',
-            '*core': 'int',
-            '*thread': 'int'
+  'data': { '*node-id': 'int',
+            '*socket-id': 'int',
+            '*core-id': 'int',
+            '*thread-id': 'int'
   }
 }

-- 
2.8.3

Re: [Qemu-devel] [PATCH v2 2/2] qapi: keep names in 'CpuInstanceProperties' in sync with struct CPUCore

[Eric Blake]

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

On 06/23/2016 03:23 PM, Peter Krempa wrote:
> struct CPUCore uses 'id' suffix in the property name. As docs for
> query-hotpluggable-cpus state that the cpu core properties should be
> passed back to device_add by management in case new members are added
> and thus the names for the fields should be kept in sync.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  hmp.c                 | 16 ++++++++--------
>  hw/ppc/spapr.c        |  4 ++--
>  include/hw/cpu/core.h |  3 +++
>  qapi-schema.json      | 19 ++++++++++---------
>  4 files changed, 23 insertions(+), 19 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -4267,20 +4267,21 @@
>  # 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.
> +# interface extension. This also requires the filed names to be kept in sync
> +# sync with the properties passed to -device/device_add.

s/sync sync/sync/

>  #
> -# @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
> +# @node-id: #optional NUMA node ID the CPU belongs to
> +# @socket-id: #optional socket number within node/board the CPU belongs to
> +# @core-id: #optional core number within socket the CPU belongs to
> +# @thread-id: #optional thread number within core the CPU belongs to
>  #
>  # Since: 2.7
>  ##
>  { 'struct': 'CpuInstanceProperties',
> -  'data': { '*node': 'int',
> -            '*socket': 'int',
> -            '*core': 'int',
> -            '*thread': 'int'
> +  'data': { '*node-id': 'int',
> +            '*socket-id': 'int',
> +            '*core-id': 'int',
> +            '*thread-id': 'int'
>    }
>  }
> 

with the duplicate word fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH v2 2/2] qapi: keep names in 'CpuInstanceProperties' in sync with struct CPUCore

[Igor Mammedov]

On Thu, 23 Jun 2016 23:23:34 +0200
Peter Krempa <pkrempa@redhat.com> wrote:

> struct CPUCore uses 'id' suffix in the property name. As docs for
> query-hotpluggable-cpus state that the cpu core properties should be
> passed back to device_add by management in case new members are added
> and thus the names for the fields should be kept in sync.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  hmp.c                 | 16 ++++++++--------
>  hw/ppc/spapr.c        |  4 ++--
>  include/hw/cpu/core.h |  3 +++
>  qapi-schema.json      | 19 ++++++++++---------
>  4 files changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 997a768..925601a 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2457,17 +2457,17 @@ void hmp_hotpluggable_cpus(Monitor *mon,
> const QDict *qdict)
> 
>          c = l->value->props;
>          monitor_printf(mon, "  CPUInstance Properties:\n");
> -        if (c->has_node) {
> -            monitor_printf(mon, "    node: \"%" PRIu64 "\"\n",
> c->node);
> +        if (c->has_node_id) {
> +            monitor_printf(mon, "    node-id: \"%" PRIu64 "\"\n",
> c->node_id); }
> -        if (c->has_socket) {
> -            monitor_printf(mon, "    socket: \"%" PRIu64 "\"\n",
> c->socket);
> +        if (c->has_socket_id) {
> +            monitor_printf(mon, "    socket-id: \"%" PRIu64 "\"\n",
> c->socket_id); }
> -        if (c->has_core) {
> -            monitor_printf(mon, "    core: \"%" PRIu64 "\"\n",
> c->core);
> +        if (c->has_core_id) {
> +            monitor_printf(mon, "    core-id: \"%" PRIu64 "\"\n",
> c->core_id); }
> -        if (c->has_thread) {
> -            monitor_printf(mon, "    thread: \"%" PRIu64 "\"\n",
> c->thread);
> +        if (c->has_thread_id) {
> +            monitor_printf(mon, "    thread-id: \"%" PRIu64 "\"\n",
> c->thread_id); }
> 
>          l = l->next;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 778fa25..0b6bb9c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2367,8 +2367,8 @@ static HotpluggableCPUList
> *spapr_query_hotpluggable_cpus(MachineState *machine)
> 
>          cpu_item->type = spapr_get_cpu_core_type(machine->cpu_model);
>          cpu_item->vcpus_count = smp_threads;
> -        cpu_props->has_core = true;
> -        cpu_props->core = i * smt;
> +        cpu_props->has_core_id = true;
> +        cpu_props->core_id = i * smt;
>          /* TODO: add 'has_node/node' here to describe
>             to which node core belongs */
> 
> diff --git a/include/hw/cpu/core.h b/include/hw/cpu/core.h
> index 4540a7d..79ac79c 100644
> --- a/include/hw/cpu/core.h
> +++ b/include/hw/cpu/core.h
> @@ -26,6 +26,9 @@ typedef struct CPUCore {
>      int nr_threads;
>  } CPUCore;
> 
> +/* Note: topology field names need to be kept in sync with
> + * 'CpuInstanceProperties' */
> +
>  #define CPU_CORE_PROP_CORE_ID "core-id"
> 
>  #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 24ede28..d0c4be1 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4267,20 +4267,21 @@
>  # 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.
> +# interface extension. This also requires the filed names to be kept
> in sync +# sync with the properties passed to -device/device_add.
>  #
> -# @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
> +# @node-id: #optional NUMA node ID the CPU belongs to
> +# @socket-id: #optional socket number within node/board the CPU
> belongs to +# @core-id: #optional core number within socket the CPU
> belongs to +# @thread-id: #optional thread number within core the CPU
> belongs to #
>  # Since: 2.7
>  ##
>  { 'struct': 'CpuInstanceProperties',
> -  'data': { '*node': 'int',
> -            '*socket': 'int',
> -            '*core': 'int',
> -            '*thread': 'int'
> +  'data': { '*node-id': 'int',
> +            '*socket-id': 'int',
> +            '*core-id': 'int',
> +            '*thread-id': 'int'
>    }
>  }
> 

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

Re: [Qemu-devel] [PATCH v2 0/2] qapi: Fix up cpu hotplug property names and add witness for cpu hotplug support

[David Gibson]

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

On Thu, 23 Jun 2016 23:23:32 +0200
Peter Krempa <pkrempa@redhat.com> wrote:

> Version 2:
> - fix typos/incompetence/drowsiness based language errors in commit message
> - select version 1 as prefered way
> - add -id suffix to all members of CpuInstanceProperties
> - note in qapi-schema the need to keep members in sync
> - fix output text field names in HMP impl
> 
> Peter Krempa (2):
>   qapi: Report support for -device cpu hotplug in query-machines
>   qapi: keep names in 'CpuInstanceProperties' in sync with struct
>     CPUCore

Adding Bharata to CC.

Igor, should I take these through my tree?

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

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

Re: [Qemu-devel] [PATCH v2 0/2] qapi: Fix up cpu hotplug property names and add witness for cpu hotplug support

[Igor Mammedov]

On Fri, 24 Jun 2016 13:00:56 +1000
David Gibson <dgibson@redhat.com> wrote:

> On Thu, 23 Jun 2016 23:23:32 +0200
> Peter Krempa <pkrempa@redhat.com> wrote:
> 
> > Version 2:
> > - fix typos/incompetence/drowsiness based language errors in commit
> > message
> > - select version 1 as prefered way
> > - add -id suffix to all members of CpuInstanceProperties
> > - note in qapi-schema the need to keep members in sync
> > - fix output text field names in HMP impl
> > 
> > Peter Krempa (2):
> >   qapi: Report support for -device cpu hotplug in query-machines
> >   qapi: keep names in 'CpuInstanceProperties' in sync with struct
> >     CPUCore
> 
> Adding Bharata to CC.
> 
> Igor, should I take these through my tree?
Yep, please do so.
It' would be even better if it's merged into master ASAP
(for libvirt and for x86 device_add series that introduces these
properties for x86 CPUs)

Re: [Qemu-devel] [PATCH v2 0/2] qapi: Fix up cpu hotplug property names and add witness for cpu hotplug support

[David Gibson]

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

On Fri, 24 Jun 2016 07:31:39 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Fri, 24 Jun 2016 13:00:56 +1000
> David Gibson <dgibson@redhat.com> wrote:
> 
> > On Thu, 23 Jun 2016 23:23:32 +0200
> > Peter Krempa <pkrempa@redhat.com> wrote:
> >   
> > > Version 2:
> > > - fix typos/incompetence/drowsiness based language errors in commit
> > > message
> > > - select version 1 as prefered way
> > > - add -id suffix to all members of CpuInstanceProperties
> > > - note in qapi-schema the need to keep members in sync
> > > - fix output text field names in HMP impl
> > > 
> > > Peter Krempa (2):
> > >   qapi: Report support for -device cpu hotplug in query-machines
> > >   qapi: keep names in 'CpuInstanceProperties' in sync with struct
> > >     CPUCore  
> > 
> > Adding Bharata to CC.
> > 
> > Igor, should I take these through my tree?  
> Yep, please do so.
> It' would be even better if it's merged into master ASAP
> (for libvirt and for x86 device_add series that introduces these
> properties for x86 CPUs)

Yes, that's my intention.  Just want to check what the final word is on
whether we need the extra witness, or whether checking what's returned
from query-hotpluggable-cpus is sufficient.

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

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

Re: [Qemu-devel] [PATCH v2 0/2] qapi: Fix up cpu hotplug property names and add witness for cpu hotplug support

[David Gibson]

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

On Fri, Jun 24, 2016 at 03:38:51PM +1000, David Gibson wrote:
> On Fri, 24 Jun 2016 07:31:39 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Fri, 24 Jun 2016 13:00:56 +1000
> > David Gibson <dgibson@redhat.com> wrote:
> > 
> > > On Thu, 23 Jun 2016 23:23:32 +0200
> > > Peter Krempa <pkrempa@redhat.com> wrote:
> > >   
> > > > Version 2:
> > > > - fix typos/incompetence/drowsiness based language errors in commit
> > > > message
> > > > - select version 1 as prefered way
> > > > - add -id suffix to all members of CpuInstanceProperties
> > > > - note in qapi-schema the need to keep members in sync
> > > > - fix output text field names in HMP impl
> > > > 
> > > > Peter Krempa (2):
> > > >   qapi: Report support for -device cpu hotplug in query-machines
> > > >   qapi: keep names in 'CpuInstanceProperties' in sync with struct
> > > >     CPUCore  
> > > 
> > > Adding Bharata to CC.
> > > 
> > > Igor, should I take these through my tree?  
> > Yep, please do so.
> > It' would be even better if it's merged into master ASAP
> > (for libvirt and for x86 device_add series that introduces these
> > properties for x86 CPUs)
> 
> Yes, that's my intention.  Just want to check what the final word is on
> whether we need the extra witness, or whether checking what's returned
> from query-hotpluggable-cpus is sufficient.

Which it looks like we've done.  I've merged both these into my
ppc-for-2.7 tree, and I'll try to send a push request before the end
of my day.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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