[PATCH] kvm_stat: always display non tracepoint based stats

[PATCH] kvm_stat: always display non tracepoint based stats

[Paolo Bonzini]

The old stats contain information not available in the tracepoints.

Inspired by a patch from Marcelo Tosatti.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/kvm/kvm_stat | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 762544b..b97542f 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -352,8 +352,8 @@ class TracepointProvider(object):
         return ret
 
 class Stats:
-    def __init__(self, provider, fields = None):
-        self.provider = provider
+    def __init__(self, providers, fields = None):
+        self.providers = providers
         self.fields_filter = fields
         self._update()
     def _update(self):
@@ -362,22 +362,25 @@ class Stats:
             if not self.fields_filter:
                 return True
             return re.match(self.fields_filter, key) is not None
-        self.values = dict([(key, None)
-                            for key in provider.fields()
-                            if wanted(key)])
-        self.provider.select(self.values.keys())
+        self.values = dict()
+        for d in providers:
+            provider_fields = [key for key in d.fields() if wanted(key)]
+            for key in provider_fields:
+                self.values[key] = None
+            d.select(provider_fields)
     def set_fields_filter(self, fields_filter):
         self.fields_filter = fields_filter
         self._update()
     def get(self):
-        new = self.provider.read()
-        for key in self.provider.fields():
-            oldval = self.values.get(key, (0, 0))
-            newval = new[key]
-            newdelta = None
-            if oldval is not None:
-                newdelta = newval - oldval[0]
-            self.values[key] = (newval, newdelta)
+        for d in providers:
+            new = d.read()
+            for key in d.fields():
+                oldval = self.values.get(key, (0, 0))
+                newval = new[key]
+                newdelta = None
+                if oldval is not None:
+                    newdelta = newval - oldval[0]
+                self.values[key] = (newval, newdelta)
         return self.values
 
 if not os.access('/sys/kernel/debug', os.F_OK):
@@ -496,11 +499,11 @@ options.add_option('-f', '--fields',
 (options, args) = options.parse_args(sys.argv)
 
 try:
-    provider = TracepointProvider()
+    providers = [TracepointProvider(), DebugfsProvider()]
 except:
-    provider = DebugfsProvider()
+    providers = [DebugfsProvider()]
 
-stats = Stats(provider, fields = options.fields)
+stats = Stats(providers, fields = options.fields)
 
 if options.log:
     log(stats)
-- 
1.8.3.1

Re: [PATCH] kvm_stat: always display non tracepoint based stats

[Marcelo Tosatti]

On Wed, May 21, 2014 at 12:44:51PM +0200, Paolo Bonzini wrote:
> The old stats contain information not available in the tracepoints.
> 
> Inspired by a patch from Marcelo Tosatti.

I avoided that because there is overlapping information.

Now you report kvm_exit count twice, for example.

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/kvm/kvm_stat | 37 ++++++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
> index 762544b..b97542f 100755
> --- a/scripts/kvm/kvm_stat
> +++ b/scripts/kvm/kvm_stat
> @@ -352,8 +352,8 @@ class TracepointProvider(object):
>          return ret
>  
>  class Stats:
> -    def __init__(self, provider, fields = None):
> -        self.provider = provider
> +    def __init__(self, providers, fields = None):
> +        self.providers = providers
>          self.fields_filter = fields
>          self._update()
>      def _update(self):
> @@ -362,22 +362,25 @@ class Stats:
>              if not self.fields_filter:
>                  return True
>              return re.match(self.fields_filter, key) is not None
> -        self.values = dict([(key, None)
> -                            for key in provider.fields()
> -                            if wanted(key)])
> -        self.provider.select(self.values.keys())
> +        self.values = dict()
> +        for d in providers:
> +            provider_fields = [key for key in d.fields() if wanted(key)]
> +            for key in provider_fields:
> +                self.values[key] = None
> +            d.select(provider_fields)
>      def set_fields_filter(self, fields_filter):
>          self.fields_filter = fields_filter
>          self._update()
>      def get(self):
> -        new = self.provider.read()
> -        for key in self.provider.fields():
> -            oldval = self.values.get(key, (0, 0))
> -            newval = new[key]
> -            newdelta = None
> -            if oldval is not None:
> -                newdelta = newval - oldval[0]
> -            self.values[key] = (newval, newdelta)
> +        for d in providers:
> +            new = d.read()
> +            for key in d.fields():
> +                oldval = self.values.get(key, (0, 0))
> +                newval = new[key]
> +                newdelta = None
> +                if oldval is not None:
> +                    newdelta = newval - oldval[0]
> +                self.values[key] = (newval, newdelta)
>          return self.values
>  
>  if not os.access('/sys/kernel/debug', os.F_OK):
> @@ -496,11 +499,11 @@ options.add_option('-f', '--fields',
>  (options, args) = options.parse_args(sys.argv)
>  
>  try:
> -    provider = TracepointProvider()
> +    providers = [TracepointProvider(), DebugfsProvider()]
>  except:
> -    provider = DebugfsProvider()
> +    providers = [DebugfsProvider()]
>  
> -stats = Stats(provider, fields = options.fields)
> +stats = Stats(providers, fields = options.fields)
>  
>  if options.log:
>      log(stats)
> -- 
> 1.8.3.1

Re: [PATCH] kvm_stat: always display non tracepoint based stats

[Paolo Bonzini]

Il 21/05/2014 14:27, Marcelo Tosatti ha scritto:
> On Wed, May 21, 2014 at 12:44:51PM +0200, Paolo Bonzini wrote:
>> The old stats contain information not available in the tracepoints.
>>
>> Inspired by a patch from Marcelo Tosatti.
>
> I avoided that because there is overlapping information.
>
> Now you report kvm_exit count twice, for example.

What about doing both, i.e. adding the options and the ability to choose 
multiple providers?  That is, adding --tracepoint and --debugfs options 
on top of my patch?  So if you use --tracepoint and tracing is not 
available, you get an error unlike the current behavior, and you can 
also use --tracepoint --debugfs together.

Paolo

Re: [PATCH] kvm_stat: always display non tracepoint based stats

[Marcelo Tosatti]

On Wed, May 21, 2014 at 02:42:35PM +0200, Paolo Bonzini wrote:
> Il 21/05/2014 14:27, Marcelo Tosatti ha scritto:
> >On Wed, May 21, 2014 at 12:44:51PM +0200, Paolo Bonzini wrote:
> >>The old stats contain information not available in the tracepoints.
> >>
> >>Inspired by a patch from Marcelo Tosatti.
> >
> >I avoided that because there is overlapping information.
> >
> >Now you report kvm_exit count twice, for example.
> 
> What about doing both, i.e. adding the options and the ability to
> choose multiple providers?  That is, adding --tracepoint and
> --debugfs options on top of my patch?  So if you use --tracepoint
> and tracing is not available, you get an error unlike the current
> behavior, and you can also use --tracepoint --debugfs together.
> 
> Paolo

Sounds good to me.