Avoid unnecessary calling of function vshCommandOptStringReq.
In the current code the function vshCommandOptStringReq is
called irrespective of whether --enable and/or --disable is
present in the command line. Eg: 'virsh perf domainName'
also results in calling this function twice. This patch
fixes this.
Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com>
---
tools/virsh-domain.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3a6fa5c..91de532 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -8862,6 +8862,8 @@ cmdPerf(vshControl *ctl, const vshCmd *cmd)
bool current = vshCommandOptBool(cmd, "current");
bool config = vshCommandOptBool(cmd, "config");
bool live = vshCommandOptBool(cmd, "live");
+ bool shouldEnable = vshCommandOptBool(cmd, "enable");
+ bool shouldDisable = vshCommandOptBool(cmd, "disable");
VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
VSH_EXCLUSIVE_OPTIONS_VAR(current, config);
@@ -8871,9 +8873,15 @@ cmdPerf(vshControl *ctl, const vshCmd *cmd)
if (live)
flags |= VIR_DOMAIN_AFFECT_LIVE;
- if (vshCommandOptStringReq(ctl, cmd, "enable", &enable) < 0 ||
- vshCommandOptStringReq(ctl, cmd, "disable", &disable) < 0)
- return false;
+ if (shouldEnable) {
+ if (vshCommandOptStringReq(ctl, cmd, "enable", &enable) < 0)
+ return false;
+ }
+
+ if (shouldDisable) {
+ if (vshCommandOptStringReq(ctl, cmd, "disable", &disable) < 0)
+ return false;
+ }
if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
return false;
--
1.9.3
Show replies by date
---
tools/virsh.pod | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index ef91223..9f9a0c6 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2274,7 +2274,7 @@ including context-switches, minor-faults, etc.. Now dozens of
events
from different sources can be supported by perf.
Currently only QEMU/KVM supports this command. The I<--enable> and
I<--disable>
-option combined with B<eventSpec> can be used to enabled or disable specific
+option combined with B<eventSpec> can be used to enable or disable specific
performance event. B<eventSpec> is a string list of one or more events
separated by commas. Valid event names are as follows:
--
1.9.3
On 12/22/2016 09:06 AM, Nitesh Konkar wrote:
---
tools/virsh.pod | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK and pushed,
John
On Thu, Dec 22, 2016 at 07:36:14PM +0530, Nitesh Konkar wrote:
Avoid unnecessary calling of function vshCommandOptStringReq.
In the current code the function vshCommandOptStringReq is
called irrespective of whether --enable and/or --disable is
present in the command line. Eg: 'virsh perf domainName'
also results in calling this function twice. This patch
fixes this.
With this patch you're now calling vshCommandOpt() anyway, which is the
biggest part of vshCommandOpt*() and in addition you call it multiple
times if the option is on the command line. That feels stupid.