Re: [Xen-devel] [PATCH] xl: check for meaningful combination of sedf config file parameters

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-06-04 at 15:01 +0100, Dario Faggioli wrote:
> As we do it in the implementation of `xl sched-sedf -d ...', some
> consistency checking is needed while parsing the sedf scheduling
> parameters provided via config file. Not doing this results in the call
> libxl_domain_sched_params_set() to fail, and no parameters being
> enforced for the domain.
> 
> Note we do this at config file parsing time as that gives us the chance
> of bailing out early. It would have been pointless to add it within
> sched_sedf_domain_set() (in libxl), as the very same thing is
> done in the hypervisor, and the result is being checked and returned
> to the caller already.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -561,6 +561,7 @@ static void parse_config_data(const char
>      long l;
>      XLU_Config *config;
>      XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids;
> +    int opt_w = 0, opt_p = 0, opt_s = 0;

These names don't make much sense in this context.

Perhaps you can just check each interesting option against the
corresponding LIBXL_DOAIN_SCHED_PARAM_DEFAULT? That might make some long
lines. Perhaps pulling this out into a separate valid_sched_params()
would help with that?

>      int pci_power_mgmt = 0;
>      int pci_msitranslate = 1;
>      int pci_permissive = 0;
> @@ -632,18 +633,36 @@ static void parse_config_data(const char
>  
>      /* the following is the actual config parsing with overriding
>       * values in the structures */
> -    if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0))
> +    if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0)) {
>          b_info->sched_params.weight = l;
> +        opt_w = 1;
> +    }
>      if (!xlu_cfg_get_long (config, "cap", &l, 0))
>          b_info->sched_params.cap = l;
> -    if (!xlu_cfg_get_long (config, "period", &l, 0))
> +    if (!xlu_cfg_get_long (config, "period", &l, 0)) {
>          b_info->sched_params.period = l;
> -    if (!xlu_cfg_get_long (config, "slice", &l, 0))
> +        opt_p = 1;
> +    }
> +    if (!xlu_cfg_get_long (config, "slice", &l, 0)) {
>          b_info->sched_params.slice = l;
> +        opt_s = 1;
> +    }
>      if (!xlu_cfg_get_long (config, "latency", &l, 0))
>          b_info->sched_params.latency = l;
>      if (!xlu_cfg_get_long (config, "extratime", &l, 0))
>          b_info->sched_params.extratime = l;
> +    /* The sedf scheduler needs some more consistency checking */
> +    if (opt_w && (opt_p || opt_s)) {
> +        fprintf(stderr, "Either specify a weight OR a period and slice\n");

Does this constrain you from setting valid combinations of credit*
parameters? I think not since period and slice are SEDF specific.


> +        exit(1);
> +    }
> +    if (opt_w) {
> +        b_info->sched_params.slice = 0;
> +        b_info->sched_params.period = 0;
> +    }
> +    if (opt_p || opt_s)
> +        b_info->sched_params.weight = 0;
> +
>  
>      if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
>          b_info->max_vcpus = l;



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