Re: [Qemu-devel] [PULL 23/29] numa: use possible_cpus for not mapped CPUs check

[Markus Armbruster]

Eduardo Habkost <address@hidden> writes:

> From: Igor Mammedov <address@hidden>
>
> and remove corresponding part in numa.c that uses
> node_cpu bitmaps.
>
> Signed-off-by: Igor Mammedov <address@hidden>
> Reviewed-by: David Gibson <address@hidden>
> Reviewed-by: Andrew Jones <address@hidden>
> Message-Id: <address@hidden>
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
>  hw/core/machine.c | 58 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  numa.c            | 10 ----------
>  2 files changed, 58 insertions(+), 10 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 64e2a4ff2c..fd6a436064 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -20,6 +20,7 @@
>  #include "sysemu/numa.h"
>  #include "qemu/error-report.h"
>  #include "qemu/cutils.h"
> +#include "sysemu/numa.h"
>  
>  static char *machine_get_accel(Object *obj, Error **errp)
>  {
> @@ -678,9 +679,66 @@ bool machine_mem_merge(MachineState *machine)
>      return machine->mem_merge;
>  }
>  
> +static char *cpu_slot_to_string(const CPUArchId *cpu)
> +{
> +    GString *s = g_string_new(NULL);
> +    if (cpu->props.has_socket_id) {
> +        g_string_append_printf(s, "socket-id: %"PRId64, 
> cpu->props.socket_id);
> +    }
> +    if (cpu->props.has_core_id) {
> +        if (s->len) {
> +            g_string_append_printf(s, ", ");
> +        }
> +        g_string_append_printf(s, "core-id: %"PRId64, cpu->props.core_id);
> +    }
> +    if (cpu->props.has_thread_id) {
> +        if (s->len) {
> +            g_string_append_printf(s, ", ");
> +        }
> +        g_string_append_printf(s, "thread-id: %"PRId64, 
> cpu->props.thread_id);
> +    }
> +    return g_string_free(s, false);
> +}
> +
> +static void machine_numa_validate(MachineState *machine)
> +{
> +    int i;
> +    GString *s = g_string_new(NULL);
> +    MachineClass *mc = MACHINE_GET_CLASS(machine);
> +    const CPUArchIdList *possible_cpus = mc->possible_cpu_arch_ids(machine);
> +
> +    assert(nb_numa_nodes);
> +    for (i = 0; i < possible_cpus->len; i++) {
> +        const CPUArchId *cpu_slot = &possible_cpus->cpus[i];
> +
> +        /* at this point numa mappings are initilized by CLI options
> +         * or with default mappings so it's sufficient to list
> +         * all not yet mapped CPUs here */
> +        /* TODO: make it hard error in future */
> +        if (!cpu_slot->props.has_node_id) {
> +            char *cpu_str = cpu_slot_to_string(cpu_slot);
> +            g_string_append_printf(s, "%sCPU %d [%s]", s->len ? ", " : "", i,
> +                                   cpu_str);
> +            g_free(cpu_str);
> +        }
> +    }
> +    if (s->len) {
> +        error_report("warning: CPU(s) not present in any NUMA nodes: %s",
> +                     s->str);
> +        error_report("warning: All CPU(s) up to maxcpus should be described "
> +                     "in NUMA config, ability to start up with partial NUMA "
> +                     "mappings is obsoleted and will be removed in future");

I can see this gripes in output of make check.  Shouldn't you suppress
them on qtest_enabled(), like we do elsewhere?

> +    }
> +    g_string_free(s, true);
> +}

[...]