From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH] xl cpupool-list: add option to list domains Date: Tue, 11 Mar 2014 08:33:33 +0100 Message-ID: <531EBC4D.7020306@ts.fujitsu.com> References: <1392709117-30019-1-git-send-email-juergen.gross@ts.fujitsu.com> <21277.43260.538618.696040@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <21277.43260.538618.696040@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org 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 prints a list of CPUs used by I. >> +If I<-d> is specified, B prints a list of domains in I 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