diff mbox

[3/8] tools/kvm_stat: mark private methods as such

Message ID 20180205130004.29691-4-raspl@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Stefan Raspl Feb. 5, 2018, 12:59 p.m. UTC
From: Stefan Raspl <stefan.raspl@de.ibm.com>

Helps quite a bit reading the code when it's obvious when a method is
intended for internal use only.

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

Comments

Paolo Bonzini Feb. 13, 2018, 2:48 p.m. UTC | #1
On 05/02/2018 13:59, Stefan Raspl wrote:
> From: Stefan Raspl <stefan.raspl@de.ibm.com>
> 
> Helps quite a bit reading the code when it's obvious when a method is
> intended for internal use only.
> 
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>

Since no subclassing is involved, should these be _single_underscore
methods rather than __double_underscore?

Thanks,

Paolo
Stefan Raspl Feb. 14, 2018, 9:24 p.m. UTC | #2
On 13.02.2018 15:48, Paolo Bonzini wrote:
> On 05/02/2018 13:59, Stefan Raspl wrote:
>> From: Stefan Raspl <stefan.raspl@de.ibm.com>
>>
>> Helps quite a bit reading the code when it's obvious when a method is
>> intended for internal use only.
>>
>> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
> 
> Since no subclassing is involved, should these be _single_underscore
> methods rather than __double_underscore?

Yup - one could argue that this way it's more future proof. But the reality is
that I thought I had read that this is the way to go. Probably listened to the
wrong voices in my head...
Will fix.

Ciao,
Stefan
diff mbox

Patch

diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 0d92c12f2b0f..26615d7e63aa 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -376,8 +376,8 @@  class Event(object):
         self.syscall = self.libc.syscall
         self.name = name
         self.fd = None
-        self.setup_event(group, trace_cpu, trace_pid, trace_point,
-                         trace_filter, trace_set)
+        self.__setup_event(group, trace_cpu, trace_pid, trace_point,
+                           trace_filter, trace_set)
 
     def __del__(self):
         """Closes the event's file descriptor.
@@ -390,7 +390,7 @@  class Event(object):
         if self.fd:
             os.close(self.fd)
 
-    def perf_event_open(self, attr, pid, cpu, group_fd, flags):
+    def __perf_event_open(self, attr, pid, cpu, group_fd, flags):
         """Wrapper for the sys_perf_evt_open() syscall.
 
         Used to set up performance events, returns a file descriptor or -1
@@ -409,7 +409,7 @@  class Event(object):
                             ctypes.c_int(pid), ctypes.c_int(cpu),
                             ctypes.c_int(group_fd), ctypes.c_long(flags))
 
-    def setup_event_attribute(self, trace_set, trace_point):
+    def __setup_event_attribute(self, trace_set, trace_point):
         """Returns an initialized ctype perf_event_attr struct."""
 
         id_path = os.path.join(PATH_DEBUGFS_TRACING, 'events', trace_set,
@@ -419,8 +419,8 @@  class Event(object):
         event_attr.config = int(open(id_path).read())
         return event_attr
 
-    def setup_event(self, group, trace_cpu, trace_pid, trace_point,
-                    trace_filter, trace_set):
+    def __setup_event(self, group, trace_cpu, trace_pid, trace_point,
+                      trace_filter, trace_set):
         """Sets up the perf event in Linux.
 
         Issues the syscall to register the event in the kernel and
@@ -428,7 +428,7 @@  class Event(object):
 
         """
 
-        event_attr = self.setup_event_attribute(trace_set, trace_point)
+        event_attr = self.__setup_event_attribute(trace_set, trace_point)
 
         # First event will be group leader.
         group_leader = -1
@@ -437,8 +437,8 @@  class Event(object):
         if group.events:
             group_leader = group.events[0].fd
 
-        fd = self.perf_event_open(event_attr, trace_pid,
-                                  trace_cpu, group_leader, 0)
+        fd = self.__perf_event_open(event_attr, trace_pid,
+                                    trace_cpu, group_leader, 0)
         if fd == -1:
             err = ctypes.get_errno()
             raise OSError(err, os.strerror(err),
@@ -500,12 +500,12 @@  class TracepointProvider(Provider):
     """
     def __init__(self, pid, fields_filter):
         self.group_leaders = []
-        self.filters = self.get_filters()
+        self.filters = self.__get_filters()
         self.update_fields(fields_filter)
         self.pid = pid
 
     @staticmethod
-    def get_filters():
+    def __get_filters():
         """Returns a dict of trace events, their filter ids and
         the values that can be filtered.
 
@@ -521,7 +521,7 @@  class TracepointProvider(Provider):
             filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
         return filters
 
-    def get_available_fields(self):
+    def __get_available_fields(self):
         """Returns a list of available event's of format 'event name(filter
         name)'.
 
@@ -549,11 +549,11 @@  class TracepointProvider(Provider):
 
     def update_fields(self, fields_filter):
         """Refresh fields, applying fields_filter"""
-        self.fields = [field for field in self.get_available_fields()
+        self.fields = [field for field in self.__get_available_fields()
                        if self.is_field_wanted(fields_filter, field)]
 
     @staticmethod
-    def get_online_cpus():
+    def __get_online_cpus():
         """Returns a list of cpu id integers."""
         def parse_int_list(list_string):
             """Returns an int list from a string of comma separated integers and
@@ -575,17 +575,17 @@  class TracepointProvider(Provider):
             cpu_string = cpu_list.readline()
             return parse_int_list(cpu_string)
 
-    def setup_traces(self):
+    def __setup_traces(self):
         """Creates all event and group objects needed to be able to retrieve
         data."""
-        fields = self.get_available_fields()
+        fields = self.__get_available_fields()
         if self._pid > 0:
             # Fetch list of all threads of the monitored pid, as qemu
             # starts a thread for each vcpu.
             path = os.path.join('/proc', str(self._pid), 'task')
             groupids = self.walkdir(path)[1]
         else:
-            groupids = self.get_online_cpus()
+            groupids = self.__get_online_cpus()
 
         # The constant is needed as a buffer for python libs, std
         # streams and other files that the script opens.
@@ -663,7 +663,7 @@  class TracepointProvider(Provider):
         # The garbage collector will get rid of all Event/Group
         # objects and open files after removing the references.
         self.group_leaders = []
-        self.setup_traces()
+        self.__setup_traces()
         self.fields = self._fields
 
     def read(self, by_guest=0):
@@ -692,9 +692,9 @@  class DebugfsProvider(Provider):
         self.paths = []
         self.pid = pid
         if include_past:
-            self.restore()
+            self.__restore()
 
-    def get_available_fields(self):
+    def __get_available_fields(self):
         """"Returns a list of available fields.
 
         The fields are all available KVM debugfs files
@@ -704,7 +704,7 @@  class DebugfsProvider(Provider):
 
     def update_fields(self, fields_filter):
         """Refresh fields, applying fields_filter"""
-        self._fields = [field for field in self.get_available_fields()
+        self._fields = [field for field in self.__get_available_fields()
                         if self.is_field_wanted(fields_filter, field)]
 
     @property
@@ -758,7 +758,7 @@  class DebugfsProvider(Provider):
                     paths.append(dir)
         for path in paths:
             for field in self._fields:
-                value = self.read_field(field, path)
+                value = self.__read_field(field, path)
                 key = path + field
                 if reset == 1:
                     self._baseline[key] = value
@@ -779,7 +779,7 @@  class DebugfsProvider(Provider):
 
         return results
 
-    def read_field(self, field, path):
+    def __read_field(self, field, path):
         """Returns the value of a single field from a specific VM."""
         try:
             return int(open(os.path.join(PATH_DEBUGFS_KVM,
@@ -794,7 +794,7 @@  class DebugfsProvider(Provider):
         self._baseline = {}
         self.read(1)
 
-    def restore(self):
+    def __restore(self):
         """Reset field counters"""
         self._baseline = {}
         self.read(2)
@@ -811,13 +811,12 @@  class Stats(object):
 
     """
     def __init__(self, options):
-        self.providers = self.get_providers(options)
+        self.providers = self.__get_providers(options)
         self._pid_filter = options.pid
         self._fields_filter = options.fields
         self.values = {}
 
-    @staticmethod
-    def get_providers(options):
+    def __get_providers(self, options):
         """Returns a list of data providers depending on the passed options."""
         providers = []
 
@@ -829,7 +828,7 @@  class Stats(object):
 
         return providers
 
-    def update_provider_filters(self):
+    def __update_provider_filters(self):
         """Propagates fields filters to providers."""
         # As we reset the counters when updating the fields we can
         # also clear the cache of old values.
@@ -850,7 +849,7 @@  class Stats(object):
     def fields_filter(self, fields_filter):
         if fields_filter != self._fields_filter:
             self._fields_filter = fields_filter
-            self.update_provider_filters()
+            self.__update_provider_filters()
 
     @property
     def pid_filter(self):
@@ -972,7 +971,7 @@  class Tui(object):
 
         return res
 
-    def print_all_gnames(self, row):
+    def __print_all_gnames(self, row):
         """Print a list of all running guests along with their pids."""
         self.screen.addstr(row, 2, '%8s  %-60s' %
                            ('Pid', 'Guest Name (fuzzy list, might be '
@@ -1035,7 +1034,7 @@  class Tui(object):
 
         return name
 
-    def update_drilldown(self):
+    def __update_drilldown(self):
         """Sets or removes a filter that only allows fields without braces."""
         if not self.stats.fields_filter:
             self.stats.fields_filter = DEFAULT_REGEX
@@ -1043,11 +1042,11 @@  class Tui(object):
         elif self.stats.fields_filter == DEFAULT_REGEX:
             self.stats.fields_filter = None
 
-    def update_pid(self, pid):
+    def __update_pid(self, pid):
         """Propagates pid selection to stats object."""
         self.stats.pid_filter = pid
 
-    def refresh_header(self, pid=None):
+    def __refresh_header(self, pid=None):
         """Refreshes the header."""
         if pid is None:
             pid = self.stats.pid_filter
@@ -1078,7 +1077,7 @@  class Tui(object):
         self.screen.addstr(4, 1, 'Collecting data...')
         self.screen.refresh()
 
-    def refresh_body(self, sleeptime):
+    def __refresh_body(self, sleeptime):
         row = 3
         self.screen.move(row, 0)
         self.screen.clrtobot()
@@ -1127,7 +1126,7 @@  class Tui(object):
                                curses.A_BOLD)
         self.screen.refresh()
 
-    def show_msg(self, text):
+    def __show_msg(self, text):
         """Display message centered text and exit on key press"""
         hint = 'Press any key to continue'
         curses.cbreak()
@@ -1142,7 +1141,7 @@  class Tui(object):
                            curses.A_STANDOUT)
         self.screen.getkey()
 
-    def show_help_interactive(self):
+    def __show_help_interactive(self):
         """Display help with list of interactive commands"""
         msg = ('   b     toggle events by guests (debugfs only, honors'
                ' filters)',
@@ -1168,9 +1167,9 @@  class Tui(object):
             self.screen.addstr(row, 0, line)
             row += 1
         self.screen.getkey()
-        self.refresh_header()
+        self.__refresh_header()
 
-    def show_filter_selection(self):
+    def __show_filter_selection(self):
         """Draws filter selection mask.
 
         Asks for a valid regex and sets the fields filter accordingly.
@@ -1192,18 +1191,18 @@  class Tui(object):
             curses.noecho()
             if len(regex) == 0:
                 self.stats.fields_filter = DEFAULT_REGEX
-                self.refresh_header()
+                self.__refresh_header()
                 return
             try:
                 re.compile(regex)
                 self.stats.fields_filter = regex
-                self.refresh_header()
+                self.__refresh_header()
                 return
             except re.error:
                 msg = '"' + regex + '": Not a valid regular expression'
                 continue
 
-    def show_vm_selection_by_pid(self):
+    def __show_vm_selection_by_pid(self):
         """Draws PID selection mask.
 
         Asks for a pid until a valid pid or 0 has been entered.
@@ -1219,7 +1218,7 @@  class Tui(object):
                                'This might limit the shown data to the trace '
                                'statistics.')
             self.screen.addstr(5, 0, msg)
-            self.print_all_gnames(7)
+            self.__print_all_gnames(7)
 
             curses.echo()
             self.screen.addstr(3, 0, "Pid [0 or pid]: ")
@@ -1235,13 +1234,13 @@  class Tui(object):
                         continue
                 else:
                     pid = 0
-                self.refresh_header(pid)
-                self.update_pid(pid)
+                self.__refresh_header(pid)
+                self.__update_pid(pid)
                 break
             except ValueError:
                 msg = '"' + str(pid) + '": Not a valid pid'
 
-    def show_set_update_interval(self):
+    def __show_set_update_interval(self):
         """Draws update interval selection mask."""
         msg = ''
         while True:
@@ -1271,9 +1270,9 @@  class Tui(object):
 
             except ValueError:
                 msg = '"' + str(val) + '": Invalid value'
-        self.refresh_header()
+        self.__refresh_header()
 
-    def show_vm_selection_by_guest_name(self):
+    def __show_vm_selection_by_guest_name(self):
         """Draws guest selection mask.
 
         Asks for a guest name until a valid guest name or '' is entered.
@@ -1289,15 +1288,15 @@  class Tui(object):
                                'This might limit the shown data to the trace '
                                'statistics.')
             self.screen.addstr(5, 0, msg)
-            self.print_all_gnames(7)
+            self.__print_all_gnames(7)
             curses.echo()
             self.screen.addstr(3, 0, "Guest [ENTER or guest]: ")
             gname = self.screen.getstr().decode(ENCODING)
             curses.noecho()
 
             if not gname:
-                self.refresh_header(0)
-                self.update_pid(0)
+                self.__refresh_header(0)
+                self.__update_pid(0)
                 break
             else:
                 pids = []
@@ -1314,17 +1313,17 @@  class Tui(object):
                     msg = '"' + gname + '": Multiple matches found, use pid ' \
                           'filter instead'
                     continue
-                self.refresh_header(pids[0])
-                self.update_pid(pids[0])
+                self.__refresh_header(pids[0])
+                self.__update_pid(pids[0])
                 break
 
     def show_stats(self):
         """Refreshes the screen and processes user input."""
         sleeptime = self._delay_initial
-        self.refresh_header()
+        self.__refresh_header()
         start = 0.0  # result based on init value never appears on screen
         while True:
-            self.refresh_body(time.time() - start)
+            self.__refresh_body(time.time() - start)
             curses.halfdelay(int(sleeptime * 10))
             start = time.time()
             sleeptime = self._delay_regular
@@ -1333,32 +1332,33 @@  class Tui(object):
                 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.__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()
+                    self.__refresh_header()
                 if char == 'c':
                     self.stats.fields_filter = DEFAULT_REGEX
-                    self.refresh_header(0)
-                    self.update_pid(0)
+                    self.__refresh_header(0)
+                    self.__update_pid(0)
                 if char == 'f':
                     curses.curs_set(1)
-                    self.show_filter_selection()
+                    self.__show_filter_selection()
                     curses.curs_set(0)
                     sleeptime = self._delay_initial
                 if char == 'g':
                     curses.curs_set(1)
-                    self.show_vm_selection_by_guest_name()
+                    self.__show_vm_selection_by_guest_name()
                     curses.curs_set(0)
                     sleeptime = self._delay_initial
                 if char == 'h':
-                    self.show_help_interactive()
+                    self.__show_help_interactive()
                 if char == 'o':
                     self._sorting = not self._sorting
                 if char == 'p':
                     curses.curs_set(1)
-                    self.show_vm_selection_by_pid()
+                    self.__show_vm_selection_by_pid()
                     curses.curs_set(0)
                     sleeptime = self._delay_initial
                 if char == 'q':
@@ -1367,11 +1367,11 @@  class Tui(object):
                     self.stats.reset()
                 if char == 's':
                     curses.curs_set(1)
-                    self.show_set_update_interval()
+                    self.__show_set_update_interval()
                     curses.curs_set(0)
                     sleeptime = self._delay_initial
                 if char == 'x':
-                    self.update_drilldown()
+                    self.__update_drilldown()
                     # prevents display of current values on next refresh
                     self.stats.get(self._display_guests)
             except KeyboardInterrupt: