* [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup
@ 2015-12-10 12:12 Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 01/34] scripts/kvm/kvm_stat: Cleanup of multiple imports Janosch Frank
` (34 more replies)
0 siblings, 35 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Kvm_stat is a very helpful script for checking the state of VMs, but
when I tried to introduce new features it broke every few lines.
This patch series aims to make the script more readable and durable,
so future additions to it will break it less likely. It also fixes
input/output problems for all of its interface modes.
Testing was done rarely on X86_64 RHEL 6.7 and mostly on s390. Tests
on other architectures would be beneficial.
Janosch Frank (34):
scripts/kvm/kvm_stat: Cleanup of multiple imports
scripts/kvm/kvm_stat: Replaced os.listdir with os.walk
scripts/kvm/kvm_stat: Make constants uppercase
scripts/kvm/kvm_stat: Removed unneeded PERF constants
scripts/kvm/kvm_stat: Mark globals in functions
scripts/kvm/kvm_stat: Invert dictionaries
scripts/kvm/kvm_stat: Cleanup of path variables
scripts/kvm/kvm_stat: Improve debugfs access checking
scripts/kvm/kvm_stat: Introduce main function
scripts/kvm/kvm_stat: Fix spaces around keyword assignments
scripts/kvm/kvm_stat: Rename variables that redefine globals
scripts/kvm/kvm_stat: Moved DebugfsProvider
scripts/kvm/kvm_stat: Fixup syscall error reporting
scripts/kvm/kvm_stat: Set sensible no. files rlimit
scripts/kvm/kvm_stat: Cleanup of platform detection
scripts/kvm/kvm_stat: Make cpu detection a function
scripts/kvm/kvm_stat: Rename _perf_event_open
scripts/kvm/kvm_stat: Introduce properties for providers
scripts/kvm/kvm_stat: Cleanup of TracepointProvider
scripts/kvm/kvm_stat: Cleanup cpu list retrieval
scripts/kvm/kvm_stat: Encapsulate filters variable
scripts/kvm/kvm_stat: Cleanup of Stats class
scripts/kvm/kvm_stat: Cleanup of Groups class
scripts/kvm/kvm_stat: Cleanup of Event class
scripts/kvm/kvm_stat: Group arch specific data
scripts/kvm/kvm_stat: Remove unneeded X86_EXIT_REASONS
scripts/kvm/kvm_stat: Make tui function a class
scripts/kvm/kvm_stat: Fix output formatting
scripts/kvm/kvm_stat: Move to argparse and add description
scripts/kvm/kvm_stat: Cleanup and pre-init perf_event_attr
scripts/kvm/kvm_stat: Read event values as u64
scripts/kvm/kvm_stat: Fix rlimit for unprivileged users
scripts/kvm/kvm_stat: Fixup filtering
scripts/kvm/kvm_stat: Add interactive filtering
scripts/kvm/kvm_stat | 1129 ++++++++++++++++++++++++++++----------------------
1 file changed, 626 insertions(+), 503 deletions(-)
--
2.3.0
^ permalink raw reply [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 01/34] scripts/kvm/kvm_stat: Cleanup of multiple imports
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 02/34] scripts/kvm/kvm_stat: Replaced os.listdir with os.walk Janosch Frank
` (33 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Removed multiple imports of the same module and moved all imports to
the top.
It is not necessary to import a module each time one of its
functions/classes is used.
For readability each import should get its own line.
---
scripts/kvm/kvm_stat | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 7e5d256..3fadbfb 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -12,8 +12,16 @@
# the COPYING file in the top-level directory.
import curses
-import sys, os, time, optparse, ctypes
-from ctypes import *
+import sys
+import os
+import time
+import optparse
+import ctypes
+import fcntl
+import resource
+import struct
+import re
+from collections import defaultdict
class DebugfsProvider(object):
def __init__(self):
@@ -285,12 +293,10 @@ filters['kvm_userspace_exit'] = ('reason', invert(userspace_exit_reasons))
if exit_reasons:
filters['kvm_exit'] = ('exit_reason', invert(exit_reasons))
-import struct, array
-
libc = ctypes.CDLL('libc.so.6')
syscall = libc.syscall
get_errno = libc.__errno_location
-get_errno.restype = POINTER(c_int)
+get_errno.restype = ctypes.POINTER(ctypes.c_int)
class perf_event_attr(ctypes.Structure):
_fields_ = [('type', ctypes.c_uint32),
@@ -334,8 +340,6 @@ PERF_FORMAT_TOTAL_TIME_RUNNING = 1 << 1
PERF_FORMAT_ID = 1 << 2
PERF_FORMAT_GROUP = 1 << 3
-import re
-
sys_tracing = '/sys/kernel/debug/tracing'
class Group(object):
@@ -378,17 +382,13 @@ class Event(object):
err = get_errno()[0]
raise Exception('perf_event_open failed, errno = ' + err.__str__())
if filter:
- import fcntl
fcntl.ioctl(fd, ioctl_numbers['SET_FILTER'], filter)
self.fd = fd
def enable(self):
- import fcntl
fcntl.ioctl(self.fd, ioctl_numbers['ENABLE'], 0)
def disable(self):
- import fcntl
fcntl.ioctl(self.fd, ioctl_numbers['DISABLE'], 0)
def reset(self):
- import fcntl
fcntl.ioctl(self.fd, ioctl_numbers['RESET'], 0)
class TracepointProvider(object):
@@ -426,7 +426,6 @@ class TracepointProvider(object):
def _setup(self, _fields):
self._fields = _fields
cpus = self._online_cpus()
- import resource
nfiles = len(cpus) * 1000
resource.setrlimit(resource.RLIMIT_NOFILE, (nfiles, nfiles))
events = []
@@ -454,7 +453,6 @@ class TracepointProvider(object):
else:
event.disable()
def read(self):
- from collections import defaultdict
ret = defaultdict(int)
for group in self.group_leaders:
for name, val in group.read().iteritems():
@@ -468,7 +466,6 @@ class Stats:
self._update()
def _update(self):
def wanted(key):
- import re
if not self.fields_filter:
return True
return re.match(self.fields_filter, key) is not None
@@ -640,7 +637,6 @@ stats = Stats(providers, fields = options.fields)
if options.log:
log(stats)
elif not options.once:
- import curses.wrapper
curses.wrapper(tui, stats)
else:
batch(stats)
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 02/34] scripts/kvm/kvm_stat: Replaced os.listdir with os.walk
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 01/34] scripts/kvm/kvm_stat: Cleanup of multiple imports Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 03/34] scripts/kvm/kvm_stat: Make constants uppercase Janosch Frank
` (32 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Os.walk gives back lists of directories and files, no need to filter
directories from the list that listdir gives back.
To make it better understandable a wrapper with docstring was
introduced.
---
scripts/kvm/kvm_stat | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 3fadbfb..6323276 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -26,7 +26,7 @@ from collections import defaultdict
class DebugfsProvider(object):
def __init__(self):
self.base = '/sys/kernel/debug/kvm'
- self._fields = os.listdir(self.base)
+ self._fields = walkdir(self.base)[2]
def fields(self):
return self._fields
def select(self, fields):
@@ -285,6 +285,15 @@ def detect_platform():
detect_platform()
+
+def walkdir(path):
+ """Returns os.walk() data for specified directory.
+
+ As it is only a wrapper it returns the same 3-tuple of (dirpath,
+ dirnames, filenames).
+ """
+ return next(os.walk(path))
+
def invert(d):
return dict((x[1], x[0]) for x in d.iteritems())
@@ -394,9 +403,7 @@ class Event(object):
class TracepointProvider(object):
def __init__(self):
path = os.path.join(sys_tracing, 'events', 'kvm')
- fields = [f
- for f in os.listdir(path)
- if os.path.isdir(os.path.join(path, f))]
+ fields = walkdir(path)[1]
extra = []
for f in fields:
if f in filters:
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 03/34] scripts/kvm/kvm_stat: Make constants uppercase
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 01/34] scripts/kvm/kvm_stat: Cleanup of multiple imports Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 02/34] scripts/kvm/kvm_stat: Replaced os.listdir with os.walk Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 04/34] scripts/kvm/kvm_stat: Removed unneeded PERF constants Janosch Frank
` (31 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Constants should be uppercase with separating underscores, as
requested in PEP8. This helps identifying them when reading the code.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
---
scripts/kvm/kvm_stat | 64 ++++++++++++++++++++++++++--------------------------
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 6323276..c4bf900 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -36,7 +36,7 @@ class DebugfsProvider(object):
return int(file(self.base + '/' + key).read())
return dict([(key, val(key)) for key in self._fields])
-vmx_exit_reasons = {
+VMX_EXIT_REASONS = {
0: 'EXCEPTION_NMI',
1: 'EXTERNAL_INTERRUPT',
2: 'TRIPLE_FAULT',
@@ -78,7 +78,7 @@ vmx_exit_reasons = {
58: 'INVPCID',
}
-svm_exit_reasons = {
+SVM_EXIT_REASONS = {
0x000: 'READ_CR0',
0x003: 'READ_CR3',
0x004: 'READ_CR4',
@@ -154,7 +154,7 @@ svm_exit_reasons = {
}
# EC definition of HSR (from arch/arm64/include/asm/kvm_arm.h)
-aarch64_exit_reasons = {
+AARCH64_EXIT_REASONS = {
0x00: 'UNKNOWN',
0x01: 'WFI',
0x03: 'CP15_32',
@@ -193,7 +193,7 @@ aarch64_exit_reasons = {
}
# From include/uapi/linux/kvm.h, KVM_EXIT_xxx
-userspace_exit_reasons = {
+USERSPACE_EXIT_REASONS = {
0: 'UNKNOWN',
1: 'EXCEPTION',
2: 'IO',
@@ -221,15 +221,15 @@ userspace_exit_reasons = {
24: 'SYSTEM_EVENT',
}
-x86_exit_reasons = {
- 'vmx': vmx_exit_reasons,
- 'svm': svm_exit_reasons,
+X86_EXIT_REASONS = {
+ 'vmx': VMX_EXIT_REASONS,
+ 'svm': SVM_EXIT_REASONS,
}
-sc_perf_evt_open = None
-exit_reasons = None
+SC_PERF_EVT_OPEN = None
+EXIT_REASONS = None
-ioctl_numbers = {
+IOCTL_NUMBERS = {
'SET_FILTER' : 0x40082406,
'ENABLE' : 0x00002400,
'DISABLE' : 0x00002401,
@@ -238,19 +238,19 @@ ioctl_numbers = {
def x86_init(flag):
globals().update({
- 'sc_perf_evt_open' : 298,
- 'exit_reasons' : x86_exit_reasons[flag],
+ 'SC_PERF_EVT_OPEN' : 298,
+ 'EXIT_REASONS' : X86_EXIT_REASONS[flag],
})
def s390_init():
globals().update({
- 'sc_perf_evt_open' : 331
+ 'SC_PERF_EVT_OPEN' : 331
})
def ppc_init():
globals().update({
- 'sc_perf_evt_open' : 319,
- 'ioctl_numbers' : {
+ 'SC_PERF_EVT_OPEN' : 319,
+ 'IOCTL_NUMBERS' : {
'SET_FILTER' : 0x80002406 | (ctypes.sizeof(ctypes.c_char_p) << 16),
'ENABLE' : 0x20002400,
'DISABLE' : 0x20002401,
@@ -259,8 +259,8 @@ def ppc_init():
def aarch64_init():
globals().update({
- 'sc_perf_evt_open' : 241,
- 'exit_reasons' : aarch64_exit_reasons,
+ 'SC_PERF_EVT_OPEN' : 241,
+ 'EXIT_REASONS' : AARCH64_EXIT_REASONS,
})
def detect_platform():
@@ -274,7 +274,7 @@ def detect_platform():
for line in file('/proc/cpuinfo').readlines():
if line.startswith('flags'):
for flag in line.split():
- if flag in x86_exit_reasons:
+ if flag in X86_EXIT_REASONS:
x86_init(flag)
return
elif line.startswith('vendor_id'):
@@ -298,9 +298,9 @@ def invert(d):
return dict((x[1], x[0]) for x in d.iteritems())
filters = {}
-filters['kvm_userspace_exit'] = ('reason', invert(userspace_exit_reasons))
-if exit_reasons:
- filters['kvm_exit'] = ('exit_reason', invert(exit_reasons))
+filters['kvm_userspace_exit'] = ('reason', invert(USERSPACE_EXIT_REASONS))
+if EXIT_REASONS:
+ filters['kvm_exit'] = ('exit_reason', invert(EXIT_REASONS))
libc = ctypes.CDLL('libc.so.6')
syscall = libc.syscall
@@ -321,7 +321,7 @@ class perf_event_attr(ctypes.Structure):
('bp_len', ctypes.c_uint64),
]
def _perf_event_open(attr, pid, cpu, group_fd, flags):
- return syscall(sc_perf_evt_open, ctypes.pointer(attr), ctypes.c_int(pid),
+ return syscall(SC_PERF_EVT_OPEN, ctypes.pointer(attr), ctypes.c_int(pid),
ctypes.c_int(cpu), ctypes.c_int(group_fd),
ctypes.c_long(flags))
@@ -391,14 +391,14 @@ class Event(object):
err = get_errno()[0]
raise Exception('perf_event_open failed, errno = ' + err.__str__())
if filter:
- fcntl.ioctl(fd, ioctl_numbers['SET_FILTER'], filter)
+ fcntl.ioctl(fd, IOCTL_NUMBERS['SET_FILTER'], filter)
self.fd = fd
def enable(self):
- fcntl.ioctl(self.fd, ioctl_numbers['ENABLE'], 0)
+ fcntl.ioctl(self.fd, IOCTL_NUMBERS['ENABLE'], 0)
def disable(self):
- fcntl.ioctl(self.fd, ioctl_numbers['DISABLE'], 0)
+ fcntl.ioctl(self.fd, IOCTL_NUMBERS['DISABLE'], 0)
def reset(self):
- fcntl.ioctl(self.fd, ioctl_numbers['RESET'], 0)
+ fcntl.ioctl(self.fd, IOCTL_NUMBERS['RESET'], 0)
class TracepointProvider(object):
def __init__(self):
@@ -505,8 +505,8 @@ if not os.access('/sys/kernel/debug/kvm', os.F_OK):
print "and ensure the kvm modules are loaded"
sys.exit(1)
-label_width = 40
-number_width = 10
+LABEL_WIDTH = 40
+NUMBER_WIDTH = 10
def tui(screen, stats):
curses.use_default_colors()
@@ -524,8 +524,8 @@ def tui(screen, stats):
screen.erase()
screen.addstr(0, 0, 'kvm statistics')
screen.addstr(2, 1, 'Event')
- screen.addstr(2, 1 + label_width + number_width - len('Total'), 'Total')
- screen.addstr(2, 1 + label_width + number_width + 8 - len('Current'), 'Current')
+ screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH - len('Total'), 'Total')
+ screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH + 8 - len('Current'), 'Current')
row = 3
s = stats.get()
def sortkey(x):
@@ -541,9 +541,9 @@ def tui(screen, stats):
break
col = 1
screen.addstr(row, col, key)
- col += label_width
+ col += LABEL_WIDTH
screen.addstr(row, col, '%10d' % (values[0],))
- col += number_width
+ col += NUMBER_WIDTH
if values[1] is not None:
screen.addstr(row, col, '%8d' % (values[1] / sleeptime,))
row += 1
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 04/34] scripts/kvm/kvm_stat: Removed unneeded PERF constants
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (2 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 03/34] scripts/kvm/kvm_stat: Make constants uppercase Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 05/34] scripts/kvm/kvm_stat: Mark globals in functions Janosch Frank
` (30 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Only two of the constants are actually needed to set up the events, so
the others were removed. All variables that used them were also removed.
---
scripts/kvm/kvm_stat | 28 ++--------------------------
1 file changed, 2 insertions(+), 26 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index c4bf900..7a8617d 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -325,29 +325,8 @@ def _perf_event_open(attr, pid, cpu, group_fd, flags):
ctypes.c_int(cpu), ctypes.c_int(group_fd),
ctypes.c_long(flags))
-PERF_TYPE_HARDWARE = 0
-PERF_TYPE_SOFTWARE = 1
-PERF_TYPE_TRACEPOINT = 2
-PERF_TYPE_HW_CACHE = 3
-PERF_TYPE_RAW = 4
-PERF_TYPE_BREAKPOINT = 5
-
-PERF_SAMPLE_IP = 1 << 0
-PERF_SAMPLE_TID = 1 << 1
-PERF_SAMPLE_TIME = 1 << 2
-PERF_SAMPLE_ADDR = 1 << 3
-PERF_SAMPLE_READ = 1 << 4
-PERF_SAMPLE_CALLCHAIN = 1 << 5
-PERF_SAMPLE_ID = 1 << 6
-PERF_SAMPLE_CPU = 1 << 7
-PERF_SAMPLE_PERIOD = 1 << 8
-PERF_SAMPLE_STREAM_ID = 1 << 9
-PERF_SAMPLE_RAW = 1 << 10
-
-PERF_FORMAT_TOTAL_TIME_ENABLED = 1 << 0
-PERF_FORMAT_TOTAL_TIME_RUNNING = 1 << 1
-PERF_FORMAT_ID = 1 << 2
-PERF_FORMAT_GROUP = 1 << 3
+PERF_TYPE_TRACEPOINT = 2
+PERF_FORMAT_GROUP = 1 << 3
sys_tracing = '/sys/kernel/debug/tracing'
@@ -378,9 +357,6 @@ class Event(object):
tracepoint, 'id')
id = int(file(id_path).read())
attr.config = id
- attr.sample_type = (PERF_SAMPLE_RAW
- | PERF_SAMPLE_TIME
- | PERF_SAMPLE_CPU)
attr.sample_period = 1
attr.read_format = PERF_FORMAT_GROUP
group_leader = -1
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 05/34] scripts/kvm/kvm_stat: Mark globals in functions
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (3 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 04/34] scripts/kvm/kvm_stat: Removed unneeded PERF constants Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 06/34] scripts/kvm/kvm_stat: Invert dictionaries Janosch Frank
` (29 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Updating globals over the globals().update() method is not the
standard way of changing globals. Marking variables as global and
modifying them the standard way is better readable.
---
scripts/kvm/kvm_stat | 41 ++++++++++++++++++++++-------------------
1 file changed, 22 insertions(+), 19 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 7a8617d..83450be 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -237,31 +237,34 @@ IOCTL_NUMBERS = {
}
def x86_init(flag):
- globals().update({
- 'SC_PERF_EVT_OPEN' : 298,
- 'EXIT_REASONS' : X86_EXIT_REASONS[flag],
- })
+ global SC_PERF_EVT_OPEN
+ global EXIT_REASONS
+
+ SC_PERF_EVT_OPEN = 298
+ EXIT_REASONS = X86_EXIT_REASONS[flag]
def s390_init():
- globals().update({
- 'SC_PERF_EVT_OPEN' : 331
- })
+ global SC_PERF_EVT_OPEN
+
+ SC_PERF_EVT_OPEN = 331
def ppc_init():
- globals().update({
- 'SC_PERF_EVT_OPEN' : 319,
- 'IOCTL_NUMBERS' : {
- 'SET_FILTER' : 0x80002406 | (ctypes.sizeof(ctypes.c_char_p) << 16),
- 'ENABLE' : 0x20002400,
- 'DISABLE' : 0x20002401,
- }
- })
+ global SC_PERF_EVT_OPEN
+ global IOCTL_NUMBERS
+
+ SC_PERF_EVT_OPEN = 319
+
+ IOCTL_NUMBERS['ENABLE'] = 0x20002400
+ IOCTL_NUMBERS['DISABLE'] = 0x20002401
+ IOCTL_NUMBERS['SET_FILTER'] = 0x80002406 | (ctypes.sizeof(ctypes.c_char_p)
+ << 16)
def aarch64_init():
- globals().update({
- 'SC_PERF_EVT_OPEN' : 241,
- 'EXIT_REASONS' : AARCH64_EXIT_REASONS,
- })
+ global SC_PERF_EVT_OPEN
+ global EXIT_REASONS
+
+ SC_PERF_EVT_OPEN = 241
+ EXIT_REASONS = AARCH64_EXIT_REASONS
def detect_platform():
if os.uname()[4].startswith('ppc'):
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 06/34] scripts/kvm/kvm_stat: Invert dictionaries
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (4 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 05/34] scripts/kvm/kvm_stat: Mark globals in functions Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 07/34] scripts/kvm/kvm_stat: Cleanup of path variables Janosch Frank
` (28 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The exit reasons dictionaries were defined number -> value but later
on were accessed the other way around. Therefore a invert function
inverted them.
Defining them the right way removes the need to invert them and
therefore also speeds up the script's setup process.
---
scripts/kvm/kvm_stat | 349 +++++++++++++++++++++++++--------------------------
1 file changed, 173 insertions(+), 176 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 83450be..d53945e 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -37,188 +37,188 @@ class DebugfsProvider(object):
return dict([(key, val(key)) for key in self._fields])
VMX_EXIT_REASONS = {
- 0: 'EXCEPTION_NMI',
- 1: 'EXTERNAL_INTERRUPT',
- 2: 'TRIPLE_FAULT',
- 7: 'PENDING_INTERRUPT',
- 8: 'NMI_WINDOW',
- 9: 'TASK_SWITCH',
- 10: 'CPUID',
- 12: 'HLT',
- 14: 'INVLPG',
- 15: 'RDPMC',
- 16: 'RDTSC',
- 18: 'VMCALL',
- 19: 'VMCLEAR',
- 20: 'VMLAUNCH',
- 21: 'VMPTRLD',
- 22: 'VMPTRST',
- 23: 'VMREAD',
- 24: 'VMRESUME',
- 25: 'VMWRITE',
- 26: 'VMOFF',
- 27: 'VMON',
- 28: 'CR_ACCESS',
- 29: 'DR_ACCESS',
- 30: 'IO_INSTRUCTION',
- 31: 'MSR_READ',
- 32: 'MSR_WRITE',
- 33: 'INVALID_STATE',
- 36: 'MWAIT_INSTRUCTION',
- 39: 'MONITOR_INSTRUCTION',
- 40: 'PAUSE_INSTRUCTION',
- 41: 'MCE_DURING_VMENTRY',
- 43: 'TPR_BELOW_THRESHOLD',
- 44: 'APIC_ACCESS',
- 48: 'EPT_VIOLATION',
- 49: 'EPT_MISCONFIG',
- 54: 'WBINVD',
- 55: 'XSETBV',
- 56: 'APIC_WRITE',
- 58: 'INVPCID',
+ 'EXCEPTION_NMI': 0,
+ 'EXTERNAL_INTERRUPT': 1,
+ 'TRIPLE_FAULT': 2,
+ 'PENDING_INTERRUPT': 7,
+ 'NMI_WINDOW': 8,
+ 'TASK_SWITCH': 9,
+ 'CPUID': 10,
+ 'HLT': 12,
+ 'INVLPG': 14,
+ 'RDPMC': 15,
+ 'RDTSC': 16,
+ 'VMCALL': 18,
+ 'VMCLEAR': 19,
+ 'VMLAUNCH': 20,
+ 'VMPTRLD': 21,
+ 'VMPTRST': 22,
+ 'VMREAD': 23,
+ 'VMRESUME': 24,
+ 'VMWRITE': 25,
+ 'VMOFF': 26,
+ 'VMON': 27,
+ 'CR_ACCESS': 28,
+ 'DR_ACCESS': 29,
+ 'IO_INSTRUCTION': 30,
+ 'MSR_READ': 31,
+ 'MSR_WRITE': 32,
+ 'INVALID_STATE': 33,
+ 'MWAIT_INSTRUCTION': 36,
+ 'MONITOR_INSTRUCTION': 39,
+ 'PAUSE_INSTRUCTION': 40,
+ 'MCE_DURING_VMENTRY': 41,
+ 'TPR_BELOW_THRESHOLD': 43,
+ 'APIC_ACCESS': 44,
+ 'EPT_VIOLATION': 48,
+ 'EPT_MISCONFIG': 49,
+ 'WBINVD': 54,
+ 'XSETBV': 55,
+ 'APIC_WRITE': 56,
+ 'INVPCID': 58,
}
SVM_EXIT_REASONS = {
- 0x000: 'READ_CR0',
- 0x003: 'READ_CR3',
- 0x004: 'READ_CR4',
- 0x008: 'READ_CR8',
- 0x010: 'WRITE_CR0',
- 0x013: 'WRITE_CR3',
- 0x014: 'WRITE_CR4',
- 0x018: 'WRITE_CR8',
- 0x020: 'READ_DR0',
- 0x021: 'READ_DR1',
- 0x022: 'READ_DR2',
- 0x023: 'READ_DR3',
- 0x024: 'READ_DR4',
- 0x025: 'READ_DR5',
- 0x026: 'READ_DR6',
- 0x027: 'READ_DR7',
- 0x030: 'WRITE_DR0',
- 0x031: 'WRITE_DR1',
- 0x032: 'WRITE_DR2',
- 0x033: 'WRITE_DR3',
- 0x034: 'WRITE_DR4',
- 0x035: 'WRITE_DR5',
- 0x036: 'WRITE_DR6',
- 0x037: 'WRITE_DR7',
- 0x040: 'EXCP_BASE',
- 0x060: 'INTR',
- 0x061: 'NMI',
- 0x062: 'SMI',
- 0x063: 'INIT',
- 0x064: 'VINTR',
- 0x065: 'CR0_SEL_WRITE',
- 0x066: 'IDTR_READ',
- 0x067: 'GDTR_READ',
- 0x068: 'LDTR_READ',
- 0x069: 'TR_READ',
- 0x06a: 'IDTR_WRITE',
- 0x06b: 'GDTR_WRITE',
- 0x06c: 'LDTR_WRITE',
- 0x06d: 'TR_WRITE',
- 0x06e: 'RDTSC',
- 0x06f: 'RDPMC',
- 0x070: 'PUSHF',
- 0x071: 'POPF',
- 0x072: 'CPUID',
- 0x073: 'RSM',
- 0x074: 'IRET',
- 0x075: 'SWINT',
- 0x076: 'INVD',
- 0x077: 'PAUSE',
- 0x078: 'HLT',
- 0x079: 'INVLPG',
- 0x07a: 'INVLPGA',
- 0x07b: 'IOIO',
- 0x07c: 'MSR',
- 0x07d: 'TASK_SWITCH',
- 0x07e: 'FERR_FREEZE',
- 0x07f: 'SHUTDOWN',
- 0x080: 'VMRUN',
- 0x081: 'VMMCALL',
- 0x082: 'VMLOAD',
- 0x083: 'VMSAVE',
- 0x084: 'STGI',
- 0x085: 'CLGI',
- 0x086: 'SKINIT',
- 0x087: 'RDTSCP',
- 0x088: 'ICEBP',
- 0x089: 'WBINVD',
- 0x08a: 'MONITOR',
- 0x08b: 'MWAIT',
- 0x08c: 'MWAIT_COND',
- 0x08d: 'XSETBV',
- 0x400: 'NPF',
+ 'READ_CR0': 0x000,
+ 'READ_CR3': 0x003,
+ 'READ_CR4': 0x004,
+ 'READ_CR8': 0x008,
+ 'WRITE_CR0': 0x010,
+ 'WRITE_CR3': 0x013,
+ 'WRITE_CR4': 0x014,
+ 'WRITE_CR8': 0x018,
+ 'READ_DR0': 0x020,
+ 'READ_DR1': 0x021,
+ 'READ_DR2': 0x022,
+ 'READ_DR3': 0x023,
+ 'READ_DR4': 0x024,
+ 'READ_DR5': 0x025,
+ 'READ_DR6': 0x026,
+ 'READ_DR7': 0x027,
+ 'WRITE_DR0': 0x030,
+ 'WRITE_DR1': 0x031,
+ 'WRITE_DR2': 0x032,
+ 'WRITE_DR3': 0x033,
+ 'WRITE_DR4': 0x034,
+ 'WRITE_DR5': 0x035,
+ 'WRITE_DR6': 0x036,
+ 'WRITE_DR7': 0x037,
+ 'EXCP_BASE': 0x040,
+ 'INTR': 0x060,
+ 'NMI': 0x061,
+ 'SMI': 0x062,
+ 'INIT': 0x063,
+ 'VINTR': 0x064,
+ 'CR0_SEL_WRITE': 0x065,
+ 'IDTR_READ': 0x066,
+ 'GDTR_READ': 0x067,
+ 'LDTR_READ': 0x068,
+ 'TR_READ': 0x069,
+ 'IDTR_WRITE': 0x06a,
+ 'GDTR_WRITE': 0x06b,
+ 'LDTR_WRITE': 0x06c,
+ 'TR_WRITE': 0x06d,
+ 'RDTSC': 0x06e,
+ 'RDPMC': 0x06f,
+ 'PUSHF': 0x070,
+ 'POPF': 0x071,
+ 'CPUID': 0x072,
+ 'RSM': 0x073,
+ 'IRET': 0x074,
+ 'SWINT': 0x075,
+ 'INVD': 0x076,
+ 'PAUSE': 0x077,
+ 'HLT': 0x078,
+ 'INVLPG': 0x079,
+ 'INVLPGA': 0x07a,
+ 'IOIO': 0x07b,
+ 'MSR': 0x07c,
+ 'TASK_SWITCH': 0x07d,
+ 'FERR_FREEZE': 0x07e,
+ 'SHUTDOWN': 0x07f,
+ 'VMRUN': 0x080,
+ 'VMMCALL': 0x081,
+ 'VMLOAD': 0x082,
+ 'VMSAVE': 0x083,
+ 'STGI': 0x084,
+ 'CLGI': 0x085,
+ 'SKINIT': 0x086,
+ 'RDTSCP': 0x087,
+ 'ICEBP': 0x088,
+ 'WBINVD': 0x089,
+ 'MONITOR': 0x08a,
+ 'MWAIT': 0x08b,
+ 'MWAIT_COND': 0x08c,
+ 'XSETBV': 0x08d,
+ 'NPF': 0x400,
}
# EC definition of HSR (from arch/arm64/include/asm/kvm_arm.h)
AARCH64_EXIT_REASONS = {
- 0x00: 'UNKNOWN',
- 0x01: 'WFI',
- 0x03: 'CP15_32',
- 0x04: 'CP15_64',
- 0x05: 'CP14_MR',
- 0x06: 'CP14_LS',
- 0x07: 'FP_ASIMD',
- 0x08: 'CP10_ID',
- 0x0C: 'CP14_64',
- 0x0E: 'ILL_ISS',
- 0x11: 'SVC32',
- 0x12: 'HVC32',
- 0x13: 'SMC32',
- 0x15: 'SVC64',
- 0x16: 'HVC64',
- 0x17: 'SMC64',
- 0x18: 'SYS64',
- 0x20: 'IABT',
- 0x21: 'IABT_HYP',
- 0x22: 'PC_ALIGN',
- 0x24: 'DABT',
- 0x25: 'DABT_HYP',
- 0x26: 'SP_ALIGN',
- 0x28: 'FP_EXC32',
- 0x2C: 'FP_EXC64',
- 0x2F: 'SERROR',
- 0x30: 'BREAKPT',
- 0x31: 'BREAKPT_HYP',
- 0x32: 'SOFTSTP',
- 0x33: 'SOFTSTP_HYP',
- 0x34: 'WATCHPT',
- 0x35: 'WATCHPT_HYP',
- 0x38: 'BKPT32',
- 0x3A: 'VECTOR32',
- 0x3C: 'BRK64',
+ 'UNKNOWN': 0x00,
+ 'WFI': 0x01,
+ 'CP15_32': 0x03,
+ 'CP15_64': 0x04,
+ 'CP14_MR': 0x05,
+ 'CP14_LS': 0x06,
+ 'FP_ASIMD': 0x07,
+ 'CP10_ID': 0x08,
+ 'CP14_64': 0x0C,
+ 'ILL_ISS': 0x0E,
+ 'SVC32': 0x11,
+ 'HVC32': 0x12,
+ 'SMC32': 0x13,
+ 'SVC64': 0x15,
+ 'HVC64': 0x16,
+ 'SMC64': 0x17,
+ 'SYS64': 0x18,
+ 'IABT': 0x20,
+ 'IABT_HYP': 0x21,
+ 'PC_ALIGN': 0x22,
+ 'DABT': 0x24,
+ 'DABT_HYP': 0x25,
+ 'SP_ALIGN': 0x26,
+ 'FP_EXC32': 0x28,
+ 'FP_EXC64': 0x2C,
+ 'SERROR': 0x2F,
+ 'BREAKPT': 0x30,
+ 'BREAKPT_HYP': 0x31,
+ 'SOFTSTP': 0x32,
+ 'SOFTSTP_HYP': 0x33,
+ 'WATCHPT': 0x34,
+ 'WATCHPT_HYP': 0x35,
+ 'BKPT32': 0x38,
+ 'VECTOR32': 0x3A,
+ 'BRK64': 0x3C,
}
# From include/uapi/linux/kvm.h, KVM_EXIT_xxx
USERSPACE_EXIT_REASONS = {
- 0: 'UNKNOWN',
- 1: 'EXCEPTION',
- 2: 'IO',
- 3: 'HYPERCALL',
- 4: 'DEBUG',
- 5: 'HLT',
- 6: 'MMIO',
- 7: 'IRQ_WINDOW_OPEN',
- 8: 'SHUTDOWN',
- 9: 'FAIL_ENTRY',
- 10: 'INTR',
- 11: 'SET_TPR',
- 12: 'TPR_ACCESS',
- 13: 'S390_SIEIC',
- 14: 'S390_RESET',
- 15: 'DCR',
- 16: 'NMI',
- 17: 'INTERNAL_ERROR',
- 18: 'OSI',
- 19: 'PAPR_HCALL',
- 20: 'S390_UCONTROL',
- 21: 'WATCHDOG',
- 22: 'S390_TSCH',
- 23: 'EPR',
- 24: 'SYSTEM_EVENT',
+ 'UNKNOWN': 0,
+ 'EXCEPTION': 1,
+ 'IO': 2,
+ 'HYPERCALL': 3,
+ 'DEBUG': 4,
+ 'HLT': 5,
+ 'MMIO': 6,
+ 'IRQ_WINDOW_OPEN': 7,
+ 'SHUTDOWN': 8,
+ 'FAIL_ENTRY': 9,
+ 'INTR': 10,
+ 'SET_TPR': 11,
+ 'TPR_ACCESS': 12,
+ 'S390_SIEIC': 13,
+ 'S390_RESET': 14,
+ 'DCR': 15,
+ 'NMI': 16,
+ 'INTERNAL_ERROR': 17,
+ 'OSI': 18,
+ 'PAPR_HCALL': 19,
+ 'S390_UCONTROL': 20,
+ 'WATCHDOG': 21,
+ 'S390_TSCH': 22,
+ 'EPR': 23,
+ 'SYSTEM_EVENT': 24,
}
X86_EXIT_REASONS = {
@@ -297,13 +297,10 @@ def walkdir(path):
"""
return next(os.walk(path))
-def invert(d):
- return dict((x[1], x[0]) for x in d.iteritems())
-
filters = {}
-filters['kvm_userspace_exit'] = ('reason', invert(USERSPACE_EXIT_REASONS))
+filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS)
if EXIT_REASONS:
- filters['kvm_exit'] = ('exit_reason', invert(EXIT_REASONS))
+ filters['kvm_exit'] = ('exit_reason', EXIT_REASONS)
libc = ctypes.CDLL('libc.so.6')
syscall = libc.syscall
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 07/34] scripts/kvm/kvm_stat: Cleanup of path variables
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (5 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 06/34] scripts/kvm/kvm_stat: Invert dictionaries Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2016-01-07 14:56 ` Paolo Bonzini
2015-12-10 12:12 ` [Qemu-devel] [PATCH 08/34] scripts/kvm/kvm_stat: Improve debugfs access checking Janosch Frank
` (27 subsequent siblings)
34 siblings, 1 reply; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Paths to debugfs and trace dirs are now specified globally to remove
redundancies in the code.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
---
scripts/kvm/kvm_stat | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index d53945e..e7f3595 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -25,15 +25,14 @@ from collections import defaultdict
class DebugfsProvider(object):
def __init__(self):
- self.base = '/sys/kernel/debug/kvm'
- self._fields = walkdir(self.base)[2]
+ self._fields = walkdir(PATH_DEBUGFS)[2]
def fields(self):
return self._fields
def select(self, fields):
self._fields = fields
def read(self):
def val(key):
- return int(file(self.base + '/' + key).read())
+ return int(file(PATH_DEBUGFS + '/' + key).read())
return dict([(key, val(key)) for key in self._fields])
VMX_EXIT_REASONS = {
@@ -328,7 +327,8 @@ def _perf_event_open(attr, pid, cpu, group_fd, flags):
PERF_TYPE_TRACEPOINT = 2
PERF_FORMAT_GROUP = 1 << 3
-sys_tracing = '/sys/kernel/debug/tracing'
+PATH_TRACING = '/sys/kernel/debug/tracing'
+PATH_DEBUGFS = '/sys/kernel/debug/kvm'
class Group(object):
def __init__(self, cpu):
@@ -353,7 +353,7 @@ class Event(object):
attr = perf_event_attr()
attr.type = PERF_TYPE_TRACEPOINT
attr.size = ctypes.sizeof(attr)
- id_path = os.path.join(sys_tracing, 'events', event_set,
+ id_path = os.path.join(PATH_TRACING, 'events', event_set,
tracepoint, 'id')
id = int(file(id_path).read())
attr.config = id
@@ -378,7 +378,7 @@ class Event(object):
class TracepointProvider(object):
def __init__(self):
- path = os.path.join(sys_tracing, 'events', 'kvm')
+ path = os.path.join(PATH_TRACING, 'events', 'kvm')
fields = walkdir(path)[1]
extra = []
for f in fields:
@@ -476,7 +476,7 @@ class Stats:
if not os.access('/sys/kernel/debug', os.F_OK):
print 'Please enable CONFIG_DEBUG_FS in your kernel'
sys.exit(1)
-if not os.access('/sys/kernel/debug/kvm', os.F_OK):
+if not os.access(PATH_DEBUGFS, os.F_OK):
print "Please mount debugfs ('mount -t debugfs debugfs /sys/kernel/debug')"
print "and ensure the kvm modules are loaded"
sys.exit(1)
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 08/34] scripts/kvm/kvm_stat: Improve debugfs access checking
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (6 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 07/34] scripts/kvm/kvm_stat: Cleanup of path variables Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 09/34] scripts/kvm/kvm_stat: Introduce main function Janosch Frank
` (26 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Access checking with F_OK was replaced with the better readable
os.path.exists().
On Linux exists() returns False when the user doesn't have sufficient
permissions for statting the directory. Therefore the error message
now states that sufficient rights are needed when the check fails.
Also added check for /sys/kernel/debug/tracing/.
---
scripts/kvm/kvm_stat | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index e7f3595..746a49b 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -473,12 +473,18 @@ class Stats:
self.values[key] = (newval, newdelta)
return self.values
-if not os.access('/sys/kernel/debug', os.F_OK):
- print 'Please enable CONFIG_DEBUG_FS in your kernel'
+if not os.path.exists('/sys/kernel/debug'):
+ sys.stderr.write('Please enable CONFIG_DEBUG_FS in your kernel.')
sys.exit(1)
-if not os.access(PATH_DEBUGFS, os.F_OK):
- print "Please mount debugfs ('mount -t debugfs debugfs /sys/kernel/debug')"
- print "and ensure the kvm modules are loaded"
+if not os.path.exists(PATH_DEBUGFS):
+ sys.stderr.write("Please make sure, that debugfs is mounted and "
+ "readable by the current user:\n"
+ "('mount -t debugfs debugfs /sys/kernel/debug')\n"
+ "Also ensure, that the kvm modules are loaded.\n")
+ sys.exit(1)
+if not os.path.exists(PATH_TRACING):
+ sys.stderr.write("Please make {0} readable by the current user.\n"
+ .format(PATH_TRACING))
sys.exit(1)
LABEL_WIDTH = 40
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 09/34] scripts/kvm/kvm_stat: Introduce main function
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (7 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 08/34] scripts/kvm/kvm_stat: Improve debugfs access checking Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 10/34] scripts/kvm/kvm_stat: Fix spaces around keyword assignments Janosch Frank
` (25 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The main function should be the main location for initialization and
helps encapsulating variables into a scope. This way they don't have
to be global and might be mistaken for local ones.
As the providers variable is scoped now it can't be accessed from
within the Stats class. Hence, the global access to the variable was
changed to a local one.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
---
scripts/kvm/kvm_stat | 145 +++++++++++++++++++++++++++------------------------
1 file changed, 78 insertions(+), 67 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 746a49b..1eb982a 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -285,8 +285,6 @@ def detect_platform():
s390_init()
return
-detect_platform()
-
def walkdir(path):
"""Returns os.walk() data for specified directory.
@@ -453,7 +451,7 @@ class Stats:
return True
return re.match(self.fields_filter, key) is not None
self.values = dict()
- for d in providers:
+ for d in self.providers:
provider_fields = [key for key in d.fields() if wanted(key)]
for key in provider_fields:
self.values[key] = None
@@ -462,7 +460,7 @@ class Stats:
self.fields_filter = fields_filter
self._update()
def get(self):
- for d in providers:
+ for d in self.providers:
new = d.read()
for key in d.fields():
oldval = self.values.get(key, (0, 0))
@@ -473,20 +471,6 @@ class Stats:
self.values[key] = (newval, newdelta)
return self.values
-if not os.path.exists('/sys/kernel/debug'):
- sys.stderr.write('Please enable CONFIG_DEBUG_FS in your kernel.')
- sys.exit(1)
-if not os.path.exists(PATH_DEBUGFS):
- sys.stderr.write("Please make sure, that debugfs is mounted and "
- "readable by the current user:\n"
- "('mount -t debugfs debugfs /sys/kernel/debug')\n"
- "Also ensure, that the kvm modules are loaded.\n")
- sys.exit(1)
-if not os.path.exists(PATH_TRACING):
- sys.stderr.write("Please make {0} readable by the current user.\n"
- .format(PATH_TRACING))
- sys.exit(1)
-
LABEL_WIDTH = 40
NUMBER_WIDTH = 10
@@ -576,56 +560,83 @@ def log(stats):
statline()
line += 1
-options = optparse.OptionParser()
-options.add_option('-1', '--once', '--batch',
- action = 'store_true',
- default = False,
- dest = 'once',
- help = 'run in batch mode for one second',
- )
-options.add_option('-l', '--log',
- action = 'store_true',
- default = False,
- dest = 'log',
- help = 'run in logging mode (like vmstat)',
- )
-options.add_option('-t', '--tracepoints',
- action = 'store_true',
- default = False,
- dest = 'tracepoints',
- help = 'retrieve statistics from tracepoints',
- )
-options.add_option('-d', '--debugfs',
- action = 'store_true',
- default = False,
- dest = 'debugfs',
- help = 'retrieve statistics from debugfs',
- )
-options.add_option('-f', '--fields',
- action = 'store',
- default = None,
- dest = 'fields',
- help = 'fields to display (regex)',
- )
-(options, args) = options.parse_args(sys.argv)
+def get_options():
+ optparser = optparse.OptionParser()
+ optparser.add_option('-1', '--once', '--batch',
+ action = 'store_true',
+ default = False,
+ dest = 'once',
+ help = 'run in batch mode for one second',
+ )
+ optparser.add_option('-l', '--log',
+ action = 'store_true',
+ default = False,
+ dest = 'log',
+ help = 'run in logging mode (like vmstat)',
+ )
+ optparser.add_option('-t', '--tracepoints',
+ action = 'store_true',
+ default = False,
+ dest = 'tracepoints',
+ help = 'retrieve statistics from tracepoints',
+ )
+ optparser.add_option('-d', '--debugfs',
+ action = 'store_true',
+ default = False,
+ dest = 'debugfs',
+ help = 'retrieve statistics from debugfs',
+ )
+ optparser.add_option('-f', '--fields',
+ action = 'store',
+ default = None,
+ dest = 'fields',
+ help = 'fields to display (regex)',
+ )
+ (options, _) = optparser.parse_args(sys.argv)
+ return options
-providers = []
-if options.tracepoints:
- providers.append(TracepointProvider())
-if options.debugfs:
- providers.append(DebugfsProvider())
+def get_providers(options):
+ providers = []
-if len(providers) == 0:
- try:
- providers = [TracepointProvider()]
- except:
- providers = [DebugfsProvider()]
+ if options.tracepoints:
+ providers.append(TracepointProvider())
+ if options.debugfs:
+ providers.append(DebugfsProvider())
+ if len(providers) == 0:
+ providers.append(TracepointProvider())
-stats = Stats(providers, fields = options.fields)
+ return providers
-if options.log:
- log(stats)
-elif not options.once:
- curses.wrapper(tui, stats)
-else:
- batch(stats)
+def check_access():
+ if not os.path.exists('/sys/kernel/debug'):
+ sys.stderr.write('Please enable CONFIG_DEBUG_FS in your kernel.')
+ sys.exit(1)
+
+ if not os.path.exists(PATH_DEBUGFS):
+ sys.stderr.write("Please make sure, that debugfs is mounted and "
+ "readable by the current user:\n"
+ "('mount -t debugfs debugfs /sys/kernel/debug')\n"
+ "Also ensure, that the kvm modules are loaded.\n")
+ sys.exit(1)
+
+ if not os.path.exists(PATH_TRACING):
+ sys.stderr.write("Please make {0} readable by the current user.\n"
+ .format(PATH_TRACING))
+ sys.exit(1)
+
+def main():
+ check_access()
+ detect_platform()
+ options = get_options()
+ providers = get_providers(options)
+ stats = Stats(providers, fields = options.fields)
+
+ if options.log:
+ log(stats)
+ elif not options.once:
+ curses.wrapper(tui, stats)
+ else:
+ batch(stats)
+
+if __name__ == "__main__":
+ main()
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 10/34] scripts/kvm/kvm_stat: Fix spaces around keyword assignments
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (8 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 09/34] scripts/kvm/kvm_stat: Introduce main function Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 11/34] scripts/kvm/kvm_stat: Rename variables that redefine globals Janosch Frank
` (24 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Keyword assignments should not not have spaces around the equal
character according to PEP8.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
---
scripts/kvm/kvm_stat | 62 ++++++++++++++++++++++++++--------------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 1eb982a..92e09fb 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -333,10 +333,10 @@ class Group(object):
self.events = []
self.group_leader = None
self.cpu = cpu
- def add_event(self, name, event_set, tracepoint, filter = None):
- self.events.append(Event(group = self,
- name = name, event_set = event_set,
- tracepoint = tracepoint, filter = filter))
+ def add_event(self, name, event_set, tracepoint, filter=None):
+ self.events.append(Event(group=self,
+ name=name, event_set=event_set,
+ tracepoint=tracepoint, filter=filter))
if len(self.events) == 1:
self.file = os.fdopen(self.events[0].fd)
def read(self):
@@ -346,7 +346,7 @@ class Group(object):
struct.unpack(fmt, self.file.read(bytes))))
class Event(object):
- def __init__(self, group, name, event_set, tracepoint, filter = None):
+ def __init__(self, group, name, event_set, tracepoint, filter=None):
self.name = name
attr = perf_event_attr()
attr.type = PERF_TYPE_TRACEPOINT
@@ -421,9 +421,9 @@ class TracepointProvider(object):
tracepoint, sub = m.groups()
filter = '%s==%d\0' % (filters[tracepoint][0],
filters[tracepoint][1][sub])
- event = group.add_event(name, event_set = 'kvm',
- tracepoint = tracepoint,
- filter = filter)
+ event = group.add_event(name, event_set='kvm',
+ tracepoint=tracepoint,
+ filter=filter)
self.group_leaders.append(group)
def select(self, fields):
for group in self.group_leaders:
@@ -441,7 +441,7 @@ class TracepointProvider(object):
return ret
class Stats:
- def __init__(self, providers, fields = None):
+ def __init__(self, providers, fields=None):
self.providers = providers
self.fields_filter = fields
self._update()
@@ -499,7 +499,7 @@ def tui(screen, stats):
return (-s[x][1], -s[x][0])
else:
return (0, -s[x][0])
- for key in sorted(s.keys(), key = sortkey):
+ for key in sorted(s.keys(), key=sortkey):
if row >= screen.getmaxyx()[0]:
break
values = s[key]
@@ -563,34 +563,34 @@ def log(stats):
def get_options():
optparser = optparse.OptionParser()
optparser.add_option('-1', '--once', '--batch',
- action = 'store_true',
- default = False,
- dest = 'once',
- help = 'run in batch mode for one second',
+ action='store_true',
+ default=False,
+ dest='once',
+ help='run in batch mode for one second',
)
optparser.add_option('-l', '--log',
- action = 'store_true',
- default = False,
- dest = 'log',
- help = 'run in logging mode (like vmstat)',
+ action='store_true',
+ default=False,
+ dest='log',
+ help='run in logging mode (like vmstat)',
)
optparser.add_option('-t', '--tracepoints',
- action = 'store_true',
- default = False,
- dest = 'tracepoints',
- help = 'retrieve statistics from tracepoints',
+ action='store_true',
+ default=False,
+ dest='tracepoints',
+ help='retrieve statistics from tracepoints',
)
optparser.add_option('-d', '--debugfs',
- action = 'store_true',
- default = False,
- dest = 'debugfs',
- help = 'retrieve statistics from debugfs',
+ action='store_true',
+ default=False,
+ dest='debugfs',
+ help='retrieve statistics from debugfs',
)
optparser.add_option('-f', '--fields',
- action = 'store',
- default = None,
- dest = 'fields',
- help = 'fields to display (regex)',
+ action='store',
+ default=None,
+ dest='fields',
+ help='fields to display (regex)',
)
(options, _) = optparser.parse_args(sys.argv)
return options
@@ -629,7 +629,7 @@ def main():
detect_platform()
options = get_options()
providers = get_providers(options)
- stats = Stats(providers, fields = options.fields)
+ stats = Stats(providers, fields=options.fields)
if options.log:
log(stats)
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 11/34] scripts/kvm/kvm_stat: Rename variables that redefine globals
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (9 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 10/34] scripts/kvm/kvm_stat: Fix spaces around keyword assignments Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 12/34] scripts/kvm/kvm_stat: Moved DebugfsProvider Janosch Frank
` (23 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Filter, id and byte are builtin python modules which should not be
redefined by local variables.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
---
scripts/kvm/kvm_stat | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 92e09fb..d18418a 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -333,20 +333,21 @@ class Group(object):
self.events = []
self.group_leader = None
self.cpu = cpu
- def add_event(self, name, event_set, tracepoint, filter=None):
+ def add_event(self, name, event_set, tracepoint, tracefilter=None):
self.events.append(Event(group=self,
name=name, event_set=event_set,
- tracepoint=tracepoint, filter=filter))
+ tracepoint=tracepoint,
+ tracefilter=tracefilter))
if len(self.events) == 1:
self.file = os.fdopen(self.events[0].fd)
def read(self):
- bytes = 8 * (1 + len(self.events))
+ length = 8 * (1 + len(self.events))
fmt = 'xxxxxxxx' + 'q' * len(self.events)
return dict(zip([event.name for event in self.events],
- struct.unpack(fmt, self.file.read(bytes))))
+ struct.unpack(fmt, self.file.read(length))))
class Event(object):
- def __init__(self, group, name, event_set, tracepoint, filter=None):
+ def __init__(self, group, name, event_set, tracepoint, tracefilter=None):
self.name = name
attr = perf_event_attr()
attr.type = PERF_TYPE_TRACEPOINT
@@ -364,8 +365,8 @@ class Event(object):
if fd == -1:
err = get_errno()[0]
raise Exception('perf_event_open failed, errno = ' + err.__str__())
- if filter:
- fcntl.ioctl(fd, IOCTL_NUMBERS['SET_FILTER'], filter)
+ if tracefilter:
+ fcntl.ioctl(fd, IOCTL_NUMBERS['SET_FILTER'], tracefilter)
self.fd = fd
def enable(self):
fcntl.ioctl(self.fd, IOCTL_NUMBERS['ENABLE'], 0)
@@ -415,15 +416,15 @@ class TracepointProvider(object):
group = Group(cpu)
for name in _fields:
tracepoint = name
- filter = None
+ tracefilter = None
m = re.match(r'(.*)\((.*)\)', name)
if m:
tracepoint, sub = m.groups()
- filter = '%s==%d\0' % (filters[tracepoint][0],
- filters[tracepoint][1][sub])
+ tracefilter = '%s==%d\0' % (filters[tracepoint][0],
+ filters[tracepoint][1][sub])
event = group.add_event(name, event_set='kvm',
tracepoint=tracepoint,
- filter=filter)
+ tracefilter=tracefilter)
self.group_leaders.append(group)
def select(self, fields):
for group in self.group_leaders:
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 12/34] scripts/kvm/kvm_stat: Moved DebugfsProvider
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (10 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 11/34] scripts/kvm/kvm_stat: Rename variables that redefine globals Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 13/34] scripts/kvm/kvm_stat: Fixup syscall error reporting Janosch Frank
` (22 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
When it is next to the TracepointProvider less scrolling is needed to
change related, surrounding code.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
---
scripts/kvm/kvm_stat | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index d18418a..83d15a1 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -23,18 +23,6 @@ import struct
import re
from collections import defaultdict
-class DebugfsProvider(object):
- def __init__(self):
- self._fields = walkdir(PATH_DEBUGFS)[2]
- def fields(self):
- return self._fields
- def select(self, fields):
- self._fields = fields
- def read(self):
- def val(key):
- return int(file(PATH_DEBUGFS + '/' + key).read())
- return dict([(key, val(key)) for key in self._fields])
-
VMX_EXIT_REASONS = {
'EXCEPTION_NMI': 0,
'EXTERNAL_INTERRUPT': 1,
@@ -441,6 +429,18 @@ class TracepointProvider(object):
ret[name] += val
return ret
+class DebugfsProvider(object):
+ def __init__(self):
+ self._fields = walkdir(PATH_DEBUGFS)[2]
+ def fields(self):
+ return self._fields
+ def select(self, fields):
+ self._fields = fields
+ def read(self):
+ def val(key):
+ return int(file(PATH_DEBUGFS + '/' + key).read())
+ return dict([(key, val(key)) for key in self._fields])
+
class Stats:
def __init__(self, providers, fields=None):
self.providers = providers
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 13/34] scripts/kvm/kvm_stat: Fixup syscall error reporting
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (11 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 12/34] scripts/kvm/kvm_stat: Moved DebugfsProvider Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 14/34] scripts/kvm/kvm_stat: Set sensible no. files rlimit Janosch Frank
` (21 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
In 2008 a patch was written that introduced ctypes.get_errno() and
set_errno() as official interfaces to the libc errno variable. Using
them we can avoid accessing private libc variables.
The patch was included in python 2.6.
Also we need to raise the right exception, with the right parameters
and a helpful message.
---
scripts/kvm/kvm_stat | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 83d15a1..62ca853 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -287,10 +287,8 @@ filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS)
if EXIT_REASONS:
filters['kvm_exit'] = ('exit_reason', EXIT_REASONS)
-libc = ctypes.CDLL('libc.so.6')
+libc = ctypes.CDLL('libc.so.6', use_errno=True)
syscall = libc.syscall
-get_errno = libc.__errno_location
-get_errno.restype = ctypes.POINTER(ctypes.c_int)
class perf_event_attr(ctypes.Structure):
_fields_ = [('type', ctypes.c_uint32),
@@ -351,8 +349,9 @@ class Event(object):
group_leader = group.events[0].fd
fd = _perf_event_open(attr, -1, group.cpu, group_leader, 0)
if fd == -1:
- err = get_errno()[0]
- raise Exception('perf_event_open failed, errno = ' + err.__str__())
+ err = ctypes.get_errno()
+ raise OSError(err, os.os.strerror(err),
+ 'while calling sys_perf_event_open().')
if tracefilter:
fcntl.ioctl(fd, IOCTL_NUMBERS['SET_FILTER'], tracefilter)
self.fd = fd
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 14/34] scripts/kvm/kvm_stat: Set sensible no. files rlimit
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (12 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 13/34] scripts/kvm/kvm_stat: Fixup syscall error reporting Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 15/34] scripts/kvm/kvm_stat: Cleanup of platform detection Janosch Frank
` (20 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
As num cpus * 1000 is NOT a sensible rlimit, we need to calculate a
more accurate rlimit.
The number of open files is directly dependent on the cpu count and on
the number of trace points per cpu. A additional constant works as a
buffer for files that are needed by python or do get opened when the
script runs.
Hence we have:
cpus * traces + constant
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
---
scripts/kvm/kvm_stat | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 62ca853..b8c2a8f 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -395,8 +395,15 @@ class TracepointProvider(object):
def _setup(self, _fields):
self._fields = _fields
cpus = self._online_cpus()
- nfiles = len(cpus) * 1000
- resource.setrlimit(resource.RLIMIT_NOFILE, (nfiles, nfiles))
+
+ # The constant is needed as a buffer for python libs, std
+ # streams and other files that the script opens.
+ rlimit = len(cpus) * len(_fields) + 50
+ try:
+ resource.setrlimit(resource.RLIMIT_NOFILE, (rlimit, rlimit))
+ except ValueError:
+ sys.exit("NOFILE rlimit could not be raised to {0}".format(rlimit))
+
events = []
self.group_leaders = []
for cpu in cpus:
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 15/34] scripts/kvm/kvm_stat: Cleanup of platform detection
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (13 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 14/34] scripts/kvm/kvm_stat: Set sensible no. files rlimit Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 16/34] scripts/kvm/kvm_stat: Make cpu detection a function Janosch Frank
` (19 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
s390 machines can also be detected via uname -m, i.e. python's
os.uname, no need for more complicated checks.
Calling uname once and saving its value for multiple checks is
perfectly sufficient. We don't expect the machine's architecture to
change when the script is running anyway.
On multi-cpu systems x86_init currently will get called multiple
times, returning makes sure we don't waste cicles on that.
---
scripts/kvm/kvm_stat | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index b8c2a8f..4cb18e1 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -254,24 +254,21 @@ def aarch64_init():
EXIT_REASONS = AARCH64_EXIT_REASONS
def detect_platform():
- if os.uname()[4].startswith('ppc'):
- ppc_init()
- return
- elif os.uname()[4].startswith('aarch64'):
- aarch64_init()
- return
+ machine = os.uname()[4]
- for line in file('/proc/cpuinfo').readlines():
- if line.startswith('flags'):
- for flag in line.split():
- if flag in X86_EXIT_REASONS:
- x86_init(flag)
- return
- elif line.startswith('vendor_id'):
- for flag in line.split():
- if flag == 'IBM/S390':
- s390_init()
- return
+ if machine.startswith('ppc'):
+ ppc_init()
+ elif machine.startswith('aarch64'):
+ aarch64_init()
+ elif machine.startswith('s390'):
+ s390_init()
+ else:
+ for line in file('/proc/cpuinfo').readlines():
+ if line.startswith('flags'):
+ for flag in line.split():
+ if flag in X86_EXIT_REASONS:
+ x86_init(flag)
+ return
def walkdir(path):
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 16/34] scripts/kvm/kvm_stat: Make cpu detection a function
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (14 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 15/34] scripts/kvm/kvm_stat: Cleanup of platform detection Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 17/34] scripts/kvm/kvm_stat: Rename _perf_event_open Janosch Frank
` (18 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The online cpus detection method is in the Stats class but does not
use any class variables.
Moving it out of the class to the platform detection function makes
the Stats class more readable.
---
scripts/kvm/kvm_stat | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 4cb18e1..ef23474 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -279,6 +279,20 @@ def walkdir(path):
"""
return next(os.walk(path))
+
+def get_online_cpus():
+ cpulist = []
+ pattern = r'cpu([0-9]+)'
+ basedir = '/sys/devices/system/cpu'
+ for entry in os.listdir(basedir):
+ match = re.match(pattern, entry)
+ if not match:
+ continue
+ path = os.path.join(basedir, entry, 'online')
+ if os.path.isfile(path) and open(path).read().strip() == '1':
+ cpulist.append(int(match.group(1)))
+ return cpulist
+
filters = {}
filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS)
if EXIT_REASONS:
@@ -375,23 +389,9 @@ class TracepointProvider(object):
def fields(self):
return self._fields
- def _online_cpus(self):
- l = []
- pattern = r'cpu([0-9]+)'
- basedir = '/sys/devices/system/cpu'
- for entry in os.listdir(basedir):
- match = re.match(pattern, entry)
- if not match:
- continue
- path = os.path.join(basedir, entry, 'online')
- if os.path.exists(path) and open(path).read().strip() != '1':
- continue
- l.append(int(match.group(1)))
- return l
-
def _setup(self, _fields):
self._fields = _fields
- cpus = self._online_cpus()
+ cpus = get_online_cpus()
# The constant is needed as a buffer for python libs, std
# streams and other files that the script opens.
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 17/34] scripts/kvm/kvm_stat: Rename _perf_event_open
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (15 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 16/34] scripts/kvm/kvm_stat: Make cpu detection a function Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 18/34] scripts/kvm/kvm_stat: Introduce properties for providers Janosch Frank
` (17 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The underscore in front of the function name does not comply with the
python coding guidelines.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
---
scripts/kvm/kvm_stat | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index ef23474..59fa39c 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -314,7 +314,7 @@ class perf_event_attr(ctypes.Structure):
('bp_addr', ctypes.c_uint64),
('bp_len', ctypes.c_uint64),
]
-def _perf_event_open(attr, pid, cpu, group_fd, flags):
+def perf_event_open(attr, pid, cpu, group_fd, flags):
return syscall(SC_PERF_EVT_OPEN, ctypes.pointer(attr), ctypes.c_int(pid),
ctypes.c_int(cpu), ctypes.c_int(group_fd),
ctypes.c_long(flags))
@@ -358,7 +358,7 @@ class Event(object):
group_leader = -1
if group.events:
group_leader = group.events[0].fd
- fd = _perf_event_open(attr, -1, group.cpu, group_leader, 0)
+ fd = perf_event_open(attr, -1, group.cpu, group_leader, 0)
if fd == -1:
err = ctypes.get_errno()
raise OSError(err, os.os.strerror(err),
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 18/34] scripts/kvm/kvm_stat: Introduce properties for providers
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (16 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 17/34] scripts/kvm/kvm_stat: Rename _perf_event_open Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 19/34] scripts/kvm/kvm_stat: Cleanup of TracepointProvider Janosch Frank
` (16 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
As previous commit authors used a mixture of setters/getters and
direct access to class variables consolidating them the python way
improved readability.
Properties allow us to assign a value to a class variable through a
setter without the need to call the setter ourselves.
Reviewed-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>
---
scripts/kvm/kvm_stat | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 59fa39c..8414b53 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -385,9 +385,7 @@ class TracepointProvider(object):
extra.append(f + '(' + name + ')')
fields += extra
self._setup(fields)
- self.select(fields)
- def fields(self):
- return self._fields
+ self.fields = fields
def _setup(self, _fields):
self._fields = _fields
@@ -417,7 +415,14 @@ class TracepointProvider(object):
tracepoint=tracepoint,
tracefilter=tracefilter)
self.group_leaders.append(group)
- def select(self, fields):
+
+ @property
+ def fields(self):
+ return self._fields
+
+ @fields.setter
+ def fields(self, fields):
+ self._fields = fields
for group in self.group_leaders:
for event in group.events:
if event.name in fields:
@@ -425,6 +430,7 @@ class TracepointProvider(object):
event.enable()
else:
event.disable()
+
def read(self):
ret = defaultdict(int)
for group in self.group_leaders:
@@ -435,10 +441,15 @@ class TracepointProvider(object):
class DebugfsProvider(object):
def __init__(self):
self._fields = walkdir(PATH_DEBUGFS)[2]
+
+ @property
def fields(self):
return self._fields
- def select(self, fields):
+
+ @fields.setter
+ def fields(self, fields):
self._fields = fields
+
def read(self):
def val(key):
return int(file(PATH_DEBUGFS + '/' + key).read())
@@ -456,17 +467,17 @@ class Stats:
return re.match(self.fields_filter, key) is not None
self.values = dict()
for d in self.providers:
- provider_fields = [key for key in d.fields() if wanted(key)]
+ provider_fields = [key for key in d.fields if wanted(key)]
for key in provider_fields:
self.values[key] = None
- d.select(provider_fields)
+ d.fields = provider_fields
def set_fields_filter(self, fields_filter):
self.fields_filter = fields_filter
self._update()
def get(self):
for d in self.providers:
new = d.read()
- for key in d.fields():
+ for key in d.fields:
oldval = self.values.get(key, (0, 0))
newval = new[key]
newdelta = None
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 19/34] scripts/kvm/kvm_stat: Cleanup of TracepointProvider
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (17 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 18/34] scripts/kvm/kvm_stat: Introduce properties for providers Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 20/34] scripts/kvm/kvm_stat: Cleanup cpu list retrieval Janosch Frank
` (15 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Variables with bad names like f and m were renamed to their full name,
so it is clearer which data they contain.
Unneeded variables were removed and the field generating code was
moved in an own function.
dict.iteritems() was removed as directly iterating over a dictionary
also yields the needed keys.
---
scripts/kvm/kvm_stat | 40 +++++++++++++++++++++-------------------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 8414b53..7bd76b3 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -375,45 +375,47 @@ class Event(object):
class TracepointProvider(object):
def __init__(self):
+ self.group_leaders = []
+ self._fields = self.get_available_fields()
+ self.setup_traces()
+ self.fields = self._fields
+
+ def get_available_fields(self):
path = os.path.join(PATH_TRACING, 'events', 'kvm')
fields = walkdir(path)[1]
extra = []
- for f in fields:
- if f in filters:
- subfield, values = filters[f]
- for name, number in values.iteritems():
- extra.append(f + '(' + name + ')')
+ for field in fields:
+ if field in filters:
+ filter_name_, filter_dicts = filters[field]
+ for name in filter_dicts:
+ extra.append(field + '(' + name + ')')
fields += extra
- self._setup(fields)
- self.fields = fields
+ return fields
- def _setup(self, _fields):
- self._fields = _fields
+ def setup_traces(self):
cpus = get_online_cpus()
# The constant is needed as a buffer for python libs, std
# streams and other files that the script opens.
- rlimit = len(cpus) * len(_fields) + 50
+ rlimit = len(cpus) * len(self._fields) + 50
try:
resource.setrlimit(resource.RLIMIT_NOFILE, (rlimit, rlimit))
except ValueError:
sys.exit("NOFILE rlimit could not be raised to {0}".format(rlimit))
- events = []
- self.group_leaders = []
for cpu in cpus:
group = Group(cpu)
- for name in _fields:
+ for name in self._fields:
tracepoint = name
tracefilter = None
- m = re.match(r'(.*)\((.*)\)', name)
- if m:
- tracepoint, sub = m.groups()
+ match = re.match(r'(.*)\((.*)\)', name)
+ if match:
+ tracepoint, sub = match.groups()
tracefilter = '%s==%d\0' % (filters[tracepoint][0],
filters[tracepoint][1][sub])
- event = group.add_event(name, event_set='kvm',
- tracepoint=tracepoint,
- tracefilter=tracefilter)
+ group.add_event(name, event_set='kvm',
+ tracepoint=tracepoint,
+ tracefilter=tracefilter)
self.group_leaders.append(group)
@property
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 20/34] scripts/kvm/kvm_stat: Cleanup cpu list retrieval
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (18 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 19/34] scripts/kvm/kvm_stat: Cleanup of TracepointProvider Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2016-01-07 15:21 ` Paolo Bonzini
2015-12-10 12:12 ` [Qemu-devel] [PATCH 21/34] scripts/kvm/kvm_stat: Encapsulate filters variable Janosch Frank
` (14 subsequent siblings)
34 siblings, 1 reply; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Reading /sys/devices/system/cpu/online makes opening the cpu
directories unnecessary and works on more/older systems.
---
scripts/kvm/kvm_stat | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 7bd76b3..20fc5c9 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -282,15 +282,18 @@ def walkdir(path):
def get_online_cpus():
cpulist = []
- pattern = r'cpu([0-9]+)'
- basedir = '/sys/devices/system/cpu'
- for entry in os.listdir(basedir):
- match = re.match(pattern, entry)
- if not match:
- continue
- path = os.path.join(basedir, entry, 'online')
- if os.path.isfile(path) and open(path).read().strip() == '1':
- cpulist.append(int(match.group(1)))
+
+ with open('/sys/devices/system/cpu/online') as cpu_list:
+ cpu_string = cpu_list.readline()
+ cpus = cpu_string.split(',')
+
+ for cpu in cpus:
+ if '-' not in cpu:
+ cpulist.append(int(cpu))
+ else:
+ cpu_range = cpu.split('-')
+ cpulist.extend(range(int(cpu_range[0]),
+ int(cpu_range[1]) + 1))
return cpulist
filters = {}
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 21/34] scripts/kvm/kvm_stat: Encapsulate filters variable
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (19 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 20/34] scripts/kvm/kvm_stat: Cleanup cpu list retrieval Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 22/34] scripts/kvm/kvm_stat: Cleanup of Stats class Janosch Frank
` (13 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The variable was only used in one class but still was defined
globally. Additionaly the detect_platform routine which prepares the
data that goes into the variable was called on each start of the
script, no matter if the class was needed.
To make the variable local to the TracepointProvider class, a new
function that calls detect_platform and returns the filters was
introduced.
---
scripts/kvm/kvm_stat | 23 ++++++++++++++---------
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 20fc5c9..868c6a5 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -296,10 +296,14 @@ def get_online_cpus():
int(cpu_range[1]) + 1))
return cpulist
-filters = {}
-filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS)
-if EXIT_REASONS:
- filters['kvm_exit'] = ('exit_reason', EXIT_REASONS)
+
+def get_filters():
+ detect_platform()
+ filters = {}
+ filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS)
+ if EXIT_REASONS:
+ filters['kvm_exit'] = ('exit_reason', EXIT_REASONS)
+ return filters
libc = ctypes.CDLL('libc.so.6', use_errno=True)
syscall = libc.syscall
@@ -379,6 +383,7 @@ class Event(object):
class TracepointProvider(object):
def __init__(self):
self.group_leaders = []
+ self.filters = get_filters()
self._fields = self.get_available_fields()
self.setup_traces()
self.fields = self._fields
@@ -388,8 +393,8 @@ class TracepointProvider(object):
fields = walkdir(path)[1]
extra = []
for field in fields:
- if field in filters:
- filter_name_, filter_dicts = filters[field]
+ if field in self.filters:
+ filter_name_, filter_dicts = self.filters[field]
for name in filter_dicts:
extra.append(field + '(' + name + ')')
fields += extra
@@ -414,8 +419,9 @@ class TracepointProvider(object):
match = re.match(r'(.*)\((.*)\)', name)
if match:
tracepoint, sub = match.groups()
- tracefilter = '%s==%d\0' % (filters[tracepoint][0],
- filters[tracepoint][1][sub])
+ tracefilter = ('%s==%d\0' %
+ (self.filters[tracepoint][0],
+ self.filters[tracepoint][1][sub]))
group.add_event(name, event_set='kvm',
tracepoint=tracepoint,
tracefilter=tracefilter)
@@ -646,7 +652,6 @@ def check_access():
def main():
check_access()
- detect_platform()
options = get_options()
providers = get_providers(options)
stats = Stats(providers, fields=options.fields)
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 22/34] scripts/kvm/kvm_stat: Cleanup of Stats class
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (20 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 21/34] scripts/kvm/kvm_stat: Encapsulate filters variable Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 23/34] scripts/kvm/kvm_stat: Cleanup of Groups class Janosch Frank
` (12 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Converted class definition to new style and renamed improper named
variables.
Introduced property for fields_filter.
Moved member variable declaration to init, so one can see all class
variables when reading the init method.
Completely clear the values dict, as we don't need to keep single values.
---
scripts/kvm/kvm_stat | 52 +++++++++++++++++++++++++++++++---------------------
1 file changed, 31 insertions(+), 21 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 868c6a5..f14af44 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -466,31 +466,41 @@ class DebugfsProvider(object):
return int(file(PATH_DEBUGFS + '/' + key).read())
return dict([(key, val(key)) for key in self._fields])
-class Stats:
+class Stats(object):
def __init__(self, providers, fields=None):
self.providers = providers
- self.fields_filter = fields
- self._update()
- def _update(self):
+ self._fields_filter = fields
+ self.values = {}
+ self.update_provider_filters()
+
+ def update_provider_filters(self):
def wanted(key):
- if not self.fields_filter:
+ if not self._fields_filter:
return True
- return re.match(self.fields_filter, key) is not None
- self.values = dict()
- for d in self.providers:
- provider_fields = [key for key in d.fields if wanted(key)]
- for key in provider_fields:
- self.values[key] = None
- d.fields = provider_fields
- def set_fields_filter(self, fields_filter):
- self.fields_filter = fields_filter
- self._update()
+ return re.match(self._fields_filter, key) is not None
+
+ # As we reset the counters when updating the fields we can
+ # also clear the cache of old values.
+ self.values = {}
+ for provider in self.providers:
+ provider_fields = [key for key in provider.fields if wanted(key)]
+ provider.fields = provider_fields
+
+ @property
+ def fields_filter(self):
+ return self._fields_filter
+
+ @fields_filter.setter
+ def fields_filter(self, fields_filter):
+ self._fields_filter = fields_filter
+ self.update_provider_filters()
+
def get(self):
- for d in self.providers:
- new = d.read()
- for key in d.fields:
+ for provider in self.providers:
+ new = provider.read()
+ for key in provider.fields:
oldval = self.values.get(key, (0, 0))
- newval = new[key]
+ newval = new.get(key, 0)
newdelta = None
if oldval is not None:
newdelta = newval - oldval[0]
@@ -508,9 +518,9 @@ def tui(screen, stats):
def update_drilldown():
if not fields_filter:
if drilldown:
- stats.set_fields_filter(None)
+ stats.fields_filter = None
else:
- stats.set_fields_filter(r'^[^\(]*$')
+ stats.fields_filter = r'^[^\(]*$'
update_drilldown()
def refresh(sleeptime):
screen.erase()
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 23/34] scripts/kvm/kvm_stat: Cleanup of Groups class
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (21 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 22/34] scripts/kvm/kvm_stat: Cleanup of Stats class Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 24/34] scripts/kvm/kvm_stat: Cleanup of Event class Janosch Frank
` (11 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Introduced separating newlines for readability and removed special
treatment/variable of the group leader. Renamed fmt to read_format.
The group leader's file descriptor will not be turned into a file
object anymore, instead os.read is used to read from the descriptor.
---
scripts/kvm/kvm_stat | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index f14af44..5d45604 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -335,20 +335,20 @@ PATH_DEBUGFS = '/sys/kernel/debug/kvm'
class Group(object):
def __init__(self, cpu):
self.events = []
- self.group_leader = None
self.cpu = cpu
+
def add_event(self, name, event_set, tracepoint, tracefilter=None):
self.events.append(Event(group=self,
name=name, event_set=event_set,
tracepoint=tracepoint,
tracefilter=tracefilter))
- if len(self.events) == 1:
- self.file = os.fdopen(self.events[0].fd)
+
def read(self):
length = 8 * (1 + len(self.events))
- fmt = 'xxxxxxxx' + 'q' * len(self.events)
+ read_format = 'xxxxxxxx' + 'q' * len(self.events)
return dict(zip([event.name for event in self.events],
- struct.unpack(fmt, self.file.read(length))))
+ struct.unpack(read_format,
+ os.read(self.events[0].fd, length))))
class Event(object):
def __init__(self, group, name, event_set, tracepoint, tracefilter=None):
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 24/34] scripts/kvm/kvm_stat: Cleanup of Event class
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (22 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 23/34] scripts/kvm/kvm_stat: Cleanup of Groups class Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2016-01-07 15:25 ` Paolo Bonzini
2015-12-10 12:12 ` [Qemu-devel] [PATCH 25/34] scripts/kvm/kvm_stat: Group arch specific data Janosch Frank
` (10 subsequent siblings)
34 siblings, 1 reply; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Added additional newlines for readability.
Factored out attribute and event setup code into own methods. All
data necessary for setting up the events was consolidated into one
dictionary. That way we get rid of the large argument list of the
functions that handle the data.
Exchanged file() with preferred open().
---
scripts/kvm/kvm_stat | 73 +++++++++++++++++++++++++++++++++-------------------
1 file changed, 47 insertions(+), 26 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 5d45604..70a27da 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -333,15 +333,11 @@ PATH_TRACING = '/sys/kernel/debug/tracing'
PATH_DEBUGFS = '/sys/kernel/debug/kvm'
class Group(object):
- def __init__(self, cpu):
+ def __init__(self):
self.events = []
- self.cpu = cpu
- def add_event(self, name, event_set, tracepoint, tracefilter=None):
- self.events.append(Event(group=self,
- name=name, event_set=event_set,
- tracepoint=tracepoint,
- tracefilter=tracefilter))
+ def add_event(self, event_data):
+ self.events.append(Event(event_data))
def read(self):
length = 8 * (1 + len(self.events))
@@ -351,32 +347,50 @@ class Group(object):
os.read(self.events[0].fd, length))))
class Event(object):
- def __init__(self, group, name, event_set, tracepoint, tracefilter=None):
- self.name = name
- attr = perf_event_attr()
- attr.type = PERF_TYPE_TRACEPOINT
- attr.size = ctypes.sizeof(attr)
+ def __init__(self, event_data):
+ self.name = event_data['name']
+ self.fd = None
+ self.setup_event(event_data)
+
+ def setup_event_attribute(self, event_set, tracepoint):
id_path = os.path.join(PATH_TRACING, 'events', event_set,
tracepoint, 'id')
- id = int(file(id_path).read())
- attr.config = id
- attr.sample_period = 1
- attr.read_format = PERF_FORMAT_GROUP
+
+ event_attr = perf_event_attr()
+ event_attr.type = PERF_TYPE_TRACEPOINT
+ event_attr.size = ctypes.sizeof(event_attr)
+ event_attr.config = int(open(id_path).read())
+ event_attr.sample_period = 1
+ event_attr.read_format = PERF_FORMAT_GROUP
+ return event_attr
+
+ def setup_event(self, event_data):
+ event_attr = self.setup_event_attribute(event_data['set'],
+ event_data['tracepoint'])
+
group_leader = -1
- if group.events:
- group_leader = group.events[0].fd
- fd = perf_event_open(attr, -1, group.cpu, group_leader, 0)
+ if event_data['group'].events:
+ group_leader = event_data['group'].events[0].fd
+
+ fd = perf_event_open(event_attr, -1, event_data['cpu'],
+ group_leader, 0)
if fd == -1:
err = ctypes.get_errno()
- raise OSError(err, os.os.strerror(err),
+ raise OSError(err, os.strerror(err),
'while calling sys_perf_event_open().')
- if tracefilter:
- fcntl.ioctl(fd, IOCTL_NUMBERS['SET_FILTER'], tracefilter)
+
+ if event_data['filter']:
+ fcntl.ioctl(fd, IOCTL_NUMBERS['SET_FILTER'],
+ event_data['filter'])
+
self.fd = fd
+
def enable(self):
fcntl.ioctl(self.fd, IOCTL_NUMBERS['ENABLE'], 0)
+
def disable(self):
fcntl.ioctl(self.fd, IOCTL_NUMBERS['DISABLE'], 0)
+
def reset(self):
fcntl.ioctl(self.fd, IOCTL_NUMBERS['RESET'], 0)
@@ -412,7 +426,7 @@ class TracepointProvider(object):
sys.exit("NOFILE rlimit could not be raised to {0}".format(rlimit))
for cpu in cpus:
- group = Group(cpu)
+ group = Group()
for name in self._fields:
tracepoint = name
tracefilter = None
@@ -422,9 +436,16 @@ class TracepointProvider(object):
tracefilter = ('%s==%d\0' %
(self.filters[tracepoint][0],
self.filters[tracepoint][1][sub]))
- group.add_event(name, event_set='kvm',
- tracepoint=tracepoint,
- tracefilter=tracefilter)
+
+ event_data = {
+ 'cpu': cpu,
+ 'name': name,
+ 'group': group,
+ 'set': 'kvm',
+ 'tracepoint': tracepoint,
+ 'filter': tracefilter or None,
+ }
+ group.add_event(event_data)
self.group_leaders.append(group)
@property
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 25/34] scripts/kvm/kvm_stat: Group arch specific data
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (23 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 24/34] scripts/kvm/kvm_stat: Cleanup of Event class Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2016-01-07 15:30 ` Paolo Bonzini
2015-12-10 12:12 ` [Qemu-devel] [PATCH 26/34] scripts/kvm/kvm_stat: Remove unneeded X86_EXIT_REASONS Janosch Frank
` (9 subsequent siblings)
34 siblings, 1 reply; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Using global variables and multiple initialization functions for arch
specific data makes the code hard to read. By grouping them in the
Arch class we encapsulate and initialize them in one place.
---
scripts/kvm/kvm_stat | 106 +++++++++++++++++++++++++--------------------------
1 file changed, 51 insertions(+), 55 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 70a27da..d81e7b6 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -213,62 +213,59 @@ X86_EXIT_REASONS = {
'svm': SVM_EXIT_REASONS,
}
-SC_PERF_EVT_OPEN = None
-EXIT_REASONS = None
-IOCTL_NUMBERS = {
- 'SET_FILTER' : 0x40082406,
- 'ENABLE' : 0x00002400,
- 'DISABLE' : 0x00002401,
- 'RESET' : 0x00002403,
-}
+class Arch(object):
+ """Class that encapsulates global architecture specific data like
+ syscall and ioctl numbers.
-def x86_init(flag):
- global SC_PERF_EVT_OPEN
- global EXIT_REASONS
+ """
+ def __init__(self):
+ self.exit_reasons = None
+ self.sc_perf_evt_open = None
+ self.ioctl_numbers = {
+ 'SET_FILTER': 0x40082406,
+ 'ENABLE': 0x00002400,
+ 'DISABLE': 0x00002401,
+ 'RESET': 0x00002403,
+ }
+ self.set_arch_data()
- SC_PERF_EVT_OPEN = 298
- EXIT_REASONS = X86_EXIT_REASONS[flag]
+ def set_arch_data(self):
+ machine = os.uname()[4]
-def s390_init():
- global SC_PERF_EVT_OPEN
+ if machine.startswith('ppc'):
+ self.sc_perf_evt_open = 319
+ self.ioctl_numbers['ENABLE'] = 0x20002400
+ self.ioctl_numbers['DISABLE'] = 0x20002401
- SC_PERF_EVT_OPEN = 331
+ # PPC comes in 32 and 64 bit and some generated ioctl
+ # numbers depend on the wordsize.
+ char_ptr_size = ctypes.sizeof(ctypes.c_char_p)
+ self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16
-def ppc_init():
- global SC_PERF_EVT_OPEN
- global IOCTL_NUMBERS
+ elif machine.startswith('aarch64'):
+ self.sc_perf_evt_open = 241
+ self.exit_reasons = AARCH64_EXIT_REASONS
- SC_PERF_EVT_OPEN = 319
+ elif machine.startswith('s390'):
+ self.sc_perf_evt_open = 331
- IOCTL_NUMBERS['ENABLE'] = 0x20002400
- IOCTL_NUMBERS['DISABLE'] = 0x20002401
- IOCTL_NUMBERS['SET_FILTER'] = 0x80002406 | (ctypes.sizeof(ctypes.c_char_p)
- << 16)
+ else:
+ # X86_64
+ for line in open('/proc/cpuinfo'):
+ if not line.startswith('flags'):
+ continue
-def aarch64_init():
- global SC_PERF_EVT_OPEN
- global EXIT_REASONS
+ self.sc_perf_evt_open = 298
- SC_PERF_EVT_OPEN = 241
- EXIT_REASONS = AARCH64_EXIT_REASONS
+ flags = line.split()
+ if 'vmx' in flags:
+ self.exit_reasons = VMX_EXIT_REASONS
+ if 'svm' in flags:
+ self.exit_reasons = SVM_EXIT_REASONS
+ return
-def detect_platform():
- machine = os.uname()[4]
-
- if machine.startswith('ppc'):
- ppc_init()
- elif machine.startswith('aarch64'):
- aarch64_init()
- elif machine.startswith('s390'):
- s390_init()
- else:
- for line in file('/proc/cpuinfo').readlines():
- if line.startswith('flags'):
- for flag in line.split():
- if flag in X86_EXIT_REASONS:
- x86_init(flag)
- return
+ARCH = Arch()
def walkdir(path):
@@ -298,11 +295,10 @@ def get_online_cpus():
def get_filters():
- detect_platform()
filters = {}
filters['kvm_userspace_exit'] = ('reason', USERSPACE_EXIT_REASONS)
- if EXIT_REASONS:
- filters['kvm_exit'] = ('exit_reason', EXIT_REASONS)
+ if ARCH.exit_reasons:
+ filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
return filters
libc = ctypes.CDLL('libc.so.6', use_errno=True)
@@ -322,9 +318,9 @@ class perf_event_attr(ctypes.Structure):
('bp_len', ctypes.c_uint64),
]
def perf_event_open(attr, pid, cpu, group_fd, flags):
- return syscall(SC_PERF_EVT_OPEN, ctypes.pointer(attr), ctypes.c_int(pid),
- ctypes.c_int(cpu), ctypes.c_int(group_fd),
- ctypes.c_long(flags))
+ return syscall(ARCH.sc_perf_evt_open, ctypes.pointer(attr),
+ ctypes.c_int(pid), ctypes.c_int(cpu),
+ ctypes.c_int(group_fd), ctypes.c_long(flags))
PERF_TYPE_TRACEPOINT = 2
PERF_FORMAT_GROUP = 1 << 3
@@ -380,19 +376,19 @@ class Event(object):
'while calling sys_perf_event_open().')
if event_data['filter']:
- fcntl.ioctl(fd, IOCTL_NUMBERS['SET_FILTER'],
+ fcntl.ioctl(fd, ARCH.ioctl_numbers['SET_FILTER'],
event_data['filter'])
self.fd = fd
def enable(self):
- fcntl.ioctl(self.fd, IOCTL_NUMBERS['ENABLE'], 0)
+ fcntl.ioctl(self.fd, ARCH.ioctl_numbers['ENABLE'], 0)
def disable(self):
- fcntl.ioctl(self.fd, IOCTL_NUMBERS['DISABLE'], 0)
+ fcntl.ioctl(self.fd, ARCH.ioctl_numbers['DISABLE'], 0)
def reset(self):
- fcntl.ioctl(self.fd, IOCTL_NUMBERS['RESET'], 0)
+ fcntl.ioctl(self.fd, ARCH.ioctl_numbers['RESET'], 0)
class TracepointProvider(object):
def __init__(self):
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 26/34] scripts/kvm/kvm_stat: Remove unneeded X86_EXIT_REASONS
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (24 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 25/34] scripts/kvm/kvm_stat: Group arch specific data Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 27/34] scripts/kvm/kvm_stat: Make tui function a class Janosch Frank
` (8 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The architecture detection method directly accesses vmx and smv exit
reason constants. Therefore we don't need it anymore.
---
scripts/kvm/kvm_stat | 5 -----
1 file changed, 5 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index d81e7b6..345ead8 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -208,11 +208,6 @@ USERSPACE_EXIT_REASONS = {
'SYSTEM_EVENT': 24,
}
-X86_EXIT_REASONS = {
- 'vmx': VMX_EXIT_REASONS,
- 'svm': SVM_EXIT_REASONS,
-}
-
class Arch(object):
"""Class that encapsulates global architecture specific data like
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 27/34] scripts/kvm/kvm_stat: Make tui function a class
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (25 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 26/34] scripts/kvm/kvm_stat: Remove unneeded X86_EXIT_REASONS Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2016-01-07 15:40 ` Paolo Bonzini
2015-12-10 12:12 ` [Qemu-devel] [PATCH 28/34] scripts/kvm/kvm_stat: Fix output formatting Janosch Frank
` (7 subsequent siblings)
34 siblings, 1 reply; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The tui function itself had a few sub-functions and therefore
basically already was class-like. Making it an actual one with proper
methods improved readability.
The tui function lives on as a wrapper for the class.
Also renamed single character variable name, so the name reflects the
content.
---
scripts/kvm/kvm_stat | 99 +++++++++++++++++++++++++++++-----------------------
1 file changed, 56 insertions(+), 43 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 345ead8..4968941 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -522,63 +522,76 @@ class Stats(object):
LABEL_WIDTH = 40
NUMBER_WIDTH = 10
-def tui(screen, stats):
- curses.use_default_colors()
- curses.noecho()
- drilldown = False
- fields_filter = stats.fields_filter
- def update_drilldown():
- if not fields_filter:
- if drilldown:
- stats.fields_filter = None
+class Tui(object):
+ def __init__(self, screen, stats):
+ self.stats = stats
+ self.screen = screen
+ self.drilldown = False
+ curses.use_default_colors()
+ curses.noecho()
+ self.fields_filter = self.stats.fields_filter
+ self.update_drilldown()
+
+ def update_drilldown(self):
+ if not self.fields_filter:
+ if self.drilldown:
+ self.stats.fields_filter = None
else:
- stats.fields_filter = r'^[^\(]*$'
- update_drilldown()
- def refresh(sleeptime):
- screen.erase()
- screen.addstr(0, 0, 'kvm statistics')
- screen.addstr(2, 1, 'Event')
- screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH - len('Total'), 'Total')
- screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH + 8 - len('Current'), 'Current')
+ self.stats.fields_filter = r'^[^\(]*$'
+
+ def refresh(self, sleeptime):
+ self.screen.erase()
+ self.screen.addstr(0, 0, 'kvm statistics - summary', curses.A_BOLD)
+ self.screen.addstr(2, 1, 'Event')
+ self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH -
+ len('Total'), 'Total')
+ self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH + 8 -
+ len('Current'), 'Current')
row = 3
- s = stats.get()
+ stats = self.stats.get()
def sortkey(x):
- if s[x][1]:
- return (-s[x][1], -s[x][0])
+ if stats[x][1]:
+ return (-stats[x][1], -stats[x][0])
else:
- return (0, -s[x][0])
- for key in sorted(s.keys(), key=sortkey):
- if row >= screen.getmaxyx()[0]:
+ return (0, -stats[x][0])
+ for key in sorted(stats.keys(), key=sortkey):
+
+ if row >= self.screen.getmaxyx()[0]:
break
- values = s[key]
+ values = stats[key]
if not values[0] and not values[1]:
break
col = 1
- screen.addstr(row, col, key)
+ self.screen.addstr(row, col, key)
col += LABEL_WIDTH
- screen.addstr(row, col, '%10d' % (values[0],))
+ self.screen.addstr(row, col, '%10d' % (values[0],))
col += NUMBER_WIDTH
if values[1] is not None:
- screen.addstr(row, col, '%8d' % (values[1] / sleeptime,))
+ self.screen.addstr(row, col, '%8d' % (values[1] / sleeptime,))
row += 1
- screen.refresh()
+ self.screen.refresh()
- sleeptime = 0.25
- while True:
- refresh(sleeptime)
- curses.halfdelay(int(sleeptime * 10))
- sleeptime = 3
- try:
- c = screen.getkey()
- if c == 'x':
- drilldown = not drilldown
- update_drilldown()
- if c == 'q':
+ def show_stats(self):
+ sleeptime = 0.25
+ while True:
+ self.refresh(sleeptime)
+ curses.halfdelay(int(sleeptime * 10))
+ sleeptime = 3
+ try:
+ char = self.screen.getkey()
+ if char == 'x':
+ self.drilldown = not self.drilldown
+ self.update_drilldown()
+ if char == 'q':
+ break
+ except KeyboardInterrupt:
break
- except KeyboardInterrupt:
- break
- except curses.error:
- continue
+ except curses.error:
+ continue
+
+def tui(screen, stats):
+ interface = Tui(screen, stats)
+ interface.show_stats()
def batch(stats):
s = stats.get()
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 28/34] scripts/kvm/kvm_stat: Fix output formatting
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (26 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 27/34] scripts/kvm/kvm_stat: Make tui function a class Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 29/34] scripts/kvm/kvm_stat: Move to argparse and add description Janosch Frank
` (6 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The key names in log mode were capped to 10 characters which is not
enough for distinguishing between keys. Capping was therefore removed.
In batch mode the spacing between keys and values was too narrow and
therefore had to be extended to 42.
---
scripts/kvm/kvm_stat | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 4968941..aa3de71 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -599,13 +599,13 @@ def batch(stats):
s = stats.get()
for key in sorted(s.keys()):
values = s[key]
- print '%-22s%10d%10d' % (key, values[0], values[1])
+ print '%-42s%10d%10d' % (key, values[0], values[1])
def log(stats):
keys = sorted(stats.get().iterkeys())
def banner():
for k in keys:
- print '%10s' % k[0:9],
+ print '%s' % k,
print
def statline():
s = stats.get()
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 29/34] scripts/kvm/kvm_stat: Move to argparse and add description
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (27 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 28/34] scripts/kvm/kvm_stat: Fix output formatting Janosch Frank
@ 2015-12-10 12:12 ` Janosch Frank
2016-01-07 15:41 ` Paolo Bonzini
2015-12-10 12:13 ` [Qemu-devel] [PATCH 30/34] scripts/kvm/kvm_stat: Cleanup and pre-init perf_event_attr Janosch Frank
` (5 subsequent siblings)
34 siblings, 1 reply; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:12 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The OptionParser is deprecated since the introduction of the
ArgumentParser in 2.7.
Additionally added a description text for the script, so new users
don't have to guess its purpose and inner workings.
---
scripts/kvm/kvm_stat | 86 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 53 insertions(+), 33 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index aa3de71..109c22c 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -15,7 +15,7 @@ import curses
import sys
import os
import time
-import optparse
+import argparse
import ctypes
import fcntl
import resource
@@ -622,38 +622,58 @@ def log(stats):
line += 1
def get_options():
- optparser = optparse.OptionParser()
- optparser.add_option('-1', '--once', '--batch',
- action='store_true',
- default=False,
- dest='once',
- help='run in batch mode for one second',
- )
- optparser.add_option('-l', '--log',
- action='store_true',
- default=False,
- dest='log',
- help='run in logging mode (like vmstat)',
- )
- optparser.add_option('-t', '--tracepoints',
- action='store_true',
- default=False,
- dest='tracepoints',
- help='retrieve statistics from tracepoints',
- )
- optparser.add_option('-d', '--debugfs',
- action='store_true',
- default=False,
- dest='debugfs',
- help='retrieve statistics from debugfs',
- )
- optparser.add_option('-f', '--fields',
- action='store',
- default=None,
- dest='fields',
- help='fields to display (regex)',
- )
- (options, _) = optparser.parse_args(sys.argv)
+ description_text = """
+This script displays various statistics about VMs running under KVM.
+The statistics are gathered from the KVM debugfs entries and / or the
+currently available perf traces.
+
+The monitoring takes additional cpu cycles and might affect the VM's
+performance.
+
+Requirements:
+- Access to:
+ /sys/kernel/debug/kvm
+ /sys/kernel/debug/trace/events/*
+ /proc/pid/task
+- /proc/sys/kernel/perf_event_paranoid < 1 if user has no
+ CAP_SYS_ADMIN and perf events are used.
+- CAP_SYS_RESOURCE if the hard limit is not high enough to allow
+ the large number of files that are possibly opened.
+"""
+ formatter = argparse.RawTextHelpFormatter
+ parser = argparse.ArgumentParser(description=description_text,
+ formatter_class=formatter)
+ parser.add_argument('-1', '--once', '--batch',
+ action='store_true',
+ default=False,
+ dest='once',
+ help='run in batch mode for one second',
+ )
+ parser.add_argument('-l', '--log',
+ action='store_true',
+ default=False,
+ dest='log',
+ help='run in logging mode (like vmstat)',
+ )
+ parser.add_argument('-t', '--tracepoints',
+ action='store_true',
+ default=False,
+ dest='tracepoints',
+ help='retrieve statistics from tracepoints',
+ )
+ parser.add_argument('-d', '--debugfs',
+ action='store_true',
+ default=False,
+ dest='debugfs',
+ help='retrieve statistics from debugfs',
+ )
+ parser.add_argument('-f', '--fields',
+ action='store',
+ default=None,
+ dest='fields',
+ help='fields to display (regex)',
+ )
+ options = parser.parse_args()
return options
def get_providers(options):
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 30/34] scripts/kvm/kvm_stat: Cleanup and pre-init perf_event_attr
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (28 preceding siblings ...)
2015-12-10 12:12 ` [Qemu-devel] [PATCH 29/34] scripts/kvm/kvm_stat: Move to argparse and add description Janosch Frank
@ 2015-12-10 12:13 ` Janosch Frank
2015-12-10 12:13 ` [Qemu-devel] [PATCH 31/34] scripts/kvm/kvm_stat: Read event values as u64 Janosch Frank
` (4 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:13 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
All initializations of the ctypes struct that don't need additional
information were moved to its init method. The unneeded
initializations for sample_type and sample_period were removed as they
do not affect the counters that are read.
This improves readability of the setup_event_attribute by halfing its
LOC.
---
scripts/kvm/kvm_stat | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 109c22c..ab10b1a 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -312,6 +312,13 @@ class perf_event_attr(ctypes.Structure):
('bp_addr', ctypes.c_uint64),
('bp_len', ctypes.c_uint64),
]
+
+ def __init__(self):
+ super(self.__class__, self).__init__()
+ self.type = PERF_TYPE_TRACEPOINT
+ self.size = ctypes.sizeof(self)
+ self.read_format = PERF_FORMAT_GROUP
+
def perf_event_open(attr, pid, cpu, group_fd, flags):
return syscall(ARCH.sc_perf_evt_open, ctypes.pointer(attr),
ctypes.c_int(pid), ctypes.c_int(cpu),
@@ -348,11 +355,7 @@ class Event(object):
tracepoint, 'id')
event_attr = perf_event_attr()
- event_attr.type = PERF_TYPE_TRACEPOINT
- event_attr.size = ctypes.sizeof(event_attr)
event_attr.config = int(open(id_path).read())
- event_attr.sample_period = 1
- event_attr.read_format = PERF_FORMAT_GROUP
return event_attr
def setup_event(self, event_data):
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 31/34] scripts/kvm/kvm_stat: Read event values as u64
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (29 preceding siblings ...)
2015-12-10 12:13 ` [Qemu-devel] [PATCH 30/34] scripts/kvm/kvm_stat: Cleanup and pre-init perf_event_attr Janosch Frank
@ 2015-12-10 12:13 ` Janosch Frank
2015-12-10 12:13 ` [Qemu-devel] [PATCH 32/34] scripts/kvm/kvm_stat: Fix rlimit for unprivileged users Janosch Frank
` (3 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:13 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
The struct read_format, which denotes the returned values on a read
states that the values are u64 and not long long which is used for
struct unpacking.
Therefore the 'q' long long formatter was exchanged with 'Q' which is
the format for u64 data.
---
scripts/kvm/kvm_stat | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index ab10b1a..ee4cf31 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -339,7 +339,7 @@ class Group(object):
def read(self):
length = 8 * (1 + len(self.events))
- read_format = 'xxxxxxxx' + 'q' * len(self.events)
+ read_format = 'xxxxxxxx' + 'Q' * len(self.events)
return dict(zip([event.name for event in self.events],
struct.unpack(read_format,
os.read(self.events[0].fd, length))))
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 32/34] scripts/kvm/kvm_stat: Fix rlimit for unprivileged users
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (30 preceding siblings ...)
2015-12-10 12:13 ` [Qemu-devel] [PATCH 31/34] scripts/kvm/kvm_stat: Read event values as u64 Janosch Frank
@ 2015-12-10 12:13 ` Janosch Frank
2015-12-10 12:13 ` [Qemu-devel] [PATCH 33/34] scripts/kvm/kvm_stat: Fixup filtering Janosch Frank
` (2 subsequent siblings)
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:13 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Setting the hard limit as a unprivileged user either returns an error
when it is higher than the current one or irreversibly sets it lower.
Therefore we leave the hardlimit untouched as long as we don't need to
raise it as this needs CAP_SYS_RESOURCE.
This gives admins the possibility to run the script as an unprivileged
user to increase security.
---
scripts/kvm/kvm_stat | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index ee4cf31..616ecb4 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -413,11 +413,19 @@ class TracepointProvider(object):
# The constant is needed as a buffer for python libs, std
# streams and other files that the script opens.
- rlimit = len(cpus) * len(self._fields) + 50
+ newlim = len(cpus) * len(self._fields) + 50
try:
- resource.setrlimit(resource.RLIMIT_NOFILE, (rlimit, rlimit))
+ softlim_, hardlim = resource.getrlimit(resource.RLIMIT_NOFILE)
+
+ if hardlim < newlim:
+ # Now we need CAP_SYS_RESOURCE, to increase the hard limit.
+ resource.setrlimit(resource.RLIMIT_NOFILE, (newlim, newlim))
+ else:
+ # Raising the soft limit is sufficient.
+ resource.setrlimit(resource.RLIMIT_NOFILE, (newlim, hardlim))
+
except ValueError:
- sys.exit("NOFILE rlimit could not be raised to {0}".format(rlimit))
+ sys.exit("NOFILE rlimit could not be raised to {0}".format(newlim))
for cpu in cpus:
group = Group()
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 33/34] scripts/kvm/kvm_stat: Fixup filtering
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (31 preceding siblings ...)
2015-12-10 12:13 ` [Qemu-devel] [PATCH 32/34] scripts/kvm/kvm_stat: Fix rlimit for unprivileged users Janosch Frank
@ 2015-12-10 12:13 ` Janosch Frank
2015-12-10 12:13 ` [Qemu-devel] [PATCH 34/34] scripts/kvm/kvm_stat: Add interactive filtering Janosch Frank
2015-12-15 9:56 ` [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Cornelia Huck
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:13 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
When filtering, the group leader event should not be disabled, as all
other events under it will also be disabled. Also we should make sure
that values from disabled fields will not be displayed.
This also filters the fields from the log and batch output for better
readability.
Also the drilldown update now directly checks for the stats' field
filter and (un)sets drilldown accordingly.
---
scripts/kvm/kvm_stat | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 616ecb4..556e5ca 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -450,6 +450,9 @@ class TracepointProvider(object):
group.add_event(event_data)
self.group_leaders.append(group)
+ def available_fields(self):
+ return self.get_available_fields()
+
@property
def fields(self):
return self._fields
@@ -458,23 +461,30 @@ class TracepointProvider(object):
def fields(self, fields):
self._fields = fields
for group in self.group_leaders:
- for event in group.events:
+ for index, event in enumerate(group.events):
if event.name in fields:
event.reset()
event.enable()
else:
- event.disable()
+ # Do not disable the group leader.
+ # It would disable all of its events.
+ if index != 0:
+ event.disable()
def read(self):
ret = defaultdict(int)
for group in self.group_leaders:
for name, val in group.read().iteritems():
- ret[name] += val
+ if name in self._fields:
+ ret[name] += val
return ret
class DebugfsProvider(object):
def __init__(self):
- self._fields = walkdir(PATH_DEBUGFS)[2]
+ self._fields = self.get_available_fields()
+
+ def get_available_fields(self):
+ return walkdir(PATH_DEBUGFS)[2]
@property
def fields(self):
@@ -506,7 +516,8 @@ class Stats(object):
# also clear the cache of old values.
self.values = {}
for provider in self.providers:
- provider_fields = [key for key in provider.fields if wanted(key)]
+ provider_fields = [key for key in provider.get_available_fields()
+ if wanted(key)]
provider.fields = provider_fields
@property
@@ -540,15 +551,14 @@ class Tui(object):
self.drilldown = False
curses.use_default_colors()
curses.noecho()
- self.fields_filter = self.stats.fields_filter
self.update_drilldown()
def update_drilldown(self):
- if not self.fields_filter:
- if self.drilldown:
- self.stats.fields_filter = None
- else:
- self.stats.fields_filter = r'^[^\(]*$'
+ if not self.stats.fields_filter:
+ self.stats.fields_filter = r'^[^\(]*$'
+
+ elif self.stats.fields_filter == r'^[^\(]*$':
+ self.stats.fields_filter = None
def refresh(self, sleeptime):
self.screen.erase()
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [Qemu-devel] [PATCH 34/34] scripts/kvm/kvm_stat: Add interactive filtering
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (32 preceding siblings ...)
2015-12-10 12:13 ` [Qemu-devel] [PATCH 33/34] scripts/kvm/kvm_stat: Fixup filtering Janosch Frank
@ 2015-12-10 12:13 ` Janosch Frank
2015-12-15 9:56 ` [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Cornelia Huck
34 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2015-12-10 12:13 UTC (permalink / raw)
To: qemu-devel; +Cc: cornelia.huck, frankja
Interactively changing the filter is much more useful than the
drilldown, because it is more versatile.
With this patch, the filter can be changed by pressing 'f' in the text
ui and entering a new filter regex.
---
scripts/kvm/kvm_stat | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index 556e5ca..cd5c4d9 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -592,6 +592,28 @@ class Tui(object):
row += 1
self.screen.refresh()
+ def show_filter_selection(self):
+ while True:
+ self.screen.erase()
+ self.screen.addstr(0, 0,
+ "Show statistics for events matching a regex.",
+ curses.A_BOLD)
+ self.screen.addstr(2, 0,
+ "Current regex: {0}"
+ .format(self.stats.fields_filter))
+ self.screen.addstr(3, 0, "New regex: ")
+ curses.echo()
+ regex = self.screen.getstr()
+ curses.noecho()
+ if len(regex) == 0:
+ return
+ try:
+ re.compile(regex)
+ self.stats.fields_filter = regex
+ return
+ except re.error:
+ continue
+
def show_stats(self):
sleeptime = 0.25
while True:
@@ -605,6 +627,10 @@ class Tui(object):
self.update_drilldown()
if char == 'q':
break
+ if char == 'p':
+ self.show_vm_selection()
+ if char == 'f':
+ self.show_filter_selection()
except KeyboardInterrupt:
break
except curses.error:
--
2.3.0
^ permalink raw reply related [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
` (33 preceding siblings ...)
2015-12-10 12:13 ` [Qemu-devel] [PATCH 34/34] scripts/kvm/kvm_stat: Add interactive filtering Janosch Frank
@ 2015-12-15 9:56 ` Cornelia Huck
2016-01-07 13:41 ` Cornelia Huck
34 siblings, 1 reply; 50+ messages in thread
From: Cornelia Huck @ 2015-12-15 9:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Janosch Frank, qemu-devel
On Thu, 10 Dec 2015 13:12:30 +0100
Janosch Frank <frankja@linux.vnet.ibm.com> wrote:
> Kvm_stat is a very helpful script for checking the state of VMs, but
> when I tried to introduce new features it broke every few lines.
>
> This patch series aims to make the script more readable and durable,
> so future additions to it will break it less likely. It also fixes
> input/output problems for all of its interface modes.
>
> Testing was done rarely on X86_64 RHEL 6.7 and mostly on s390. Tests
> on other architectures would be beneficial.
The cleanup and fixes look good to me, but I'm most certainly not a
python expert :)
Paolo, would you take these through the kvm tree?
>
> Janosch Frank (34):
> scripts/kvm/kvm_stat: Cleanup of multiple imports
> scripts/kvm/kvm_stat: Replaced os.listdir with os.walk
> scripts/kvm/kvm_stat: Make constants uppercase
> scripts/kvm/kvm_stat: Removed unneeded PERF constants
> scripts/kvm/kvm_stat: Mark globals in functions
> scripts/kvm/kvm_stat: Invert dictionaries
> scripts/kvm/kvm_stat: Cleanup of path variables
> scripts/kvm/kvm_stat: Improve debugfs access checking
> scripts/kvm/kvm_stat: Introduce main function
> scripts/kvm/kvm_stat: Fix spaces around keyword assignments
> scripts/kvm/kvm_stat: Rename variables that redefine globals
> scripts/kvm/kvm_stat: Moved DebugfsProvider
> scripts/kvm/kvm_stat: Fixup syscall error reporting
> scripts/kvm/kvm_stat: Set sensible no. files rlimit
> scripts/kvm/kvm_stat: Cleanup of platform detection
> scripts/kvm/kvm_stat: Make cpu detection a function
> scripts/kvm/kvm_stat: Rename _perf_event_open
> scripts/kvm/kvm_stat: Introduce properties for providers
> scripts/kvm/kvm_stat: Cleanup of TracepointProvider
> scripts/kvm/kvm_stat: Cleanup cpu list retrieval
> scripts/kvm/kvm_stat: Encapsulate filters variable
> scripts/kvm/kvm_stat: Cleanup of Stats class
> scripts/kvm/kvm_stat: Cleanup of Groups class
> scripts/kvm/kvm_stat: Cleanup of Event class
> scripts/kvm/kvm_stat: Group arch specific data
> scripts/kvm/kvm_stat: Remove unneeded X86_EXIT_REASONS
> scripts/kvm/kvm_stat: Make tui function a class
> scripts/kvm/kvm_stat: Fix output formatting
> scripts/kvm/kvm_stat: Move to argparse and add description
> scripts/kvm/kvm_stat: Cleanup and pre-init perf_event_attr
> scripts/kvm/kvm_stat: Read event values as u64
> scripts/kvm/kvm_stat: Fix rlimit for unprivileged users
> scripts/kvm/kvm_stat: Fixup filtering
> scripts/kvm/kvm_stat: Add interactive filtering
>
> scripts/kvm/kvm_stat | 1129 ++++++++++++++++++++++++++++----------------------
> 1 file changed, 626 insertions(+), 503 deletions(-)
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup
2015-12-15 9:56 ` [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Cornelia Huck
@ 2016-01-07 13:41 ` Cornelia Huck
2016-01-07 13:50 ` Paolo Bonzini
2016-01-07 15:44 ` Paolo Bonzini
0 siblings, 2 replies; 50+ messages in thread
From: Cornelia Huck @ 2016-01-07 13:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, Janosch Frank
On Tue, 15 Dec 2015 10:56:35 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> On Thu, 10 Dec 2015 13:12:30 +0100
> Janosch Frank <frankja@linux.vnet.ibm.com> wrote:
>
> > Kvm_stat is a very helpful script for checking the state of VMs, but
> > when I tried to introduce new features it broke every few lines.
> >
> > This patch series aims to make the script more readable and durable,
> > so future additions to it will break it less likely. It also fixes
> > input/output problems for all of its interface modes.
> >
> > Testing was done rarely on X86_64 RHEL 6.7 and mostly on s390. Tests
> > on other architectures would be beneficial.
>
> The cleanup and fixes look good to me, but I'm most certainly not a
> python expert :)
>
> Paolo, would you take these through the kvm tree?
Anybody interested in these? I can't really comment on python stuff,
but it would be a shame if these patches were left out in the cold...
>
> >
> > Janosch Frank (34):
> > scripts/kvm/kvm_stat: Cleanup of multiple imports
> > scripts/kvm/kvm_stat: Replaced os.listdir with os.walk
> > scripts/kvm/kvm_stat: Make constants uppercase
> > scripts/kvm/kvm_stat: Removed unneeded PERF constants
> > scripts/kvm/kvm_stat: Mark globals in functions
> > scripts/kvm/kvm_stat: Invert dictionaries
> > scripts/kvm/kvm_stat: Cleanup of path variables
> > scripts/kvm/kvm_stat: Improve debugfs access checking
> > scripts/kvm/kvm_stat: Introduce main function
> > scripts/kvm/kvm_stat: Fix spaces around keyword assignments
> > scripts/kvm/kvm_stat: Rename variables that redefine globals
> > scripts/kvm/kvm_stat: Moved DebugfsProvider
> > scripts/kvm/kvm_stat: Fixup syscall error reporting
> > scripts/kvm/kvm_stat: Set sensible no. files rlimit
> > scripts/kvm/kvm_stat: Cleanup of platform detection
> > scripts/kvm/kvm_stat: Make cpu detection a function
> > scripts/kvm/kvm_stat: Rename _perf_event_open
> > scripts/kvm/kvm_stat: Introduce properties for providers
> > scripts/kvm/kvm_stat: Cleanup of TracepointProvider
> > scripts/kvm/kvm_stat: Cleanup cpu list retrieval
> > scripts/kvm/kvm_stat: Encapsulate filters variable
> > scripts/kvm/kvm_stat: Cleanup of Stats class
> > scripts/kvm/kvm_stat: Cleanup of Groups class
> > scripts/kvm/kvm_stat: Cleanup of Event class
> > scripts/kvm/kvm_stat: Group arch specific data
> > scripts/kvm/kvm_stat: Remove unneeded X86_EXIT_REASONS
> > scripts/kvm/kvm_stat: Make tui function a class
> > scripts/kvm/kvm_stat: Fix output formatting
> > scripts/kvm/kvm_stat: Move to argparse and add description
> > scripts/kvm/kvm_stat: Cleanup and pre-init perf_event_attr
> > scripts/kvm/kvm_stat: Read event values as u64
> > scripts/kvm/kvm_stat: Fix rlimit for unprivileged users
> > scripts/kvm/kvm_stat: Fixup filtering
> > scripts/kvm/kvm_stat: Add interactive filtering
> >
> > scripts/kvm/kvm_stat | 1129 ++++++++++++++++++++++++++++----------------------
> > 1 file changed, 626 insertions(+), 503 deletions(-)
> >
>
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup
2016-01-07 13:41 ` Cornelia Huck
@ 2016-01-07 13:50 ` Paolo Bonzini
2016-01-07 15:44 ` Paolo Bonzini
1 sibling, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 13:50 UTC (permalink / raw)
To: Cornelia Huck, qemu-devel; +Cc: Janosch Frank
On 07/01/2016 14:41, Cornelia Huck wrote:
>>> > > Testing was done rarely on X86_64 RHEL 6.7 and mostly on s390. Tests
>>> > > on other architectures would be beneficial.
>> >
>> > The cleanup and fixes look good to me, but I'm most certainly not a
>> > python expert :)
>> >
>> > Paolo, would you take these through the kvm tree?
> Anybody interested in these? I can't really comment on python stuff,
> but it would be a shame if these patches were left out in the cold...
Yes, they're on my list... I've still got a bit of backlog after the
holidays.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 07/34] scripts/kvm/kvm_stat: Cleanup of path variables
2015-12-10 12:12 ` [Qemu-devel] [PATCH 07/34] scripts/kvm/kvm_stat: Cleanup of path variables Janosch Frank
@ 2016-01-07 14:56 ` Paolo Bonzini
2016-01-07 16:58 ` Janosch Frank
0 siblings, 1 reply; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 14:56 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: cornelia.huck
On 10/12/2015 13:12, Janosch Frank wrote:
> if not os.access('/sys/kernel/debug', os.F_OK):
PATH_DEBUGFS should be /sys/kernel/debug, while...
> print 'Please enable CONFIG_DEBUG_FS in your kernel'
> sys.exit(1)
> -if not os.access('/sys/kernel/debug/kvm', os.F_OK):
> +if not os.access(PATH_DEBUGFS, os.F_OK):
... this should be PATH_DEBUGFS_KVM.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 20/34] scripts/kvm/kvm_stat: Cleanup cpu list retrieval
2015-12-10 12:12 ` [Qemu-devel] [PATCH 20/34] scripts/kvm/kvm_stat: Cleanup cpu list retrieval Janosch Frank
@ 2016-01-07 15:21 ` Paolo Bonzini
2016-01-07 16:56 ` Janosch Frank
0 siblings, 1 reply; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 15:21 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: cornelia.huck
On 10/12/2015 13:12, Janosch Frank wrote:
> + with open('/sys/devices/system/cpu/online') as cpu_list:
> + cpu_string = cpu_list.readline()
> + cpus = cpu_string.split(',')
> +
> + for cpu in cpus:
> + if '-' not in cpu:
> + cpulist.append(int(cpu))
> + else:
> + cpu_range = cpu.split('-')
> + cpulist.extend(range(int(cpu_range[0]),
> + int(cpu_range[1]) + 1))
Perhaps you can move everything after readline() to a separate function?
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 24/34] scripts/kvm/kvm_stat: Cleanup of Event class
2015-12-10 12:12 ` [Qemu-devel] [PATCH 24/34] scripts/kvm/kvm_stat: Cleanup of Event class Janosch Frank
@ 2016-01-07 15:25 ` Paolo Bonzini
0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 15:25 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: cornelia.huck
On 10/12/2015 13:12, Janosch Frank wrote:
> + event_data = {
> + 'cpu': cpu,
> + 'name': name,
> + 'group': group,
> + 'set': 'kvm',
> + 'tracepoint': tracepoint,
> + 'filter': tracefilter or None,
> + }
> + group.add_event(event_data)
I'd rather have the Event passed directly to add_event, and the event
constructor use keyword arguments instead of a dictionary.
Paolo
> self.group_leaders.append(group)
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 25/34] scripts/kvm/kvm_stat: Group arch specific data
2015-12-10 12:12 ` [Qemu-devel] [PATCH 25/34] scripts/kvm/kvm_stat: Group arch specific data Janosch Frank
@ 2016-01-07 15:30 ` Paolo Bonzini
0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 15:30 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: cornelia.huck
On 10/12/2015 13:12, Janosch Frank wrote:
> + self.exit_reasons = None
> + self.sc_perf_evt_open = None
> + self.ioctl_numbers = {
> + 'SET_FILTER': 0x40082406,
> + 'ENABLE': 0x00002400,
> + 'DISABLE': 0x00002401,
> + 'RESET': 0x00002403,
> + }
> + self.set_arch_data()
>
> - SC_PERF_EVT_OPEN = 298
> - EXIT_REASONS = X86_EXIT_REASONS[flag]
> + def set_arch_data(self):
> + machine = os.uname()[4]
>
> -def s390_init():
> - global SC_PERF_EVT_OPEN
> + if machine.startswith('ppc'):
> + self.sc_perf_evt_open = 319
> + self.ioctl_numbers['ENABLE'] = 0x20002400
> + self.ioctl_numbers['DISABLE'] = 0x20002401
Please keep the separate foo_init() methods. Even better, change them
to __init__ of Arch subclasses, and add a static get_arch() method that
instantiates the appropriate subclass. Then get_arch() can be used to
initialize ARCH.
Paolo
>
> - SC_PERF_EVT_OPEN = 331
> + # PPC comes in 32 and 64 bit and some generated ioctl
> + # numbers depend on the wordsize.
> + char_ptr_size = ctypes.sizeof(ctypes.c_char_p)
> + self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16
>
> -def ppc_init():
> - global SC_PERF_EVT_OPEN
> - global IOCTL_NUMBERS
> + elif machine.startswith('aarch64'):
> + self.sc_perf_evt_open = 241
> + self.exit_reasons = AARCH64_EXIT_REASONS
>
> - SC_PERF_EVT_OPEN = 319
> + elif machine.startswith('s390'):
> + self.sc_perf_evt_open = 331
>
> - IOCTL_NUMBERS['ENABLE'] = 0x20002400
> - IOCTL_NUMBERS['DISABLE'] = 0x20002401
> - IOCTL_NUMBERS['SET_FILTER'] = 0x80002406 | (ctypes.sizeof(ctypes.c_char_p)
> - << 16)
> + else:
> + # X86_64
> + for line in open('/proc/cpuinfo'):
> + if not line.startswith('flags'):
> + continue
>
> -def aarch64_init():
> - global SC_PERF_EVT_OPEN
> - global EXIT_REASONS
> + self.sc_perf_evt_open = 298
>
> - SC_PERF_EVT_OPEN = 241
> - EXIT_REASONS = AARCH64_EXIT_REASONS
> + flags = line.split()
> + if 'vmx' in flags:
> + self.exit_reasons = VMX_EXIT_REASONS
> + if 'svm' in flags:
> + self.exit_reasons = SVM_EXIT_REASONS
> + return
>
> -def detect_platform():
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 27/34] scripts/kvm/kvm_stat: Make tui function a class
2015-12-10 12:12 ` [Qemu-devel] [PATCH 27/34] scripts/kvm/kvm_stat: Make tui function a class Janosch Frank
@ 2016-01-07 15:40 ` Paolo Bonzini
0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 15:40 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: cornelia.huck
On 10/12/2015 13:12, Janosch Frank wrote:
> +
> +def tui(screen, stats):
> + interface = Tui(screen, stats)
> + interface.show_stats()
>
I think it would make sense to "reimplement" curses.wrapper as
__enter__/__exit__ methods in Tui(), and then use with here.
It's really just a bunch of curses calls:
self.stdscr = curses.initscr()
curses.noecho()
curses.cbreak()
# Start color, too. Harmless if the terminal doesn't have
# color; user can test with has_color() later on. The try/catch
# works around a minor bit of over-conscientiousness in the curses
# module -- the error return from C start_color() is ignorable.
try:
curses.start_color()
except:
pass
curses.use_default_colors()
---
# Set everything back to normal
if self.stdscr:
self.stdscr.keypad(0)
curses.echo()
curses.nocbreak()
curses.endwin()
where the existing calls to curses.use_default_colors and curses.noecho
can then be removed.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 29/34] scripts/kvm/kvm_stat: Move to argparse and add description
2015-12-10 12:12 ` [Qemu-devel] [PATCH 29/34] scripts/kvm/kvm_stat: Move to argparse and add description Janosch Frank
@ 2016-01-07 15:41 ` Paolo Bonzini
2016-01-07 15:54 ` Janosch Frank
0 siblings, 1 reply; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 15:41 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: cornelia.huck
On 10/12/2015 13:12, Janosch Frank wrote:
> The OptionParser is deprecated since the introduction of the
> ArgumentParser in 2.7.
>
> Additionally added a description text for the script, so new users
> don't have to guess its purpose and inner workings.
QEMU still supports 2.6, so this cannot be done.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup
2016-01-07 13:41 ` Cornelia Huck
2016-01-07 13:50 ` Paolo Bonzini
@ 2016-01-07 15:44 ` Paolo Bonzini
1 sibling, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 15:44 UTC (permalink / raw)
To: Cornelia Huck, qemu-devel; +Cc: Janosch Frank
On 07/01/2016 14:41, Cornelia Huck wrote:
> On Tue, 15 Dec 2015 10:56:35 +0100
> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
>
>> On Thu, 10 Dec 2015 13:12:30 +0100
>> Janosch Frank <frankja@linux.vnet.ibm.com> wrote:
>>
>>> Kvm_stat is a very helpful script for checking the state of VMs, but
>>> when I tried to introduce new features it broke every few lines.
>>>
>>> This patch series aims to make the script more readable and durable,
>>> so future additions to it will break it less likely. It also fixes
>>> input/output problems for all of its interface modes.
>>>
>>> Testing was done rarely on X86_64 RHEL 6.7 and mostly on s390. Tests
>>> on other architectures would be beneficial.
>>
>> The cleanup and fixes look good to me, but I'm most certainly not a
>> python expert :)
>>
>> Paolo, would you take these through the kvm tree?
>
> Anybody interested in these? I can't really comment on python stuff,
> but it would be a shame if these patches were left out in the cold...
Ok, I've now reviewed it. I didn't have many comments, but the
Signed-off-by was missing so I prefer to have a v2 instead of follow-up
patches.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 29/34] scripts/kvm/kvm_stat: Move to argparse and add description
2016-01-07 15:41 ` Paolo Bonzini
@ 2016-01-07 15:54 ` Janosch Frank
2016-01-07 16:02 ` Paolo Bonzini
0 siblings, 1 reply; 50+ messages in thread
From: Janosch Frank @ 2016-01-07 15:54 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: cornelia.huck
On 01/07/2016 04:41 PM, Paolo Bonzini wrote:
>
>
> On 10/12/2015 13:12, Janosch Frank wrote:
>> The OptionParser is deprecated since the introduction of the
>> ArgumentParser in 2.7.
>>
>> Additionally added a description text for the script, so new users
>> don't have to guess its purpose and inner workings.
>
> QEMU still supports 2.6, so this cannot be done.
>
> Paolo
>
I guess you don't want to have the dependency on the python-argparse
package which is provided down to 2.3?
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 29/34] scripts/kvm/kvm_stat: Move to argparse and add description
2016-01-07 15:54 ` Janosch Frank
@ 2016-01-07 16:02 ` Paolo Bonzini
0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 16:02 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: cornelia.huck
On 07/01/2016 16:54, Janosch Frank wrote:
> On 01/07/2016 04:41 PM, Paolo Bonzini wrote:
>>
>>
>> On 10/12/2015 13:12, Janosch Frank wrote:
>>> The OptionParser is deprecated since the introduction of the
>>> ArgumentParser in 2.7.
>>>
>>> Additionally added a description text for the script, so new users
>>> don't have to guess its purpose and inner workings.
>>
>> QEMU still supports 2.6, so this cannot be done.
>>
>> Paolo
>>
>
> I guess you don't want to have the dependency on the python-argparse
> package which is provided down to 2.3?
I would at least make it a separate patch, because unlike other patches
in this series it would require a mention in the release notes. One
would also have to check whether the supported Linux distributions
include it (for example, RHEL6 only includes it since RHEL6.4; I don't
know about SLES-11).
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 20/34] scripts/kvm/kvm_stat: Cleanup cpu list retrieval
2016-01-07 15:21 ` Paolo Bonzini
@ 2016-01-07 16:56 ` Janosch Frank
2016-01-07 17:02 ` Paolo Bonzini
0 siblings, 1 reply; 50+ messages in thread
From: Janosch Frank @ 2016-01-07 16:56 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: cornelia.huck
On 01/07/2016 04:21 PM, Paolo Bonzini wrote:
>
>
> On 10/12/2015 13:12, Janosch Frank wrote:
>> + with open('/sys/devices/system/cpu/online') as cpu_list:
>> + cpu_string = cpu_list.readline()
>> + cpus = cpu_string.split(',')
>> +
>> + for cpu in cpus:
>> + if '-' not in cpu:
>> + cpulist.append(int(cpu))
>> + else:
>> + cpu_range = cpu.split('-')
>> + cpulist.extend(range(int(cpu_range[0]),
>> + int(cpu_range[1]) + 1))
>
> Perhaps you can move everything after readline() to a separate function?
>
> Paolo
The string analysis to extract the cpu numbers is only needed in the
get_online_cpus function where it resides, so why do you want to split
this function into two?
Its also only a few lines which are easy to read.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 07/34] scripts/kvm/kvm_stat: Cleanup of path variables
2016-01-07 14:56 ` Paolo Bonzini
@ 2016-01-07 16:58 ` Janosch Frank
0 siblings, 0 replies; 50+ messages in thread
From: Janosch Frank @ 2016-01-07 16:58 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: cornelia.huck
On 01/07/2016 03:56 PM, Paolo Bonzini wrote:
>
>
> On 10/12/2015 13:12, Janosch Frank wrote:
>> if not os.access('/sys/kernel/debug', os.F_OK):
>
> PATH_DEBUGFS should be /sys/kernel/debug, while...
>
>> print 'Please enable CONFIG_DEBUG_FS in your kernel'
>> sys.exit(1)
>> -if not os.access('/sys/kernel/debug/kvm', os.F_OK):
>> +if not os.access(PATH_DEBUGFS, os.F_OK):
>
> ... this should be PATH_DEBUGFS_KVM.
>
> Paolo
>
Sure, when I'm at it I'll also change PATH_TRACING to
PATH_DEBUGFS_TRACING so they are consistent.
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [Qemu-devel] [PATCH 20/34] scripts/kvm/kvm_stat: Cleanup cpu list retrieval
2016-01-07 16:56 ` Janosch Frank
@ 2016-01-07 17:02 ` Paolo Bonzini
0 siblings, 0 replies; 50+ messages in thread
From: Paolo Bonzini @ 2016-01-07 17:02 UTC (permalink / raw)
To: Janosch Frank, qemu-devel; +Cc: cornelia.huck
On 07/01/2016 17:56, Janosch Frank wrote:
> The string analysis to extract the cpu numbers is only needed in the
> get_online_cpus function where it resides, so why do you want to split
> this function into two?
>
> Its also only a few lines which are easy to read.
It's a common format in sysfs, so it is not implausible that it will be
reused. The function could be named parse_int_list.
Paolo
^ permalink raw reply [flat|nested] 50+ messages in thread
end of thread, other threads:[~2016-01-07 17:02 UTC | newest]
Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-10 12:12 [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 01/34] scripts/kvm/kvm_stat: Cleanup of multiple imports Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 02/34] scripts/kvm/kvm_stat: Replaced os.listdir with os.walk Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 03/34] scripts/kvm/kvm_stat: Make constants uppercase Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 04/34] scripts/kvm/kvm_stat: Removed unneeded PERF constants Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 05/34] scripts/kvm/kvm_stat: Mark globals in functions Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 06/34] scripts/kvm/kvm_stat: Invert dictionaries Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 07/34] scripts/kvm/kvm_stat: Cleanup of path variables Janosch Frank
2016-01-07 14:56 ` Paolo Bonzini
2016-01-07 16:58 ` Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 08/34] scripts/kvm/kvm_stat: Improve debugfs access checking Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 09/34] scripts/kvm/kvm_stat: Introduce main function Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 10/34] scripts/kvm/kvm_stat: Fix spaces around keyword assignments Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 11/34] scripts/kvm/kvm_stat: Rename variables that redefine globals Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 12/34] scripts/kvm/kvm_stat: Moved DebugfsProvider Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 13/34] scripts/kvm/kvm_stat: Fixup syscall error reporting Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 14/34] scripts/kvm/kvm_stat: Set sensible no. files rlimit Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 15/34] scripts/kvm/kvm_stat: Cleanup of platform detection Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 16/34] scripts/kvm/kvm_stat: Make cpu detection a function Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 17/34] scripts/kvm/kvm_stat: Rename _perf_event_open Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 18/34] scripts/kvm/kvm_stat: Introduce properties for providers Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 19/34] scripts/kvm/kvm_stat: Cleanup of TracepointProvider Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 20/34] scripts/kvm/kvm_stat: Cleanup cpu list retrieval Janosch Frank
2016-01-07 15:21 ` Paolo Bonzini
2016-01-07 16:56 ` Janosch Frank
2016-01-07 17:02 ` Paolo Bonzini
2015-12-10 12:12 ` [Qemu-devel] [PATCH 21/34] scripts/kvm/kvm_stat: Encapsulate filters variable Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 22/34] scripts/kvm/kvm_stat: Cleanup of Stats class Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 23/34] scripts/kvm/kvm_stat: Cleanup of Groups class Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 24/34] scripts/kvm/kvm_stat: Cleanup of Event class Janosch Frank
2016-01-07 15:25 ` Paolo Bonzini
2015-12-10 12:12 ` [Qemu-devel] [PATCH 25/34] scripts/kvm/kvm_stat: Group arch specific data Janosch Frank
2016-01-07 15:30 ` Paolo Bonzini
2015-12-10 12:12 ` [Qemu-devel] [PATCH 26/34] scripts/kvm/kvm_stat: Remove unneeded X86_EXIT_REASONS Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 27/34] scripts/kvm/kvm_stat: Make tui function a class Janosch Frank
2016-01-07 15:40 ` Paolo Bonzini
2015-12-10 12:12 ` [Qemu-devel] [PATCH 28/34] scripts/kvm/kvm_stat: Fix output formatting Janosch Frank
2015-12-10 12:12 ` [Qemu-devel] [PATCH 29/34] scripts/kvm/kvm_stat: Move to argparse and add description Janosch Frank
2016-01-07 15:41 ` Paolo Bonzini
2016-01-07 15:54 ` Janosch Frank
2016-01-07 16:02 ` Paolo Bonzini
2015-12-10 12:13 ` [Qemu-devel] [PATCH 30/34] scripts/kvm/kvm_stat: Cleanup and pre-init perf_event_attr Janosch Frank
2015-12-10 12:13 ` [Qemu-devel] [PATCH 31/34] scripts/kvm/kvm_stat: Read event values as u64 Janosch Frank
2015-12-10 12:13 ` [Qemu-devel] [PATCH 32/34] scripts/kvm/kvm_stat: Fix rlimit for unprivileged users Janosch Frank
2015-12-10 12:13 ` [Qemu-devel] [PATCH 33/34] scripts/kvm/kvm_stat: Fixup filtering Janosch Frank
2015-12-10 12:13 ` [Qemu-devel] [PATCH 34/34] scripts/kvm/kvm_stat: Add interactive filtering Janosch Frank
2015-12-15 9:56 ` [Qemu-devel] [PATCH 00/34] kvm_stat: Cleanup and fixup Cornelia Huck
2016-01-07 13:41 ` Cornelia Huck
2016-01-07 13:50 ` Paolo Bonzini
2016-01-07 15:44 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.