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

[Chong Li <lichong659@xxxxxxxxx>]

On Tue, May 12, 2015 at 5:01 AM, Dario Faggioli
<dario.faggioli@xxxxxxxxxx> 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).
>

>>  libxl_domain_sched_params = Struct("domain_sched_params",[
>>      ("sched",        libxl_scheduler),
>>      ("weight",       integer, {'init_val': 
>> 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT'}),
>> @@ -356,6 +366,7 @@ libxl_domain_sched_params = 
>> Struct("domain_sched_params",[
>>      ("latency",      integer, {'init_val': 
>> 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}),
>>      ("extratime",    integer, {'init_val': 
>> 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}),
>>      ("budget",       integer, {'init_val': 
>> 'LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT'}),
>> +    ("rtds",             libxl_domain_sched_rtds_params),
>>      ])
>>
> I've got the same concerns I've already expressed about the hypervisor
> interface. Doing things like this, we will have
> libxl_domain_sched_params that hosts both per-domain and per-vcpu
> parameters, without a clear way to know what happens if one specifies
> both.
>
> Also, if at some point other schedulers want to support per-vcpu
> parameters, the said information duplication (and ambiguity) will apply
> to them too!
>
> Finally, I don't think I would call the way the result returned by a
> call to libxl_domain_sched_params_{get,set}() changes backward
> compatible, which OTOH is something we want to retain in libxl.
>
> 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;
>
> [*]
>
> And I'll deal with this in new API functions:
>
> int libxl_vcpu_sched_params_get(libxl_ctx *ctx, uint32_t domid,
>                                 libxl_domain_vcpu_params *params);
> int libxl_vcpu_sched_params_set(libxl_ctx *ctx, uint32_t domid,
>                                 const libxl_vcpu_sched_params *params);
>
> At this point, if, say, Credit2 wants to start supporting per-vcpu
> scheduling parameters, we're all set for that!
>
> What do the tools' maintainers think?
>
> The only thing I'm not sure about, in the described scenario, is what a
> call to libxl_domain_sched_params_get() should return for a scheduler
> which supports per-vcpu parameters, and actually has different
> scheduling parameters for the various vcpus...
>
> I mean, a call to libxl_domain_sched_params_set() can be interpreted
> like 'set the passed params for all the vcpus', _get(), I don't know...
> perhaps it should return some sort of ERROR_INVAL, to inform the caller
> that it is libxl_vcpu_sched_params_get() that should be used?
>
> Again, tools maintainers? :-)
>
> Oh, and in any case, you need to mark that something is being added to
> the libxl API using the '#define LIBXL_HAVE_*' mechanism. Look for
> examples in libxl.h, there are several. If going for my proposal, I'd
> call the constant LIBXL_HAVE_VCPU_SCHED_PARAMS.
>
> 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.

Hi, Dario,

Yes. To be consistent with the hypervisor interface (seperate per-dom
and per-vcpu data
structures), this idea seems better.

I can do it quick after getting feedbacks from tools maintainers.

Chong


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

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