* [PATCH 01/17] tools/kvm_stat: hide cursor
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
@ 2017-02-20 15:41 ` Stefan Raspl
2017-02-20 15:41 ` [PATCH 02/17] tools/kvm_stat: catch curses exceptions only Stefan Raspl
` (17 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:41 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
When running kvm_stat in interactive mode, the cursor appears at the lower
left corner, which looks a bit distracting.
This patch hides the cursor by turning it invisible.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-By: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 581278c..8cc83a30 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -812,6 +812,13 @@ class Tui(object):
except:
pass
+ # Hide cursor in extra statement as some monochrome terminals
+ # might support hiding but not colors.
+ try:
+ curses.curs_set(0)
+ except curses.error:
+ pass
+
curses.use_default_colors()
return self
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 02/17] tools/kvm_stat: catch curses exceptions only
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
2017-02-20 15:41 ` [PATCH 01/17] tools/kvm_stat: hide cursor Stefan Raspl
@ 2017-02-20 15:41 ` Stefan Raspl
2017-02-20 15:41 ` [PATCH 03/17] tools/kvm_stat: handle SIGINT in log and batch modes Stefan Raspl
` (16 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:41 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
The previous version was catching all exceptions, including SIGINT.
We only want to catch the curses exceptions here.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 8cc83a30..ef47ad7 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -809,7 +809,7 @@ class Tui(object):
# return from C start_color() is ignorable.
try:
curses.start_color()
- except:
+ except curses.error:
pass
# Hide cursor in extra statement as some monochrome terminals
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 03/17] tools/kvm_stat: handle SIGINT in log and batch modes
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
2017-02-20 15:41 ` [PATCH 01/17] tools/kvm_stat: hide cursor Stefan Raspl
2017-02-20 15:41 ` [PATCH 02/17] tools/kvm_stat: catch curses exceptions only Stefan Raspl
@ 2017-02-20 15:41 ` Stefan Raspl
2017-02-20 15:41 ` [PATCH 04/17] tools/kvm_stat: fix misc glitches Stefan Raspl
` (15 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:41 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
SIGINT causes ugly unhandled exceptions in log and batch mode, which we
prevent by catching the exceptions accordingly.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index ef47ad7..14536c0 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -969,12 +969,15 @@ class Tui(object):
def batch(stats):
"""Prints statistics in a key, value format."""
- s = stats.get()
- time.sleep(1)
- s = stats.get()
- for key in sorted(s.keys()):
- values = s[key]
- print '%-42s%10d%10d' % (key, values[0], values[1])
+ try:
+ s = stats.get()
+ time.sleep(1)
+ s = stats.get()
+ for key in sorted(s.keys()):
+ values = s[key]
+ print '%-42s%10d%10d' % (key, values[0], values[1])
+ except KeyboardInterrupt:
+ pass
def log(stats):
"""Prints statistics as reiterating key block, multiple value blocks."""
@@ -991,11 +994,14 @@ def log(stats):
line = 0
banner_repeat = 20
while True:
- time.sleep(1)
- if line % banner_repeat == 0:
- banner()
- statline()
- line += 1
+ try:
+ time.sleep(1)
+ if line % banner_repeat == 0:
+ banner()
+ statline()
+ line += 1
+ except KeyboardInterrupt:
+ break
def get_options():
"""Returns processed program arguments."""
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 04/17] tools/kvm_stat: fix misc glitches
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (2 preceding siblings ...)
2017-02-20 15:41 ` [PATCH 03/17] tools/kvm_stat: handle SIGINT in log and batch modes Stefan Raspl
@ 2017-02-20 15:41 ` Stefan Raspl
2017-03-09 16:51 ` Paolo Bonzini
2017-02-20 15:41 ` [PATCH 05/17] tools/kvm_stat: fix trace setup glitch on field updates in TracepointProvider Stefan Raspl
` (14 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:41 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Addresses
- eliminate extra import
- missing variable initialization
- type redefinition from int to float
- passing of int type argument instead of string
- a couple of PEP8-reported indentation/formatting glitches
- remove unused variable drilldown in class Tui
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 14536c0..a09179e 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -31,7 +31,6 @@ import resource
import struct
import re
from collections import defaultdict
-from time import sleep
VMX_EXIT_REASONS = {
'EXCEPTION_NMI': 0,
@@ -339,8 +338,7 @@ def get_filters():
filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
return filters
-libc = ctypes.CDLL('libc.so.6', use_errno=True)
-syscall = libc.syscall
+syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall
class perf_event_attr(ctypes.Structure):
"""Struct that holds the necessary data to set up a trace event.
@@ -657,6 +655,7 @@ class DebugfsProvider(object):
self._fields = self.get_available_fields()
self._pid = 0
self.do_read = True
+ self.paths = []
def get_available_fields(self):
""""Returns a list of available fields.
@@ -794,7 +793,6 @@ class Tui(object):
def __init__(self, stats):
self.stats = stats
self.screen = None
- self.drilldown = False
self.update_drilldown()
def __enter__(self):
@@ -950,11 +948,10 @@ class Tui(object):
while True:
self.refresh(sleeptime)
curses.halfdelay(int(sleeptime * 10))
- sleeptime = 3
+ sleeptime = 3.
try:
char = self.screen.getkey()
if char == 'x':
- self.drilldown = not self.drilldown
self.update_drilldown()
if char == 'q':
break
@@ -1064,12 +1061,12 @@ Requirements:
help='fields to display (regex)',
)
optparser.add_option('-p', '--pid',
- action='store',
- default=0,
- type=int,
- dest='pid',
- help='restrict statistics to pid',
- )
+ action='store',
+ default=0,
+ type='int',
+ dest='pid',
+ help='restrict statistics to pid',
+ )
(options, _) = optparser.parse_args(sys.argv)
return options
@@ -1099,8 +1096,8 @@ def check_access(options):
"Also ensure, that the kvm modules are loaded.\n")
sys.exit(1)
- if not os.path.exists(PATH_DEBUGFS_TRACING) and (options.tracepoints
- or not options.debugfs):
+ if not os.path.exists(PATH_DEBUGFS_TRACING) and (options.tracepoints or
+ not options.debugfs):
sys.stderr.write("Please enable CONFIG_TRACING in your kernel "
"when using the option -t (default).\n"
"If it is enabled, make {0} readable by the "
@@ -1111,7 +1108,7 @@ def check_access(options):
sys.stderr.write("Falling back to debugfs statistics!\n")
options.debugfs = True
- sleep(5)
+ time.sleep(5)
return options
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
2017-02-20 15:41 ` [PATCH 04/17] tools/kvm_stat: fix misc glitches Stefan Raspl
@ 2017-03-09 16:51 ` Paolo Bonzini
2017-03-10 6:04 ` Stefan Raspl
0 siblings, 1 reply; 35+ messages in thread
From: Paolo Bonzini @ 2017-03-09 16:51 UTC (permalink / raw)
To: Stefan Raspl, kvm; +Cc: rkrcmar, frankja
On 20/02/2017 16:41, Stefan Raspl wrote:
> @@ -339,8 +338,7 @@ def get_filters():
> filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
> return filters
>
> -libc = ctypes.CDLL('libc.so.6', use_errno=True)
> -syscall = libc.syscall
> +syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall
>
> class perf_event_attr(ctypes.Structure):
> """Struct that holds the necessary data to set up a trace event.
> @@ -950,11 +948,10 @@ class Tui(object):
> while True:
> self.refresh(sleeptime)
> curses.halfdelay(int(sleeptime * 10))
> - sleeptime = 3
> + sleeptime = 3.
> try:
> char = self.screen.getkey()
> if char == 'x':
> - self.drilldown = not self.drilldown
> self.update_drilldown()
> if char == 'q':
> break
I'm not sure I understand the point of these; the rest is fine.
Paolo
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
2017-03-09 16:51 ` Paolo Bonzini
@ 2017-03-10 6:04 ` Stefan Raspl
2017-03-10 8:14 ` Paolo Bonzini
0 siblings, 1 reply; 35+ messages in thread
From: Stefan Raspl @ 2017-03-10 6:04 UTC (permalink / raw)
To: Paolo Bonzini, kvm; +Cc: rkrcmar, frankja
On 09.03.2017 17:51, Paolo Bonzini wrote:
>
>
> On 20/02/2017 16:41, Stefan Raspl wrote:
>> @@ -339,8 +338,7 @@ def get_filters():
>> filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
>> return filters
>>
>> -libc = ctypes.CDLL('libc.so.6', use_errno=True)
>> -syscall = libc.syscall
>> +syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall
>>
>> class perf_event_attr(ctypes.Structure):
>> """Struct that holds the necessary data to set up a trace event.
>> @@ -950,11 +948,10 @@ class Tui(object):
>> while True:
>> self.refresh(sleeptime)
>> curses.halfdelay(int(sleeptime * 10))
>> - sleeptime = 3
>> + sleeptime = 3.
>> try:
>> char = self.screen.getkey()
>> if char == 'x':
>> - self.drilldown = not self.drilldown
>> self.update_drilldown()
>> if char == 'q':
>> break
>
> I'm not sure I understand the point of these; the rest is fine.
'sleeptime' starts out as a float (sleeptime = 0.25), but is here
re-defined to an int - so we make it float all the way.
The variable 'drilldown' is never used, so we remove its
initialization in __init__() and the sole place where it is ever
used, which is the line above.
Ciao,
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
2017-03-10 6:04 ` Stefan Raspl
@ 2017-03-10 8:14 ` Paolo Bonzini
2017-03-10 8:33 ` Stefan Raspl
0 siblings, 1 reply; 35+ messages in thread
From: Paolo Bonzini @ 2017-03-10 8:14 UTC (permalink / raw)
To: raspl; +Cc: kvm, rkrcmar, frankja
----- Original Message -----
> From: "Stefan Raspl" <raspl@linux.vnet.ibm.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>, kvm@vger.kernel.org
> Cc: rkrcmar@redhat.com, frankja@linux.vnet.ibm.com
> Sent: Friday, March 10, 2017 7:04:46 AM
> Subject: Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
>
> On 09.03.2017 17:51, Paolo Bonzini wrote:
> >
> >
> > On 20/02/2017 16:41, Stefan Raspl wrote:
> >> @@ -339,8 +338,7 @@ def get_filters():
> >> filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
> >> return filters
> >>
> >> -libc = ctypes.CDLL('libc.so.6', use_errno=True)
> >> -syscall = libc.syscall
> >> +syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall
> >>
> >> class perf_event_attr(ctypes.Structure):
> >> """Struct that holds the necessary data to set up a trace event.
> >> @@ -950,11 +948,10 @@ class Tui(object):
> >> while True:
> >> self.refresh(sleeptime)
> >> curses.halfdelay(int(sleeptime * 10))
> >> - sleeptime = 3
> >> + sleeptime = 3.
> >> try:
> >> char = self.screen.getkey()
> >> if char == 'x':
> >> - self.drilldown = not self.drilldown
> >> self.update_drilldown()
> >> if char == 'q':
> >> break
> >
> > I'm not sure I understand the point of these; the rest is fine.
>
> 'sleeptime' starts out as a float (sleeptime = 0.25), but is here
> re-defined to an int - so we make it float all the way.
> The variable 'drilldown' is never used, so we remove its
> initialization in __init__() and the sole place where it is ever
> used, which is the line above.
Yes, I was referring to libc and sleeptime. I don't like the
"3.", especially since Python 3 has "3/2" return a float. Is
this a PEP8 complaint? Does it still complain if you do
"from __future__ import division"?
Paolo
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
2017-03-10 8:14 ` Paolo Bonzini
@ 2017-03-10 8:33 ` Stefan Raspl
2017-03-10 8:38 ` Paolo Bonzini
0 siblings, 1 reply; 35+ messages in thread
From: Stefan Raspl @ 2017-03-10 8:33 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, rkrcmar, frankja
On 10.03.2017 09:14, Paolo Bonzini wrote:
>
>
> ----- Original Message -----
>> From: "Stefan Raspl" <raspl@linux.vnet.ibm.com>
>> To: "Paolo Bonzini" <pbonzini@redhat.com>, kvm@vger.kernel.org
>> Cc: rkrcmar@redhat.com, frankja@linux.vnet.ibm.com
>> Sent: Friday, March 10, 2017 7:04:46 AM
>> Subject: Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
>>
>> On 09.03.2017 17:51, Paolo Bonzini wrote:
>>>
>>>
>>> On 20/02/2017 16:41, Stefan Raspl wrote:
>>>> @@ -339,8 +338,7 @@ def get_filters():
>>>> filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
>>>> return filters
>>>>
>>>> -libc = ctypes.CDLL('libc.so.6', use_errno=True)
>>>> -syscall = libc.syscall
>>>> +syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall
>>>>
>>>> class perf_event_attr(ctypes.Structure):
>>>> """Struct that holds the necessary data to set up a trace event.
>>>> @@ -950,11 +948,10 @@ class Tui(object):
>>>> while True:
>>>> self.refresh(sleeptime)
>>>> curses.halfdelay(int(sleeptime * 10))
>>>> - sleeptime = 3
>>>> + sleeptime = 3.
>>>> try:
>>>> char = self.screen.getkey()
>>>> if char == 'x':
>>>> - self.drilldown = not self.drilldown
>>>> self.update_drilldown()
>>>> if char == 'q':
>>>> break
>>>
>>> I'm not sure I understand the point of these; the rest is fine.
>>
>> 'sleeptime' starts out as a float (sleeptime = 0.25), but is here
>> re-defined to an int - so we make it float all the way.
>> The variable 'drilldown' is never used, so we remove its
>> initialization in __init__() and the sole place where it is ever
>> used, which is the line above.
>
> Yes, I was referring to libc and sleeptime. I don't like the
> "3.", especially since Python 3 has "3/2" return a float. Is
> this a PEP8 complaint? Does it still complain if you do
> "from __future__ import division"?
Ah, sorry, missed the first chunk: That was to eliminate unused
variable 'libc'.
As for the "3.": I noticed through pylint, PEP8 is fine. Seemed
valid to me, but in the grand scheme of things it certainly doesn't
matter (and wasn't aware it becomes moot in Python 3), so I'd take
it out if you want.
Sidenote: I only ever hear the complaint about the '3.' from kernel
folks, seemed quite common to me elsewhere - maybe 'cause there's
usually no floats in the kernel?! But as I said, I'd rather take it
out altogether in here instead of adding more imports, and fix 07/17
to redefine to 3 (instead of 3.), OK?
Ciao,
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
2017-03-10 8:33 ` Stefan Raspl
@ 2017-03-10 8:38 ` Paolo Bonzini
2017-03-10 9:42 ` Stefan Raspl
0 siblings, 1 reply; 35+ messages in thread
From: Paolo Bonzini @ 2017-03-10 8:38 UTC (permalink / raw)
To: raspl; +Cc: kvm, rkrcmar, frankja
On 10/03/2017 09:33, Stefan Raspl wrote:
> On 10.03.2017 09:14, Paolo Bonzini wrote:
>>
>>
>> ----- Original Message -----
>>> From: "Stefan Raspl" <raspl@linux.vnet.ibm.com>
>>> To: "Paolo Bonzini" <pbonzini@redhat.com>, kvm@vger.kernel.org
>>> Cc: rkrcmar@redhat.com, frankja@linux.vnet.ibm.com
>>> Sent: Friday, March 10, 2017 7:04:46 AM
>>> Subject: Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
>>>
>>> On 09.03.2017 17:51, Paolo Bonzini wrote:
>>>>
>>>>
>>>> On 20/02/2017 16:41, Stefan Raspl wrote:
>>>>> @@ -339,8 +338,7 @@ def get_filters():
>>>>> filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
>>>>> return filters
>>>>>
>>>>> -libc = ctypes.CDLL('libc.so.6', use_errno=True)
>>>>> -syscall = libc.syscall
>>>>> +syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall
>>>>>
>>>>> class perf_event_attr(ctypes.Structure):
>>>>> """Struct that holds the necessary data to set up a trace event.
>>>>> @@ -950,11 +948,10 @@ class Tui(object):
>>>>> while True:
>>>>> self.refresh(sleeptime)
>>>>> curses.halfdelay(int(sleeptime * 10))
>>>>> - sleeptime = 3
>>>>> + sleeptime = 3.
>>>>> try:
>>>>> char = self.screen.getkey()
>>>>> if char == 'x':
>>>>> - self.drilldown = not self.drilldown
>>>>> self.update_drilldown()
>>>>> if char == 'q':
>>>>> break
>>>>
>>>> I'm not sure I understand the point of these; the rest is fine.
>>>
>>> 'sleeptime' starts out as a float (sleeptime = 0.25), but is here
>>> re-defined to an int - so we make it float all the way.
>>> The variable 'drilldown' is never used, so we remove its
>>> initialization in __init__() and the sole place where it is ever
>>> used, which is the line above.
>>
>> Yes, I was referring to libc and sleeptime. I don't like the
>> "3.", especially since Python 3 has "3/2" return a float. Is
>> this a PEP8 complaint? Does it still complain if you do
>> "from __future__ import division"?
>
>
> Ah, sorry, missed the first chunk: That was to eliminate unused
> variable 'libc'.
It's not unused, it's used on the following line. :)
> As for the "3.": I noticed through pylint, PEP8 is fine. Seemed
> valid to me, but in the grand scheme of things it certainly doesn't
> matter (and wasn't aware it becomes moot in Python 3), so I'd take
> it out if you want.
I think it's a valid complaint with Python 2 division. If pylint
complaints even with "from __future__ import division", it would be a
pylint bug in my opinion.
Maybe it's just me... I wouldn't have complained about "3.0" for example. :)
Paolo
> Sidenote: I only ever hear the complaint about the '3.' from kernel
> folks, seemed quite common to me elsewhere - maybe 'cause there's
> usually no floats in the kernel?! But as I said, I'd rather take it
> out altogether in here instead of adding more imports, and fix 07/17
> to redefine to 3 (instead of 3.), OK?
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
2017-03-10 8:38 ` Paolo Bonzini
@ 2017-03-10 9:42 ` Stefan Raspl
2017-03-10 10:05 ` Paolo Bonzini
0 siblings, 1 reply; 35+ messages in thread
From: Stefan Raspl @ 2017-03-10 9:42 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, rkrcmar, frankja
On 10.03.2017 09:38, Paolo Bonzini wrote:
>
>
> On 10/03/2017 09:33, Stefan Raspl wrote:
>> On 10.03.2017 09:14, Paolo Bonzini wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Stefan Raspl" <raspl@linux.vnet.ibm.com>
>>>> To: "Paolo Bonzini" <pbonzini@redhat.com>, kvm@vger.kernel.org
>>>> Cc: rkrcmar@redhat.com, frankja@linux.vnet.ibm.com
>>>> Sent: Friday, March 10, 2017 7:04:46 AM
>>>> Subject: Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
>>>>
>>>> On 09.03.2017 17:51, Paolo Bonzini wrote:
>>>>>
>>>>>
>>>>> On 20/02/2017 16:41, Stefan Raspl wrote:
>>>>>> @@ -339,8 +338,7 @@ def get_filters():
>>>>>> filters['kvm_exit'] = ('exit_reason', ARCH.exit_reasons)
>>>>>> return filters
>>>>>>
>>>>>> -libc = ctypes.CDLL('libc.so.6', use_errno=True)
>>>>>> -syscall = libc.syscall
>>>>>> +syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall
>>>>>>
>>>>>> class perf_event_attr(ctypes.Structure):
>>>>>> """Struct that holds the necessary data to set up a trace event.
>>>>>> @@ -950,11 +948,10 @@ class Tui(object):
>>>>>> while True:
>>>>>> self.refresh(sleeptime)
>>>>>> curses.halfdelay(int(sleeptime * 10))
>>>>>> - sleeptime = 3
>>>>>> + sleeptime = 3.
>>>>>> try:
>>>>>> char = self.screen.getkey()
>>>>>> if char == 'x':
>>>>>> - self.drilldown = not self.drilldown
>>>>>> self.update_drilldown()
>>>>>> if char == 'q':
>>>>>> break
>>>>>
>>>>> I'm not sure I understand the point of these; the rest is fine.
>>>>
>>>> 'sleeptime' starts out as a float (sleeptime = 0.25), but is here
>>>> re-defined to an int - so we make it float all the way.
>>>> The variable 'drilldown' is never used, so we remove its
>>>> initialization in __init__() and the sole place where it is ever
>>>> used, which is the line above.
>>>
>>> Yes, I was referring to libc and sleeptime. I don't like the
>>> "3.", especially since Python 3 has "3/2" return a float. Is
>>> this a PEP8 complaint? Does it still complain if you do
>>> "from __future__ import division"?
>>
>>
>> Ah, sorry, missed the first chunk: That was to eliminate unused
>> variable 'libc'.
>
> It's not unused, it's used on the following line. :)
Sorry, don't want to get annoying: Yeah, right, so I was eliminating
that variable - do you want me to remove the chunk from the patch?
>> As for the "3.": I noticed through pylint, PEP8 is fine. Seemed
>> valid to me, but in the grand scheme of things it certainly doesn't
>> matter (and wasn't aware it becomes moot in Python 3), so I'd take
>> it out if you want.
>
> I think it's a valid complaint with Python 2 division. If pylint
> complaints even with "from __future__ import division", it would be a
> pylint bug in my opinion.
>
> Maybe it's just me... I wouldn't have complained about "3.0" for example. :)
Tried the import, pylint still complains :O
So: Just switch to 3.0 here and in 07/17, and repost?
Ciao,
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 04/17] tools/kvm_stat: fix misc glitches
2017-03-10 9:42 ` Stefan Raspl
@ 2017-03-10 10:05 ` Paolo Bonzini
0 siblings, 0 replies; 35+ messages in thread
From: Paolo Bonzini @ 2017-03-10 10:05 UTC (permalink / raw)
To: raspl; +Cc: kvm, rkrcmar, frankja
On 10/03/2017 10:42, Stefan Raspl wrote:
>>> Ah, sorry, missed the first chunk: That was to eliminate unused
>>> variable 'libc'.
>> It's not unused, it's used on the following line. :)
> Sorry, don't want to get annoying: Yeah, right, so I was eliminating
> that variable - do you want me to remove the chunk from the patch?
Yes, please.
>>> As for the "3.": I noticed through pylint, PEP8 is fine. Seemed
>>> valid to me, but in the grand scheme of things it certainly doesn't
>>> matter (and wasn't aware it becomes moot in Python 3), so I'd take
>>> it out if you want.
>> I think it's a valid complaint with Python 2 division. If pylint
>> complaints even with "from __future__ import division", it would be a
>> pylint bug in my opinion.
>>
>> Maybe it's just me... I wouldn't have complained about "3.0" for example. :)
> Tried the import, pylint still complains :O
> So: Just switch to 3.0 here and in 07/17, and repost?
And this too.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 05/17] tools/kvm_stat: fix trace setup glitch on field updates in TracepointProvider
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (3 preceding siblings ...)
2017-02-20 15:41 ` [PATCH 04/17] tools/kvm_stat: fix misc glitches Stefan Raspl
@ 2017-02-20 15:41 ` Stefan Raspl
2017-02-20 15:42 ` [PATCH 06/17] tools/kvm_stat: full PEP8 compliance Stefan Raspl
` (13 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:41 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Updating the fields of the TracepointProvider does not propagate changes to the
tracepoints. This shows when a pid filter is enabled, whereby subsequent
extensions of the fields of the Tracepoint provider (e.g. by toggling
drilldown) will not modify the tracepoints as required.
To reproduce, select a specific process via interactive command 'p', and
enable drilldown via 'x' - none of the fields with the braces will appear
although they should.
The fix will always leave all available fields in the TracepointProvider
enabled.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Based-on-text-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index a09179e..a1fa86e 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -549,6 +549,7 @@ class TracepointProvider(object):
def setup_traces(self):
"""Creates all event and group objects needed to be able to retrieve
data."""
+ fields = self.get_available_fields()
if self._pid > 0:
# Fetch list of all threads of the monitored pid, as qemu
# starts a thread for each vcpu.
@@ -559,7 +560,7 @@ class TracepointProvider(object):
# The constant is needed as a buffer for python libs, std
# streams and other files that the script opens.
- newlim = len(groupids) * len(self._fields) + 50
+ newlim = len(groupids) * len(fields) + 50
try:
softlim_, hardlim = resource.getrlimit(resource.RLIMIT_NOFILE)
@@ -575,7 +576,7 @@ class TracepointProvider(object):
for groupid in groupids:
group = Group()
- for name in self._fields:
+ for name in fields:
tracepoint = name
tracefilter = None
match = re.match(r'(.*)\((.*)\)', name)
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 06/17] tools/kvm_stat: full PEP8 compliance
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (4 preceding siblings ...)
2017-02-20 15:41 ` [PATCH 05/17] tools/kvm_stat: fix trace setup glitch on field updates in TracepointProvider Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-02-20 15:42 ` [PATCH 07/17] tools/kvm_stat: reduce perceived idle time on filter updates Stefan Raspl
` (12 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Provides all missing empty lines as required for full PEP compliance.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index a1fa86e..e0503c8 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -224,6 +224,7 @@ IOCTL_NUMBERS = {
'RESET': 0x00002403,
}
+
class Arch(object):
"""Encapsulates global architecture specific data.
@@ -254,12 +255,14 @@ class Arch(object):
return ArchX86(SVM_EXIT_REASONS)
return
+
class ArchX86(Arch):
def __init__(self, exit_reasons):
self.sc_perf_evt_open = 298
self.ioctl_numbers = IOCTL_NUMBERS
self.exit_reasons = exit_reasons
+
class ArchPPC(Arch):
def __init__(self):
self.sc_perf_evt_open = 319
@@ -274,12 +277,14 @@ class ArchPPC(Arch):
self.ioctl_numbers['SET_FILTER'] = 0x80002406 | char_ptr_size << 16
self.exit_reasons = {}
+
class ArchA64(Arch):
def __init__(self):
self.sc_perf_evt_open = 241
self.ioctl_numbers = IOCTL_NUMBERS
self.exit_reasons = AARCH64_EXIT_REASONS
+
class ArchS390(Arch):
def __init__(self):
self.sc_perf_evt_open = 331
@@ -340,6 +345,7 @@ def get_filters():
syscall = ctypes.CDLL('libc.so.6', use_errno=True).syscall
+
class perf_event_attr(ctypes.Structure):
"""Struct that holds the necessary data to set up a trace event.
@@ -368,6 +374,7 @@ class perf_event_attr(ctypes.Structure):
self.size = ctypes.sizeof(self)
self.read_format = PERF_FORMAT_GROUP
+
def perf_event_open(attr, pid, cpu, group_fd, flags):
"""Wrapper for the sys_perf_evt_open() syscall.
@@ -393,6 +400,7 @@ PERF_FORMAT_GROUP = 1 << 3
PATH_DEBUGFS_TRACING = '/sys/kernel/debug/tracing'
PATH_DEBUGFS_KVM = '/sys/kernel/debug/kvm'
+
class Group(object):
"""Represents a perf event group."""
@@ -425,6 +433,7 @@ class Group(object):
struct.unpack(read_format,
os.read(self.events[0].fd, length))))
+
class Event(object):
"""Represents a performance event and manages its life cycle."""
def __init__(self, name, group, trace_cpu, trace_pid, trace_point,
@@ -508,6 +517,7 @@ class Event(object):
"""Resets the count of the trace event in the kernel."""
fcntl.ioctl(self.fd, ARCH.ioctl_numbers['RESET'], 0)
+
class TracepointProvider(object):
"""Data provider for the stats class.
@@ -649,6 +659,7 @@ class TracepointProvider(object):
ret[name] += val
return ret
+
class DebugfsProvider(object):
"""Provides data from the files that KVM creates in the kvm debugfs
folder."""
@@ -718,6 +729,7 @@ class DebugfsProvider(object):
except IOError:
return 0
+
class Stats(object):
"""Manages the data providers and the data they provide.
@@ -789,6 +801,7 @@ class Stats(object):
LABEL_WIDTH = 40
NUMBER_WIDTH = 10
+
class Tui(object):
"""Instruments curses to draw a nice text ui."""
def __init__(self, stats):
@@ -858,6 +871,7 @@ class Tui(object):
len('Current'), 'Current')
row = 3
stats = self.stats.get()
+
def sortkey(x):
if stats[x][1]:
return (-stats[x][1], -stats[x][0])
@@ -965,6 +979,7 @@ class Tui(object):
except curses.error:
continue
+
def batch(stats):
"""Prints statistics in a key, value format."""
try:
@@ -977,13 +992,16 @@ def batch(stats):
except KeyboardInterrupt:
pass
+
def log(stats):
"""Prints statistics as reiterating key block, multiple value blocks."""
keys = sorted(stats.get().iterkeys())
+
def banner():
for k in keys:
print '%s' % k,
print
+
def statline():
s = stats.get()
for k in keys:
@@ -1001,6 +1019,7 @@ def log(stats):
except KeyboardInterrupt:
break
+
def get_options():
"""Returns processed program arguments."""
description_text = """
@@ -1071,6 +1090,7 @@ Requirements:
(options, _) = optparser.parse_args(sys.argv)
return options
+
def get_providers(options):
"""Returns a list of data providers depending on the passed options."""
providers = []
@@ -1084,6 +1104,7 @@ def get_providers(options):
return providers
+
def check_access(options):
"""Exits if the current user can't access all needed directories."""
if not os.path.exists('/sys/kernel/debug'):
@@ -1113,6 +1134,7 @@ def check_access(options):
return options
+
def main():
options = get_options()
options = check_access(options)
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 07/17] tools/kvm_stat: reduce perceived idle time on filter updates
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (5 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 06/17] tools/kvm_stat: full PEP8 compliance Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-03-09 16:54 ` Paolo Bonzini
2017-02-20 15:42 ` [PATCH 08/17] tools/kvm_stat: document list of interactive commands Stefan Raspl
` (11 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Whenever a user adds a filter, we
* redraw the header immediately for a snappy response
* print a message indicating to the user that we're busy while the
noticeable delay induced by updating all of the stats objects takes place
* update the statistics ASAP (i.e. after 0.25s instead of 3s) to be
consistent with behavior on startup
To do so, we split the Tui's refresh() method to allow for drawing header
and stats separately, and trigger a header refresh whenever we are about
to do something that takes a while - like updating filters.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 48 ++++++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 18 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index e0503c8..eeb5fe5 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -800,6 +800,8 @@ class Stats(object):
LABEL_WIDTH = 40
NUMBER_WIDTH = 10
+DELAY_INITIAL = 0.25
+DELAY_REGULAR = 3.
class Tui(object):
@@ -855,13 +857,14 @@ class Tui(object):
"""Propagates pid selection to stats object."""
self.stats.pid_filter = pid
- def refresh(self, sleeptime):
- """Refreshes on-screen data."""
+ def refresh_header(self, pid=None):
+ """Refreshes the header."""
+ if pid is None:
+ pid = self.stats.pid_filter
self.screen.erase()
- if self.stats.pid_filter > 0:
+ if pid > 0:
self.screen.addstr(0, 0, 'kvm statistics - pid {0}'
- .format(self.stats.pid_filter),
- curses.A_BOLD)
+ .format(pid), curses.A_BOLD)
else:
self.screen.addstr(0, 0, 'kvm statistics - summary', curses.A_BOLD)
self.screen.addstr(2, 1, 'Event')
@@ -869,7 +872,13 @@ class Tui(object):
len('Total'), 'Total')
self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH + 8 -
len('Current'), 'Current')
+ self.screen.addstr(4, 1, 'Collecting data...')
+ self.screen.refresh()
+
+ def refresh_body(self, sleeptime):
row = 3
+ self.screen.move(row, 0)
+ self.screen.clrtobot()
stats = self.stats.get()
def sortkey(x):
@@ -913,10 +922,12 @@ class Tui(object):
regex = self.screen.getstr()
curses.noecho()
if len(regex) == 0:
+ self.refresh_header()
return
try:
re.compile(regex)
self.stats.fields_filter = regex
+ self.refresh_header()
return
except re.error:
continue
@@ -943,37 +954,38 @@ class Tui(object):
try:
pid = int(pid)
-
- if pid == 0:
- self.update_pid(pid)
- break
- else:
- if not os.path.isdir(os.path.join('/proc/', str(pid))):
- continue
- else:
- self.update_pid(pid)
- break
+ if pid != 0 and not os.path.isdir(os.path.join('/proc/',
+ str(pid))):
+ continue
+ self.refresh_header(pid)
+ self.update_pid(pid)
+ break
except ValueError:
continue
def show_stats(self):
"""Refreshes the screen and processes user input."""
- sleeptime = 0.25
+ sleeptime = DELAY_INITIAL
+ self.refresh_header()
while True:
- self.refresh(sleeptime)
+ self.refresh_body(sleeptime)
curses.halfdelay(int(sleeptime * 10))
- sleeptime = 3.
+ sleeptime = DELAY_REGULAR
try:
char = self.screen.getkey()
if char == 'x':
+ self.refresh_header()
self.update_drilldown()
+ sleeptime = DELAY_INITIAL
if char == 'q':
break
if char == 'f':
self.show_filter_selection()
+ sleeptime = DELAY_INITIAL
if char == 'p':
self.show_vm_selection()
+ sleeptime = DELAY_INITIAL
except KeyboardInterrupt:
break
except curses.error:
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH 07/17] tools/kvm_stat: reduce perceived idle time on filter updates
2017-02-20 15:42 ` [PATCH 07/17] tools/kvm_stat: reduce perceived idle time on filter updates Stefan Raspl
@ 2017-03-09 16:54 ` Paolo Bonzini
0 siblings, 0 replies; 35+ messages in thread
From: Paolo Bonzini @ 2017-03-09 16:54 UTC (permalink / raw)
To: Stefan Raspl, kvm; +Cc: rkrcmar, frankja
On 20/02/2017 16:42, Stefan Raspl wrote:
> Whenever a user adds a filter, we
> * redraw the header immediately for a snappy response
> * print a message indicating to the user that we're busy while the
> noticeable delay induced by updating all of the stats objects takes place
> * update the statistics ASAP (i.e. after 0.25s instead of 3s) to be
> consistent with behavior on startup
> To do so, we split the Tui's refresh() method to allow for drawing header
> and stats separately, and trigger a header refresh whenever we are about
> to do something that takes a while - like updating filters.
>
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
> ---
> tools/kvm/kvm_stat/kvm_stat | 48 ++++++++++++++++++++++++++++-----------------
> 1 file changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
> index e0503c8..eeb5fe5 100755
> --- a/tools/kvm/kvm_stat/kvm_stat
> +++ b/tools/kvm/kvm_stat/kvm_stat
> @@ -800,6 +800,8 @@ class Stats(object):
>
> LABEL_WIDTH = 40
> NUMBER_WIDTH = 10
> +DELAY_INITIAL = 0.25
> +DELAY_REGULAR = 3.
Same here, the float constant isn't strictly necessary and the trailing
dot is somewhat distracting.
Nothing to complain about the change, which is a nice touch.
Paolo
>
> class Tui(object):
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 08/17] tools/kvm_stat: document list of interactive commands
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (6 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 07/17] tools/kvm_stat: reduce perceived idle time on filter updates Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-02-20 15:42 ` [PATCH 09/17] tools/kvm_stat: display guest name when using pid filter Stefan Raspl
` (10 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Apart from the source code, there does not seem to be a place that documents
the interactive capabilities of kvm_stat yet.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 7 +++++++
tools/kvm/kvm_stat/kvm_stat.txt | 16 ++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index eeb5fe5..1db7f38 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -1051,6 +1051,13 @@ Requirements:
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.
+
+Interactive Commands:
+ f filter by regular expression
+ p filter by PID
+ q quit
+ x toggle reporting of stats for individual child trace events
+Press any other key to refresh statistics immediately.
"""
class PlainHelpFormatter(optparse.IndentedHelpFormatter):
diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index b92a153..077bcc7 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -18,11 +18,27 @@ state transitions such as guest mode entry and exit.
This tool is useful for observing guest behavior from the host perspective.
Often conclusions about performance or buggy behavior can be drawn from the
output.
+While running in regular mode, use any of the keys listed in section
+'Interactive Commands' below.
+Use batch and logging modes for scripting purposes.
The set of KVM kernel module trace events may be specific to the kernel version
or architecture. It is best to check the KVM kernel module source code for the
meaning of events.
+INTERACTIVE COMMANDS
+--------------------
+[horizontal]
+*f*:: filter by regular expression
+
+*p*:: filter by PID
+
+*q*:: quit
+
+*x*:: toggle reporting of stats for child trace events
+
+Press any other key to refresh statistics immediately.
+
OPTIONS
-------
-1::
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 09/17] tools/kvm_stat: display guest name when using pid filter
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (7 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 08/17] tools/kvm_stat: document list of interactive commands Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-03-09 16:54 ` Paolo Bonzini
2017-02-20 15:42 ` [PATCH 10/17] tools/kvm_stat: remove pid filter on empty input Stefan Raspl
` (9 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
When running kvm_stat with option '-p' to filter per process, display
the QEMU guest name next to the pid, if available.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-By: Janosch Frank <frankja@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 41 +++++++++++++++++++++++++++++++++++++++--
1 file changed, 39 insertions(+), 2 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 1db7f38..2ef7308 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -320,6 +320,37 @@ def parse_int_list(list_string):
return integers
+def get_gname_from_pid(pid):
+ """Returns the guest name for a QEMU process pid.
+
+ Extracts the guest name from the QEMU comma line by processing the '-name'
+ option. Will also handle names specified out of sequence.
+
+ """
+ name = ''
+ try:
+ line = open('/proc/{}/cmdline'.format(pid), 'rb').read().split('\0')
+ parms = line[line.index('-name') + 1].split(',')
+ while '' in parms:
+ # commas are escaped (i.e. ',,'), hence e.g. 'foo,bar' results in
+ # ['foo', '', 'bar'], which we revert here
+ idx = parms.index('')
+ parms[idx - 1] += ',' + parms[idx + 1]
+ del parms[idx:idx+2]
+ # the '-name' switch allows for two ways to specify the guest name,
+ # where the plain name overrides the name specified via 'guest='
+ for arg in parms:
+ if '=' not in arg:
+ name = arg
+ break
+ if arg[:6] == 'guest=':
+ name = arg[6:]
+ except (ValueError, IOError, IndexError):
+ pass
+
+ return name
+
+
def get_online_cpus():
"""Returns a list of cpu id integers."""
with open('/sys/devices/system/cpu/online') as cpu_list:
@@ -802,6 +833,7 @@ LABEL_WIDTH = 40
NUMBER_WIDTH = 10
DELAY_INITIAL = 0.25
DELAY_REGULAR = 3.
+MAX_GUEST_NAME_LEN = 48
class Tui(object):
@@ -862,9 +894,14 @@ class Tui(object):
if pid is None:
pid = self.stats.pid_filter
self.screen.erase()
+ gname = get_gname_from_pid(pid)
+ if gname:
+ gname = ('({})'.format(gname[:MAX_GUEST_NAME_LEN] + '...'
+ if len(gname) > MAX_GUEST_NAME_LEN
+ else gname))
if pid > 0:
- self.screen.addstr(0, 0, 'kvm statistics - pid {0}'
- .format(pid), curses.A_BOLD)
+ self.screen.addstr(0, 0, 'kvm statistics - pid {0} {1}'
+ .format(pid, gname), curses.A_BOLD)
else:
self.screen.addstr(0, 0, 'kvm statistics - summary', curses.A_BOLD)
self.screen.addstr(2, 1, 'Event')
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH 09/17] tools/kvm_stat: display guest name when using pid filter
2017-02-20 15:42 ` [PATCH 09/17] tools/kvm_stat: display guest name when using pid filter Stefan Raspl
@ 2017-03-09 16:54 ` Paolo Bonzini
2017-03-10 6:08 ` Stefan Raspl
0 siblings, 1 reply; 35+ messages in thread
From: Paolo Bonzini @ 2017-03-09 16:54 UTC (permalink / raw)
To: Stefan Raspl, kvm; +Cc: rkrcmar, frankja
On 20/02/2017 16:42, Stefan Raspl wrote:
> +def get_gname_from_pid(pid):
> + """Returns the guest name for a QEMU process pid.
> +
> + Extracts the guest name from the QEMU comma line by processing the '-name'
> + option. Will also handle names specified out of sequence.
> +
> + """
> + name = ''
> + try:
> + line = open('/proc/{}/cmdline'.format(pid), 'rb').read().split('\0')
> + parms = line[line.index('-name') + 1].split(',')
> + while '' in parms:
> + # commas are escaped (i.e. ',,'), hence e.g. 'foo,bar' results in
> + # ['foo', '', 'bar'], which we revert here
> + idx = parms.index('')
> + parms[idx - 1] += ',' + parms[idx + 1]
> + del parms[idx:idx+2]
> + # the '-name' switch allows for two ways to specify the guest name,
> + # where the plain name overrides the name specified via 'guest='
> + for arg in parms:
> + if '=' not in arg:
> + name = arg
> + break
> + if arg[:6] == 'guest=':
> + name = arg[6:]
> + except (ValueError, IOError, IndexError):
> + pass
> +
> + return name
> +
> +
That's as hideous as it is useful! :)
Paolo
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 09/17] tools/kvm_stat: display guest name when using pid filter
2017-03-09 16:54 ` Paolo Bonzini
@ 2017-03-10 6:08 ` Stefan Raspl
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-03-10 6:08 UTC (permalink / raw)
To: Paolo Bonzini, kvm; +Cc: rkrcmar, frankja
On 09.03.2017 17:54, Paolo Bonzini wrote:
>
>
> On 20/02/2017 16:42, Stefan Raspl wrote:
>> +def get_gname_from_pid(pid):
>> + """Returns the guest name for a QEMU process pid.
>> +
>> + Extracts the guest name from the QEMU comma line by processing the '-name'
>> + option. Will also handle names specified out of sequence.
>> +
>> + """
>> + name = ''
>> + try:
>> + line = open('/proc/{}/cmdline'.format(pid), 'rb').read().split('\0')
>> + parms = line[line.index('-name') + 1].split(',')
>> + while '' in parms:
>> + # commas are escaped (i.e. ',,'), hence e.g. 'foo,bar' results in
>> + # ['foo', '', 'bar'], which we revert here
>> + idx = parms.index('')
>> + parms[idx - 1] += ',' + parms[idx + 1]
>> + del parms[idx:idx+2]
>> + # the '-name' switch allows for two ways to specify the guest name,
>> + # where the plain name overrides the name specified via 'guest='
>> + for arg in parms:
>> + if '=' not in arg:
>> + name = arg
>> + break
>> + if arg[:6] == 'guest=':
>> + name = arg[6:]
>> + except (ValueError, IOError, IndexError):
>> + pass
>> +
>> + return name
>> +
>> +
>
> That's as hideous as it is useful! :)
I'm not going to argue with that!
Ciao,
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 10/17] tools/kvm_stat: remove pid filter on empty input
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (8 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 09/17] tools/kvm_stat: display guest name when using pid filter Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-02-20 15:42 ` [PATCH 11/17] tools/kvm_stat: print error messages on faulty pid filter input Stefan Raspl
` (8 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Improve consistency in the interactive dialogue for pid filtering by
removing any filters on empty input (in addition to entering 0).
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 2ef7308..8271b41 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -990,10 +990,13 @@ class Tui(object):
curses.noecho()
try:
- pid = int(pid)
- if pid != 0 and not os.path.isdir(os.path.join('/proc/',
- str(pid))):
- continue
+ if len(pid) > 0:
+ pid = int(pid)
+ if pid != 0 and not os.path.isdir(os.path.join('/proc/',
+ str(pid))):
+ continue
+ else:
+ pid = 0
self.refresh_header(pid)
self.update_pid(pid)
break
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 11/17] tools/kvm_stat: print error messages on faulty pid filter input
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (9 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 10/17] tools/kvm_stat: remove pid filter on empty input Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-02-20 15:42 ` [PATCH 12/17] tools/kvm_stat: display regex when set to non-default Stefan Raspl
` (7 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Print helpful messages in case users enter invalid input or invalid pids in
the interactive pid filter dialogue.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 8271b41..db6a624 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -975,6 +975,7 @@ class Tui(object):
Asks for a pid until a valid pid or 0 has been entered.
"""
+ msg = ''
while True:
self.screen.erase()
self.screen.addstr(0, 0,
@@ -983,6 +984,7 @@ class Tui(object):
self.screen.addstr(1, 0,
'This might limit the shown data to the trace '
'statistics.')
+ self.screen.addstr(5, 0, msg)
curses.echo()
self.screen.addstr(3, 0, "Pid [0 or pid]: ")
@@ -994,6 +996,7 @@ class Tui(object):
pid = int(pid)
if pid != 0 and not os.path.isdir(os.path.join('/proc/',
str(pid))):
+ msg = '"' + str(pid) + '": Not a running process'
continue
else:
pid = 0
@@ -1002,6 +1005,7 @@ class Tui(object):
break
except ValueError:
+ msg = '"' + str(pid) + '": Not a valid pid'
continue
def show_stats(self):
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 12/17] tools/kvm_stat: display regex when set to non-default
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (10 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 11/17] tools/kvm_stat: print error messages on faulty pid filter input Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-02-20 15:42 ` [PATCH 13/17] tools/kvm_stat: remove regex filter on empty input Stefan Raspl
` (6 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
If a user defines a regex filter through the interactive command, display
the active regex in the header's second line.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index db6a624..da7bd0b 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -834,6 +834,7 @@ NUMBER_WIDTH = 10
DELAY_INITIAL = 0.25
DELAY_REGULAR = 3.
MAX_GUEST_NAME_LEN = 48
+MAX_REGEX_LEN = 44
class Tui(object):
@@ -904,6 +905,11 @@ class Tui(object):
.format(pid, gname), curses.A_BOLD)
else:
self.screen.addstr(0, 0, 'kvm statistics - summary', curses.A_BOLD)
+ if self.stats.fields_filter and self.stats.fields_filter != '^[^\(]*$':
+ regex = self.stats.fields_filter
+ if len(regex) > MAX_REGEX_LEN:
+ regex = regex[:MAX_REGEX_LEN] + '...'
+ self.screen.addstr(1, 17, 'regex filter: {0}'.format(regex))
self.screen.addstr(2, 1, 'Event')
self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH -
len('Total'), 'Total')
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 13/17] tools/kvm_stat: remove regex filter on empty input
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (11 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 12/17] tools/kvm_stat: display regex when set to non-default Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-02-20 15:42 ` [PATCH 14/17] tools/kvm_stat: add option '--guest' Stefan Raspl
` (5 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Behavior on empty/0 input for regex and pid filtering was inconsistent, as
the former would keep the current filter, while the latter would (naturally)
remove any pid filtering.
Make things consistent by falling back to the default filter on empty input
for the regex filter dialogue.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index da7bd0b..17d2eff 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -965,6 +965,7 @@ class Tui(object):
regex = self.screen.getstr()
curses.noecho()
if len(regex) == 0:
+ self.stats.fields_filter = r'^[^\(]*$'
self.refresh_header()
return
try:
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 14/17] tools/kvm_stat: add option '--guest'
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (12 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 13/17] tools/kvm_stat: remove regex filter on empty input Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-03-10 11:52 ` Janosch Frank
2017-02-20 15:42 ` [PATCH 15/17] tools/kvm_stat: add interactive command 'c' Stefan Raspl
` (4 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Add a new option '-g'/'--guest' to select a particular process by providing
the QEMU guest name.
Notes:
- The logic to figure out the pid corresponding to the guest name might look
scary, but works pretty reliably in practice; in the unlikely event that it
returns add'l flukes, it will bail out and hint at using '-p' instead, no
harm done.
- Mixing '-g' and '-p' is possible, and the final instance specified on the
command line is the significant one. This is consistent with current
behavior for '-p' which, if specified multiple times, also regards the final
instance as the significant one.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 101 +++++++++++++++++++++++++++++++++++++++-
tools/kvm/kvm_stat/kvm_stat.txt | 6 +++
2 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index 17d2eff..ec77aac 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -30,6 +30,7 @@ import fcntl
import resource
import struct
import re
+import subprocess
from collections import defaultdict
VMX_EXIT_REASONS = {
@@ -320,6 +321,30 @@ def parse_int_list(list_string):
return integers
+def get_pid_from_gname(gname):
+ """Fuzzy function to convert guest name to QEMU process pid.
+
+ Returns a list of potential pids, can be empty if no match found.
+ Throws an exception on processing errors.
+
+ """
+ pids = []
+ try:
+ child = subprocess.Popen(['ps', '-A', '--format', 'pid,args'],
+ stdout=subprocess.PIPE)
+ except:
+ raise Exception
+ for line in child.stdout:
+ line = line.lstrip().split(' ', 1)
+ # perform a sanity check before calling the more expensive
+ # function to possibly extract the guest name
+ if ' -name ' in line[1] and gname == get_gname_from_pid(line[0]):
+ pids.append(int(line[0]))
+ child.stdout.close()
+
+ return pids
+
+
def get_gname_from_pid(pid):
"""Returns the guest name for a QEMU process pid.
@@ -976,7 +1001,7 @@ class Tui(object):
except re.error:
continue
- def show_vm_selection(self):
+ def show_vm_selection_by_pid(self):
"""Draws PID selection mask.
Asks for a pid until a valid pid or 0 has been entered.
@@ -1015,6 +1040,50 @@ class Tui(object):
msg = '"' + str(pid) + '": Not a valid pid'
continue
+ def show_vm_selection_by_guest_name(self):
+ """Draws guest selection mask.
+
+ Asks for a guest name until a valid guest name or '' is entered.
+
+ """
+ msg = ''
+ while True:
+ self.screen.erase()
+ self.screen.addstr(0, 0,
+ 'Show statistics for specific guest.',
+ curses.A_BOLD)
+ self.screen.addstr(1, 0,
+ 'This might limit the shown data to the trace '
+ 'statistics.')
+ self.screen.addstr(5, 0, msg)
+ curses.echo()
+ self.screen.addstr(3, 0, "Guest [ENTER or guest]: ")
+ gname = self.screen.getstr()
+ curses.noecho()
+
+ if not gname:
+ self.refresh_header(0)
+ self.update_pid(0)
+ break
+ else:
+ pids = []
+ try:
+ pids = get_pid_from_gname(gname)
+ except:
+ msg = '"' + gname + '": Internal error while searching, ' \
+ 'use pid filter instead'
+ continue
+ if len(pids) == 0:
+ msg = '"' + gname + '": Not an active guest'
+ continue
+ if len(pids) > 1:
+ msg = '"' + gname + '": Multiple matches found, use pid ' \
+ 'filter instead'
+ continue
+ self.refresh_header(pids[0])
+ self.update_pid(pids[0])
+ break
+
def show_stats(self):
"""Refreshes the screen and processes user input."""
sleeptime = DELAY_INITIAL
@@ -1034,8 +1103,11 @@ class Tui(object):
if char == 'f':
self.show_filter_selection()
sleeptime = DELAY_INITIAL
+ if char == 'g':
+ self.show_vm_selection_by_guest_name()
+ sleeptime = DELAY_INITIAL
if char == 'p':
- self.show_vm_selection()
+ self.show_vm_selection_by_pid()
sleeptime = DELAY_INITIAL
except KeyboardInterrupt:
break
@@ -1105,6 +1177,7 @@ Requirements:
Interactive Commands:
f filter by regular expression
+ g filter by guest name
p filter by PID
q quit
x toggle reporting of stats for individual child trace events
@@ -1118,6 +1191,22 @@ Press any other key to refresh statistics immediately.
else:
return ""
+ def cb_guest_to_pid(option, opt, val, parser):
+ try:
+ pids = get_pid_from_gname(val)
+ except:
+ raise optparse.OptionValueError('Error while searching for guest '
+ '"{}", use "-p" to specify a pid '
+ 'instead'.format(val))
+ if len(pids) == 0:
+ raise optparse.OptionValueError('No guest by the name "{}" '
+ 'found'.format(val))
+ if len(pids) > 1:
+ raise optparse.OptionValueError('Multiple processes found (pids: '
+ '{}) - use "-p" to specify a pid '
+ 'instead'.format(" ".join(pids)))
+ parser.values.pid = pids[0]
+
optparser = optparse.OptionParser(description=description_text,
formatter=PlainHelpFormatter())
optparser.add_option('-1', '--once', '--batch',
@@ -1157,6 +1246,14 @@ Press any other key to refresh statistics immediately.
dest='pid',
help='restrict statistics to pid',
)
+ optparser.add_option('-g', '--guest',
+ action='callback',
+ type='string',
+ dest='pid',
+ metavar='GUEST',
+ help='restrict statistics to guest by name',
+ callback=cb_guest_to_pid,
+ )
(options, _) = optparser.parse_args(sys.argv)
return options
diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index 077bcc7..35587c3 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -31,6 +31,8 @@ INTERACTIVE COMMANDS
[horizontal]
*f*:: filter by regular expression
+*g*:: filter by guest name
+
*p*:: filter by PID
*q*:: quit
@@ -62,6 +64,10 @@ OPTIONS
--pid=<pid>::
limit statistics to one virtual machine (pid)
+-g<guest>::
+--guest=<guest_name>::
+ limit statistics to one virtual machine (guest name)
+
-f<fields>::
--fields=<fields>::
fields to display (regex)
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH 14/17] tools/kvm_stat: add option '--guest'
2017-02-20 15:42 ` [PATCH 14/17] tools/kvm_stat: add option '--guest' Stefan Raspl
@ 2017-03-10 11:52 ` Janosch Frank
0 siblings, 0 replies; 35+ messages in thread
From: Janosch Frank @ 2017-03-10 11:52 UTC (permalink / raw)
To: Stefan Raspl, kvm; +Cc: pbonzini, rkrcmar
[-- Attachment #1.1: Type: text/plain, Size: 1055 bytes --]
On 20.02.2017 16:42, Stefan Raspl wrote:
> Add a new option '-g'/'--guest' to select a particular process by providing
> the QEMU guest name.
> Notes:
> - The logic to figure out the pid corresponding to the guest name might look
> scary, but works pretty reliably in practice; in the unlikely event that it
> returns add'l flukes, it will bail out and hint at using '-p' instead, no
> harm done.
> - Mixing '-g' and '-p' is possible, and the final instance specified on the
> command line is the significant one. This is consistent with current
> behavior for '-p' which, if specified multiple times, also regards the final
> instance as the significant one.
>
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
This is really helpful, no more extra terminals for searching guest pids
anymore. And I hope qemu will not change the name passing for a very
long time, I want to avoid having to find solutions to such terrible
string parsing problems again.
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 15/17] tools/kvm_stat: add interactive command 'c'
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (13 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 14/17] tools/kvm_stat: add option '--guest' Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-02-20 15:42 ` [PATCH 16/17] tools/kvm_stat: add interactive command 'r' Stefan Raspl
` (3 subsequent siblings)
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Provide a real simple way to erase any active filter.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 16 ++++++++++++----
tools/kvm/kvm_stat/kvm_stat.txt | 2 ++
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index ec77aac..e0f28e1 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -860,6 +860,7 @@ DELAY_INITIAL = 0.25
DELAY_REGULAR = 3.
MAX_GUEST_NAME_LEN = 48
MAX_REGEX_LEN = 44
+DEFAULT_REGEX = r'^[^\(]*$'
class Tui(object):
@@ -906,9 +907,9 @@ class Tui(object):
def update_drilldown(self):
"""Sets or removes a filter that only allows fields without braces."""
if not self.stats.fields_filter:
- self.stats.fields_filter = r'^[^\(]*$'
+ self.stats.fields_filter = DEFAULT_REGEX
- elif self.stats.fields_filter == r'^[^\(]*$':
+ elif self.stats.fields_filter == DEFAULT_REGEX:
self.stats.fields_filter = None
def update_pid(self, pid):
@@ -930,7 +931,8 @@ class Tui(object):
.format(pid, gname), curses.A_BOLD)
else:
self.screen.addstr(0, 0, 'kvm statistics - summary', curses.A_BOLD)
- if self.stats.fields_filter and self.stats.fields_filter != '^[^\(]*$':
+ if self.stats.fields_filter and self.stats.fields_filter \
+ != DEFAULT_REGEX:
regex = self.stats.fields_filter
if len(regex) > MAX_REGEX_LEN:
regex = regex[:MAX_REGEX_LEN] + '...'
@@ -990,7 +992,7 @@ class Tui(object):
regex = self.screen.getstr()
curses.noecho()
if len(regex) == 0:
- self.stats.fields_filter = r'^[^\(]*$'
+ self.stats.fields_filter = DEFAULT_REGEX
self.refresh_header()
return
try:
@@ -1100,6 +1102,11 @@ class Tui(object):
sleeptime = DELAY_INITIAL
if char == 'q':
break
+ if char == 'c':
+ self.stats.fields_filter = DEFAULT_REGEX
+ self.refresh_header(0)
+ self.update_pid(0)
+ sleeptime = DELAY_INITIAL
if char == 'f':
self.show_filter_selection()
sleeptime = DELAY_INITIAL
@@ -1176,6 +1183,7 @@ Requirements:
the large number of files that are possibly opened.
Interactive Commands:
+ c clear filter
f filter by regular expression
g filter by guest name
p filter by PID
diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index 35587c3..c3ab6a2 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -29,6 +29,8 @@ meaning of events.
INTERACTIVE COMMANDS
--------------------
[horizontal]
+*c*:: clear filter
+
*f*:: filter by regular expression
*g*:: filter by guest name
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH 16/17] tools/kvm_stat: add interactive command 'r'
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (14 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 15/17] tools/kvm_stat: add interactive command 'c' Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-03-10 11:37 ` Janosch Frank
2017-02-20 15:42 ` [PATCH 17/17] tools/kvm_stat: add '%Total' column Stefan Raspl
` (2 subsequent siblings)
18 siblings, 1 reply; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Provide an interactive command to reset the tracepoint statistics.
Requires some extra work for debugfs, as the counters cannot be reset.
On the up side, offers us the opportunity to have debugfs values reset
on startup and whenever a filter is modified, becoming consistent with
the tracepoint provider. As a bonus, 'kvmstat -dt' will now provide
useful output, instead of mixing values in totally different orders of
magnitude.
Furthermore, we avoid unnecessary resets when any of the filters is
"changed" interactively to the previous value.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 66 +++++++++++++++++++++++++++++++----------
tools/kvm/kvm_stat/kvm_stat.txt | 2 ++
2 files changed, 53 insertions(+), 15 deletions(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index e0f28e1..f9f653a 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -715,15 +715,23 @@ class TracepointProvider(object):
ret[name] += val
return ret
+ def reset(self):
+ """Reset all field counters"""
+ for group in self.group_leaders:
+ for event in group.events:
+ event.reset()
+
class DebugfsProvider(object):
"""Provides data from the files that KVM creates in the kvm debugfs
folder."""
def __init__(self):
self._fields = self.get_available_fields()
+ self._baseline = {}
self._pid = 0
self.do_read = True
self.paths = []
+ self.reset()
def get_available_fields(self):
""""Returns a list of available fields.
@@ -740,6 +748,7 @@ class DebugfsProvider(object):
@fields.setter
def fields(self, fields):
self._fields = fields
+ self.reset()
@property
def pid(self):
@@ -757,10 +766,11 @@ class DebugfsProvider(object):
self.paths = filter(lambda x: "{}-".format(pid) in x, vms)
else:
- self.paths = ['']
+ self.paths = []
self.do_read = True
+ self.reset()
- def read(self):
+ def read(self, reset=0):
"""Returns a dict with format:'file name / field -> current value'."""
results = {}
@@ -768,10 +778,22 @@ class DebugfsProvider(object):
if not self.do_read:
return results
- for path in self.paths:
+ paths = self.paths
+ if self._pid == 0:
+ paths = []
+ for entry in os.walk(PATH_DEBUGFS_KVM):
+ for dir in entry[1]:
+ paths.append(dir)
+ for path in paths:
for field in self._fields:
- results[field] = results.get(field, 0) \
- + self.read_field(field, path)
+ value = self.read_field(field, path)
+ key = path + field
+ if reset:
+ self._baseline[key] = value
+ if self._baseline.get(key, -1) == -1:
+ self._baseline[key] = value
+ results[field] = (results.get(field, 0) + value -
+ self._baseline.get(key, 0))
return results
@@ -785,6 +807,11 @@ class DebugfsProvider(object):
except IOError:
return 0
+ def reset(self):
+ """Reset field counters"""
+ self._baseline = {}
+ self.read(1)
+
class Stats(object):
"""Manages the data providers and the data they provide.
@@ -821,14 +848,20 @@ class Stats(object):
for provider in self.providers:
provider.pid = self._pid_filter
+ def reset(self):
+ self.values = {}
+ for provider in self.providers:
+ provider.reset()
+
@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()
+ if fields_filter != self._fields_filter:
+ self._fields_filter = fields_filter
+ self.update_provider_filters()
@property
def pid_filter(self):
@@ -836,9 +869,10 @@ class Stats(object):
@pid_filter.setter
def pid_filter(self, pid):
- self._pid_filter = pid
- self.values = {}
- self.update_provider_pid()
+ if pid != self._pid_filter:
+ self._pid_filter = pid
+ self.values = {}
+ self.update_provider_pid()
def get(self):
"""Returns a dict with field -> (value, delta to last value) of all
@@ -846,11 +880,9 @@ class Stats(object):
for provider in self.providers:
new = provider.read()
for key in provider.fields:
- oldval = self.values.get(key, (0, 0))
+ oldval = self.values.get(key, (0, 0))[0]
newval = new.get(key, 0)
- newdelta = None
- if oldval is not None:
- newdelta = newval - oldval[0]
+ newdelta = newval - oldval
self.values[key] = (newval, newdelta)
return self.values
@@ -1116,6 +1148,10 @@ class Tui(object):
if char == 'p':
self.show_vm_selection_by_pid()
sleeptime = DELAY_INITIAL
+ if char == 'r':
+ self.refresh_header()
+ self.stats.reset()
+ sleeptime = DELAY_INITIAL
except KeyboardInterrupt:
break
except curses.error:
@@ -1188,7 +1224,7 @@ Interactive Commands:
g filter by guest name
p filter by PID
q quit
- x toggle reporting of stats for individual child trace events
+ r reset stats
Press any other key to refresh statistics immediately.
"""
diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
index c3ab6a2..109431b 100644
--- a/tools/kvm/kvm_stat/kvm_stat.txt
+++ b/tools/kvm/kvm_stat/kvm_stat.txt
@@ -39,6 +39,8 @@ INTERACTIVE COMMANDS
*q*:: quit
+*r*:: reset stats
+
*x*:: toggle reporting of stats for child trace events
Press any other key to refresh statistics immediately.
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH 16/17] tools/kvm_stat: add interactive command 'r'
2017-02-20 15:42 ` [PATCH 16/17] tools/kvm_stat: add interactive command 'r' Stefan Raspl
@ 2017-03-10 11:37 ` Janosch Frank
2017-03-10 12:32 ` Stefan Raspl
0 siblings, 1 reply; 35+ messages in thread
From: Janosch Frank @ 2017-03-10 11:37 UTC (permalink / raw)
To: Stefan Raspl, kvm; +Cc: pbonzini, rkrcmar
[-- Attachment #1.1: Type: text/plain, Size: 3188 bytes --]
On 20.02.2017 16:42, Stefan Raspl wrote:
> Provide an interactive command to reset the tracepoint statistics.
> Requires some extra work for debugfs, as the counters cannot be reset.
> On the up side, offers us the opportunity to have debugfs values reset
^
add "this"
> on startup and whenever a filter is modified, becoming consistent with
> the tracepoint provider. As a bonus, 'kvmstat -dt' will now provide
> useful output, instead of mixing values in totally different orders of
> magnitude.
> Furthermore, we avoid unnecessary resets when any of the filters is
> "changed" interactively to the previous value.
Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
>
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
> ---
> tools/kvm/kvm_stat/kvm_stat | 66 +++++++++++++++++++++++++++++++----------
> tools/kvm/kvm_stat/kvm_stat.txt | 2 ++
> 2 files changed, 53 insertions(+), 15 deletions(-)
>
> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
> index e0f28e1..f9f653a 100755
> --- a/tools/kvm/kvm_stat/kvm_stat
> +++ b/tools/kvm/kvm_stat/kvm_stat
> @@ -715,15 +715,23 @@ class TracepointProvider(object):
> ret[name] += val
> return ret
>
> + def reset(self):
> + """Reset all field counters"""
> + for group in self.group_leaders:
> + for event in group.events:
> + event.reset()
> +
>
> class DebugfsProvider(object):
> """Provides data from the files that KVM creates in the kvm debugfs
> folder."""
> def __init__(self):
> self._fields = self.get_available_fields()
> + self._baseline = {}
> self._pid = 0
> self.do_read = True
> self.paths = []
> + self.reset()
>
> def get_available_fields(self):
> """"Returns a list of available fields.
> @@ -740,6 +748,7 @@ class DebugfsProvider(object):
> @fields.setter
> def fields(self, fields):
> self._fields = fields
> + self.reset()
>
> @property
> def pid(self):
> @@ -757,10 +766,11 @@ class DebugfsProvider(object):
> self.paths = filter(lambda x: "{}-".format(pid) in x, vms)
>
> else:
> - self.paths = ['']
> + self.paths = []
This is more a cleanup than new code, isn't it?
[...]
> @@ -1188,7 +1224,7 @@ Interactive Commands:
> g filter by guest name
> p filter by PID
> q quit
> - x toggle reporting of stats for individual child trace events
> + r reset stats
> Press any other key to refresh statistics immediately.
> """
This is inconsistent to the part below.
> diff --git a/tools/kvm/kvm_stat/kvm_stat.txt b/tools/kvm/kvm_stat/kvm_stat.txt
> index c3ab6a2..109431b 100644
> --- a/tools/kvm/kvm_stat/kvm_stat.txt
> +++ b/tools/kvm/kvm_stat/kvm_stat.txt
> @@ -39,6 +39,8 @@ INTERACTIVE COMMANDS
>
> *q*:: quit
>
> +*r*:: reset stats
> +
> *x*:: toggle reporting of stats for child trace events
>
> Press any other key to refresh statistics immediately.
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 16/17] tools/kvm_stat: add interactive command 'r'
2017-03-10 11:37 ` Janosch Frank
@ 2017-03-10 12:32 ` Stefan Raspl
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-03-10 12:32 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: pbonzini, rkrcmar
On 10.03.2017 12:37, Janosch Frank wrote:
>> @@ -757,10 +766,11 @@ class DebugfsProvider(object):
>> self.paths = filter(lambda x: "{}-".format(pid) in x, vms)
>>
>> else:
>> - self.paths = ['']
>> + self.paths = []
>
> This is more a cleanup than new code, isn't it?
Not quite: The former holds an element, while the latter doesn't.
Therefore a subsequent 'for path in self.paths:' would execute with
the former version, but won't with the latter.
> [...]
>> @@ -1188,7 +1224,7 @@ Interactive Commands:
>> g filter by guest name
>> p filter by PID
>> q quit
>> - x toggle reporting of stats for individual child trace events
>> + r reset stats
>> Press any other key to refresh statistics immediately.
>> """
>
> This is inconsistent to the part below.
Yup, accidental deletion - good catch, thanks!
Ciao,
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 17/17] tools/kvm_stat: add '%Total' column
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (15 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 16/17] tools/kvm_stat: add interactive command 'r' Stefan Raspl
@ 2017-02-20 15:42 ` Stefan Raspl
2017-03-06 8:08 ` [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
2017-03-09 17:00 ` Paolo Bonzini
18 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-02-20 15:42 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
Add column '%Total' next to 'Total' for easier comparison of numbers between
hosts.
Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Marc Hartmayer <mhartmay@linux.vnet.ibm.com>
---
tools/kvm/kvm_stat/kvm_stat | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
index f9f653a..9e40c8a 100755
--- a/tools/kvm/kvm_stat/kvm_stat
+++ b/tools/kvm/kvm_stat/kvm_stat
@@ -972,7 +972,9 @@ class Tui(object):
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 -
+ self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH + 7 -
+ len('%Total'), '%Total')
+ self.screen.addstr(2, 1 + LABEL_WIDTH + NUMBER_WIDTH + 7 + 8 -
len('Current'), 'Current')
self.screen.addstr(4, 1, 'Collecting data...')
self.screen.refresh()
@@ -988,6 +990,9 @@ class Tui(object):
return (-stats[x][1], -stats[x][0])
else:
return (0, -stats[x][0])
+ total = 0.
+ for val in stats.values():
+ total += val[0]
for key in sorted(stats.keys(), key=sortkey):
if row >= self.screen.getmaxyx()[0]:
@@ -1000,6 +1005,8 @@ class Tui(object):
col += LABEL_WIDTH
self.screen.addstr(row, col, '%10d' % (values[0],))
col += NUMBER_WIDTH
+ self.screen.addstr(row, col, '%7.1f' % (values[0] * 100 / total,))
+ col += 7
if values[1] is not None:
self.screen.addstr(row, col, '%8d' % (values[1] / sleeptime,))
row += 1
--
2.8.4
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH 00/17] tools/kvm_stat: Misc Patches
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (16 preceding siblings ...)
2017-02-20 15:42 ` [PATCH 17/17] tools/kvm_stat: add '%Total' column Stefan Raspl
@ 2017-03-06 8:08 ` Stefan Raspl
2017-03-06 16:05 ` Paolo Bonzini
2017-03-09 17:00 ` Paolo Bonzini
18 siblings, 1 reply; 35+ messages in thread
From: Stefan Raspl @ 2017-03-06 8:08 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, rkrcmar, frankja
*ping*
On 20.02.2017 16:41, Stefan Raspl wrote:
> Assortment of various fixes and improvements for the kvm_stat utility.
> Patches 1-5 are bugfixes.
> Patch 6 provides full PEP8 compliance by inserting the required empty
> lines. It is included as an extra for easy disposal, since that kind of
> patch might be a bit controversial.
> There are some subtle aspects regarding patch 7, as its effects might
> differ per platform. However, I didn't notice any negative effects, only
> improvements.
> Patch 8 is a simple doc update.
> Patches 9-13 provide usability improvements.
> Patches 14-17 add new features.
>
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
>
> ---
>
> Stefan Raspl (17):
> tools/kvm_stat: hide cursor
> tools/kvm_stat: catch curses exceptions only
> tools/kvm_stat: handle SIGINT in log and batch modes
> tools/kvm_stat: fix misc glitches
> tools/kvm_stat: fix trace setup glitch on field updates in
> TracepointProvider
> tools/kvm_stat: full PEP8 compliance
> tools/kvm_stat: reduce perceived idle time on filter updates
> tools/kvm_stat: document list of interactive commands
> tools/kvm_stat: display guest name when using pid filter
> tools/kvm_stat: remove pid filter on empty input
> tools/kvm_stat: print error messages on faulty pid filter input
> tools/kvm_stat: display regex when set to non-default
> tools/kvm_stat: remove regex filter on empty input
> tools/kvm_stat: add option '--guest'
> tools/kvm_stat: add interactive command 'c'
> tools/kvm_stat: add interactive command 'r'
> tools/kvm_stat: add '%Total' column
>
> tools/kvm/kvm_stat/kvm_stat | 383 +++++++++++++++++++++++++++++++++-------
> tools/kvm/kvm_stat/kvm_stat.txt | 26 +++
> 2 files changed, 343 insertions(+), 66 deletions(-)
>
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 00/17] tools/kvm_stat: Misc Patches
2017-03-06 8:08 ` [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
@ 2017-03-06 16:05 ` Paolo Bonzini
0 siblings, 0 replies; 35+ messages in thread
From: Paolo Bonzini @ 2017-03-06 16:05 UTC (permalink / raw)
To: Stefan Raspl, kvm; +Cc: rkrcmar, frankja
On 06/03/2017 09:08, Stefan Raspl wrote:
> *ping*
It's on the to-do list, but typically only very urgent patches get
reviewed during the merge window.
Paolo
>
> On 20.02.2017 16:41, Stefan Raspl wrote:
>> Assortment of various fixes and improvements for the kvm_stat utility.
>> Patches 1-5 are bugfixes.
>> Patch 6 provides full PEP8 compliance by inserting the required empty
>> lines. It is included as an extra for easy disposal, since that kind of
>> patch might be a bit controversial.
>> There are some subtle aspects regarding patch 7, as its effects might
>> differ per platform. However, I didn't notice any negative effects, only
>> improvements.
>> Patch 8 is a simple doc update.
>> Patches 9-13 provide usability improvements.
>> Patches 14-17 add new features.
>>
>> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
>>
>> ---
>>
>> Stefan Raspl (17):
>> tools/kvm_stat: hide cursor
>> tools/kvm_stat: catch curses exceptions only
>> tools/kvm_stat: handle SIGINT in log and batch modes
>> tools/kvm_stat: fix misc glitches
>> tools/kvm_stat: fix trace setup glitch on field updates in
>> TracepointProvider
>> tools/kvm_stat: full PEP8 compliance
>> tools/kvm_stat: reduce perceived idle time on filter updates
>> tools/kvm_stat: document list of interactive commands
>> tools/kvm_stat: display guest name when using pid filter
>> tools/kvm_stat: remove pid filter on empty input
>> tools/kvm_stat: print error messages on faulty pid filter input
>> tools/kvm_stat: display regex when set to non-default
>> tools/kvm_stat: remove regex filter on empty input
>> tools/kvm_stat: add option '--guest'
>> tools/kvm_stat: add interactive command 'c'
>> tools/kvm_stat: add interactive command 'r'
>> tools/kvm_stat: add '%Total' column
>>
>> tools/kvm/kvm_stat/kvm_stat | 383 +++++++++++++++++++++++++++++++++-------
>> tools/kvm/kvm_stat/kvm_stat.txt | 26 +++
>> 2 files changed, 343 insertions(+), 66 deletions(-)
>>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/17] tools/kvm_stat: Misc Patches
2017-02-20 15:41 [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
` (17 preceding siblings ...)
2017-03-06 8:08 ` [PATCH 00/17] tools/kvm_stat: Misc Patches Stefan Raspl
@ 2017-03-09 17:00 ` Paolo Bonzini
2017-03-10 6:13 ` Stefan Raspl
18 siblings, 1 reply; 35+ messages in thread
From: Paolo Bonzini @ 2017-03-09 17:00 UTC (permalink / raw)
To: Stefan Raspl, kvm; +Cc: rkrcmar, frankja
On 20/02/2017 16:41, Stefan Raspl wrote:
> Assortment of various fixes and improvements for the kvm_stat utility.
> Patches 1-5 are bugfixes.
> Patch 6 provides full PEP8 compliance by inserting the required empty
> lines. It is included as an extra for easy disposal, since that kind of
> patch might be a bit controversial.
> There are some subtle aspects regarding patch 7, as its effects might
> differ per platform. However, I didn't notice any negative effects, only
> improvements.
> Patch 8 is a simple doc update.
> Patches 9-13 provide usability improvements.
> Patches 14-17 add new features.
>
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
Really nice -- just two very small comments.
The hard-coded knowledge of QEMU is not the nicest thing (after all
kvm_stat is part of the Linux repository), but it is useful so I'm not
going to complain too loudly.
Paolo
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 00/17] tools/kvm_stat: Misc Patches
2017-03-09 17:00 ` Paolo Bonzini
@ 2017-03-10 6:13 ` Stefan Raspl
0 siblings, 0 replies; 35+ messages in thread
From: Stefan Raspl @ 2017-03-10 6:13 UTC (permalink / raw)
To: Paolo Bonzini, kvm; +Cc: rkrcmar, frankja
On 09.03.2017 18:00, Paolo Bonzini wrote:
>
>
> On 20/02/2017 16:41, Stefan Raspl wrote:
>> Assortment of various fixes and improvements for the kvm_stat utility.
>> Patches 1-5 are bugfixes.
>> Patch 6 provides full PEP8 compliance by inserting the required empty
>> lines. It is included as an extra for easy disposal, since that kind of
>> patch might be a bit controversial.
>> There are some subtle aspects regarding patch 7, as its effects might
>> differ per platform. However, I didn't notice any negative effects, only
>> improvements.
>> Patch 8 is a simple doc update.
>> Patches 9-13 provide usability improvements.
>> Patches 14-17 add new features.
>>
>> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
>
> Really nice -- just two very small comments.
>
> The hard-coded knowledge of QEMU is not the nicest thing (after all
> kvm_stat is part of the Linux repository), but it is useful so I'm not
> going to complain too loudly.
Thx for the credit :)
Have to admit that I totally missed the QEMU-only aspect. Still, if
somebody
is running something else, he'll get no result/a proper hint, so we
should be
fine.
Ciao,
Stefan
^ permalink raw reply [flat|nested] 35+ messages in thread