Re: [Patch][resend] implementation of cpupool support in xl

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

On 09/15/10 10:29, Ian Campbell wrote:
> On Wed, 2010-09-15 at 08:26 +0100, Juergen Gross wrote:
>> diff -r 3985fea87987 tools/libxl/libxl.idl
>> --- a/tools/libxl/libxl.idl     Fri Sep 10 19:06:33 2010 +0100
>> +++ b/tools/libxl/libxl.idl     Wed Sep 15 09:19:02 2010 +0200
>> @@ -43,7 +43,11 @@ SHUTDOWN_* constant."""),
>>       ], destructor_fn=None)
>>
>>   libxl_poolinfo = Struct("poolinfo", [
>> -    ("poolid", uint32)
>> +    ("poolid",      uint32),
>> +    ("sched_id",    uint32),
>> +    ("n_dom",       uint32),
>> +    ("cpumap_size", uint32),
>> +    ("cpumap",      libxl_cpumap)
>>       ], destructor_fn=None)
>>
>>   libxl_vminfo = Struct("vminfo", [
>
> Does the addition of the cpumap field here mean that we now need to
> generate a destructor function (by removing destructor_fn=None) and call
> it e.g. from main_pool*?

I took care of this by allocating the space for the cpumap(s) together with
the poolinfo structure(s).
If you don't like this, a destructor would be the correct solution, I think.

> Would it make sense to turn libxl_cpumap into a struct containing both
> the size and the data pointer?

IMO this would make sense. You ALWAYS need the size of a cpumap to handle it
correctly.

>
>> diff -r 3985fea87987 tools/libxl/libxl.h
>> --- a/tools/libxl/libxl.h	Fri Sep 10 19:06:33 2010 +0100
>> +++ b/tools/libxl/libxl.h	Wed Sep 15 09:19:02 2010 +0200
>> @@ -471,6 +471,15 @@ int libxl_device_net2_del(libxl_ctx *ctx
>>   int libxl_device_net2_del(libxl_ctx *ctx, libxl_device_net2 *net2,
>>                             int wait);
>>
>> +int libxl_get_freecpus(libxl_ctx *ctx, int *n_cpus, uint64_t **cpumap);
>> +int libxl_create_cpupool(libxl_ctx *ctx, char *name, int schedid,
>> +                         uint64_t *cpumap, int n_cpus, libxl_uuid *uuid,
>> +                         uint32_t *poolid);
>
> Should these cpumap parameters be libxl_cpumap* or are they a different sort of cpumap?

You are right, these should be libxl_cpumap.
I'll update the patch. It would be nice to know whether you are planning to
change libxl_cpumap to include the size or not.


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