xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xl cpupool-list: add option to list domains
@ 2014-02-18  7:38 Juergen Gross
  2014-02-19 15:07 ` Nate Studer
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Juergen Gross @ 2014-02-18  7:38 UTC (permalink / raw)
  To: xen-devel, Ian.Jackson; +Cc: Juergen Gross

It is rather complicated to obtain the cpupool a domain lives in. Add an
option -d (or --domains) to list all domains running in a cpupool.

Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
---
 docs/man/xl.pod.1         |    5 ++++-
 tools/libxl/xl_cmdimpl.c  |   47 ++++++++++++++++++++++++++++++++++++++-------
 tools/libxl/xl_cmdtable.c |    5 +++--
 3 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index e7b9de2..547af6d 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1019,10 +1019,13 @@ Use the given configuration file.
 
 =back
 
-=item B<cpupool-list> [I<-c|--cpus>] [I<cpu-pool>]
+=item B<cpupool-list> [I<-c|--cpus>] [I<-d|--domains>] [I<cpu-pool>]
 
 List CPU pools on the host.
 If I<-c> is specified, B<xl> prints a list of CPUs used by I<cpu-pool>.
+If I<-d> is specified, B<xl> prints a list of domains in I<cpu-pool> instead
+of the domain count.
+I<-c> and I<-d> are mutually exclusive.
 
 =item B<cpupool-destroy> I<cpu-pool>
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index aff6f90..c7b9fce 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -6754,23 +6754,32 @@ int main_cpupoollist(int argc, char **argv)
     int opt;
     static struct option opts[] = {
         {"cpus", 0, 0, 'c'},
+        {"domains", 0, 0, 'd'},
         COMMON_LONG_OPTS,
         {0, 0, 0, 0}
     };
-    int opt_cpus = 0;
+    int opt_cpus = 0, opt_domains = 0;
     const char *pool = NULL;
     libxl_cpupoolinfo *poolinfo;
-    int n_pools, p, c, n;
+    libxl_dominfo *dominfo = NULL;
+    int n_pools, n_domains, p, c, n;
     uint32_t poolid;
     char *name;
     int ret = 0;
 
-    SWITCH_FOREACH_OPT(opt, "hc", opts, "cpupool-list", 0) {
+    SWITCH_FOREACH_OPT(opt, "hcd", opts, "cpupool-list", 0) {
     case 'c':
         opt_cpus = 1;
         break;
+    case 'd':
+        opt_domains = 1;
+        break;
     }
 
+    if (opt_cpus && opt_domains) {
+        fprintf(stderr, "specifying both cpu- and domain-list not allowed\n");
+        return -ERROR_FAIL;
+    }
     if (optind < argc) {
         pool = argv[optind];
         if (libxl_name_to_cpupoolid(ctx, pool, &poolid)) {
@@ -6784,12 +6793,21 @@ int main_cpupoollist(int argc, char **argv)
         fprintf(stderr, "error getting cpupool info\n");
         return -ERROR_NOMEM;
     }
+    if (opt_domains) {
+        dominfo = libxl_list_domain(ctx, &n_domains);
+        if (!dominfo) {
+            fprintf(stderr, "error getting domain info\n");
+            ret = -ERROR_NOMEM;
+            goto out;
+        }
+    }
 
     printf("%-19s", "Name");
     if (opt_cpus)
         printf("CPU list\n");
     else
-        printf("CPUs   Sched     Active   Domain count\n");
+        printf("CPUs   Sched     Active   Domain %s\n",
+               opt_domains ? "list" : "count");
 
     for (p = 0; p < n_pools; p++) {
         if (!ret && (!pool || (poolinfo[p].poolid == poolid))) {
@@ -6808,15 +6826,30 @@ int main_cpupoollist(int argc, char **argv)
                         n++;
                     }
                 if (!opt_cpus) {
-                    printf("%3d %9s       y       %4d", n,
-                           libxl_scheduler_to_string(poolinfo[p].sched),
-                           poolinfo[p].n_dom);
+                    printf("%3d %9s       y     ", n,
+                           libxl_scheduler_to_string(poolinfo[p].sched));
+                    if (opt_domains) {
+                        c = 0;
+                        for (n = 0; n < n_domains; n++) {
+                            if (poolinfo[p].poolid == dominfo[n].cpupool) {
+                                name = libxl_domid_to_name(ctx, dominfo[n].domid);
+                                printf("%s%s", c ? ", " : "", name);
+                                free(name);
+                                c++;
+                            }
+                        }
+                    }
+                    else
+                        printf("  %4d", poolinfo[p].n_dom);
                 }
                 printf("\n");
             }
         }
     }
 
+    if (dominfo)
+        libxl_dominfo_list_free(dominfo, n_domains);
+out:
     libxl_cpupoolinfo_list_free(poolinfo, n_pools);
 
     return ret;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index ebe0220..8a52d26 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -426,8 +426,9 @@ struct cmd_spec cmd_table[] = {
     { "cpupool-list",
       &main_cpupoollist, 0, 0,
       "List CPU pools on host",
-      "[-c|--cpus] [<CPU Pool>]",
-      "-c, --cpus                     Output list of CPUs used by a pool"
+      "[-c|--cpus] [-d|--domains] [<CPU Pool>]",
+      "-c, --cpus                     Output list of CPUs used by a pool\n"
+      "-d, --domains                  Output list of domains running in a pool"
     },
     { "cpupool-destroy",
       &main_cpupooldestroy, 0, 1,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] xl cpupool-list: add option to list domains
  2014-02-18  7:38 [PATCH] xl cpupool-list: add option to list domains Juergen Gross
@ 2014-02-19 15:07 ` Nate Studer
  2014-03-04 12:19 ` Juergen Gross
  2014-03-10 11:58 ` Ian Jackson
  2 siblings, 0 replies; 7+ messages in thread
From: Nate Studer @ 2014-02-19 15:07 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, Ian.Jackson

On 2/18/2014 2:38 AM, Juergen Gross wrote:
> It is rather complicated to obtain the cpupool a domain lives in. Add an
> option -d (or --domains) to list all domains running in a cpupool.
> 
> Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>

FWIW, this patch works as advertised.

arlx@arlx-dw-47:~$ sudo xl cpupool-list -d
Name               CPUs   Sched     Active   Domain list
Pool-0               1    credit       y     Domain-0
test                 1  arinc653       y     dom1, dom2
arlx@arlx-dw-47:~$ sudo xl cpupool-list -c
Name               CPU list
Pool-0             0
test               1
arlx@arlx-dw-47:~$ sudo xl cpupool-list -c -d
specifying both cpu- and domain-list not allowed
arlx@arlx-dw-47:~$ sudo xl cpupool-list -c test
Name               CPU list
test               1
arlx@arlx-dw-47:~$ sudo xl cpupool-list -d test
Name               CPUs   Sched     Active   Domain list
test                 1  arinc653       y     dom1, dom2

-- 
Nathan Studer

> ---
>  docs/man/xl.pod.1         |    5 ++++-
>  tools/libxl/xl_cmdimpl.c  |   47 ++++++++++++++++++++++++++++++++++++++-------
>  tools/libxl/xl_cmdtable.c |    5 +++--
>  3 files changed, 47 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index e7b9de2..547af6d 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1019,10 +1019,13 @@ Use the given configuration file.
>  
>  =back
>  
> -=item B<cpupool-list> [I<-c|--cpus>] [I<cpu-pool>]
> +=item B<cpupool-list> [I<-c|--cpus>] [I<-d|--domains>] [I<cpu-pool>]
>  
>  List CPU pools on the host.
>  If I<-c> is specified, B<xl> prints a list of CPUs used by I<cpu-pool>.
> +If I<-d> is specified, B<xl> prints a list of domains in I<cpu-pool> instead
> +of the domain count.
> +I<-c> and I<-d> are mutually exclusive.
>  
>  =item B<cpupool-destroy> I<cpu-pool>
>  
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index aff6f90..c7b9fce 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -6754,23 +6754,32 @@ int main_cpupoollist(int argc, char **argv)
>      int opt;
>      static struct option opts[] = {
>          {"cpus", 0, 0, 'c'},
> +        {"domains", 0, 0, 'd'},
>          COMMON_LONG_OPTS,
>          {0, 0, 0, 0}
>      };
> -    int opt_cpus = 0;
> +    int opt_cpus = 0, opt_domains = 0;
>      const char *pool = NULL;
>      libxl_cpupoolinfo *poolinfo;
> -    int n_pools, p, c, n;
> +    libxl_dominfo *dominfo = NULL;
> +    int n_pools, n_domains, p, c, n;
>      uint32_t poolid;
>      char *name;
>      int ret = 0;
>  
> -    SWITCH_FOREACH_OPT(opt, "hc", opts, "cpupool-list", 0) {
> +    SWITCH_FOREACH_OPT(opt, "hcd", opts, "cpupool-list", 0) {
>      case 'c':
>          opt_cpus = 1;
>          break;
> +    case 'd':
> +        opt_domains = 1;
> +        break;
>      }
>  
> +    if (opt_cpus && opt_domains) {
> +        fprintf(stderr, "specifying both cpu- and domain-list not allowed\n");
> +        return -ERROR_FAIL;
> +    }
>      if (optind < argc) {
>          pool = argv[optind];
>          if (libxl_name_to_cpupoolid(ctx, pool, &poolid)) {
> @@ -6784,12 +6793,21 @@ int main_cpupoollist(int argc, char **argv)
>          fprintf(stderr, "error getting cpupool info\n");
>          return -ERROR_NOMEM;
>      }
> +    if (opt_domains) {
> +        dominfo = libxl_list_domain(ctx, &n_domains);
> +        if (!dominfo) {
> +            fprintf(stderr, "error getting domain info\n");
> +            ret = -ERROR_NOMEM;
> +            goto out;
> +        }
> +    }
>  
>      printf("%-19s", "Name");
>      if (opt_cpus)
>          printf("CPU list\n");
>      else
> -        printf("CPUs   Sched     Active   Domain count\n");
> +        printf("CPUs   Sched     Active   Domain %s\n",
> +               opt_domains ? "list" : "count");
>  
>      for (p = 0; p < n_pools; p++) {
>          if (!ret && (!pool || (poolinfo[p].poolid == poolid))) {
> @@ -6808,15 +6826,30 @@ int main_cpupoollist(int argc, char **argv)
>                          n++;
>                      }
>                  if (!opt_cpus) {
> -                    printf("%3d %9s       y       %4d", n,
> -                           libxl_scheduler_to_string(poolinfo[p].sched),
> -                           poolinfo[p].n_dom);
> +                    printf("%3d %9s       y     ", n,
> +                           libxl_scheduler_to_string(poolinfo[p].sched));
> +                    if (opt_domains) {
> +                        c = 0;
> +                        for (n = 0; n < n_domains; n++) {
> +                            if (poolinfo[p].poolid == dominfo[n].cpupool) {
> +                                name = libxl_domid_to_name(ctx, dominfo[n].domid);
> +                                printf("%s%s", c ? ", " : "", name);
> +                                free(name);
> +                                c++;
> +                            }
> +                        }
> +                    }
> +                    else
> +                        printf("  %4d", poolinfo[p].n_dom);
>                  }
>                  printf("\n");
>              }
>          }
>      }
>  
> +    if (dominfo)
> +        libxl_dominfo_list_free(dominfo, n_domains);
> +out:
>      libxl_cpupoolinfo_list_free(poolinfo, n_pools);
>  
>      return ret;
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index ebe0220..8a52d26 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -426,8 +426,9 @@ struct cmd_spec cmd_table[] = {
>      { "cpupool-list",
>        &main_cpupoollist, 0, 0,
>        "List CPU pools on host",
> -      "[-c|--cpus] [<CPU Pool>]",
> -      "-c, --cpus                     Output list of CPUs used by a pool"
> +      "[-c|--cpus] [-d|--domains] [<CPU Pool>]",
> +      "-c, --cpus                     Output list of CPUs used by a pool\n"
> +      "-d, --domains                  Output list of domains running in a pool"
>      },
>      { "cpupool-destroy",
>        &main_cpupooldestroy, 0, 1,
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xl cpupool-list: add option to list domains
  2014-02-18  7:38 [PATCH] xl cpupool-list: add option to list domains Juergen Gross
  2014-02-19 15:07 ` Nate Studer
@ 2014-03-04 12:19 ` Juergen Gross
  2014-03-10 11:58 ` Ian Jackson
  2 siblings, 0 replies; 7+ messages in thread
From: Juergen Gross @ 2014-03-04 12:19 UTC (permalink / raw)
  To: Juergen Gross; +Cc: Ian.Jackson, xen-devel

Ping?

On 18.02.2014 08:38, Juergen Gross wrote:
> It is rather complicated to obtain the cpupool a domain lives in. Add an
> option -d (or --domains) to list all domains running in a cpupool.
>
> Signed-off-by: Juergen Gross <juergen.gross@ts.fujitsu.com>
> ---
>   docs/man/xl.pod.1         |    5 ++++-
>   tools/libxl/xl_cmdimpl.c  |   47 ++++++++++++++++++++++++++++++++++++++-------
>   tools/libxl/xl_cmdtable.c |    5 +++--
>   3 files changed, 47 insertions(+), 10 deletions(-)
>
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index e7b9de2..547af6d 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1019,10 +1019,13 @@ Use the given configuration file.
>
>   =back
>
> -=item B<cpupool-list> [I<-c|--cpus>] [I<cpu-pool>]
> +=item B<cpupool-list> [I<-c|--cpus>] [I<-d|--domains>] [I<cpu-pool>]
>
>   List CPU pools on the host.
>   If I<-c> is specified, B<xl> prints a list of CPUs used by I<cpu-pool>.
> +If I<-d> is specified, B<xl> prints a list of domains in I<cpu-pool> instead
> +of the domain count.
> +I<-c> and I<-d> are mutually exclusive.
>
>   =item B<cpupool-destroy> I<cpu-pool>
>
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index aff6f90..c7b9fce 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -6754,23 +6754,32 @@ int main_cpupoollist(int argc, char **argv)
>       int opt;
>       static struct option opts[] = {
>           {"cpus", 0, 0, 'c'},
> +        {"domains", 0, 0, 'd'},
>           COMMON_LONG_OPTS,
>           {0, 0, 0, 0}
>       };
> -    int opt_cpus = 0;
> +    int opt_cpus = 0, opt_domains = 0;
>       const char *pool = NULL;
>       libxl_cpupoolinfo *poolinfo;
> -    int n_pools, p, c, n;
> +    libxl_dominfo *dominfo = NULL;
> +    int n_pools, n_domains, p, c, n;
>       uint32_t poolid;
>       char *name;
>       int ret = 0;
>
> -    SWITCH_FOREACH_OPT(opt, "hc", opts, "cpupool-list", 0) {
> +    SWITCH_FOREACH_OPT(opt, "hcd", opts, "cpupool-list", 0) {
>       case 'c':
>           opt_cpus = 1;
>           break;
> +    case 'd':
> +        opt_domains = 1;
> +        break;
>       }
>
> +    if (opt_cpus && opt_domains) {
> +        fprintf(stderr, "specifying both cpu- and domain-list not allowed\n");
> +        return -ERROR_FAIL;
> +    }
>       if (optind < argc) {
>           pool = argv[optind];
>           if (libxl_name_to_cpupoolid(ctx, pool, &poolid)) {
> @@ -6784,12 +6793,21 @@ int main_cpupoollist(int argc, char **argv)
>           fprintf(stderr, "error getting cpupool info\n");
>           return -ERROR_NOMEM;
>       }
> +    if (opt_domains) {
> +        dominfo = libxl_list_domain(ctx, &n_domains);
> +        if (!dominfo) {
> +            fprintf(stderr, "error getting domain info\n");
> +            ret = -ERROR_NOMEM;
> +            goto out;
> +        }
> +    }
>
>       printf("%-19s", "Name");
>       if (opt_cpus)
>           printf("CPU list\n");
>       else
> -        printf("CPUs   Sched     Active   Domain count\n");
> +        printf("CPUs   Sched     Active   Domain %s\n",
> +               opt_domains ? "list" : "count");
>
>       for (p = 0; p < n_pools; p++) {
>           if (!ret && (!pool || (poolinfo[p].poolid == poolid))) {
> @@ -6808,15 +6826,30 @@ int main_cpupoollist(int argc, char **argv)
>                           n++;
>                       }
>                   if (!opt_cpus) {
> -                    printf("%3d %9s       y       %4d", n,
> -                           libxl_scheduler_to_string(poolinfo[p].sched),
> -                           poolinfo[p].n_dom);
> +                    printf("%3d %9s       y     ", n,
> +                           libxl_scheduler_to_string(poolinfo[p].sched));
> +                    if (opt_domains) {
> +                        c = 0;
> +                        for (n = 0; n < n_domains; n++) {
> +                            if (poolinfo[p].poolid == dominfo[n].cpupool) {
> +                                name = libxl_domid_to_name(ctx, dominfo[n].domid);
> +                                printf("%s%s", c ? ", " : "", name);
> +                                free(name);
> +                                c++;
> +                            }
> +                        }
> +                    }
> +                    else
> +                        printf("  %4d", poolinfo[p].n_dom);
>                   }
>                   printf("\n");
>               }
>           }
>       }
>
> +    if (dominfo)
> +        libxl_dominfo_list_free(dominfo, n_domains);
> +out:
>       libxl_cpupoolinfo_list_free(poolinfo, n_pools);
>
>       return ret;
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index ebe0220..8a52d26 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -426,8 +426,9 @@ struct cmd_spec cmd_table[] = {
>       { "cpupool-list",
>         &main_cpupoollist, 0, 0,
>         "List CPU pools on host",
> -      "[-c|--cpus] [<CPU Pool>]",
> -      "-c, --cpus                     Output list of CPUs used by a pool"
> +      "[-c|--cpus] [-d|--domains] [<CPU Pool>]",
> +      "-c, --cpus                     Output list of CPUs used by a pool\n"
> +      "-d, --domains                  Output list of domains running in a pool"
>       },
>       { "cpupool-destroy",
>         &main_cpupooldestroy, 0, 1,
>


-- 
Juergen Gross                 Principal Developer Operating Systems
PBG PDG ES&S SWE OS6                   Telephone: +49 (0) 89 62060 2932
Fujitsu                                   e-mail: juergen.gross@ts.fujitsu.com
Mies-van-der-Rohe-Str. 8                Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xl cpupool-list: add option to list domains
  2014-02-18  7:38 [PATCH] xl cpupool-list: add option to list domains Juergen Gross
  2014-02-19 15:07 ` Nate Studer
  2014-03-04 12:19 ` Juergen Gross
@ 2014-03-10 11:58 ` Ian Jackson
  2014-03-11  7:33   ` Juergen Gross
  2 siblings, 1 reply; 7+ messages in thread
From: Ian Jackson @ 2014-03-10 11:58 UTC (permalink / raw)
  To: Juergen Gross; +Cc: xen-devel

Juergen Gross writes ("[Xen-devel] [PATCH] xl cpupool-list: add option to list domains"):
> It is rather complicated to obtain the cpupool a domain lives in. Add an
> option -d (or --domains) to list all domains running in a cpupool.

Thanks for this.  I have some comments:

>  List CPU pools on the host.
>  If I<-c> is specified, B<xl> prints a list of CPUs used by I<cpu-pool>.
> +If I<-d> is specified, B<xl> prints a list of domains in I<cpu-pool> instead
> +of the domain count.
> +I<-c> and I<-d> are mutually exclusive.

Couldn't we come up with an (unambiguous) output syntax that made them
nonexclusive ?

> @@ -6808,15 +6826,30 @@ int main_cpupoollist(int argc, char **argv)
>                          n++;
>                      }
>                  if (!opt_cpus) {
> -                    printf("%3d %9s       y       %4d", n,
> -                           libxl_scheduler_to_string(poolinfo[p].sched),
> -                           poolinfo[p].n_dom);
> +                    printf("%3d %9s       y     ", n,
> +                           libxl_scheduler_to_string(poolinfo[p].sched));
> +                    if (opt_domains) {
> +                        c = 0;
> +                        for (n = 0; n < n_domains; n++) {
> +                            if (poolinfo[p].poolid == dominfo[n].cpupool) {
> +                                name = libxl_domid_to_name(ctx, dominfo[n].domid);

This long source line needs wrapping.

> +                                printf("%s%s", c ? ", " : "", name);

I'm not a huge fan of this comma-separated list.  If the list were
space separated it could be cut-and-pasted into
  for f in dom1 dom2; do xl somethingorother; done
etc.

Also, I think domain names aren't guaranteed not to contain commas.
So I think you need to quote and/or escape them somehow.  I suggest
using "-quotes iff the domain name contains whitespace.

(People who put " in their domain names can send patches to \-escape
them or something.)

> +                                free(name);
> +                                c++;

And if you are going to do this delimiter checking, calling the
variable for "what delimiter to use" "c" is a bit odd.  Presumably you
mean count, but it's actually used as a boolean.

> +                            }
> +                        }
> +                    }
> +                    else

} and else should be on the same line.  (Also I'm not a huge fan of
"} else" and "else {" but we have other examples in the tree...)

Thanks,
Ian.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xl cpupool-list: add option to list domains
  2014-03-10 11:58 ` Ian Jackson
@ 2014-03-11  7:33   ` Juergen Gross
  2014-03-13 16:39     ` Ian Jackson
  0 siblings, 1 reply; 7+ messages in thread
From: Juergen Gross @ 2014-03-11  7:33 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On 10.03.2014 12:58, Ian Jackson wrote:
> Juergen Gross writes ("[Xen-devel] [PATCH] xl cpupool-list: add option to list domains"):
>> It is rather complicated to obtain the cpupool a domain lives in. Add an
>> option -d (or --domains) to list all domains running in a cpupool.
>
> Thanks for this.  I have some comments:
>
>>   List CPU pools on the host.
>>   If I<-c> is specified, B<xl> prints a list of CPUs used by I<cpu-pool>.
>> +If I<-d> is specified, B<xl> prints a list of domains in I<cpu-pool> instead
>> +of the domain count.
>> +I<-c> and I<-d> are mutually exclusive.
>
> Couldn't we come up with an (unambiguous) output syntax that made them
> nonexclusive ?

I have no idea how to print this information in a table without producing
unaligned columns.

It would be possible, however, to add something like "xl cpupool-list --long"
to avoid this problem completely. We wouldn't need the --domains option then.
At the same time your comment regarding the comma-separated list could be
addressed.

What do you think?


Juergen

>
>> @@ -6808,15 +6826,30 @@ int main_cpupoollist(int argc, char **argv)
>>                           n++;
>>                       }
>>                   if (!opt_cpus) {
>> -                    printf("%3d %9s       y       %4d", n,
>> -                           libxl_scheduler_to_string(poolinfo[p].sched),
>> -                           poolinfo[p].n_dom);
>> +                    printf("%3d %9s       y     ", n,
>> +                           libxl_scheduler_to_string(poolinfo[p].sched));
>> +                    if (opt_domains) {
>> +                        c = 0;
>> +                        for (n = 0; n < n_domains; n++) {
>> +                            if (poolinfo[p].poolid == dominfo[n].cpupool) {
>> +                                name = libxl_domid_to_name(ctx, dominfo[n].domid);
>
> This long source line needs wrapping.
>
>> +                                printf("%s%s", c ? ", " : "", name);
>
> I'm not a huge fan of this comma-separated list.  If the list were
> space separated it could be cut-and-pasted into
>    for f in dom1 dom2; do xl somethingorother; done
> etc.
>
> Also, I think domain names aren't guaranteed not to contain commas.
> So I think you need to quote and/or escape them somehow.  I suggest
> using "-quotes iff the domain name contains whitespace.
>
> (People who put " in their domain names can send patches to \-escape
> them or something.)
>
>> +                                free(name);
>> +                                c++;
>
> And if you are going to do this delimiter checking, calling the
> variable for "what delimiter to use" "c" is a bit odd.  Presumably you
> mean count, but it's actually used as a boolean.
>
>> +                            }
>> +                        }
>> +                    }
>> +                    else
>
> } and else should be on the same line.  (Also I'm not a huge fan of
> "} else" and "else {" but we have other examples in the tree...)
>
> Thanks,
> Ian.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
>


-- 
Juergen Gross                 Principal Developer Operating Systems
PBG PDG ES&S SWE OS6                   Telephone: +49 (0) 89 62060 2932
Fujitsu                                   e-mail: juergen.gross@ts.fujitsu.com
Mies-van-der-Rohe-Str. 8                Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xl cpupool-list: add option to list domains
  2014-03-11  7:33   ` Juergen Gross
@ 2014-03-13 16:39     ` Ian Jackson
  2014-03-17  7:52       ` Juergen Gross
  0 siblings, 1 reply; 7+ messages in thread
From: Ian Jackson @ 2014-03-13 16:39 UTC (permalink / raw)
  To: Juergen Gross; +Cc: Ian Jackson, xen-devel

Juergen Gross writes ("Re: [Xen-devel] [PATCH] xl cpupool-list: add option to list domains"):
> On 10.03.2014 12:58, Ian Jackson wrote:
> > Couldn't we come up with an (unambiguous) output syntax that made them
> > nonexclusive ?
> 
> I have no idea how to print this information in a table without producing
> unaligned columns.

Ah, I see, yes.

> It would be possible, however, to add something like "xl cpupool-list --long"
> to avoid this problem completely. We wouldn't need the --domains option then.
> At the same time your comment regarding the comma-separated list could be
> addressed.
> 
> What do you think?

Now I'm not sure any more.  I think you should do what you think
best.

Ian.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xl cpupool-list: add option to list domains
  2014-03-13 16:39     ` Ian Jackson
@ 2014-03-17  7:52       ` Juergen Gross
  0 siblings, 0 replies; 7+ messages in thread
From: Juergen Gross @ 2014-03-17  7:52 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

On 13.03.2014 17:39, Ian Jackson wrote:
> Juergen Gross writes ("Re: [Xen-devel] [PATCH] xl cpupool-list: add option to list domains"):
>> On 10.03.2014 12:58, Ian Jackson wrote:
>>> Couldn't we come up with an (unambiguous) output syntax that made them
>>> nonexclusive ?
>>
>> I have no idea how to print this information in a table without producing
>> unaligned columns.
>
> Ah, I see, yes.
>
>> It would be possible, however, to add something like "xl cpupool-list --long"
>> to avoid this problem completely. We wouldn't need the --domains option then.
>> At the same time your comment regarding the comma-separated list could be
>> addressed.
>>
>> What do you think?
>
> Now I'm not sure any more.  I think you should do what you think
> best.

I think the "--long" option is the best solution. There have already been
requests to show more information (resource sharing, NUMA).

I'll send a patch, but this may last some time as I'm very busy in the moment
doing know-how transfer (I'm about to leave Fujitsu).


Juergen

-- 
Juergen Gross                 Principal Developer Operating Systems
PBG PDG ES&S SWE OS6                   Telephone: +49 (0) 89 62060 2932
Fujitsu                                   e-mail: juergen.gross@ts.fujitsu.com
Mies-van-der-Rohe-Str. 8                Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-03-17  7:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-18  7:38 [PATCH] xl cpupool-list: add option to list domains Juergen Gross
2014-02-19 15:07 ` Nate Studer
2014-03-04 12:19 ` Juergen Gross
2014-03-10 11:58 ` Ian Jackson
2014-03-11  7:33   ` Juergen Gross
2014-03-13 16:39     ` Ian Jackson
2014-03-17  7:52       ` Juergen Gross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).