Re: [Xen-devel] [PATCH] xl cpupool-list: add option to list domains

[Juergen Gross <juergen.gross@xxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxx
Mies-van-der-Rohe-Str. 8                Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

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