| Message ID | 1559205199-233510-2-git-send-email-imammedo@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | numa: deprecate '-numa node, mem' and default memory distribution | expand |
Igor Mammedov <imammedo@redhat.com> writes: > Legacy '-numa node,mem' option has a number of issues and mgmt often > defaults to it. Unfortunately it's no possible to replace it with > an alternative '-numa memdev' without breaking migration compatibility. > What's possible though is to deprecate it, keeping option working with > old machine types only. > > In order to help users to find out if being deprecated CLI option > '-numa node,mem' is still supported by particular machine type, add new > "numa-mem-supported" property to MachineInfo description in QAPI schema. Suggest s/to MachineInfo description in QAPI schema/to output of query-machines/, because query-machines is the external interface people know. > "numa-mem-supported" is set to 'true' for machines that currently support > NUMA, but it will be flipped to 'false' later on, once deprecation period > expires and kept 'true' only for old machine types that used to support > the legacy option so it won't break existing configuration that are using > it. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > > Notes: > v4: > * drop idea to use "qom-list-properties" and use MachineInfo instead > which could be inspected with 'query-machines' > > include/hw/boards.h | 3 +++ > hw/arm/virt.c | 1 + > hw/i386/pc.c | 1 + > hw/ppc/spapr.c | 1 + > qapi/misc.json | 5 ++++- > vl.c | 1 + > 6 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 6f7916f..86894b6 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -158,6 +158,8 @@ typedef struct { > * @kvm_type: > * Return the type of KVM corresponding to the kvm-type string option or > * computed based on other criteria such as the host kernel capabilities. > + * @numa_mem_supported: > + * true if '--numa node.mem' option is supported and false otherwise > */ > struct MachineClass { > /*< private >*/ > @@ -210,6 +212,7 @@ struct MachineClass { > bool ignore_boot_device_suffixes; > bool smbus_no_migration_support; > bool nvdimm_supported; > + bool numa_mem_supported; > > HotplugHandler *(*get_hotplug_handler)(MachineState *machine, > DeviceState *dev); > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index bf54f10..481a603 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1943,6 +1943,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) > assert(!mc->get_hotplug_handler); > mc->get_hotplug_handler = virt_machine_get_hotplug_handler; > hc->plug = virt_machine_device_plug_cb; > + mc->numa_mem_supported = true; > } > > static void virt_instance_init(Object *obj) > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 2632b73..05b8368 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2747,6 +2747,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > nc->nmi_monitor_handler = x86_nmi; > mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE; > mc->nvdimm_supported = true; > + mc->numa_mem_supported = true; > > object_class_property_add(oc, PC_MACHINE_DEVMEM_REGION_SIZE, "int", > pc_machine_get_device_memory_region_size, NULL, > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 2ef3ce4..265ecfb 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -4336,6 +4336,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) > * in which LMBs are represented and hot-added > */ > mc->numa_mem_align_shift = 28; > + mc->numa_mem_supported = true; > > smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF; > smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON; This is correct when the TYPE_VIRT_MACHINE, TYPE_PC_MACHINE and TYPE_SPAPR_MACHINE are exactly the machines supporting NUMA. How could I check that? > diff --git a/qapi/misc.json b/qapi/misc.json > index 8b3ca4f..d0bdccb 100644 > --- a/qapi/misc.json > +++ b/qapi/misc.json > @@ -2018,12 +2018,15 @@ > # > # @hotpluggable-cpus: cpu hotplug via -device is supported (since 2.7.0) > # > +# @numa-mem-supported: true if '-numa node,mem' option is supported by machine > +# type and false otherwise (since 4.1) "by the machine type", for consistency with @cpu-max. Also, rather long line. > +# > # Since: 1.2.0 > ## > { 'struct': 'MachineInfo', > 'data': { 'name': 'str', '*alias': 'str', > '*is-default': 'bool', 'cpu-max': 'int', > - 'hotpluggable-cpus': 'bool'} } > + 'hotpluggable-cpus': 'bool', 'numa-mem-supported': 'bool'} } > > ## > # @query-machines: > diff --git a/vl.c b/vl.c > index 5550bd7..5bf17f5 100644 > --- a/vl.c > +++ b/vl.c > @@ -1520,6 +1520,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->has_hotpluggable_cpus; > + info->numa_mem_supported = mc->numa_mem_supported; > > entry = g_malloc0(sizeof(*entry)); > entry->value = info;
On Fri, Jun 07, 2019 at 07:39:17PM +0200, Markus Armbruster wrote: > This is correct when the TYPE_VIRT_MACHINE, TYPE_PC_MACHINE and > TYPE_SPAPR_MACHINE are exactly the machines supporting NUMA. How could > I check that? parse_numa_node() rejects the -numa option if the machine doesn't implement MachineClass::get_default_cpu_node_id(). Grepping for it: $ git grep -pw get_default_cpu_node_id hw/arm/virt.c=static void virt_machine_class_init(ObjectClass *oc, void *data) hw/arm/virt.c: mc->get_default_cpu_node_id = virt_get_default_cpu_node_id; hw/core/machine.c=static void machine_numa_finish_cpu_init(MachineState *machine) hw/core/machine.c: props.node_id = mc->get_default_cpu_node_id(machine, i); hw/i386/pc.c=static void pc_machine_class_init(ObjectClass *oc, void *data) hw/i386/pc.c: mc->get_default_cpu_node_id = pc_get_default_cpu_node_id; hw/ppc/spapr.c=static void spapr_machine_class_init(ObjectClass *oc, void *data) hw/ppc/spapr.c: mc->get_default_cpu_node_id = spapr_get_default_cpu_node_id; include/hw/boards.h=typedef struct { include/hw/boards.h: * @get_default_cpu_node_id: include/hw/boards.h=struct MachineClass { include/hw/boards.h: int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx); numa.c=static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, numa.c: if (!mc->cpu_index_to_instance_props || !mc->get_default_cpu_node_id) { Related: [PATCH v4 01/11] numa: move numa global variable nb_numa_nodes into MachineState which adds a MachineClass::numa_supported flag to those machines.
On Fri, 07 Jun 2019 19:39:17 +0200 Markus Armbruster <armbru@redhat.com> wrote: > Igor Mammedov <imammedo@redhat.com> writes: > > > Legacy '-numa node,mem' option has a number of issues and mgmt often > > defaults to it. Unfortunately it's no possible to replace it with > > an alternative '-numa memdev' without breaking migration compatibility. > > What's possible though is to deprecate it, keeping option working with > > old machine types only. > > > > In order to help users to find out if being deprecated CLI option > > '-numa node,mem' is still supported by particular machine type, add new > > "numa-mem-supported" property to MachineInfo description in QAPI schema. > > Suggest s/to MachineInfo description in QAPI schema/to output of > query-machines/, because query-machines is the external interface people > know. fixed > > > "numa-mem-supported" is set to 'true' for machines that currently support > > NUMA, but it will be flipped to 'false' later on, once deprecation period > > expires and kept 'true' only for old machine types that used to support > > the legacy option so it won't break existing configuration that are using > > it. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > > > Notes: > > v4: > > * drop idea to use "qom-list-properties" and use MachineInfo instead > > which could be inspected with 'query-machines' > > > > include/hw/boards.h | 3 +++ > > hw/arm/virt.c | 1 + > > hw/i386/pc.c | 1 + > > hw/ppc/spapr.c | 1 + > > qapi/misc.json | 5 ++++- > > vl.c | 1 + > > 6 files changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/include/hw/boards.h b/include/hw/boards.h > > index 6f7916f..86894b6 100644 > > --- a/include/hw/boards.h > > +++ b/include/hw/boards.h > > @@ -158,6 +158,8 @@ typedef struct { > > * @kvm_type: > > * Return the type of KVM corresponding to the kvm-type string option or > > * computed based on other criteria such as the host kernel capabilities. > > + * @numa_mem_supported: > > + * true if '--numa node.mem' option is supported and false otherwise > > */ > > struct MachineClass { > > /*< private >*/ > > @@ -210,6 +212,7 @@ struct MachineClass { > > bool ignore_boot_device_suffixes; > > bool smbus_no_migration_support; > > bool nvdimm_supported; > > + bool numa_mem_supported; > > > > HotplugHandler *(*get_hotplug_handler)(MachineState *machine, > > DeviceState *dev); > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index bf54f10..481a603 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -1943,6 +1943,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) > > assert(!mc->get_hotplug_handler); > > mc->get_hotplug_handler = virt_machine_get_hotplug_handler; > > hc->plug = virt_machine_device_plug_cb; > > + mc->numa_mem_supported = true; > > } > > > > static void virt_instance_init(Object *obj) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 2632b73..05b8368 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -2747,6 +2747,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > > nc->nmi_monitor_handler = x86_nmi; > > mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE; > > mc->nvdimm_supported = true; > > + mc->numa_mem_supported = true; > > > > object_class_property_add(oc, PC_MACHINE_DEVMEM_REGION_SIZE, "int", > > pc_machine_get_device_memory_region_size, NULL, > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 2ef3ce4..265ecfb 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -4336,6 +4336,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) > > * in which LMBs are represented and hot-added > > */ > > mc->numa_mem_align_shift = 28; > > + mc->numa_mem_supported = true; > > > > smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF; > > smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON; > > This is correct when the TYPE_VIRT_MACHINE, TYPE_PC_MACHINE and > TYPE_SPAPR_MACHINE are exactly the machines supporting NUMA. How could > I check that? We don't have an interface to communicate that to users as far as I know. Eduardo pointed out to the patch that might serve as starting point for it though. > > > diff --git a/qapi/misc.json b/qapi/misc.json > > index 8b3ca4f..d0bdccb 100644 > > --- a/qapi/misc.json > > +++ b/qapi/misc.json > > @@ -2018,12 +2018,15 @@ > > # > > # @hotpluggable-cpus: cpu hotplug via -device is supported (since 2.7.0) > > # > > +# @numa-mem-supported: true if '-numa node,mem' option is supported by machine > > +# type and false otherwise (since 4.1) > > > "by the machine type", for consistency with @cpu-max. Also, rather long > line. fixed > > > +# > > # Since: 1.2.0 > > ## > > { 'struct': 'MachineInfo', > > 'data': { 'name': 'str', '*alias': 'str', > > '*is-default': 'bool', 'cpu-max': 'int', > > - 'hotpluggable-cpus': 'bool'} } > > + 'hotpluggable-cpus': 'bool', 'numa-mem-supported': 'bool'} } > > > > ## > > # @query-machines: > > diff --git a/vl.c b/vl.c > > index 5550bd7..5bf17f5 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -1520,6 +1520,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->has_hotpluggable_cpus; > > + info->numa_mem_supported = mc->numa_mem_supported; > > > > entry = g_malloc0(sizeof(*entry)); > > entry->value = info;
Eduardo Habkost <ehabkost@redhat.com> writes: > On Fri, Jun 07, 2019 at 07:39:17PM +0200, Markus Armbruster wrote: >> This is correct when the TYPE_VIRT_MACHINE, TYPE_PC_MACHINE and >> TYPE_SPAPR_MACHINE are exactly the machines supporting NUMA. How could >> I check that? > > parse_numa_node() rejects the -numa option if the machine doesn't > implement MachineClass::get_default_cpu_node_id(). > > Grepping for it: > > $ git grep -pw get_default_cpu_node_id > hw/arm/virt.c=static void virt_machine_class_init(ObjectClass *oc, void *data) > hw/arm/virt.c: mc->get_default_cpu_node_id = virt_get_default_cpu_node_id; > hw/core/machine.c=static void machine_numa_finish_cpu_init(MachineState *machine) > hw/core/machine.c: props.node_id = mc->get_default_cpu_node_id(machine, i); > hw/i386/pc.c=static void pc_machine_class_init(ObjectClass *oc, void *data) > hw/i386/pc.c: mc->get_default_cpu_node_id = pc_get_default_cpu_node_id; > hw/ppc/spapr.c=static void spapr_machine_class_init(ObjectClass *oc, void *data) > hw/ppc/spapr.c: mc->get_default_cpu_node_id = spapr_get_default_cpu_node_id; > include/hw/boards.h=typedef struct { > include/hw/boards.h: * @get_default_cpu_node_id: > include/hw/boards.h=struct MachineClass { > include/hw/boards.h: int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx); > numa.c=static void parse_numa_node(MachineState *ms, NumaNodeOptions *node, > numa.c: if (!mc->cpu_index_to_instance_props || !mc->get_default_cpu_node_id) { > > > Related: > [PATCH v4 01/11] numa: move numa global variable nb_numa_nodes into MachineState > which adds a MachineClass::numa_supported flag to those machines. Thanks, Eduardo! Preferably with commit message and doc comment tweaked: Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff --git a/include/hw/boards.h b/include/hw/boards.h index 6f7916f..86894b6 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -158,6 +158,8 @@ typedef struct { * @kvm_type: * Return the type of KVM corresponding to the kvm-type string option or * computed based on other criteria such as the host kernel capabilities. + * @numa_mem_supported: + * true if '--numa node.mem' option is supported and false otherwise */ struct MachineClass { /*< private >*/ @@ -210,6 +212,7 @@ struct MachineClass { bool ignore_boot_device_suffixes; bool smbus_no_migration_support; bool nvdimm_supported; + bool numa_mem_supported; HotplugHandler *(*get_hotplug_handler)(MachineState *machine, DeviceState *dev); diff --git a/hw/arm/virt.c b/hw/arm/virt.c index bf54f10..481a603 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1943,6 +1943,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) assert(!mc->get_hotplug_handler); mc->get_hotplug_handler = virt_machine_get_hotplug_handler; hc->plug = virt_machine_device_plug_cb; + mc->numa_mem_supported = true; } static void virt_instance_init(Object *obj) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 2632b73..05b8368 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2747,6 +2747,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) nc->nmi_monitor_handler = x86_nmi; mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE; mc->nvdimm_supported = true; + mc->numa_mem_supported = true; object_class_property_add(oc, PC_MACHINE_DEVMEM_REGION_SIZE, "int", pc_machine_get_device_memory_region_size, NULL, diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 2ef3ce4..265ecfb 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4336,6 +4336,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) * in which LMBs are represented and hot-added */ mc->numa_mem_align_shift = 28; + mc->numa_mem_supported = true; smc->default_caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF; smc->default_caps.caps[SPAPR_CAP_VSX] = SPAPR_CAP_ON; diff --git a/qapi/misc.json b/qapi/misc.json index 8b3ca4f..d0bdccb 100644 --- a/qapi/misc.json +++ b/qapi/misc.json @@ -2018,12 +2018,15 @@ # # @hotpluggable-cpus: cpu hotplug via -device is supported (since 2.7.0) # +# @numa-mem-supported: true if '-numa node,mem' option is supported by machine +# type and false otherwise (since 4.1) +# # Since: 1.2.0 ## { 'struct': 'MachineInfo', 'data': { 'name': 'str', '*alias': 'str', '*is-default': 'bool', 'cpu-max': 'int', - 'hotpluggable-cpus': 'bool'} } + 'hotpluggable-cpus': 'bool', 'numa-mem-supported': 'bool'} } ## # @query-machines: diff --git a/vl.c b/vl.c index 5550bd7..5bf17f5 100644 --- a/vl.c +++ b/vl.c @@ -1520,6 +1520,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->has_hotpluggable_cpus; + info->numa_mem_supported = mc->numa_mem_supported; entry = g_malloc0(sizeof(*entry)); entry->value = info;
Legacy '-numa node,mem' option has a number of issues and mgmt often defaults to it. Unfortunately it's no possible to replace it with an alternative '-numa memdev' without breaking migration compatibility. What's possible though is to deprecate it, keeping option working with old machine types only. In order to help users to find out if being deprecated CLI option '-numa node,mem' is still supported by particular machine type, add new "numa-mem-supported" property to MachineInfo description in QAPI schema. "numa-mem-supported" is set to 'true' for machines that currently support NUMA, but it will be flipped to 'false' later on, once deprecation period expires and kept 'true' only for old machine types that used to support the legacy option so it won't break existing configuration that are using it. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- Notes: v4: * drop idea to use "qom-list-properties" and use MachineInfo instead which could be inspected with 'query-machines' include/hw/boards.h | 3 +++ hw/arm/virt.c | 1 + hw/i386/pc.c | 1 + hw/ppc/spapr.c | 1 + qapi/misc.json | 5 ++++- vl.c | 1 + 6 files changed, 11 insertions(+), 1 deletion(-)