Re: [PATCH 1 of 3] To be able to support arbitrary numbers of physical cpus it was necessary to

[Juergen Gross <juergen.gross@ts.fujitsu.com>]

On 10/06/10 15:41, Ian Campbell wrote:
> On Wed, 2010-10-06 at 12:21 +0100, Stefano Stabellini wrote:
>> On Tue, 5 Oct 2010, Juergen Gross wrote:
>>> diff -r fe3018c6976d -r cfce8e755505 tools/libxl/libxl_internal.h
>>> --- a/tools/libxl/libxl_internal.h	Mon Oct 04 12:52:14 2010 +0100
>>> +++ b/tools/libxl/libxl_internal.h	Tue Oct 05 14:19:13 2010 +0200
>>> @@ -239,7 +239,6 @@
>>>   _hidden char *libxl__domid_to_name(libxl__gc *gc, uint32_t domid);
>>>   _hidden char *libxl__poolid_to_name(libxl__gc *gc, uint32_t poolid);
>>>
>>> -
>>>     /* holds the CPUID response for a single CPUID leaf
>>>      * input contains the value of the EAX and ECX register,
>>>      * and each policy string contains a filter to apply to
>>
>> we don't need this change
>>
>>
>>> diff -r fe3018c6976d -r cfce8e755505 tools/libxl/xl_cmdimpl.c
>>> --- a/tools/libxl/xl_cmdimpl.c	Mon Oct 04 12:52:14 2010 +0100
>>> +++ b/tools/libxl/xl_cmdimpl.c	Tue Oct 05 14:19:13 2010 +0200
>>> @@ -3616,12 +3616,11 @@
>>>   static void vcpupin(char *d, const char *vcpu, char *cpu)
>>>   {
>>>       libxl_vcpuinfo *vcpuinfo;
>>> -    libxl_physinfo physinfo;
>>>       uint64_t *cpumap = NULL;
>>>
>>>       uint32_t vcpuid, cpuida, cpuidb;
>>>       char *endptr, *toka, *tokb;
>>> -    int i, nb_vcpu, cpusize;
>>> +    int i, nb_vcpu, cpusize, cpumapsize;
>>>
>>>       vcpuid = strtoul(vcpu,&endptr, 10);
>>>       if (vcpu == endptr) {
>>> @@ -3634,12 +3633,13 @@
>>>
>>>       find_domain(d);
>>>
>>> -    if (libxl_get_physinfo(&ctx,&physinfo) != 0) {
>>> -        fprintf(stderr, "libxl_get_physinfo failed.\n");
>>> +    if ((cpusize = xc_get_max_cpus(ctx.xch)) == 0) {
>>> +        fprintf(stderr, "xc_get_maxcpus failed.\n");
>>>           goto vcpupin_out1;
>>>       }
>>
>> You shouldn't be calling xc functions directly from xl_cmdimpl.c.
>> The basic rule is: libxl clients (such as xl) shouldn't need to call any
>> library functions other than libxenlight's functions.
>> You can add a libxl_get_max_cpus function though.
>
> Maybe we should add libxl_cpumap_alloc_phys which returns a libxl_cpumap
> big enough to hold all physical CPUs, there are a few places within
> libxl itself which might benefit from this, e.g. libxl_list_cpupool and
> libxl_list_vcpu both call xc_get_max_cpus and then use the result as a
> parameter to libxl_cpumap_alloc.
>
> Actually, given that libxl_cpumap is only ever used for PCPUs perhaps
> alloc should just always return a suitably sized map and there's no need
> for the size parameter to libxl_cpumap_alloc? Is there any plausible
> potential use for a libxl_cpumap of nrVCPU rather than nrPCPU ?

Currently there seems to be no demand for cpumasks larger than nrPCPU.
Changinf libxl_cpumap_alloc to allocate just the correct size seems
appropriate.
I think I'll do this in my planned cpumask rework.


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
TSP ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html