| Message ID | 3b55c78a3f4f12d143ae7b42209f1900bc2c6cc2.1466713052.git.pkrempa@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 23 Jun 2016 22:23:24 +0200 Peter Krempa <pkrempa@redhat.com> wrote: > struct CPUCore uses 'core-id' as 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. David also prefers core-id, one nit pls also add -id suffix to socket and thread fields in schema to be consistent. > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > --- > hmp.c | 4 ++-- > hw/ppc/spapr.c | 4 ++-- > include/hw/cpu/core.h | 3 +++ > qapi-schema.json | 4 ++-- > 4 files changed, 9 insertions(+), 6 deletions(-) > > diff --git a/hmp.c b/hmp.c > index 997a768..543f087 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -2463,8 +2463,8 @@ void hmp_hotpluggable_cpus(Monitor *mon, const > QDict *qdict) if (c->has_socket) { > monitor_printf(mon, " socket: \"%" PRIu64 "\"\n", > c->socket); } > - if (c->has_core) { > - monitor_printf(mon, " core: \"%" PRIu64 "\"\n", > c->core); > + if (c->has_core_id) { > + monitor_printf(mon, " core: \"%" PRIu64 "\"\n", > c->core_id); } > if (c->has_thread) { > monitor_printf(mon, " thread: \"%" PRIu64 "\"\n", > c->thread); 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..37ef5fd 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4271,7 +4271,7 @@ > # > # @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 > +# @core-id: #optional core number within socket the CPU belongs to > # @thread: #optional thread number within core the CPU belongs to > # > # Since: 2.7 > @@ -4279,7 +4279,7 @@ > { 'struct': 'CpuInstanceProperties', > 'data': { '*node': 'int', > '*socket': 'int', > - '*core': 'int', > + '*core-id': 'int', > '*thread': 'int' > } > }
On Thu, 23 Jun 2016 22:54:18 +0200 Igor Mammedov <imammedo@redhat.com> wrote: > On Thu, 23 Jun 2016 22:23:24 +0200 > Peter Krempa <pkrempa@redhat.com> wrote: > > > struct CPUCore uses 'core-id' as 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. > David also prefers core-id, > one nit pls also add -id suffix to socket and thread fields in schema > to be consistent. Heh. I wrote a patch almost identical to this yesterday, intending to post it today. So, Reviewed-by: David Gibson <david@gibson.dropbear.id.au> with the same comment as Igor about changing socket and thread to match. > > > > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > > --- > > hmp.c | 4 ++-- > > hw/ppc/spapr.c | 4 ++-- > > include/hw/cpu/core.h | 3 +++ > > qapi-schema.json | 4 ++-- > > 4 files changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/hmp.c b/hmp.c > > index 997a768..543f087 100644 > > --- a/hmp.c > > +++ b/hmp.c > > @@ -2463,8 +2463,8 @@ void hmp_hotpluggable_cpus(Monitor *mon, const > > QDict *qdict) if (c->has_socket) { > > monitor_printf(mon, " socket: \"%" PRIu64 "\"\n", > > c->socket); } > > - if (c->has_core) { > > - monitor_printf(mon, " core: \"%" PRIu64 "\"\n", > > c->core); > > + if (c->has_core_id) { > > + monitor_printf(mon, " core: \"%" PRIu64 "\"\n", > > c->core_id); } > > if (c->has_thread) { > > monitor_printf(mon, " thread: \"%" PRIu64 "\"\n", > > c->thread); 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..37ef5fd 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -4271,7 +4271,7 @@ > > # > > # @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 > > +# @core-id: #optional core number within socket the CPU belongs to > > # @thread: #optional thread number within core the CPU belongs to > > # > > # Since: 2.7 > > @@ -4279,7 +4279,7 @@ > > { 'struct': 'CpuInstanceProperties', > > 'data': { '*node': 'int', > > '*socket': 'int', > > - '*core': 'int', > > + '*core-id': 'int', > > '*thread': 'int' > > } > > } >
diff --git a/hmp.c b/hmp.c index 997a768..543f087 100644 --- a/hmp.c +++ b/hmp.c @@ -2463,8 +2463,8 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict) if (c->has_socket) { monitor_printf(mon, " socket: \"%" PRIu64 "\"\n", c->socket); } - if (c->has_core) { - monitor_printf(mon, " core: \"%" PRIu64 "\"\n", c->core); + if (c->has_core_id) { + monitor_printf(mon, " core: \"%" PRIu64 "\"\n", c->core_id); } if (c->has_thread) { monitor_printf(mon, " thread: \"%" PRIu64 "\"\n", c->thread); 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..37ef5fd 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4271,7 +4271,7 @@ # # @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 +# @core-id: #optional core number within socket the CPU belongs to # @thread: #optional thread number within core the CPU belongs to # # Since: 2.7 @@ -4279,7 +4279,7 @@ { 'struct': 'CpuInstanceProperties', 'data': { '*node': 'int', '*socket': 'int', - '*core': 'int', + '*core-id': 'int', '*thread': 'int' } }
struct CPUCore uses 'core-id' as 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 | 4 ++-- hw/ppc/spapr.c | 4 ++-- include/hw/cpu/core.h | 3 +++ qapi-schema.json | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-)