Re: [PATCH 16/17] tools/kvm_stat: add interactive command 'r'

[Janosch Frank <frankja@linux.vnet.ibm.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3188 bytes --]

On 20.02.2017 16:42, Stefan Raspl wrote:
> Provide an interactive command to reset the tracepoint statistics.
> Requires some extra work for debugfs, as the counters cannot be reset.
> On the up side, offers us the opportunity to have debugfs values reset
                  ^
                  add "this"

> on startup and whenever a filter is modified, becoming consistent with
> the tracepoint provider. As a bonus, 'kvmstat -dt' will now provide
> useful output, instead of mixing values in totally different orders of
> magnitude.
> Furthermore, we avoid unnecessary resets when any of the filters is
> "changed" interactively to the previous value.

Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>

> 
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
> ---
>  tools/kvm/kvm_stat/kvm_stat     | 66 +++++++++++++++++++++++++++++++----------
>  tools/kvm/kvm_stat/kvm_stat.txt |  2 ++
>  2 files changed, 53 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
> index e0f28e1..f9f653a 100755
> --- a/tools/kvm/kvm_stat/kvm_stat
> +++ b/tools/kvm/kvm_stat/kvm_stat
> @@ -715,15 +715,23 @@ class TracepointProvider(object):
>                      ret[name] += val
>          return ret
> 
> +    def reset(self):
> +        """Reset all field counters"""
> +        for group in self.group_leaders:
> +            for event in group.events:
> +                event.reset()
> +
> 
>  class DebugfsProvider(object):
>      """Provides data from the files that KVM creates in the kvm debugfs
>      folder."""
>      def __init__(self):
>          self._fields = self.get_available_fields()
> +        self._baseline = {}
>          self._pid = 0
>          self.do_read = True
>          self.paths = []
> +        self.reset()
> 
>      def get_available_fields(self):
>          """"Returns a list of available fields.
> @@ -740,6 +748,7 @@ class DebugfsProvider(object):
>      @fields.setter
>      def fields(self, fields):
>          self._fields = fields
> +        self.reset()
> 
>      @property
>      def pid(self):
> @@ -757,10 +766,11 @@ class DebugfsProvider(object):
>              self.paths = filter(lambda x: "{}-".format(pid) in x, vms)
> 
>          else:
> -            self.paths = ['']
> +            self.paths = []

This is more a cleanup than new code, isn't it?

[...]
> @@ -1188,7 +1224,7 @@ Interactive Commands:
>     g     filter by guest name
>     p     filter by PID
>     q     quit
> -   x     toggle reporting of stats for individual child trace events
> +   r     reset stats
>  Press any other key to refresh statistics immediately.
>  """

This is inconsistent to the part below.

> diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
> index c3ab6a2..109431b 100644
> --- a/tools/kvm/kvm_stat/kvm_stat.txt
> +++ b/tools/kvm/kvm_stat/kvm_stat.txt
> @@ -39,6 +39,8 @@ INTERACTIVE COMMANDS
> 
>  *q*::	quit
> 
> +*r*::	reset stats
> +
>  *x*::	toggle reporting of stats for child trace events
> 
>  Press any other key to refresh statistics immediately.
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]