Re: [PATCH v1 3/4] libxl: enabling XL to set per-VCPU parameters of a domain for RTDS scheduler

[Chong Li <lichong659@gmail.com>]

On Tue, May 12, 2015 at 5:01 AM, Dario Faggioli
<dario.faggioli@citrix.com> wrote:
> [Adjusting the Cc list:
>   - removing hypervisor only people
>   - adding more tools maintainers
>   - adding George]
>
> On Thu, 2015-05-07 at 12:05 -0500, Chong Li wrote:
>> Change sched_rtds_domain_get/set functions to support per-VCPU settings for RTDS scheduler.
>>
> More on this patch (I had to run yesterday).
>
>> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
>> index 44bd8e2..8284ce1 100644
>> --- a/tools/libxl/libxl.h
>> +++ b/tools/libxl/libxl.h
>

>> +libxl_rtds_vcpu = Struct("vcpu",[
>>
>                      ^Struct("rtds_vcpu",[
>
>> +    ("period",       uint64, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT'}),
>> +    ("budget",       uint64, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT'}),
>> +    ("index",        integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT'}),
>>
> Call this last member vcpuid, if you want to be able to pass a
> sparse/incomplete array, and hence you need to know to what vcpu each
> element refers to, or just get rid of it, it you always pass all the
> elements.
>
> I'd go with the former.
>
>
> So, if API compatibility/stability wasn't an issue, fiddling with
> libxl_domain_sched_params would probably be the best solution. Since it
> is, I'd leave that alone as much as possible, and introduce something
> completely new, for dealing with per-vcpu parameters. Something like
> this:
>
> typedef struct libxl_vcpu_sched_params {
>     libxl_domain_sched_params vcpus[];
>     int num_vcpus;
> } libxl_vcpu_sched_params;
>
> [*]
>

If we use libxl_domain_sched_params vcpus[] here, then each time we
set only one per-vcpu
param, we need to copy the whole vcpus array to the hypervisor (e.g.,
the array length is 32
when there are 32 vcpus in the domain), because
libxl_domain_sched_params has no a
member serving as "vcpuid".(so we can only locate the vcpu based on
the array index).

My idea is using "libxl_rtds_vcpu vcpus[]" (libxl_rtds_vcpu includes
budget, period and
vcpuid, as shown above) in libxl_vcpu_sched_params. Is that Ok?

>
> Regards,
> Dario
>
> [*] I don't like the fact that the vcpus[] array is of
> 'libxl_domain_sched_params' type, especially the *domain* part of the
> type name, but I don't think we can change that, for the said API
> stability reasons.



-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis