Re: [Xen-devel] [PATCH v2 12/16] libxl: get and set soft affinity

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Dario Faggioli writes ("[PATCH v2 12/16] libxl: get and set soft affinity"):
> which basically means making space for a new cpumap in both
> vcpu_info (for getting soft affinity) and build_info (for setting
> it), along with providing the get/set functions, and wiring them
> to the proper xc calls. Interface is as follows:
> 
>  * libxl_{get,set}_vcpuaffinity() deals with hard affinity, as it
>    always has happened;
>  * libxl_get,set}_vcpuaffinity_soft() deals with soft affinity.

In practice, doesn't this API plus these warnings mean that a
toolstack which wants to migrate a domain to a new set of vcpus (or,
worse, a new cpupool) will find it difficult to avoid warnings from
libxl ?

Because the toolstack will want to change both the hard and soft
affinities to new maps, perhaps entirely disjoint from the old ones,
but can only do one at a time.  So the system will definitely go
through "silly" states.

This would be solved with an API that permitted setting both sets of
affinities in a single call, even if the underlying libxc and
hypercalls are separate, because libxl would do the check only on the
overall final state.

So perhaps we should have a singe function that can change the
cpupool, hard affinity, and soft affinity, all at once ?

What if the application makes a call to change the cpupool, without
touching the affinities ?  Should the affinities be reset
automatically ?


> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index cf214bb..b7f39bd 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -5,6 +5,7 @@
>  XEN_ROOT = $(CURDIR)/../..
>  include $(XEN_ROOT)/tools/Rules.mk
>  
> +#MAJOR = 4.4
>  MAJOR = 4.3
>  MINOR = 0

Commented out ?


> +        if (libxl_cpu_bitmap_alloc(ctx, &ptr->cpumap_soft, 0)) {
> +            LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "allocating 
> cpumap_soft");
> +            return NULL;
> +        }

I went and looked at the error handling.  Normally in libxl allocators
can't fail but this one wants to do a hypercall to look up the max
cpus.

But, pulling on the thread, I wonder if it would be better to do this
error logging in libxl_cpu_bitmap_alloc rather than all the call
sites.  (If we did that in principle we should introduce a new #define
which allows applications to #ifdef out their own logging for this,
but in practice the double-logging isn't likely to be a problem.)

And looking at libxl_cpu_bitmap_alloc, it calls libxl_get_max_cpus and
does something very odd with the return value: libxl_get_max_nodes
ought to return a positive number or a libxl error code.

So I went to look at libxl_get_max_nodes and it just returns whatever
it got from libxc, which is definitely wrong!

Would you mind fixing this as part of this series ?


Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel