Re: [Xen-devel] [PATCH 3 of 5 V2] libxl: make it possible to explicitly specify default sched params

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2012-05-30 at 08:26 +0100, Dario Faggioli wrote:
> On Tue, 2012-05-29 at 14:57 +0100, Ian Campbell wrote: 
> > int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid,
> > -                                libxl_sched_sedf_domain *scinfo)
> > +                                libxl_domain_sched_params *scinfo)
> >  {
> > -    xc_domaininfo_t domaininfo;
> > -    int rc;
> > -
> > -    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo);
> > -    if (rc < 0) {
> > -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info 
> > list");
> > +    uint64_t period;
> > +    uint64_t slice;
> > +    uint64_t latency;
> > +    uint16_t extratime;
> > +    uint16_t weight;
> > +
> > +    int ret;
> > +
> > +    ret = xc_sedf_domain_get(ctx->xch, domid, &period, &slice, &latency,
> > +                            &extratime, &weight);
> > +    if (ret != 0) {
> > +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched 
> > sedf");
> >          return ERROR_FAIL;
> >      }
> > -    if (rc != 1 || domaininfo.domain != domid)
> > -        return ERROR_INVAL;
> > -
> > -
> > -    rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000,
> > -                            scinfo->slice * 1000000, scinfo->latency * 
> > 1000000,
> > -                            scinfo->extratime, scinfo->weight);
> > -    if ( rc < 0 ) {
> > +
> > +    if (scinfo->period != LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT)
> > +        period = scinfo->period * 1000000;
> > +    if (scinfo->slice != LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT)
> > +        slice = scinfo->slice * 1000000;
> > +    if (scinfo->latency != LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT)
> > +        latency = scinfo->latency * 1000000;
> > +    if (scinfo->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT)
> > +        extratime = scinfo->extratime;
> > +    if (scinfo->weight != LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT)
> > +        weight = scinfo->weight;
> > +
> > +    ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency,
> > +                            extratime, weight);
> >
> I only tested this on your last version of this series (and I am still
> trying to find the time to test this new one) and this wasn't working.
> Reason should be the fact that the SEDF code in Xen wants you to put a
> domain either in the "proper-EDF" state _or_ in the
> "weighted-best-effort-state", and it discriminates this by checking
> whether you're passing a slice+period _or_ a weight.
> 
> IOW, if your domain has a slice+period and wants a weight, either you
> set slice and period to zero, or the whole thing will fail. In fact,
> with this code, the new parameter set will include both the slice+period
> (as the domain has them set) and the new weight that is being set by the
> call... Does this sound right?

I've no idea, but I trust you..

> That's why the  current int main_sched_sedf(int argc, char **argv) is
> doing this:
> 
>             if (opt_p) {
>                 scinfo.period = period;
>                 scinfo.weight = 0;
>             }
>             if (opt_s) {
>                 scinfo.slice = slice;
>                 scinfo.weight = 0;
>             }
>             if (opt_l)
>                 scinfo.latency = latency;
>             if (opt_e)
>                 scinfo.extratime = extra;
>             if (opt_w) {
>                 scinfo.weight = weight;
>                 scinfo.period = 0;
>                 scinfo.slice = 0;
>             } 
>
> That being said, I'm not sure at what level we want to enforce something
> like the above. The lowest level toolstack seems fine to me, which would
> mean putting something like the code above in the config file parsing...
> If you agree, I'll try that and let you know whether or not it works.

If you could provide an incremental patch that would be much
appreciated.

IMHO it would be fine (and a good idea) for libxl to return ERROR_INVAL
if the conditions aren't met too. If you want to also check it in xl's
config file parsing and either fix it up like the above or error out
then please do.


> 
> Regards,
> Dario
> 



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