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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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.

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