Re: [Xen-devel] [PATCH 3 of 3] full support of setting scheduler parameters on domain creation

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-05-22 at 13:39 +0100, Juergen Gross wrote:
> On 05/22/2012 02:30 PM, Ian Campbell wrote:
> >> # HG changeset patch
> >> # User Juergen Gross<juergen.gross@xxxxxxxxxxxxxx>
> >> # Date 1337677423 -7200
> >> # Node ID 953383741ff44d97587c2e75da79b092523d6e83
> >> # Parent  19aaa30d7fdce2f1b56cb13399d603d955a61fb8
> >> full support of setting scheduler parameters on domain creation
> >>
> >> Obtains default scheduler parameters before modifying any settings
> >> specified
> >> via domain config.
> >>
> >> Corrected an error for setting sedf parameters (setting .period
> >> multiple
> >> times).
> >>
> >> Signed-off-by: Juergen Gross<juergen.gross@xxxxxxxxxxxxxx>
> >>
> >> diff -r 19aaa30d7fdc -r 953383741ff4 tools/libxl/libxl.h
> >> --- a/tools/libxl/libxl.h       Tue May 22 10:31:30 2012 +0200
> >> +++ b/tools/libxl/libxl.h       Tue May 22 11:03:43 2012 +0200
> >> @@ -605,6 +605,8 @@ int libxl_primary_console_exec(libxl_ctx
> >>   /* May be called with info_r == NULL to check for domain's existance
> >> */
> >>   int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
> >>                         uint32_t domid);
> >> +int libxl_sched_set_defaults(libxl_ctx*, uint32_t poolid,
> >> +                             libxl_sched_params *scparams);
> > This interface really makes libxl_sched_params differ from all the other
> > libxl structs (which have a public _init function and an internal
> > setdefaults function). I'm not really sure its justified either, I was
> > under the impression that you'd found that there were useful
> > discriminating values?
> 
> Dario opted for this solution, so I proposed a patch implementing it.
> I prefer this solution, too, as it isn't exporting scheduler internals to
> the tools.

I don't think "-1 is not a valid weight" is really "exporting scheduler
internals".

Anyway, I am far more concerned that the libxl API is useful to users
than I am about the API between libxl/libxc and the hypervisor (which is
not set in stone and can be fixed etc). The libxl API is the one we want
to expose and keep stable going forward etc.

> 
> > If this function was called libxl_sched_init (replacing the
> > autogenerated one) then it might be ok. Although I'm still not really
> > sure what the issue is with having a discriminating value meaning
> > default is, doing that keeps the _init function cheap too.
> 
> Okay, I can rename it.
> 
> >
> >>   int libxl__sched_set_params(libxl__gc *gc, uint32_t domid,
> >> libxl_sched_params *scparams)
> >>   {
> >>       libxl_ctx *ctx = libxl__gc_owner(gc);
> >> -    libxl_scheduler sched;
> >>       libxl_sched_sedf_domain sedf_info;
> >>       libxl_sched_credit_domain credit_info;
> >>       libxl_sched_credit2_domain credit2_info;
> >>       int ret;
> >>
> >> -    sched = libxl_get_scheduler (ctx);
> >> -    switch (sched) {
> >> +    switch (scparams->sched) {
> > What happens if scparams->sched is not the scheduler used for this
> > domain? Should it either be checked or set somewhere?
> 
> The check would be the same as the original setting of scparams->sched.
> Setting of the scheduler parameters will be rejected by the hypervisor if the
> scheduler does not match.
> 
> 
> Juergen
> 



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