[libvirt] [PATCH] virsh: fix return value error of cpu-stats

virsh cpu-stats guest --start 0 --count 3 It outputs right but the return value is 1 rather than 0 echo $? 1 Found by running libvirt-autotest ./run -t libvirt --tests virsh_cpu_stats --- tools/virsh-domain.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b29f934..bcf495c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6350,7 +6350,9 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) if (!nparams) { vshPrint(ctl, "%s", _("No per-CPU stats available")); - goto do_show_total; + if (show_total) + goto do_show_total; + goto cleanup; } if (VIR_ALLOC_N(params, nparams * MIN(show_count, 128)) < 0) @@ -6389,10 +6391,12 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) } VIR_FREE(params); -do_show_total: - if (!show_total) + if (!show_total) { + ret = true; goto cleanup; + } +do_show_total: /* get supported num of parameter for total statistics */ if ((nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, flags)) < 0) goto failed_stats; -- 1.8.3.1

On 08/23/2013 06:22 PM, Guannan Ren wrote:
virsh cpu-stats guest --start 0 --count 3 It outputs right but the return value is 1 rather than 0 echo $? 1
It's ok with libvirt-0.10.2-23.el6.x86_64 on RHEL6.y, but failed with libvirt-1.1.1-2.el7.x86_64 on RHEL7.y, it's a little wired, the commit 73b89ed8 is introduced since 2012-07-25, and Michal only switches it to c99 initialization of vshCmdDef after that, except this, nobody changes the cmdCPUStats(), maybe, we have different patches about cmdCPUStats() between RHEL6.y and RHEL7.y?
Found by running libvirt-autotest ./run -t libvirt --tests virsh_cpu_stats --- tools/virsh-domain.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b29f934..bcf495c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6350,7 +6350,9 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
if (!nparams) { vshPrint(ctl, "%s", _("No per-CPU stats available")); - goto do_show_total; + if (show_total) + goto do_show_total; + goto cleanup; }
if (VIR_ALLOC_N(params, nparams * MIN(show_count, 128))< 0) @@ -6389,10 +6391,12 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) } VIR_FREE(params);
-do_show_total: - if (!show_total) + if (!show_total) { + ret = true; goto cleanup; + }
+do_show_total: /* get supported num of parameter for total statistics */ if ((nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, flags))< 0) goto failed_stats;

On 26.8.2013 07:11, Alex Jia wrote:
On 08/23/2013 06:22 PM, Guannan Ren wrote:
virsh cpu-stats guest --start 0 --count 3 It outputs right but the return value is 1 rather than 0 echo $? 1
It's ok with libvirt-0.10.2-23.el6.x86_64 on RHEL6.y, but failed with libvirt-1.1.1-2.el7.x86_64 on RHEL7.y, it's a little wired, the commit 73b89ed8 is introduced since 2012-07-25, and Michal only switches it to c99 initialization of vshCmdDef after that, except this, nobody changes the cmdCPUStats(), maybe, we have different patches about cmdCPUStats() between RHEL6.y and RHEL7.y?
This bug has been introduced by commit a54f25a9.
Found by running libvirt-autotest ./run -t libvirt --tests virsh_cpu_stats --- tools/virsh-domain.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b29f934..bcf495c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6350,7 +6350,9 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
if (!nparams) { vshPrint(ctl, "%s", _("No per-CPU stats available")); - goto do_show_total; + if (show_total) + goto do_show_total; + goto cleanup; }
if (VIR_ALLOC_N(params, nparams * MIN(show_count, 128))< 0) @@ -6389,10 +6391,12 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) } VIR_FREE(params);
-do_show_total: - if (!show_total) + if (!show_total) { + ret = true; goto cleanup; + }
+do_show_total: /* get supported num of parameter for total statistics */ if ((nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, flags))< 0) goto failed_stats;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 08/26/2013 05:11 PM, Pavel Hrdina wrote:
On 26.8.2013 07:11, Alex Jia wrote:
On 08/23/2013 06:22 PM, Guannan Ren wrote:
virsh cpu-stats guest --start 0 --count 3 It outputs right but the return value is 1 rather than 0 echo $? 1
It's ok with libvirt-0.10.2-23.el6.x86_64 on RHEL6.y, but failed with libvirt-1.1.1-2.el7.x86_64 on RHEL7.y, it's a little wired, the commit 73b89ed8 is introduced since 2012-07-25, and Michal only switches it to c99 initialization of vshCmdDef after that, except this, nobody changes the cmdCPUStats(), maybe, we have different patches about cmdCPUStats() between RHEL6.y and RHEL7.y?
This bug has been introduced by commit a54f25a9.
Yes, thanks, I gave a error keyword 'cmdCPUStats' on the following cmdline. # git blame tools/virsh-domain.c | grep cmdCPUStats
Found by running libvirt-autotest ./run -t libvirt --tests virsh_cpu_stats --- tools/virsh-domain.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b29f934..bcf495c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6350,7 +6350,9 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
if (!nparams) { vshPrint(ctl, "%s", _("No per-CPU stats available")); - goto do_show_total; + if (show_total) + goto do_show_total; + goto cleanup; }
if (VIR_ALLOC_N(params, nparams * MIN(show_count, 128))< 0) @@ -6389,10 +6391,12 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) } VIR_FREE(params);
-do_show_total: - if (!show_total) + if (!show_total) { + ret = true; goto cleanup; + }
+do_show_total: /* get supported num of parameter for total statistics */ if ((nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, flags))< 0) goto failed_stats;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 08/23/2013 04:22 AM, Guannan Ren wrote:
virsh cpu-stats guest --start 0 --count 3 It outputs right but the return value is 1 rather than 0 echo $? 1
Found by running libvirt-autotest ./run -t libvirt --tests virsh_cpu_stats --- tools/virsh-domain.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 08/26/2013 10:05 PM, Eric Blake wrote:
On 08/23/2013 04:22 AM, Guannan Ren wrote:
virsh cpu-stats guest --start 0 --count 3 It outputs right but the return value is 1 rather than 0 echo $? 1
Found by running libvirt-autotest ./run -t libvirt --tests virsh_cpu_stats --- tools/virsh-domain.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) ACK.
Thanks, pushed Guannan
participants (4)
-
Alex Jia
-
Eric Blake
-
Guannan Ren
-
Pavel Hrdina