[PATCH v1 0/3] tools/kvm_stat: Misc patches leftovers

[PATCH v1 0/3] tools/kvm_stat: Misc patches leftovers

[Stefan Raspl]

Leftovers from the previous patch series, namely:
(1) is a fix for a bug noticed while reworking the remaining patches.
(2) patch reworked to become a command line switch rather than an
    interactive command.
(3) added a new dialog hinting at turning off tracepoints in case they
    are active when pressing 'b'. Furthermore, switch the first
    column's title from 'Event' to 'Guest Name' when pressing 'b'.


Stefan Raspl (3):
  tools/kvm_stat: fix error on interactive command 'g'
  tools/kvm_stat: add new command line switch '-i'
  tools/kvm_stat: add new interactive command 'b'

 tools/kvm/kvm_stat/kvm_stat     | 123 +++++++++++++++++++++++++++++++++++-----
 tools/kvm/kvm_stat/kvm_stat.txt |   2 +
 2 files changed, 110 insertions(+), 15 deletions(-)

-- 
2.11.2

[PATCH v1 1/3] tools/kvm_stat: fix error on interactive command 'g'

[Stefan Raspl]

Fix an instance where print_all_gnames() is called without the mandatory
argument, resulting in a stack trace.
To reproduce, simply press 'g' in interactive mode.

Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
 tools/kvm/kvm_stat/kvm_stat | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 2cf5176bbeee..39476e55f557 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -1195,7 +1195,7 @@ class Tui(object):
                                'This might limit the shown data to the trace '
                                'statistics.')
             self.screen.addstr(5, 0, msg)
-            self.print_all_gnames()
+            self.print_all_gnames(7)
             curses.echo()
             self.screen.addstr(3, 0, "Guest [ENTER or guest]: ")
             gname = self.screen.getstr()
-- 
2.11.2

[PATCH v1 2/3] tools/kvm_stat: add new command line switch '-i'

[Stefan Raspl]

It might be handy to display the full history of event stats to compare
the current event distribution against any available historic data.
Since we have that available for debugfs, we offer a respective command
line option to display what's available.

Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
 tools/kvm/kvm_stat/kvm_stat | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 39476e55f557..4065b2909085 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -681,12 +681,14 @@ class TracepointProvider(Provider):
 class DebugfsProvider(Provider):
     """Provides data from the files that KVM creates in the kvm debugfs
     folder."""
-    def __init__(self, pid, fields_filter):
+    def __init__(self, pid, fields_filter, include_past):
         self.update_fields(fields_filter)
         self._baseline = {}
         self.do_read = True
         self.paths = []
         self.pid = pid
+        if include_past:
+            self.restore()
 
     def get_available_fields(self):
         """"Returns a list of available fields.
@@ -730,7 +732,14 @@ class DebugfsProvider(Provider):
         self.reset()
 
     def read(self, reset=0):
-        """Returns a dict with format:'file name / field -> current value'."""
+        """Returns a dict with format:'file name / field -> current value'.
+
+        Parameter 'reset':
+          0   plain read
+          1   reset field counts to 0
+          2   restore the original field counts
+
+        """
         results = {}
 
         # If no debugfs filtering support is available, then don't read.
@@ -747,8 +756,10 @@ class DebugfsProvider(Provider):
             for field in self._fields:
                 value = self.read_field(field, path)
                 key = path + field
-                if reset:
+                if reset == 1:
                     self._baseline[key] = value
+                if reset == 2:
+                    self._baseline[key] = 0
                 if self._baseline.get(key, -1) == -1:
                     self._baseline[key] = value
                 results[field] = (results.get(field, 0) + value -
@@ -771,6 +782,11 @@ class DebugfsProvider(Provider):
         self._baseline = {}
         self.read(1)
 
+    def restore(self):
+        """Reset field counters"""
+        self._baseline = {}
+        self.read(2)
+
 
 class Stats(object):
     """Manages the data providers and the data they provide.
@@ -791,7 +807,8 @@ class Stats(object):
         providers = []
 
         if options.debugfs:
-            providers.append(DebugfsProvider(options.pid, options.fields))
+            providers.append(DebugfsProvider(options.pid, options.fields,
+                                             options.dbgfs_include_past))
         if options.tracepoints or not providers:
             providers.append(TracepointProvider(options.pid, options.fields))
 
@@ -1270,6 +1287,8 @@ class Tui(object):
                     sleeptime = self._delay_initial
                 if char == 'x':
                     self.update_drilldown()
+                    # prevents display of current values on next refresh
+                    self.stats.get()
             except KeyboardInterrupt:
                 break
             except curses.error:
@@ -1381,6 +1400,13 @@ Press any other key to refresh statistics immediately.
                          dest='once',
                          help='run in batch mode for one second',
                          )
+    optparser.add_option('-i', '--debugfs-include-past',
+                         action='store_true',
+                         default=False,
+                         dest='dbgfs_include_past',
+                         help='include all available data on past events for '
+                              'debugfs',
+                         )
     optparser.add_option('-l', '--log',
                          action='store_true',
                          default=False,
-- 
2.11.2

[PATCH v1 3/3] tools/kvm_stat: add new interactive command 'b'

[Stefan Raspl]

Toggle display total number of events by guest (debugfs only).
When switching to display of events by guest, field filters remain
active. I.e. the number of events per guest reported considers only
events matching the filters. Likewise with pid/guest filtering.
Note that when switching to display of events by guest, DebugfsProvider
remains to collect data for events as it did before, but the read()
method summarizes the values by pid.

Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
 tools/kvm/kvm_stat/kvm_stat     | 87 ++++++++++++++++++++++++++++++++++++-----
 tools/kvm/kvm_stat/kvm_stat.txt |  2 +
 2 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 4065b2909085..dd8f00cfb8b4 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -662,7 +662,7 @@ class TracepointProvider(Provider):
         self.setup_traces()
         self.fields = self._fields
 
-    def read(self):
+    def read(self, by_guest=0):
         """Returns 'event name: current value' for all enabled events."""
         ret = defaultdict(int)
         for group in self.group_leaders:
@@ -731,7 +731,7 @@ class DebugfsProvider(Provider):
             self.do_read = True
         self.reset()
 
-    def read(self, reset=0):
+    def read(self, reset=0, by_guest=0):
         """Returns a dict with format:'file name / field -> current value'.
 
         Parameter 'reset':
@@ -762,8 +762,16 @@ class DebugfsProvider(Provider):
                     self._baseline[key] = 0
                 if self._baseline.get(key, -1) == -1:
                     self._baseline[key] = value
-                results[field] = (results.get(field, 0) + value -
-                                  self._baseline.get(key, 0))
+                increment = (results.get(field, 0) + value -
+                             self._baseline.get(key, 0))
+                if by_guest:
+                    pid = key.split('-')[0]
+                    if pid in results:
+                        results[pid] += increment
+                    else:
+                        results[pid] = increment
+                else:
+                    results[field] = increment
 
         return results
 
@@ -849,18 +857,44 @@ class Stats(object):
             for provider in self.providers:
                 provider.pid = self._pid_filter
 
-    def get(self):
+    def get(self, by_guest=0):
         """Returns a dict with field -> (value, delta to last value) of all
         provider data."""
         for provider in self.providers:
-            new = provider.read()
-            for key in provider.fields:
+            new = provider.read(by_guest=by_guest)
+            for key in new if by_guest else provider.fields:
                 oldval = self.values.get(key, (0, 0))[0]
                 newval = new.get(key, 0)
                 newdelta = newval - oldval
                 self.values[key] = (newval, newdelta)
         return self.values
 
+    def toggle_display_guests(self, to_pid):
+        """Toggle between collection of stats by individual event and by
+        guest pid
+
+        Events reported by DebugfsProvider change when switching to/from
+        reading by guest values. Hence we have to remove the excess event
+        names from self.values.
+
+        """
+        if any(isinstance(ins, TracepointProvider) for ins in self.providers):
+            return 1
+        if to_pid:
+            for provider in self.providers:
+                if isinstance(provider, DebugfsProvider):
+                    for key in provider.fields:
+                        if key in self.values.keys():
+                            del self.values[key]
+        else:
+            oldvals = self.values.copy()
+            for key in oldvals:
+                if key.isdigit():
+                    del self.values[key]
+        # Update oldval (see get())
+        self.get(to_pid)
+        return 0
+
 DELAY_DEFAULT = 3.0
 MAX_GUEST_NAME_LEN = 48
 MAX_REGEX_LEN = 44
@@ -876,6 +910,7 @@ class Tui(object):
         self._delay_initial = 0.25
         self._delay_regular = DELAY_DEFAULT
         self._sorting = SORT_DEFAULT
+        self._display_guests = 0
 
     def __enter__(self):
         """Initialises curses for later use.  Based on curses.wrapper
@@ -1024,8 +1059,12 @@ class Tui(object):
             if len(regex) > MAX_REGEX_LEN:
                 regex = regex[:MAX_REGEX_LEN] + '...'
             self.screen.addstr(1, 17, 'regex filter: {0}'.format(regex))
+        if self._display_guests:
+            col_name = 'Guest Name'
+        else:
+            col_name = 'Event'
         self.screen.addstr(2, 1, '%-40s %10s%7s %8s' %
-                           ('Event', 'Total', '%Total', 'CurAvg/s'),
+                           (col_name, 'Total', '%Total', 'CurAvg/s'),
                            curses.A_STANDOUT)
         self.screen.addstr(4, 1, 'Collecting data...')
         self.screen.refresh()
@@ -1034,7 +1073,7 @@ class Tui(object):
         row = 3
         self.screen.move(row, 0)
         self.screen.clrtobot()
-        stats = self.stats.get()
+        stats = self.stats.get(self._display_guests)
 
         def sortCurAvg(x):
             # sort by current events if available
@@ -1062,6 +1101,8 @@ class Tui(object):
                 break
             if values[0] is not None:
                 cur = int(round(values[1] / sleeptime)) if values[1] else ''
+                if self._display_guests:
+                    key = self.get_gname_from_pid(key)
                 self.screen.addstr(row, 1, '%-40s %10d%7.1f %8s' %
                                    (key, values[0], values[0] * 100 / total,
                                     cur))
@@ -1070,9 +1111,26 @@ class Tui(object):
             self.screen.addstr(4, 1, 'No matching events reported yet')
         self.screen.refresh()
 
+    def show_msg(self, text):
+        """Display message centered text and exit on key press"""
+        hint = 'Press any key to continue'
+        curses.cbreak()
+        self.screen.erase()
+        (x, term_width) = self.screen.getmaxyx()
+        row = 2
+        for line in text:
+            start = (term_width - len(line)) / 2
+            self.screen.addstr(row, start, line)
+            row += 1
+        self.screen.addstr(row + 1, (term_width - len(hint)) / 2, hint,
+                           curses.A_STANDOUT)
+        self.screen.getkey()
+
     def show_help_interactive(self):
         """Display help with list of interactive commands"""
-        msg = ('   c     clear filter',
+        msg = ('   b     toggle events by guests (debugfs only, honors'
+               ' filters)',
+               '   c     clear filter',
                '   f     filter by regular expression',
                '   g     filter by guest name',
                '   h     display interactive commands reference',
@@ -1253,6 +1311,14 @@ class Tui(object):
             sleeptime = self._delay_regular
             try:
                 char = self.screen.getkey()
+                if char == 'b':
+                    self._display_guests = not self._display_guests
+                    if self.stats.toggle_display_guests(self._display_guests):
+                        self.show_msg(['Command not available with tracepoints'
+                                       ' enabled', 'Restart with debugfs only '
+                                       '(see option \'-d\') and try again!'])
+                        self._display_guests = not self._display_guests
+                    self.refresh_header()
                 if char == 'c':
                     self.stats.fields_filter = DEFAULT_REGEX
                     self.refresh_header(0)
@@ -1356,6 +1422,7 @@ Requirements:
   the large number of files that are possibly opened.
 
 Interactive Commands:
+   b     toggle events by guests (debugfs only, honors filters)
    c     clear filter
    f     filter by regular expression
    g     filter by guest name
diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index e24ac464d341..0eaffb9dac46 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -29,6 +29,8 @@ meaning of events.
 INTERACTIVE COMMANDS
 --------------------
 [horizontal]
+*b*::	toggle events by guests (debugfs only, honors filters)
+
 *c*::	clear filter
 
 *f*::	filter by regular expression
-- 
2.11.2

Re: [PATCH v1 0/3] tools/kvm_stat: Misc patches leftovers

[Paolo Bonzini]

On 25/06/2017 21:34, Stefan Raspl wrote:
> Leftovers from the previous patch series, namely:
> (1) is a fix for a bug noticed while reworking the remaining patches.
> (2) patch reworked to become a command line switch rather than an
>     interactive command.

Squashed this in patch 2/3 and queued:

diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index e24ac464d341..851372d263cc 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -70,6 +70,10 @@ OPTIONS
 --debugfs::
 	retrieve statistics from debugfs
 
+-i::
+--debugfs-include-past::
+	include all available data on past events for debugfs
+
 -p<pid>::
 --pid=<pid>::
 	limit statistics to one virtual machine (pid)

Paolo

> (3) added a new dialog hinting at turning off tracepoints in case they
>     are active when pressing 'b'. Furthermore, switch the first
>     column's title from 'Event' to 'Guest Name' when pressing 'b'.
> 
> 
> Stefan Raspl (3):
>   tools/kvm_stat: fix error on interactive command 'g'
>   tools/kvm_stat: add new command line switch '-i'
>   tools/kvm_stat: add new interactive command 'b'
> 
>  tools/kvm/kvm_stat/kvm_stat     | 123 +++++++++++++++++++++++++++++++++++-----
>  tools/kvm/kvm_stat/kvm_stat.txt |   2 +
>  2 files changed, 110 insertions(+), 15 deletions(-)
>