On 04/05/2013 10:40 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=907732
Also added informational message when count value is larger than number
of CPUs present. Original code commit '31047e2b' quietly changes it and
continues on.
Prior to this patch, no errors were seen for following sequences
virsh cpu-stats guest xyz
virsh cpu-stats guest --start xyz
virsh cpu-stats guest --count xyz
virsh cpu-stats guest --count 99999999999
With this patch, the following errors are displayed
error: Invalid value for start CPU
error: Invalid value for start CPU
error: Invalid value for number of CPUs to show
error: Invalid value for number of CPUs to show
Passing a value such as 9 to count will display the following:
Only 4 CPUs available to show
CPU0:
cpu_time 19.860859202 seconds
vcpu_time 17.551435620 seconds
...
---
tools/virsh-domain.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index c088468..3dbfa53 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6111,7 +6111,7 @@ static const vshCmdInfo info_cpu_stats[] = {
{.name = "desc",
.data = N_("Display per-CPU and total statistics about the domain's
CPUs")
},
- {.name = NULL},
+ {.name = NULL}
};
static const vshCmdOptDef opts_cpu_stats[] = {
@@ -6132,7 +6132,7 @@ static const vshCmdOptDef opts_cpu_stats[] = {
.type = VSH_OT_INT,
.help = N_("Number of shown CPUs at most")
},
- {.name = NULL},
+ {.name = NULL}
};
I think these two hunks would be better in a separate patch.
static bool
@@ -6149,9 +6149,18 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
return false;
show_total = vshCommandOptBool(cmd, "total");
- if (vshCommandOptInt(cmd, "start", &cpu) > 0)
+ if (vshCommandOptInt(cmd, "start", &cpu) < 0) {
+ vshError(ctl, "%s", _("Invalid value for start CPU"));
+ goto cleanup;
+ }
+ if (cpu >= 0)
show_per_cpu = true;
- if (vshCommandOptInt(cmd, "count", &show_count) > 0)
+
+ if (vshCommandOptInt(cmd, "count", &show_count) < 0) {
+ vshError(ctl, "%s", _("Invalid value for number of CPUs to
show"));
+ goto cleanup;
+ }
+ if (show_count >= 0)
show_per_cpu = true;
/* default show per_cpu and total */
This doesn't set show_per_cpu when a negative number was specified,
shouldn't we set it based on vshCommandOptInt return value instead?
@@ -6170,8 +6179,10 @@ cmdCPUStats(vshControl *ctl, const vshCmd
*cmd)
/* get number of cpus on the node */
if ((max_id = virDomainGetCPUStats(dom, NULL, 0, 0, 0, flags)) < 0)
goto failed_stats;
- if (show_count < 0 || show_count > max_id)
+ if (show_count < 0 || show_count > max_id) {
+ vshPrint(ctl, _("Only %d CPUs available to show\n"), max_id);
This message will get printed even if --count wasn't specified, since
show_count is -1 by default.
show_count = max_id;
+ }
/* get percpu information */
if ((nparams = virDomainGetCPUStats(dom, NULL, 0, 0, 1, flags)) < 0)
Jan