* [PATCH] xl: Don't output a broken result if error occurs in 'xl info'
@ 2010-04-26 9:16 Yu Zhiguo
2010-04-26 10:54 ` Andre Przywara
0 siblings, 1 reply; 3+ messages in thread
From: Yu Zhiguo @ 2010-04-26 9:16 UTC (permalink / raw)
To: Andre Przywara, Keir Fraser, xen-devel@lists.xensource.com
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;
}
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] xl: Don't output a broken result if error occurs in 'xl info'
2010-04-26 9:16 [PATCH] xl: Don't output a broken result if error occurs in 'xl info' Yu Zhiguo
@ 2010-04-26 10:54 ` Andre Przywara
2010-04-28 3:35 ` [PATCH] xl: sanity check for " Yu Zhiguo
0 siblings, 1 reply; 3+ messages in thread
From: Andre Przywara @ 2010-04-26 10:54 UTC (permalink / raw)
To: Yu Zhiguo; +Cc: xen-devel@lists.xensource.com, Keir Fraser
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
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH] xl: sanity check for 'xl info'
2010-04-26 10:54 ` Andre Przywara
@ 2010-04-28 3:35 ` Yu Zhiguo
0 siblings, 0 replies; 3+ messages in thread
From: Yu Zhiguo @ 2010-04-28 3:35 UTC (permalink / raw)
To: Andre Przywara; +Cc: xen-devel@lists.xensource.com, Keir Fraser
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;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-04-28 3:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-26 9:16 [PATCH] xl: Don't output a broken result if error occurs in 'xl info' Yu Zhiguo
2010-04-26 10:54 ` Andre Przywara
2010-04-28 3:35 ` [PATCH] xl: sanity check for " Yu Zhiguo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.