[libvirt] [PATCH 1/2] perf: Refactor perf code

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@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

--- 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 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.
participants (3)
-
John Ferlan
-
Martin Kletzander
-
Nitesh Konkar