[v5,for,Xen,4.7,4/4] xl: enable per-VCPU parameter settings for RTDS scheduler

Commit Message

Chong Li Feb. 4, 2016, 10:50 p.m. UTC

Change main_sched_rtds and related output functions to support
per-VCPU settings.

Signed-off-by: Chong Li <[e-mail]>
Signed-off-by: Meng Xu <[e-mail]>
Signed-off-by: Sisu Xi <[e-mail]>

---
Changes on PATCH v4:
1) Coding style changes

Changes on PATCH v3:
1) Support commands, e.g., "xl sched-rtds -d vm1" to output the
default scheduling parameters

Changes on PATCH v2:
1) Remove per-domain output functions for RTDS scheduler.

2) Users now use '-v all' to specify all VCPUs.

3) Support outputting a subset of the parameters of the VCPUs
of a specific domain.

4) When setting all VCPUs with the same parameters (by only one
command), no per-domain function is invoked.

CC: <[e-mail]>
CC: <[e-mail]>
CC: <[e-mail]>
CC: <[e-mail]>
CC: <[e-mail]>
CC: <[e-mail]>
---
 docs/man/xl.pod.1         |   4 +
 tools/libxl/xl_cmdimpl.c  | 305 ++++++++++++++++++++++++++++++++++++++++------
 tools/libxl/xl_cmdtable.c |  10 +-
 3 files changed, 281 insertions(+), 38 deletions(-)

Comments

Wei Liu Feb. 5, 2016, 2:51 p.m. UTC | #1

On Thu, Feb 04, 2016 at 04:50:44PM -0600, Chong Li wrote:
> Change main_sched_rtds and related output functions to support
> per-VCPU settings.
> 
> Signed-off-by: Chong Li <[e-mail]>
> Signed-off-by: Meng Xu <[e-mail]>
> Signed-off-by: Sisu Xi <[e-mail]>
> 
> ---
> Changes on PATCH v4:
> 1) Coding style changes
> 
> Changes on PATCH v3:
> 1) Support commands, e.g., "xl sched-rtds -d vm1" to output the
> default scheduling parameters
> 
> Changes on PATCH v2:
> 1) Remove per-domain output functions for RTDS scheduler.
> 
> 2) Users now use '-v all' to specify all VCPUs.
> 
> 3) Support outputting a subset of the parameters of the VCPUs
> of a specific domain.
> 
> 4) When setting all VCPUs with the same parameters (by only one
> command), no per-domain function is invoked.
> 
> CC: <[e-mail]>
> CC: <[e-mail]>
> CC: <[e-mail]>
> CC: <[e-mail]>
> CC: <[e-mail]>
> CC: <[e-mail]>
> ---
>  docs/man/xl.pod.1         |   4 +
>  tools/libxl/xl_cmdimpl.c  | 305 ++++++++++++++++++++++++++++++++++++++++------
>  tools/libxl/xl_cmdtable.c |  10 +-
>  3 files changed, 281 insertions(+), 38 deletions(-)
> 
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index 4279c7c..f9ff917 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1051,6 +1051,10 @@ B<OPTIONS>
>  Specify domain for which scheduler parameters are to be modified or retrieved.
>  Mandatory for modifying scheduler parameters.
>  
> +=item B<-v VCPUID/all>, B<--vcpuid=VCPUID/all>
> +
> +Specify vcpu for which scheduler parameters are to be modified or retrieved.
> +
>  =item B<-p PERIOD>, B<--period=PERIOD>
>  
>  Period of time, in microseconds, over which to replenish the budget.
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 2b6371d..b843fa5 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -5823,6 +5823,39 @@ static int sched_domain_set(int domid, const libxl_domain_sched_params *scinfo)
>      return 0;
>  }
>  
> +static int sched_vcpu_get(libxl_scheduler sched, int domid,
> +                            libxl_vcpu_sched_params *scinfo)

Indentation.

> +{
> +    int rc;
> +
> +    rc = libxl_vcpu_sched_params_get(ctx, domid, scinfo);
> +    if (rc) {
> +        fprintf(stderr, "libxl_vcpu_sched_params_get failed.\n");
> +        exit(-1);
> +    }
> +    if (scinfo->sched != sched) {
> +        fprintf(stderr, "libxl_vcpu_sched_params_get returned %s not %s.\n",
> +                libxl_scheduler_to_string(scinfo->sched),
> +                libxl_scheduler_to_string(sched));
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
[...]
> +
>  static int sched_rtds_pool_output(uint32_t poolid)
>  {
>      char *poolname;
> @@ -6015,6 +6080,65 @@ static int sched_domain_output(libxl_scheduler sched, int (*output)(int),
>      return 0;
>  }
>  
> +static int sched_vcpu_output(libxl_scheduler sched,
> +                               int (*output)(int, libxl_vcpu_sched_params *),
> +                               int (*pooloutput)(uint32_t), const char *cpupool)

Indentation.

> +{
> +    libxl_dominfo *info;
> +    libxl_cpupoolinfo *poolinfo = NULL;
> +    uint32_t poolid;
> +    int nb_domain, n_pools = 0, i, p;
> +    int rc = 0;
[...]
> @@ -6222,77 +6346,190 @@ 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 */
> +
> +    int *vcpus = (int *)xmalloc(sizeof(int)); /* IDs of VCPUs that change */
> +    int *periods = (int *)xmalloc(sizeof(int)); /* period is in microsecond */
> +    int *budgets = (int *)xmalloc(sizeof(int)); /* budget is in microsecond */
> +    int v_size = 1; /* size of vcpus array */
> +    int p_size = 1; /* size of periods array */
> +    int b_size = 1; /* size of budgets array */
> +    int v_index = 0; /* index in vcpus array */
> +    int p_index =0; /* index in periods array */
> +    int b_index =0; /* index for in budgets array */
>      bool opt_p = false;
>      bool opt_b = false;
> -    int opt, rc;
> +    bool opt_v = false;
> +    bool opt_all = false; /* output per-dom parameters */
> +    int opt, i;
> +    int rc = 0;
>      static struct option opts[] = {
>          {"domain", 1, 0, 'd'},
>          {"period", 1, 0, 'p'},
>          {"budget", 1, 0, 'b'},
> +        {"vcpuid",1, 0, 'v'},
>          {"cpupool", 1, 0, 'c'},
> -        COMMON_LONG_OPTS
> +        COMMON_LONG_OPTS,
> +        {0, 0, 0, 0}

This is not needed because now COMMON_LONG_OPTS includes {0,0,0,0}.

>      };
>  
> -    SWITCH_FOREACH_OPT(opt, "d:p:b:c:", opts, "sched-rtds", 0) {
> +    SWITCH_FOREACH_OPT(opt, "d:p:b:v:c", opts, "sched-rtds", 0) {
>      case 'd':
>          dom = optarg;
>          break;
>      case 'p':
> -        period = strtol(optarg, NULL, 10);
> +        if (p_index > b_index || p_index > v_index) {
> +            /* budget or vcpuID is missed */
> +            fprintf(stderr, "Must specify period, budget and vcpuID\n");
> +            rc = 1;
> +            goto out;
> +        }
> +        if (p_index >= p_size) {
> +            p_size *= 2;
> +            periods = xrealloc(periods, p_size);
> +            if (!periods) {

xreaalloc can't fail.

> +                fprintf(stderr, "Failed to realloc periods\n");
> +                rc = 1;
> +                goto out;
> +            }
> +        }
> +        periods[p_index++] = strtol(optarg, NULL, 10);

You mix option parsing and validation in same place. I think you need to
clearly separate them. It's very hard to review a function like this
one.

Wei.

Dario Faggioli Feb. 9, 2016, 6:25 p.m. UTC | #2

On Fri, 2016-02-05 at 14:51 +0000, Wei Liu wrote:
> On Thu, Feb 04, 2016 at 04:50:44PM -0600, Chong Li wrote:
> > 
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 2b6371d..b843fa5 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > 
> > -    SWITCH_FOREACH_OPT(opt, "d:p:b:c:", opts, "sched-rtds", 0) {
> > +    SWITCH_FOREACH_OPT(opt, "d:p:b:v:c", opts, "sched-rtds", 0) {
> >      case 'd':
> >          dom = optarg;
> >          break;
> >      case 'p':
> > -        period = strtol(optarg, NULL, 10);
> > +        if (p_index > b_index || p_index > v_index) {
> > +            /* budget or vcpuID is missed */
> > +            fprintf(stderr, "Must specify period, budget and
> > vcpuID\n");
> > +            rc = 1;
> > +            goto out;
> > +        }
> > +        if (p_index >= p_size) {
> > +            p_size *= 2;
> > +            periods = xrealloc(periods, p_size);
> > +            if (!periods) {
> 
> xreaalloc can't fail.
> 
> > +                fprintf(stderr, "Failed to realloc periods\n");
> > +                rc = 1;
> > +                goto out;
> > +            }
> > +        }
> > +        periods[p_index++] = strtol(optarg, NULL, 10);
> 
> You mix option parsing and validation in same place. I think you need
> to
> clearly separate them. It's very hard to review a function like this
> one.
> 
Exactly.

So, considering this, and the fact that the libxl API is changing (the
_set_all() function is being added), I don't think there is much point
in reviewing this version of this patch.

So, good work following up on this! I know, it takes more time than one
would think... but interfaces are very important, so it's normal it
takes a bit to get them right (and convince people that that's the
case! :-P).

Looking forward to v6.

Thanks and Regards,
Dario

Patch

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 4279c7c..f9ff917 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1051,6 +1051,10 @@  B<OPTIONS>
 Specify domain for which scheduler parameters are to be modified or retrieved.
 Mandatory for modifying scheduler parameters.
 
+=item B<-v VCPUID/all>, B<--vcpuid=VCPUID/all>
+
+Specify vcpu for which scheduler parameters are to be modified or retrieved.
+
 =item B<-p PERIOD>, B<--period=PERIOD>
 
 Period of time, in microseconds, over which to replenish the budget.
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2b6371d..b843fa5 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5823,6 +5823,39 @@  static int sched_domain_set(int domid, const libxl_domain_sched_params *scinfo)
     return 0;
 }
 
+static int sched_vcpu_get(libxl_scheduler sched, int domid,
+                            libxl_vcpu_sched_params *scinfo)
+{
+    int rc;
+
+    rc = libxl_vcpu_sched_params_get(ctx, domid, scinfo);
+    if (rc) {
+        fprintf(stderr, "libxl_vcpu_sched_params_get failed.\n");
+        exit(-1);
+    }
+    if (scinfo->sched != sched) {
+        fprintf(stderr, "libxl_vcpu_sched_params_get returned %s not %s.\n",
+                libxl_scheduler_to_string(scinfo->sched),
+                libxl_scheduler_to_string(sched));
+        return 1;
+    }
+
+    return 0;
+}
+
+static int sched_vcpu_set(int domid, const libxl_vcpu_sched_params *scinfo)
+{
+    int rc;
+
+    rc = libxl_vcpu_sched_params_set(ctx, domid, scinfo);
+    if (rc) {
+        fprintf(stderr, "libxl_vcpu_sched_params_set failed.\n");
+        exit(-1);
+    }
+
+    return rc;
+}
+
 static int sched_credit_params_set(int poolid, libxl_sched_credit_params *scinfo)
 {
     if (libxl_sched_credit_params_set(ctx, poolid, scinfo)) {
@@ -5942,6 +5975,38 @@  static int sched_rtds_domain_output(
     return 0;
 }
 
+static int sched_rtds_vcpu_output(
+    int domid, libxl_vcpu_sched_params *scinfo)
+{
+    char *domname;
+    int rc = 0;
+    int i;
+
+    if (domid < 0) {
+        printf("%-33s %4s %4s %9s %9s\n", "Name", "ID",
+                        "VCPU", "Period", "Budget");
+        return 0;
+    }
+
+    rc = sched_vcpu_get(LIBXL_SCHEDULER_RTDS, domid, scinfo);
+    if (rc)
+        goto out;
+
+    domname = libxl_domid_to_name(ctx, domid);
+    for( i = 0; i < scinfo->num_vcpus; i++ ) {
+        printf("%-33s %4d %4d %9"PRIu32" %9"PRIu32"\n",
+            domname,
+            domid,
+            scinfo->vcpus[i].vcpuid,
+            scinfo->vcpus[i].period,
+            scinfo->vcpus[i].budget);
+    }
+    free(domname);
+
+out:
+    return rc;
+}
+
 static int sched_rtds_pool_output(uint32_t poolid)
 {
     char *poolname;
@@ -6015,6 +6080,65 @@  static int sched_domain_output(libxl_scheduler sched, int (*output)(int),
     return 0;
 }
 
+static int sched_vcpu_output(libxl_scheduler sched,
+                               int (*output)(int, libxl_vcpu_sched_params *),
+                               int (*pooloutput)(uint32_t), const char *cpupool)
+{
+    libxl_dominfo *info;
+    libxl_cpupoolinfo *poolinfo = NULL;
+    uint32_t poolid;
+    int nb_domain, n_pools = 0, i, p;
+    int rc = 0;
+
+    if (cpupool) {
+        if (libxl_cpupool_qualifier_to_cpupoolid(ctx, cpupool, &poolid, NULL)
+            || !libxl_cpupoolid_is_valid(ctx, poolid)) {
+            fprintf(stderr, "unknown cpupool \'%s\'\n", cpupool);
+            return -ERROR_FAIL;
+        }
+    }
+
+    info = libxl_list_domain(ctx, &nb_domain);
+    if (!info) {
+        fprintf(stderr, "libxl_list_domain failed.\n");
+        return 1;
+    }
+    poolinfo = libxl_list_cpupool(ctx, &n_pools);
+    if (!poolinfo) {
+        fprintf(stderr, "error getting cpupool info\n");
+        libxl_dominfo_list_free(info, nb_domain);
+        return -ERROR_NOMEM;
+    }
+
+    for (p = 0; !rc && (p < n_pools); p++) {
+        if ((poolinfo[p].sched != sched) ||
+            (cpupool && (poolid != poolinfo[p].poolid)))
+            continue;
+
+        pooloutput(poolinfo[p].poolid);
+
+        libxl_vcpu_sched_params scinfo_out;
+        libxl_vcpu_sched_params_init(&scinfo_out);
+        output(-1, &scinfo_out);
+        libxl_vcpu_sched_params_dispose(&scinfo_out);
+        for (i = 0; i < nb_domain; i++) {
+            if (info[i].cpupool != poolinfo[p].poolid)
+                continue;
+            libxl_vcpu_sched_params scinfo;
+            libxl_vcpu_sched_params_init(&scinfo);
+            scinfo.num_vcpus = 0;
+            rc = output(info[i].domid, &scinfo);
+            libxl_vcpu_sched_params_dispose(&scinfo);
+            if (rc)
+                break;
+        }
+    }
+
+    libxl_cpupoolinfo_list_free(poolinfo, n_pools);
+    libxl_dominfo_list_free(info, nb_domain);
+    return 0;
+}
+
 /* 
  * <nothing>             : List all domain params and sched params from all pools
  * -d [domid]            : List domain params for domain
@@ -6222,77 +6346,190 @@  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 */
+
+    int *vcpus = (int *)xmalloc(sizeof(int)); /* IDs of VCPUs that change */
+    int *periods = (int *)xmalloc(sizeof(int)); /* period is in microsecond */
+    int *budgets = (int *)xmalloc(sizeof(int)); /* budget is in microsecond */
+    int v_size = 1; /* size of vcpus array */
+    int p_size = 1; /* size of periods array */
+    int b_size = 1; /* size of budgets array */
+    int v_index = 0; /* index in vcpus array */
+    int p_index =0; /* index in periods array */
+    int b_index =0; /* index for in budgets array */
     bool opt_p = false;
     bool opt_b = false;
-    int opt, rc;
+    bool opt_v = false;
+    bool opt_all = false; /* output per-dom parameters */
+    int opt, i;
+    int rc = 0;
     static struct option opts[] = {
         {"domain", 1, 0, 'd'},
         {"period", 1, 0, 'p'},
         {"budget", 1, 0, 'b'},
+        {"vcpuid",1, 0, 'v'},
         {"cpupool", 1, 0, 'c'},
-        COMMON_LONG_OPTS
+        COMMON_LONG_OPTS,
+        {0, 0, 0, 0}
     };
 
-    SWITCH_FOREACH_OPT(opt, "d:p:b:c:", opts, "sched-rtds", 0) {
+    SWITCH_FOREACH_OPT(opt, "d:p:b:v:c", opts, "sched-rtds", 0) {
     case 'd':
         dom = optarg;
         break;
     case 'p':
-        period = strtol(optarg, NULL, 10);
+        if (p_index > b_index || p_index > v_index) {
+            /* budget or vcpuID is missed */
+            fprintf(stderr, "Must specify period, budget and vcpuID\n");
+            rc = 1;
+            goto out;
+        }
+        if (p_index >= p_size) {
+            p_size *= 2;
+            periods = xrealloc(periods, p_size);
+            if (!periods) {
+                fprintf(stderr, "Failed to realloc periods\n");
+                rc = 1;
+                goto out;
+            }
+        }
+        periods[p_index++] = strtol(optarg, NULL, 10);
         opt_p = 1;
         break;
     case 'b':
-        budget = strtol(optarg, NULL, 10);
+        if (b_index > p_index || b_index > v_index) {
+            /* period or vcpuID is missed */
+            fprintf(stderr, "Must specify period, budget and vcpuID\n");
+            rc = 1;
+            goto out;
+        }
+        if (b_index >= b_size) {
+            b_size *= 2;
+            budgets = xrealloc(budgets, b_size);
+            if (!budgets) {
+                fprintf(stderr, "Failed to realloc budgets\n");
+                rc = 1;
+                goto out;
+            }
+        }
+        budgets[b_index++] = strtol(optarg, NULL, 10);
         opt_b = 1;
         break;
+    case 'v':
+        if (!strcmp(optarg, "all")) { /* output per-dom parameters*/
+            opt_all = 1;
+            break;
+        }
+        if (v_index >= v_size) {
+            v_size *= 2;
+            vcpus = xrealloc(vcpus, v_size);
+            if (!vcpus) {
+                fprintf(stderr, "Failed to realloc vcpus\n");
+                rc = 1;
+                goto out;
+            }
+        }
+        vcpus[v_index++] = strtol(optarg, NULL, 10);
+        opt_v = 1;
+        break;
     case 'c':
         cpupool = optarg;
         break;
     }
 
-    if (cpupool && (dom || opt_p || opt_b)) {
+    if (cpupool && (dom || opt_p || opt_b || opt_v || opt_all)) {
         fprintf(stderr, "Specifying a cpupool is not allowed with "
                 "other options.\n");
-        return EXIT_FAILURE;
+        rc = 1;
+        goto out;
     }
-    if (!dom && (opt_p || opt_b)) {
-        fprintf(stderr, "Must specify a domain.\n");
-        return EXIT_FAILURE;
+    if (!dom && (opt_p || opt_b || opt_v)) {
+        fprintf(stderr, "Missing parameters.\n");
+        rc = 1;
+        goto out;
     }
-    if (opt_p != opt_b) {
-        fprintf(stderr, "Must specify period and budget\n");
-        return EXIT_FAILURE;
+    if (dom && !opt_v && !opt_all && (opt_p || opt_b)) {
+        fprintf(stderr, "Must specify VCPU.\n");
+        rc = 1;
+        goto out;
+    }
+    if (opt_v && opt_all) {
+        fprintf(stderr, "Incorrect VCPU IDs.\n");
+        rc = 1;
+        goto out;
+    }
+    if (((v_index > b_index) && opt_b) || ((v_index > p_index) && opt_p)
+            || p_index != b_index) {
+        fprintf(stderr, "Incorrect number of period and budget\n");
+        rc = 1;
+        goto out;
     }
 
-    if (!dom) { /* list all domain's rt scheduler info */
-        if (sched_domain_output(LIBXL_SCHEDULER_RTDS,
-                                sched_rtds_domain_output,
-                                sched_rtds_pool_output,
-                                cpupool))
-            return EXIT_FAILURE;
+    if ((!dom) && opt_all) { /* list all domain's rtds scheduler info */
+        rc = -sched_vcpu_output(LIBXL_SCHEDULER_RTDS,
+                                    sched_rtds_vcpu_output,
+                                    sched_rtds_pool_output,
+                                    cpupool);
+        goto out;
+    } else if (!dom && !opt_all) {
+        /* list all domain's default scheduling parameters */
+        rc = -sched_domain_output(LIBXL_SCHEDULER_RTDS,
+                                    sched_rtds_domain_output,
+                                    sched_rtds_pool_output,
+                                    cpupool);
+        goto out;
     } else {
         uint32_t domid = find_domain(dom);
-        if (!opt_p && !opt_b) { /* output rt scheduler info */
+        if (!opt_v && !opt_all) { /* output default scheduling parameters */
             sched_rtds_domain_output(-1);
-            if (sched_rtds_domain_output(domid))
-                return EXIT_FAILURE;
-        } else { /* set rt scheduler paramaters */
-            libxl_domain_sched_params scinfo;
-            libxl_domain_sched_params_init(&scinfo);
+            rc = -sched_rtds_domain_output(domid);
+            goto out;
+        } else if (!opt_p && !opt_b) { /* output rtds scheduler info */
+            libxl_vcpu_sched_params scinfo;
+            libxl_vcpu_sched_params_init(&scinfo);
+            sched_rtds_vcpu_output(-1, &scinfo);
+            scinfo.num_vcpus = v_index;
+            if (v_index > 0)
+                scinfo.vcpus = (libxl_sched_params *)
+                        xmalloc(sizeof(libxl_sched_params) * (v_index));
+            for (i = 0; i < v_index; i++)
+                scinfo.vcpus[i].vcpuid = vcpus[i];
+            rc = -sched_rtds_vcpu_output(domid, &scinfo);
+            libxl_vcpu_sched_params_dispose(&scinfo);
+            goto out;
+    } else if (opt_v || opt_all) { /* set per-vcpu rtds scheduler parameters */
+            libxl_vcpu_sched_params scinfo;
+            libxl_vcpu_sched_params_init(&scinfo);
             scinfo.sched = LIBXL_SCHEDULER_RTDS;
-            scinfo.period = period;
-            scinfo.budget = budget;
+            scinfo.num_vcpus = v_index;
+            if (v_index > 0) {
+                scinfo.vcpus = (libxl_sched_params *)
+                        xmalloc(sizeof(libxl_sched_params) * (v_index));
+                for (i = 0; i < v_index; i++) {
+                    scinfo.vcpus[i].vcpuid = vcpus[i];
+                    scinfo.vcpus[i].period = periods[i];
+                    scinfo.vcpus[i].budget = budgets[i];
+                }
+            } else { /* set params for all vcpus*/
+                scinfo.vcpus = (libxl_sched_params *)
+                        xmalloc(sizeof(libxl_sched_params));
+                scinfo.vcpus[0].period = periods[0];
+                scinfo.vcpus[0].budget = budgets[0];
+            }
 
-            rc = sched_domain_set(domid, &scinfo);
-            libxl_domain_sched_params_dispose(&scinfo);
-            if (rc)
-                return EXIT_FAILURE;
+            rc = sched_vcpu_set(domid, &scinfo);
+            libxl_vcpu_sched_params_dispose(&scinfo);
+            if (rc) {
+                rc = -rc;
+                goto out;
+            }
         }
     }
 
-    return EXIT_SUCCESS;
+out:
+    free(vcpus);
+    free(periods);
+    free(budgets);
+    return rc;
 }
 
 int main_domid(int argc, char **argv)
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index fdc1ac6..c68b656 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -268,10 +268,12 @@  struct cmd_spec cmd_table[] = {
     { "sched-rtds",
       &main_sched_rtds, 0, 1,
       "Get/set rtds scheduler parameters",
-      "[-d <Domain> [-p[=PERIOD]] [-b[=BUDGET]]]",
-      "-d DOMAIN, --domain=DOMAIN     Domain to modify\n"
-      "-p PERIOD, --period=PERIOD     Period (us)\n"
-      "-b BUDGET, --budget=BUDGET     Budget (us)\n"
+      "[-d <Domain> [-v[=VCPUID]] [-p[=PERIOD]] [-b[=BUDGET]]]",
+      "-d DOMAIN, --domain=DOMAIN 	Domain to modify\n"
+      "-v VCPUID/all, --vcpuid=VCPUID/all    VCPU to modify or output;\n"
+      "                               Using '-v all' to modify/output all vcpus\n"
+      "-p PERIOD, --period=PERIOD 	Period (us)\n"
+      "-b BUDGET, --budget=BUDGET 	Budget (us)\n"
     },
     { "domid",
       &main_domid, 0, 0,