Re: [Qemu-devel] [PATCH v4 1/3] machine: show if CLI option '-numa node, mem' is supported in QAPI schema

[Igor Mammedov]

On Fri, 07 Jun 2019 19:39:17 +0200
Markus Armbruster <address@hidden> wrote:

> Igor Mammedov <address@hidden> 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 <address@hidden>
> > ---
> >
> > 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;