[libvirt] [Qemu-devel] [PATCH v3 4/6] numa: introduce "numa-mem-supported" machine property

[Eduardo Habkost]

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 at 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
> 
> 

-- 
Eduardo