Re: [Xen-devel] [PATCH v3 4/4] xl: introduce rtds scheduler

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Sun, 2014-09-14 at 17:37 -0400, Meng Xu wrote:
> Add xl command for rtds scheduler
> Note: VCPU's parameter (period, budget) is in microsecond (us).
> 
> Signed-off-by: Meng Xu <mengxu@xxxxxxxxxxxxx>
> Signed-off-by: Sisu Xi <xisisu@xxxxxxxxx>
>
Reviewed-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>

I only have two minor comments.

> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1

> +static int sched_rtds_pool_output(uint32_t poolid)
> +{
> +    char *poolname;
> +
> +    poolname = libxl_cpupoolid_to_name(ctx, poolid);
> +    printf("Cpupool %s: sched=RTDS algorithm=EDF\n", poolname);
>
Oh, so you are printing RTDS. Perhaps the cover letter is outdated
then...

In any case, just use "sched=RTDS", no need for "algorithm=FOO"

> +
> +    free(poolname);
> +    return 0;
> +}

> +int main_sched_rtds(int argc, char **argv)
> +{
> +    const char *dom = NULL;
> +    const char *cpupool = NULL;
> +    int period = 0; /* period is in microsecond */
> +    int budget = 0; /* budget is in microsecond */
> +    bool opt_p = false;
> +    bool opt_b = false;
> +    int opt, rc;
> +    static struct option opts[] = {
> +        {"domain", 1, 0, 'd'},
> +        {"period", 1, 0, 'p'},
> +        {"budget", 1, 0, 'b'},
> +        {"cpupool", 1, 0, 'c'},
> +        COMMON_LONG_OPTS,
> +        {0, 0, 0, 0}
> +    };
> +
> +    SWITCH_FOREACH_OPT(opt, "d:p:b:c:h", opts, "sched-rtds", 0) {
> +    case 'd':
> +        dom = optarg;
> +        break;
> +    case 'p':
> +        period = strtol(optarg, NULL, 10);
> +        opt_p = 1;
> +        break;
> +    case 'b':
> +        budget = strtol(optarg, NULL, 10);
> +        opt_b = 1;
> +        break;
> +    case 'c':
> +        cpupool = optarg;
> +        break;
> +    }
> +
> +    if (cpupool && (dom || opt_p || opt_b)) {
> +        fprintf(stderr, "Specifying a cpupool is not allowed with other 
> options.\n");
>
Long line.  We probably can live with this, but since I spotted it...

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

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