Yu Zhiguo
2010-Apr-26 09:16 UTC
[Xen-devel] [PATCH] xl: Don''t output a broken result if error occurs in ''xl info''
If error occurs in ''xl info'', we''d better output an
error message
rather than a broken result.
Signed-off-by: Yu Zhiguo <yuzg@cn.fujitsu.com>
diff -r c87ec146229a -r 3f97bcb08bbd tools/libxl/xl.c
--- a/tools/libxl/xl.c Fri Apr 23 15:04:26 2010 +0100
+++ b/tools/libxl/xl.c Tue Apr 27 01:11:06 2010 +0800
@@ -2642,93 +2642,88 @@
exit(0);
}
-static void output_xeninfo(void)
+static void output_info(struct utsname *utsinfo, struct libxl_physinfo
*physinfo,
+ const libxl_version_info *verinfo)
{
- const libxl_version_info *info;
+ unsigned int i;
int sched_id;
- info = libxl_get_version_info(&ctx);
if ((sched_id = libxl_get_sched_id(&ctx)) < 0) {
fprintf(stderr, "get_sched_id sysctl failed.\n");
return;
}
- printf("xen_major : %d\n",
info->xen_version_major);
- printf("xen_minor : %d\n",
info->xen_version_minor);
- printf("xen_extra : %s\n",
info->xen_version_extra);
- printf("xen_caps : %s\n", info->capabilities);
+ // output utsinfo
+ printf("host : %s\n", utsinfo->nodename);
+ printf("release : %s\n", utsinfo->release);
+ printf("version : %s\n", utsinfo->version);
+ printf("machine : %s\n", utsinfo->machine);
+
+ // output physinfo
+ printf("nr_cpus : %d\n", physinfo->nr_cpus);
+ printf("nr_nodes : %d\n", physinfo->nr_nodes);
+ printf("cores_per_socket : %d\n",
physinfo->cores_per_socket);
+ printf("threads_per_core : %d\n",
physinfo->threads_per_core);
+ printf("cpu_mhz : %d\n", physinfo->cpu_khz /
1000);
+ printf("hw_caps : ");
+ for (i = 0; i < 8; i++)
+ printf("%08x%c", physinfo->hw_cap[i], i < 7 ?
'':'' : ''\n'');
+ printf("virt_caps :");
+ if (physinfo->phys_cap & XEN_SYSCTL_PHYSCAP_hvm)
+ printf(" hvm");
+ if (physinfo->phys_cap & XEN_SYSCTL_PHYSCAP_hvm_directio)
+ printf(" hvm_directio");
+ printf("\n");
+ if (!verinfo)
+ return;
+ i = (1 << 20) / verinfo->pagesize;
+ printf("total_memory : %llu\n", (long long unsigned
int) physinfo->total_pages / i);
+ printf("free_memory : %llu\n", (long long unsigned
int) physinfo->free_pages / i);
+
+ // output verinfo
+ printf("xen_major : %d\n",
verinfo->xen_version_major);
+ printf("xen_minor : %d\n",
verinfo->xen_version_minor);
+ printf("xen_extra : %s\n",
verinfo->xen_version_extra);
+ printf("xen_caps : %s\n",
verinfo->capabilities);
printf("xen_scheduler : %s\n",
sched_id == XEN_SCHEDULER_SEDF ? "sedf" :
sched_id == XEN_SCHEDULER_CREDIT ? "credit" :
sched_id == XEN_SCHEDULER_CREDIT2 ? "credit2" :
"unknown");
- printf("xen_pagesize : %lu\n", info->pagesize);
- printf("platform_params : virt_start=0x%lx\n",
info->virt_start);
- printf("xen_changeset : %s\n", info->changeset);
- printf("xen_commandline : %s\n", info->commandline);
- printf("cc_compiler : %s\n", info->compiler);
- printf("cc_compile_by : %s\n", info->compile_by);
- printf("cc_compile_domain : %s\n", info->compile_domain);
- printf("cc_compile_date : %s\n", info->compile_date);
+ printf("xen_pagesize : %lu\n", verinfo->pagesize);
+ printf("platform_params : virt_start=0x%lx\n",
verinfo->virt_start);
+ printf("xen_changeset : %s\n", verinfo->changeset);
+ printf("xen_commandline : %s\n", verinfo->commandline);
+ printf("cc_compiler : %s\n", verinfo->compiler);
+ printf("cc_compile_by : %s\n", verinfo->compile_by);
+ printf("cc_compile_domain : %s\n",
verinfo->compile_domain);
+ printf("cc_compile_date : %s\n",
verinfo->compile_date);
- return;
-}
-
-static void output_nodeinfo(void)
-{
- struct utsname utsbuf;
-
- uname(&utsbuf);
-
- printf("host : %s\n", utsbuf.nodename);
- printf("release : %s\n", utsbuf.release);
- printf("version : %s\n", utsbuf.version);
- printf("machine : %s\n", utsbuf.machine);
-
- return;
-}
-
-static void output_physinfo(void)
-{
- struct libxl_physinfo info;
- const libxl_version_info *vinfo;
- unsigned int i;
-
- if (libxl_get_physinfo(&ctx, &info) != 0) {
- fprintf(stderr, "libxl_physinfo failed.\n");
- return;
- }
-
- printf("nr_cpus : %d\n", info.nr_cpus);
- printf("nr_nodes : %d\n", info.nr_nodes);
- printf("cores_per_socket : %d\n", info.cores_per_socket);
- printf("threads_per_core : %d\n", info.threads_per_core);
- printf("cpu_mhz : %d\n", info.cpu_khz / 1000);
- printf("hw_caps : ");
- for (i = 0; i < 8; i++)
- printf("%08x%c", info.hw_cap[i], i < 7 ?
'':'' : ''\n'');
- printf("virt_caps :");
- if (info.phys_cap & XEN_SYSCTL_PHYSCAP_hvm)
- printf(" hvm");
- if (info.phys_cap & XEN_SYSCTL_PHYSCAP_hvm_directio)
- printf(" hvm_directio");
- printf("\n");
- vinfo = libxl_get_version_info(&ctx);
- i = (1 << 20) / vinfo->pagesize;
- printf("total_memory : %lu\n", info.total_pages / i);
- printf("free_memory : %lu\n", info.free_pages / i);
+ printf("xend_config_format : 4\n");
return;
}
void info(int verbose)
{
- output_nodeinfo();
+ struct utsname utsinfo;
+ struct libxl_physinfo physinfo;
+ const libxl_version_info *verinfo;
- output_physinfo();
+ if (uname(&utsinfo) != 0) {
+ fprintf(stderr, "cannot get nodeinfo\n");
+ return;
+ }
+ if (libxl_get_physinfo(&ctx, &physinfo) != 0) {
+ fprintf(stderr, "libxl_physinfo failed.\n");
+ return;
+ }
+ verinfo = libxl_get_version_info(&ctx);
+ if (!verinfo) {
+ fprintf(stderr, "libxl_get_version_info failed.\n");
+ return;
+ }
- output_xeninfo();
-
- printf("xend_config_format : 4\n");
+ output_info(&utsinfo, &physinfo, verinfo);
return;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Andre Przywara
2010-Apr-26 10:54 UTC
[Xen-devel] Re: [PATCH] xl: Don''t output a broken result if error occurs in ''xl info''
Yu Zhiguo wrote:> If error occurs in ''xl info'', we''d better output an error message > rather than a broken result.Do you consider an incomplete output as broken? If I got this right, you want to make sure that you output everything or nothing with this patch. Wouldn''t it be sufficient to just add the missing check for utsname() and output the other infos regardless? Was there a scenario where broken output was generated? So far I only see missing Xen support or missing privileges to cause errors. Regards, Andre.> diff -r c87ec146229a -r 3f97bcb08bbd tools/libxl/xl.c > --- a/tools/libxl/xl.c Fri Apr 23 15:04:26 2010 +0100 > +++ b/tools/libxl/xl.c Tue Apr 27 01:11:06 2010 +0800 > @@ -2642,93 +2642,88 @@ > exit(0); > } > > -static void output_xeninfo(void) > +static void output_info(struct utsname *utsinfo, struct libxl_physinfo *physinfo, > + const libxl_version_info *verinfo) > { > - const libxl_version_info *info; > + unsigned int i; > int sched_id; > > - info = libxl_get_version_info(&ctx); > if ((sched_id = libxl_get_sched_id(&ctx)) < 0) { > fprintf(stderr, "get_sched_id sysctl failed.\n"); > return; > } > > - printf("xen_major : %d\n", info->xen_version_major); > - printf("xen_minor : %d\n", info->xen_version_minor); > - printf("xen_extra : %s\n", info->xen_version_extra); > - printf("xen_caps : %s\n", info->capabilities); > + // output utsinfo > + printf("host : %s\n", utsinfo->nodename); > + printf("release : %s\n", utsinfo->release); > + printf("version : %s\n", utsinfo->version); > + printf("machine : %s\n", utsinfo->machine); > + > + // output physinfo > + printf("nr_cpus : %d\n", physinfo->nr_cpus); > + printf("nr_nodes : %d\n", physinfo->nr_nodes); > + printf("cores_per_socket : %d\n", physinfo->cores_per_socket); > + printf("threads_per_core : %d\n", physinfo->threads_per_core); > + printf("cpu_mhz : %d\n", physinfo->cpu_khz / 1000); > + printf("hw_caps : "); > + for (i = 0; i < 8; i++) > + printf("%08x%c", physinfo->hw_cap[i], i < 7 ? '':'' : ''\n''); > + printf("virt_caps :"); > + if (physinfo->phys_cap & XEN_SYSCTL_PHYSCAP_hvm) > + printf(" hvm"); > + if (physinfo->phys_cap & XEN_SYSCTL_PHYSCAP_hvm_directio) > + printf(" hvm_directio"); > + printf("\n"); > + if (!verinfo) > + return; > + i = (1 << 20) / verinfo->pagesize; > + printf("total_memory : %llu\n", (long long unsigned int) physinfo->total_pages / i); > + printf("free_memory : %llu\n", (long long unsigned int) physinfo->free_pages / i); > + > + // output verinfo > + printf("xen_major : %d\n", verinfo->xen_version_major); > + printf("xen_minor : %d\n", verinfo->xen_version_minor); > + printf("xen_extra : %s\n", verinfo->xen_version_extra); > + printf("xen_caps : %s\n", verinfo->capabilities); > printf("xen_scheduler : %s\n", > sched_id == XEN_SCHEDULER_SEDF ? "sedf" : > sched_id == XEN_SCHEDULER_CREDIT ? "credit" : > sched_id == XEN_SCHEDULER_CREDIT2 ? "credit2" : "unknown"); > - printf("xen_pagesize : %lu\n", info->pagesize); > - printf("platform_params : virt_start=0x%lx\n", info->virt_start); > - printf("xen_changeset : %s\n", info->changeset); > - printf("xen_commandline : %s\n", info->commandline); > - printf("cc_compiler : %s\n", info->compiler); > - printf("cc_compile_by : %s\n", info->compile_by); > - printf("cc_compile_domain : %s\n", info->compile_domain); > - printf("cc_compile_date : %s\n", info->compile_date); > + printf("xen_pagesize : %lu\n", verinfo->pagesize); > + printf("platform_params : virt_start=0x%lx\n", verinfo->virt_start); > + printf("xen_changeset : %s\n", verinfo->changeset); > + printf("xen_commandline : %s\n", verinfo->commandline); > + printf("cc_compiler : %s\n", verinfo->compiler); > + printf("cc_compile_by : %s\n", verinfo->compile_by); > + printf("cc_compile_domain : %s\n", verinfo->compile_domain); > + printf("cc_compile_date : %s\n", verinfo->compile_date); > > - return; > -} > - > -static void output_nodeinfo(void) > -{ > - struct utsname utsbuf; > - > - uname(&utsbuf); > - > - printf("host : %s\n", utsbuf.nodename); > - printf("release : %s\n", utsbuf.release); > - printf("version : %s\n", utsbuf.version); > - printf("machine : %s\n", utsbuf.machine); > - > - return; > -} > - > -static void output_physinfo(void) > -{ > - struct libxl_physinfo info; > - const libxl_version_info *vinfo; > - unsigned int i; > - > - if (libxl_get_physinfo(&ctx, &info) != 0) { > - fprintf(stderr, "libxl_physinfo failed.\n"); > - return; > - } > - > - printf("nr_cpus : %d\n", info.nr_cpus); > - printf("nr_nodes : %d\n", info.nr_nodes); > - printf("cores_per_socket : %d\n", info.cores_per_socket); > - printf("threads_per_core : %d\n", info.threads_per_core); > - printf("cpu_mhz : %d\n", info.cpu_khz / 1000); > - printf("hw_caps : "); > - for (i = 0; i < 8; i++) > - printf("%08x%c", info.hw_cap[i], i < 7 ? '':'' : ''\n''); > - printf("virt_caps :"); > - if (info.phys_cap & XEN_SYSCTL_PHYSCAP_hvm) > - printf(" hvm"); > - if (info.phys_cap & XEN_SYSCTL_PHYSCAP_hvm_directio) > - printf(" hvm_directio"); > - printf("\n"); > - vinfo = libxl_get_version_info(&ctx); > - i = (1 << 20) / vinfo->pagesize; > - printf("total_memory : %lu\n", info.total_pages / i); > - printf("free_memory : %lu\n", info.free_pages / i); > + printf("xend_config_format : 4\n"); > > return; > } > > void info(int verbose) > { > - output_nodeinfo(); > + struct utsname utsinfo; > + struct libxl_physinfo physinfo; > + const libxl_version_info *verinfo; > > - output_physinfo(); > + if (uname(&utsinfo) != 0) { > + fprintf(stderr, "cannot get nodeinfo\n"); > + return; > + } > + if (libxl_get_physinfo(&ctx, &physinfo) != 0) { > + fprintf(stderr, "libxl_physinfo failed.\n"); > + return; > + } > + verinfo = libxl_get_version_info(&ctx); > + if (!verinfo) { > + fprintf(stderr, "libxl_get_version_info failed.\n"); > + return; > + } > > - output_xeninfo(); > - > - printf("xend_config_format : 4\n"); > + output_info(&utsinfo, &physinfo, verinfo); > > return; > } > >-- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany Tel: +49 351 448-3567-12 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andre Przywara wrote:> Yu Zhiguo wrote: >> If error occurs in ''xl info'', we''d better output an error message >> rather than a broken result. > Do you consider an incomplete output as broken? If I got this right, you > want to make sure that you output everything or nothing with this patch. > Wouldn''t it be sufficient to just add the missing check for utsname() > and output the other infos regardless? Was there a scenario where broken > output was generated? So far I only see missing Xen support or missing > privileges to cause errors.It seems that xm will always output all or nothing, so I think we''d better compatible with xm. But it isn''t so important, just add sanity check now. Regards Yu Zhiguo -------------------------------------- Add sanity check for ''xl info''. Signed-off-by: Yu Zhiguo <yuzg@cn.fujitsu.com> diff -r c87ec146229a -r c4a684b35c7b tools/libxl/xl.c --- a/tools/libxl/xl.c Fri Apr 23 15:04:26 2010 +0100 +++ b/tools/libxl/xl.c Wed Apr 28 19:24:13 2010 +0800 @@ -2647,7 +2647,10 @@ const libxl_version_info *info; int sched_id; - info = libxl_get_version_info(&ctx); + if (!(info = libxl_get_version_info(&ctx))) { + fprintf(stderr, "libxl_get_version_info failed.\n"); + return; + } if ((sched_id = libxl_get_sched_id(&ctx)) < 0) { fprintf(stderr, "get_sched_id sysctl failed.\n"); return; @@ -2677,12 +2680,12 @@ { struct utsname utsbuf; - uname(&utsbuf); - - printf("host : %s\n", utsbuf.nodename); - printf("release : %s\n", utsbuf.release); - printf("version : %s\n", utsbuf.version); - printf("machine : %s\n", utsbuf.machine); + if (uname(&utsbuf) != -1) { + printf("host : %s\n", utsbuf.nodename); + printf("release : %s\n", utsbuf.release); + printf("version : %s\n", utsbuf.version); + printf("machine : %s\n", utsbuf.machine); + } return; } @@ -2713,9 +2716,11 @@ printf(" hvm_directio"); printf("\n"); vinfo = libxl_get_version_info(&ctx); - i = (1 << 20) / vinfo->pagesize; - printf("total_memory : %lu\n", info.total_pages / i); - printf("free_memory : %lu\n", info.free_pages / i); + if (vinfo) { + i = (1 << 20) / vinfo->pagesize; + printf("total_memory : %lu\n", info.total_pages / i); + printf("free_memory : %lu\n", info.free_pages / i); + } return; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel