Re: [PATCH RESEND 12/12] xl: numa-sched: enable specifying node-affinity in VM config file

[Ian Jackson <Ian.Jackson@eu.citrix.com>]

Dario Faggioli writes ("[PATCH RESEND 12/12] xl: numa-sched: enable specifying node-affinity in VM config file"):
> in a similar way to how it is possible to specify vcpu-affinity.
...
>      /*
> -     * Check if the domain has any CPU affinity. If not, try to build
> -     * up one. In case numa_place_domain() find at least a suitable
> -     * candidate, it will affect info->nodemap accordingly; if it
> -     * does not, it just leaves it as it is. This means (unless
> -     * some weird error manifests) the subsequent call to
> -     * libxl_domain_set_nodeaffinity() will do the actual placement,
> +     * Check if the domain has any pinning or node-affinity and, if not, try
> +     * to build up one.
> +     *
> +     * In case numa_place_domain() find at least a suitable candidate, it will
> +     * affect info->nodemap accordingly; if it does not, it just leaves it as
> +     * it is. This means (unless some weird error manifests) the subsequent
> +     * call to libxl_domain_set_nodeaffinity() will do the actual placement,
>       * whatever that turns out to be.
>       */
>      if (libxl_defbool_val(info->numa_placement)) {
>  
> -        if (!libxl_bitmap_is_full(&info->cpumap)) {
> +        if (!libxl_bitmap_is_full(&info->cpumap) ||
> +            !libxl_bitmap_is_full(&info->nodemap)) {
>              LOG(ERROR, "Can run NUMA placement only if no vcpu "
> -                       "affinity is specified");
> +                       "pinning or node-affinity is specified");

I appreciate I may be a bit late with this complaint, but surely this
approach (setting the cpumap and nodemap when they aren't
all-bits-set) means that it's not possible to explicitly set "all
cpus".

If I say in the config file "all" for cpus and "all" for nodes, this
code will override that.  I don't think that can be right.

Ian.