diff mbox series

[v3,4/6] numa: introduce "numa-mem-supported" machine property

Message ID 1558079119-320634-5-git-send-email-imammedo@redhat.com (mailing list archive)
State New, archived
Headers show
Series numa: deprecate '-numa node, mem' and default memory distribution | expand

Commit Message

Igor Mammedov May 17, 2019, 7:45 a.m. UTC
'-numa 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.
Once deprecation period expires, QEMU will disable '-numa mem' option,
usage on new machine types and when the last machine type that supported
it is removed we would be able to remove '-numa mem' with associated code.

In order to help mgmt to find out if being deprecated CLI option
'-numa mem=SZ' is still supported by particular machine type, expose
this information via "numa-mem-supported" machine property.

Users can use "qom-list-properties" QMP command to list machine type
properties including initial proprety values (when probing for supported
machine types with '-machine none') or at runtime at preconfig time
before numa mapping is configured and decide if they should used legacy
'-numa mem' or alternative '-numa memdev' option.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/boards.h |  1 +
 hw/arm/virt.c       |  1 +
 hw/core/machine.c   | 12 ++++++++++++
 hw/i386/pc.c        |  1 +
 hw/ppc/spapr.c      |  1 +
 5 files changed, 16 insertions(+)

Comments

Markus Armbruster May 27, 2019, 6:38 p.m. UTC | #1
Igor Mammedov <imammedo@redhat.com> writes:

> '-numa 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.

To be precise: -numa node,mem=... and -numa node,memdev=...  Correct?

>                                                          What's possible
> though is to deprecate it, keeping option working with old machine types.
> Once deprecation period expires, QEMU will disable '-numa mem' option,
> usage on new machine types and when the last machine type that supported
> it is removed we would be able to remove '-numa mem' with associated code.
>
> In order to help mgmt to find out if being deprecated CLI option
> '-numa mem=SZ' is still supported by particular machine type, expose
> this information via "numa-mem-supported" machine property.
>
> Users can use "qom-list-properties" QMP command to list machine type
> properties including initial proprety values (when probing for supported
> machine types with '-machine none') or at runtime at preconfig time
> before numa mapping is configured and decide if they should used legacy
> '-numa mem' or alternative '-numa memdev' option.

This sentence is impenetrable, I'm afraid :)

If we only want to convey whether a machine type supports -numa
node,mem=..., then adding a flag to query-machines suffices.  Since I'm
pretty sure you'd have figured that out yourself, I suspect I'm missing
something.  Can you give me some examples of intended usage?
Eduardo Habkost May 27, 2019, 6:52 p.m. UTC | #2
On Fri, May 17, 2019 at 09:45:17AM +0200, Igor Mammedov wrote:
> '-numa 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.
> Once deprecation period expires, QEMU will disable '-numa mem' option,
> usage on new machine types and when the last machine type that supported
> it is removed we would be able to remove '-numa mem' with associated code.
> 
> In order to help mgmt to find out if being deprecated CLI option
> '-numa mem=SZ' is still supported by particular machine type, expose
> this information via "numa-mem-supported" machine property.
> 
> Users can use "qom-list-properties" QMP command to list machine type
> properties including initial proprety values (when probing for supported
> machine types with '-machine none') or at runtime at preconfig time
> before numa mapping is configured and decide if they should used legacy
> '-numa mem' or alternative '-numa memdev' option.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/boards.h |  1 +
>  hw/arm/virt.c       |  1 +
>  hw/core/machine.c   | 12 ++++++++++++
>  hw/i386/pc.c        |  1 +
>  hw/ppc/spapr.c      |  1 +
>  5 files changed, 16 insertions(+)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 6f7916f..9e347cf 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -210,6 +210,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 5331ab7..2e86c78 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/core/machine.c b/hw/core/machine.c
> index 5d046a4..8bc53ba 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -506,6 +506,13 @@ static char *machine_get_nvdimm_persistence(Object *obj, Error **errp)
>      return g_strdup(ms->nvdimms_state->persistence_string);
>  }
>  
> +static bool machine_get_numa_mem_supported(Object *obj, Error **errp)
> +{
> +    MachineClass *mc = MACHINE_GET_CLASS(obj);
> +
> +    return mc->numa_mem_supported;

I don't remember other cases where a property value is constant
for all instances of a given class, but still requires
instantiating an object just to look at the value of the
property.  Is this really a pattern we want to start using in
QEMU?

Why hiding numa_mem_supported behind qom-list-properties is
better than simply extending the QAPI schema to add a new field
to MachineInfo?  Extending MachineInfo is simple and obvious, and
it makes the new machine class attribute be explicitly documented
in the QAPI schema.


> +}
> +
>  static void machine_set_nvdimm_persistence(Object *obj, const char *value,
>                                             Error **errp)
>  {
> @@ -810,6 +817,11 @@ static void machine_class_init(ObjectClass *oc, void *data)
>          &error_abort);
>      object_class_property_set_description(oc, "memory-encryption",
>          "Set memory encryption object to use", &error_abort);
> +
> +    object_class_property_add_bool(oc, "numa-mem-supported",
> +        machine_get_numa_mem_supported, NULL, &error_abort);
> +    object_class_property_set_description(oc, "numa-mem-supported",
> +        "Shows if legacy '-numa mem=SIZE option is supported", &error_abort);
>  }
>  
>  static void machine_class_base_init(ObjectClass *oc, void *data)
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index de91e90..bec0055 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2756,6 +2756,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;
> -- 
> 2.7.4
> 
>
Igor Mammedov May 28, 2019, 1:14 p.m. UTC | #3
On Mon, 27 May 2019 20:38:57 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > '-numa 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.  
> 
> To be precise: -numa node,mem=... and -numa node,memdev=...  Correct?
yep, I'll try to use full syntax since so it would be clear to others.


> >                                                          What's possible
> > though is to deprecate it, keeping option working with old machine types.
> > Once deprecation period expires, QEMU will disable '-numa mem' option,
> > usage on new machine types and when the last machine type that supported
> > it is removed we would be able to remove '-numa mem' with associated code.
> >
> > In order to help mgmt to find out if being deprecated CLI option
> > '-numa mem=SZ' is still supported by particular machine type, expose
> > this information via "numa-mem-supported" machine property.
> >
> > Users can use "qom-list-properties" QMP command to list machine type
> > properties including initial proprety values (when probing for supported
> > machine types with '-machine none') or at runtime at preconfig time
> > before numa mapping is configured and decide if they should used legacy
> > '-numa mem' or alternative '-numa memdev' option.  
> 
> This sentence is impenetrable, I'm afraid :)
> 
> If we only want to convey whether a machine type supports -numa
> node,mem=..., then adding a flag to query-machines suffices.  Since I'm
> pretty sure you'd have figured that out yourself, I suspect I'm missing
I didn't know about query-machines, hence implemented "qom-list-properties"
approach as was discussed at https://www.mail-archive.com/qemu-devel@nongnu.org/msg601220.html

For the purpose of deprecating '-numa node,mem" query-machines is more than
enough. I'll drop 1-3 patches and respin series using query-machines.

> something.  Can you give me some examples of intended usage?
Perhaps there will in future use cases when introspecting 'defaults'
of objects will be needed, then we could look back into qom-list-properties
if there aren't a better alternative.
diff mbox series

Patch

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 6f7916f..9e347cf 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -210,6 +210,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 5331ab7..2e86c78 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/core/machine.c b/hw/core/machine.c
index 5d046a4..8bc53ba 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -506,6 +506,13 @@  static char *machine_get_nvdimm_persistence(Object *obj, Error **errp)
     return g_strdup(ms->nvdimms_state->persistence_string);
 }
 
+static bool machine_get_numa_mem_supported(Object *obj, Error **errp)
+{
+    MachineClass *mc = MACHINE_GET_CLASS(obj);
+
+    return mc->numa_mem_supported;
+}
+
 static void machine_set_nvdimm_persistence(Object *obj, const char *value,
                                            Error **errp)
 {
@@ -810,6 +817,11 @@  static void machine_class_init(ObjectClass *oc, void *data)
         &error_abort);
     object_class_property_set_description(oc, "memory-encryption",
         "Set memory encryption object to use", &error_abort);
+
+    object_class_property_add_bool(oc, "numa-mem-supported",
+        machine_get_numa_mem_supported, NULL, &error_abort);
+    object_class_property_set_description(oc, "numa-mem-supported",
+        "Shows if legacy '-numa mem=SIZE option is supported", &error_abort);
 }
 
 static void machine_class_base_init(ObjectClass *oc, void *data)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index de91e90..bec0055 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2756,6 +2756,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;