* 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