tools/kvm_stat: add logfile support

PATCH Superseded Stefan Raspl 2 participants 10 messages Mar 31, 2020 – Apr 1, 2020 KVM

Share & tools

Stefan Raspl

author

2020-03-31 20:00 UTC #0 lore raw

cover tools/kvm_stat: add logfile support

Next attempt to come up with support for logfiles that can be combined with a solution for rotating logs.
Adding another patch to skip records with all zeros to preserve space.

Stefan Raspl (3):
tools/kvm_stat: add command line switch '-z' to skip zero records tools/kvm_stat: Add command line switch '-L' to log to file tools/kvm_stat: add sample systemd unit file

 tools/kvm/kvm_stat/kvm_stat         | 110 ++++++++++++++++++++++++----
 tools/kvm/kvm_stat/kvm_stat.service |  16 ++++
 tools/kvm/kvm_stat/kvm_stat.txt     |   9 +++
 3 files changed, 121 insertions(+), 14 deletions(-)
 create mode 100644 tools/kvm/kvm_stat/kvm_stat.service

Signature · 1 line

2.17.1

Stefan Raspl

author

2020-03-31 20:00 UTC #1 lore raw

PATCH 1/3 tools/kvm_stat: add command line switch '-z' to skip zero records

From: Stefan Raspl <[email protected]>

When running in logging mode, skip records with all zeros (=empty records) to preserve space when logging to files.

Signed-off-by Stefan Raspl <[email protected]>

---
 tools/kvm/kvm_stat/kvm_stat     | 47 +++++++++++++++++++++++++--------
 tools/kvm/kvm_stat/kvm_stat.txt |  4 +++
 2 files changed, 40 insertions(+), 11 deletions(-)

tools/kvm/kvm_stat/kvm_stat +36 ~~-11~~

index 7fe767bd2625..54000ac508f9 100755
@@ -1488,7 +1488,8 @@ def batch(stats):
 
 
 class StdFormat(object):
-    def __init__(self, keys):
+    def __init__(self, keys, skip_zero_records):
+        self._skip_zero_records = skip_zero_records
         self._banner = ''
         for key in keys:
             self._banner += key.split(' ')[0] + ' '

@@ -1496,16 +1497,21 @@ class StdFormat(object):
     def get_banner(self):
         return self._banner
 
-    @staticmethod
-    def get_statline(keys, s):
+    def get_statline(self, keys, s):
         res = ''
+        non_zero = False
         for key in keys:
+            if s[key].delta != 0:
+                non_zero = True
             res += ' %9d' % s[key].delta
+        if self._skip_zero_records and not non_zero:
+            return ''
         return res
 
 
 class CSVFormat(object):
-    def __init__(self, keys):
+    def __init__(self, keys, skip_zero_records):
+        self._skip_zero_records = skip_zero_records
         self._banner = 'timestamp'
         self._banner += reduce(lambda res, key: "{},{!s}".format(res,
                                key.split(' ')[0]), keys, '')

@@ -1513,8 +1519,14 @@ class CSVFormat(object):
     def get_banner(self):
         return self._banner
 
-    @staticmethod
-    def get_statline(keys, s):
+    def get_statline(self, keys, s):
+        if self._skip_zero_records:
+            non_zero = False
+            for key in keys:
+                if s[key].delta != 0:
+                    non_zero = True
+            if self._skip_zero_records and not non_zero:
+                return ''
         return reduce(lambda res, key: "{},{!s}".format(res, s[key].delta),
                       keys, '')

@@ -1523,14 +1535,20 @@ def log(stats, opts, frmt, keys):
     """Prints statistics as reiterating key block, multiple value blocks."""
     line = 0
     banner_repeat = 20
+    banner_printed = False
+
     while True:
         try:
             time.sleep(opts.set_delay)
-            if line % banner_repeat == 0:
+            if line % banner_repeat == 0 and not banner_printed:
                 print(frmt.get_banner())
-            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") +
-                  frmt.get_statline(keys, stats.get()))
+                banner_printed = True
+            statline = frmt.get_statline(keys, stats.get())
+            if len(statline) == 0:
+                continue
+            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)
             line += 1
+            banner_printed = False
         except KeyboardInterrupt:
             break

@@ -1651,9 +1669,16 @@ Press any other key to refresh statistics immediately.
                            default=False,
                            help='retrieve statistics from tracepoints',
                            )
+    argparser.add_argument('-z', '--skip-zero-records',
+                           action='store_true',
+                           default=False,
+                           help='omit records with all zeros in logging mode',
+                           )
     options = argparser.parse_args()
     if options.csv and not options.log:
         sys.exit('Error: Option -c/--csv requires -l/--log')
+    if options.skip_zero_records and not options.log:
+        sys.exit('Error: Option -z/--skip-zero-records requires -l/--log')
     try:
         # verify that we were passed a valid regex up front
         re.compile(options.fields)

@@ -1736,9 +1761,9 @@ def main():
     if options.log:
         keys = sorted(stats.get().keys())
         if options.csv:
-            frmt = CSVFormat(keys)
+            frmt = CSVFormat(keys, options.skip_zero_records)
         else:
-            frmt = StdFormat(keys)
+            frmt = StdFormat(keys, options.skip_zero_records)
         log(stats, options, frmt, keys)
     elif not options.once:
         with Tui(stats, options) as tui:

tools/kvm/kvm_stat/kvm_stat.txt +4 -0

index a97ded2aedad..24296dccc00a 100644
@@ -104,6 +104,10 @@ OPTIONS
 --tracepoints::
         retrieve statistics from tracepoints
 
+*z*::
+--skip-zero-records::
+        omit records with all zeros in logging mode
+
 SEE ALSO
 --------
 'perf'(1), 'trace-cmd'(1)

Signature · 1 line

2.17.1

Paolo Bonzini

maintainer

2020-03-31 21:45 UTC #2 lore raw

Re: [PATCH 1/3] tools/kvm_stat: add command line switch '-z' to skip zero records

Quoted 5 lines · Stefan Raspl

tools/kvm/kvm_stat/kvm_stat +1 -0

@@ -1523,14 +1535,20 @@ def log(stats, opts, frmt, keys):
     """Prints statistics as reiterating key block, multiple value blocks."""
     line = 0
     banner_repeat = 20
+    banner_printed = False

tools/kvm/kvm_stat/kvm_stat def log(stats, opts, frmt, keys): · line 1545

Quoted 10 lines

tools/kvm/kvm_stat/kvm_stat +3 -3

+
     while True:
         try:
             time.sleep(opts.set_delay)
-            if line % banner_repeat == 0:
+            if line % banner_repeat == 0 and not banner_printed:
                 print(frmt.get_banner())
-            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") +
-                  frmt.get_statline(keys, stats.get()))
+                banner_printed = True

Can't skip_zero_records be handled here instead?

    values = stats.get()
    if not opts.skip_zero_records or \
        any((values[k].delta != 0 for k in keys):
       statline = frmt.get_statline(keys, values)
       print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)

Paolo

Quoted 2 lines

tools/kvm/kvm_stat/kvm_stat +1 -0

+            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)
             line += 1

Stefan Raspl

author

2020-04-01 08:19 UTC #3 lore raw

Re: [PATCH 1/3] tools/kvm_stat: add command line switch '-z' to skip zero records

Quoted 19 lines · Paolo Bonzini

Quoted 5 lines · Stefan Raspl

tools/kvm/kvm_stat/kvm_stat +1 -0

@@ -1523,14 +1535,20 @@ def log(stats, opts, frmt, keys):
     """Prints statistics as reiterating key block, multiple value blocks."""
     line = 0
     banner_repeat = 20
+    banner_printed = False

tools/kvm/kvm_stat/kvm_stat def log(stats, opts, frmt, keys): · line 1545

Quoted 10 lines

tools/kvm/kvm_stat/kvm_stat +3 -3

+
     while True:
         try:
             time.sleep(opts.set_delay)
-            if line % banner_repeat == 0:
+            if line % banner_repeat == 0 and not banner_printed:
                 print(frmt.get_banner())
-            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") +
-                  frmt.get_statline(keys, stats.get()))
+                banner_printed = True

Can't skip_zero_records be handled here instead?

Quoted 5 lines

    values = stats.get()
    if not opts.skip_zero_records or \
        any((values[k].delta != 0 for k in keys):
       statline = frmt.get_statline(keys, values)
       print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)

I wanted to avoid such an extra check for performance reasons. Granted, I have to do something likewise (i.e. checking for non-zero values) in my patch for csv records, but at least for the standard format things are a bit less costly (avoiding an extra pass over the data).

Ciao,
Stefan

Paolo Bonzini

maintainer

2020-04-01 13:23 UTC #4 lore raw

Re: [PATCH 1/3] tools/kvm_stat: add command line switch '-z' to skip zero records

Quoted 26 lines · Stefan Raspl

Quoted 19 lines · Paolo Bonzini

Quoted 5 lines · Stefan Raspl

tools/kvm/kvm_stat/kvm_stat +1 -0

@@ -1523,14 +1535,20 @@ def log(stats, opts, frmt, keys):
     """Prints statistics as reiterating key block, multiple value blocks."""
     line = 0
     banner_repeat = 20
+    banner_printed = False

tools/kvm/kvm_stat/kvm_stat def log(stats, opts, frmt, keys): · line 1545

Quoted 10 lines

tools/kvm/kvm_stat/kvm_stat +3 -3

+
     while True:
         try:
             time.sleep(opts.set_delay)
-            if line % banner_repeat == 0:
+            if line % banner_repeat == 0 and not banner_printed:
                 print(frmt.get_banner())
-            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") +
-                  frmt.get_statline(keys, stats.get()))
+                banner_printed = True

Can't skip_zero_records be handled here instead?

Quoted 5 lines

    values = stats.get()
    if not opts.skip_zero_records or \
        any((values[k].delta != 0 for k in keys):
       statline = frmt.get_statline(keys, values)
       print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)

Quoted 4 lines

I wanted to avoid such an extra check for performance reasons. Granted, I have to do something likewise (i.e. checking for non-zero values) in my patch for csv records, but at least for the standard format things are a bit less costly (avoiding an extra pass over the data).

I don't think it's a perceivable difference, and it simplifies the code noticeably.

Paolo

Stefan Raspl

author

2020-03-31 20:00 UTC #5 lore raw

PATCH 2/3 tools/kvm_stat: Add command line switch '-L' to log to file

In reply to Stefan Raspl

From: Stefan Raspl <[email protected]>

To integrate with logrotate, we have a signal handler that will re-open the logfile.
Assuming we have a systemd unit file with
ExecStart=kvm_stat -dtc -s 10 -L /var/log/kvm_stat.csv ExecReload=/bin/kill -HUP $MAINPID
and a logrotate config featuring
postrotate
/bin/systemctl restart kvm_stat.service
endscript
Then the overall flow will look like this:
(1) systemd starts kvm_stat, logging to A.
(2) At some point, logrotate runs, moving A to B.
kvm_stat continues to write to B at this point.
(3) After rotating, logrotate restarts the kvm_stat unit via systemctl.
(4) The kvm_stat unit sends a SIGHUP to kvm_stat, finally making it switch over to writing to A again.
Note that in order to keep the structure of the cvs output in tact, we make sure to, in contrast to the standard log format, only write the header once at the beginning of a file. This implies that the header is suppressed when appending to an existing file. Unlike the standard format, where we append to an existing file by starting out with a header.

Signed-off-by Stefan Raspl <[email protected]>

---
 tools/kvm/kvm_stat/kvm_stat     | 75 +++++++++++++++++++++++++++++----
 tools/kvm/kvm_stat/kvm_stat.txt |  5 +++
 2 files changed, 71 insertions(+), 9 deletions(-)

tools/kvm/kvm_stat/kvm_stat +66 -9

index 54000ac508f9..e4f67f0629ee 100755
@@ -32,6 +32,7 @@ import resource
 import struct
 import re
 import subprocess
+import signal
 from collections import defaultdict, namedtuple
 from functools import reduce
 from datetime import datetime

@@ -228,6 +229,8 @@ IOCTL_NUMBERS = {
     'RESET':       0x00002403,
 }
 
+signal_received = False
+
 ENCODING = locale.getpreferredencoding(False)
 TRACE_FILTER = re.compile(r'^[^\(]*$')

@@ -1533,25 +1536,73 @@ class CSVFormat(object):
 
 def log(stats, opts, frmt, keys):
     """Prints statistics as reiterating key block, multiple value blocks."""
+    global signal_received
     line = 0
     banner_repeat = 20
-    banner_printed = False
+    f = None
+
+    def do_banner(opts):
+        nonlocal f
+        if opts.log_to_file:
+            if not f:
+                try:
+                     f = open(opts.log_to_file, 'a')
+                except (IOError, OSError):
+                    sys.exit("Error: Could not open file: %s" %
+                             opts.log_to_file)
+                if isinstance(frmt, CSVFormat) and f.tell() != 0:
+                    return
+            f.write(frmt.get_banner())
+            f.write('\n')
+        else:
+            print(frmt.get_banner())
+
+    def do_statline(opts):
+        statline = frmt.get_statline(keys, stats.get())
+        if len(statline) == 0:
+            return False
+        statline = datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline
+        if opts.log_to_file:
+            f.write(statline)
+            f.write('\n')
+        else:
+            print(statline)
 
+        return True
+
+    do_banner(opts)
+    banner_printed = True
     while True:
         try:
             time.sleep(opts.set_delay)
-            if line % banner_repeat == 0 and not banner_printed:
-                print(frmt.get_banner())
+            if signal_received:
+                banner_printed = True
+                line = 0
+                f.close()
+                do_banner(opts)
+                signal_received = False
+            if (line % banner_repeat == 0 and not banner_printed and
+                not (opts.log_to_file and isinstance(frmt, CSVFormat))):
+                do_banner(opts)
                 banner_printed = True
-            statline = frmt.get_statline(keys, stats.get())
-            if len(statline) == 0:
+            if not do_statline(opts):
                 continue
-            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)
             line += 1
             banner_printed = False
         except KeyboardInterrupt:
             break
 
+    if opts.log_to_file:
+        f.close()
+
+
+def handle_signal(sig, frame):
+    global signal_received
+
+    signal_received = True
+
+    return
+
 
 def is_delay_valid(delay):
     """Verify delay is in valid value range."""

@@ -1652,6 +1703,10 @@ Press any other key to refresh statistics immediately.
                            default=False,
                            help='run in logging mode (like vmstat)',
                            )
+    argparser.add_argument('-L', '--log-to-file',
+                           type=str,
+                           help="like '--log', but logging to a file"
+                           )
     argparser.add_argument('-p', '--pid',
                            type=int,
                            default=0,

@@ -1675,9 +1730,9 @@ Press any other key to refresh statistics immediately.
                            help='omit records with all zeros in logging mode',
                            )
     options = argparser.parse_args()
-    if options.csv and not options.log:
+    if options.csv and not (options.log or options.log_to_file):
         sys.exit('Error: Option -c/--csv requires -l/--log')
-    if options.skip_zero_records and not options.log:
+    if options.skip_zero_records and not (options.log or options.log_to_file):
         sys.exit('Error: Option -z/--skip-zero-records requires -l/--log')
     try:
         # verify that we were passed a valid regex up front

@@ -1758,7 +1813,9 @@ def main():
         sys.stdout.write('  ' + '\n  '.join(sorted(set(event_list))) + '\n')
         sys.exit(0)
 
-    if options.log:
+    if options.log or options.log_to_file:
+        if options.log_to_file:
+            signal.signal(signal.SIGHUP, handle_signal)
         keys = sorted(stats.get().keys())
         if options.csv:
             frmt = CSVFormat(keys, options.skip_zero_records)

tools/kvm/kvm_stat/kvm_stat.txt +5 -0

index 24296dccc00a..de7c4a2497f9 100644
@@ -92,6 +92,11 @@ OPTIONS
 --log::
         run in logging mode (like vmstat)
 
+
+-L::
+--log-to-file::
+        like '--log', but logging to a file
+
 -p<pid>::
 --pid=<pid>::
 	limit statistics to one virtual machine (pid)

Signature · 1 line

2.17.1

Paolo Bonzini

maintainer

2020-03-31 21:02 UTC #6 lore raw

Re: [PATCH 2/3] tools/kvm_stat: Add command line switch '-L' to log to file

Quoted 10 lines · Stefan Raspl

From: Stefan Raspl <[email protected]>
To integrate with logrotate, we have a signal handler that will re-open the logfile.
Assuming we have a systemd unit file with
ExecStart=kvm_stat -dtc -s 10 -L /var/log/kvm_stat.csv ExecReload=/bin/kill -HUP $MAINPID
and a logrotate config featuring
postrotate
/bin/systemctl restart kvm_stat.service

Does reload work too?

Regarding the code, I only have a few nits.

tools/kvm/kvm_stat/kvm_stat class CSVFormat(object): · line 1558

Quoted 4 lines

tools/kvm/kvm_stat/kvm_stat +4 -0

+            f.write(frmt.get_banner())
+            f.write('\n')
+        else:
+            print(frmt.get_banner())

What about

print(frmt.get_banner(), file=f or sys.stdout)

tools/kvm/kvm_stat/kvm_stat class CSVFormat(object): · line 1569

Quoted 11 lines

tools/kvm/kvm_stat/kvm_stat +11 -0

+
+    def do_statline(opts):
+        statline = frmt.get_statline(keys, stats.get())
+        if len(statline) == 0:
+            return False
+        statline = datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline
+        if opts.log_to_file:
+            f.write(statline)
+            f.write('\n')
+        else:
+            print(statline)

... and likewise here? (

Quoted 22 lines

tools/kvm/kvm_stat/kvm_stat +13 -4

+        return True
+
+    do_banner(opts)
+    banner_printed = True
     while True:
         try:
             time.sleep(opts.set_delay)
-            if line % banner_repeat == 0 and not banner_printed:
-                print(frmt.get_banner())
+            if signal_received:
+                banner_printed = True
+                line = 0
+                f.close()
+                do_banner(opts)
+                signal_received = False
+            if (line % banner_repeat == 0 and not banner_printed and
+                not (opts.log_to_file and isinstance(frmt, CSVFormat))):
+                do_banner(opts)
                 banner_printed = True
-            statline = frmt.get_statline(keys, stats.get())
-            if len(statline) == 0:

tools/kvm/kvm_stat/kvm_stat class CSVFormat(object): · line 1596

Quoted 10 lines

tools/kvm/kvm_stat/kvm_stat +3 -1

+            if not do_statline(opts):
                 continue
-            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)
             line += 1
             banner_printed = False
         except KeyboardInterrupt:
             break
 
+    if opts.log_to_file:
+        f.close()

"if f:"?

Quoted 24 lines

tools/kvm/kvm_stat/kvm_stat +13 -0

+
+
+def handle_signal(sig, frame):
+    global signal_received
+
+    signal_received = True
+
+    return
+
 
 def is_delay_valid(delay):
     """Verify delay is in valid value range."""

@@ -1652,6 +1703,10 @@ Press any other key to refresh statistics immediately.
                            default=False,
                            help='run in logging mode (like vmstat)',
                            )
+    argparser.add_argument('-L', '--log-to-file',
+                           type=str,
+                           help="like '--log', but logging to a file"
+                           )
     argparser.add_argument('-p', '--pid',
                            type=int,
                            default=0,

tools/kvm/kvm_stat/kvm_stat Press any other key to refresh statistics immediately. · line 1734

Quoted 6 lines

tools/kvm/kvm_stat/kvm_stat +1 -1

@@ -1675,9 +1730,9 @@ Press any other key to refresh statistics immediately.
                            help='omit records with all zeros in logging mode',
                            )
     options = argparser.parse_args()
-    if options.csv and not options.log:
+    if options.csv and not (options.log or options.log_to_file):
         sys.exit('Error: Option -c/--csv requires -l/--log')

"requires -l/-L"?

tools/kvm/kvm_stat/kvm_stat Press any other key to refresh statistics immediately. · line 1736

Quoted 3 lines

tools/kvm/kvm_stat/kvm_stat +1 -1

-    if options.skip_zero_records and not options.log:
+    if options.skip_zero_records and not (options.log or options.log_to_file):
         sys.exit('Error: Option -z/--skip-zero-records requires -l/--log')

Likewise.

Quoted 18 lines

tools/kvm/kvm_stat/kvm_stat +3 -1

     try:
         # verify that we were passed a valid regex up front

@@ -1758,7 +1813,9 @@ def main():
         sys.stdout.write('  ' + '\n  '.join(sorted(set(event_list))) + '\n')
         sys.exit(0)
 
-    if options.log:
+    if options.log or options.log_to_file:
+        if options.log_to_file:
+            signal.signal(signal.SIGHUP, handle_signal)
         keys = sorted(stats.get().keys())
         if options.csv:
             frmt = CSVFormat(keys, options.skip_zero_records)

tools/kvm/kvm_stat/kvm_stat.txt +0 -0

index 24296dccc00a..de7c4a2497f9 100644

tools/kvm/kvm_stat/kvm_stat.txt OPTIONS · line 98

Quoted 7 lines

tools/kvm/kvm_stat/kvm_stat.txt +4 -0

@@ -92,6 +92,11 @@ OPTIONS
 --log::
         run in logging mode (like vmstat)
 
+
+-L::
+--log-to-file::
+        like '--log', but logging to a file

-L<file>:: / --log-to-file=<file>

Quoted 5 lines

tools/kvm/kvm_stat/kvm_stat.txt +1 -0

+
 -p<pid>::
 --pid=<pid>::
 	limit statistics to one virtual machine (pid)

Stefan Raspl

author

2020-04-01 12:59 UTC #7 lore raw

Re: [PATCH 2/3] tools/kvm_stat: Add command line switch '-L' to log to file

Quoted 12 lines · Paolo Bonzini

Quoted 10 lines · Stefan Raspl
From: Stefan Raspl <[email protected]>
To integrate with logrotate, we have a signal handler that will re-open the logfile.
Assuming we have a systemd unit file with
ExecStart=kvm_stat -dtc -s 10 -L /var/log/kvm_stat.csv ExecReload=/bin/kill -HUP $MAINPID
and a logrotate config featuring
postrotate
/bin/systemctl restart kvm_stat.service

Quoted 1 line

Does reload work too?

Reload and restart work fine - any specific concerns or areas to look for?

Quoted 11 lines

Regarding the code, I only have a few nits.
tools/kvm/kvm_stat/kvm_stat class CSVFormat(object): · line 1558
Quoted 4 lines
tools/kvm/kvm_stat/kvm_stat +4 -0
+            f.write(frmt.get_banner())
+            f.write('\n')
+        else:
+            print(frmt.get_banner())
What about
print(frmt.get_banner(), file=f or sys.stdout)

Yup, thx!

Quoted 12 lines

tools/kvm/kvm_stat/kvm_stat class CSVFormat(object): · line 1569

Quoted 11 lines

tools/kvm/kvm_stat/kvm_stat +11 -0

+
+    def do_statline(opts):
+        statline = frmt.get_statline(keys, stats.get())
+        if len(statline) == 0:
+            return False
+        statline = datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline
+        if opts.log_to_file:
+            f.write(statline)
+            f.write('\n')
+        else:
+            print(statline)

Quoted 1 line

... and likewise here? (

Sure

Quoted 30 lines

tools/kvm/kvm_stat/kvm_stat +14 -5

+        return True
+
+    do_banner(opts)
+    banner_printed = True
     while True:
         try:
             time.sleep(opts.set_delay)
-            if line % banner_repeat == 0 and not banner_printed:
-                print(frmt.get_banner())
+            if signal_received:
+                banner_printed = True
+                line = 0
+                f.close()
+                do_banner(opts)
+                signal_received = False
+            if (line % banner_repeat == 0 and not banner_printed and
+                not (opts.log_to_file and isinstance(frmt, CSVFormat))):
+                do_banner(opts)
                 banner_printed = True
-            statline = frmt.get_statline(keys, stats.get())
-            if len(statline) == 0:
+            if not do_statline(opts):
                 continue
-            print(datetime.now().strftime("%Y-%m-%d %H:%M:%S") + statline)
             line += 1
             banner_printed = False
         except KeyboardInterrupt:
             break

Quoted 4 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+    if opts.log_to_file:
+        f.close()
"if f:"?

I'd argue the former is a lot more telling/easier to read - one of the downsides of using extremely short variable names like 'f'.

Quoted 24 lines

tools/kvm/kvm_stat/kvm_stat +13 -0

+
+
+def handle_signal(sig, frame):
+    global signal_received
+
+    signal_received = True
+
+    return
+
 
 def is_delay_valid(delay):
     """Verify delay is in valid value range."""

@@ -1652,6 +1703,10 @@ Press any other key to refresh statistics immediately.
                            default=False,
                            help='run in logging mode (like vmstat)',
                            )
+    argparser.add_argument('-L', '--log-to-file',
+                           type=str,
+                           help="like '--log', but logging to a file"
+                           )
     argparser.add_argument('-p', '--pid',
                            type=int,
                            default=0,

@@ -1675,9 +1730,9 @@ Press any other key to refresh statistics immediately.

Quoted 8 lines

tools/kvm/kvm_stat/kvm_stat Press any other key to refresh statistics immediately. · line 1734

Quoted 6 lines

tools/kvm/kvm_stat/kvm_stat +1 -1

                            help='omit records with all zeros in logging mode',
                            )
     options = argparser.parse_args()
-    if options.csv and not options.log:
+    if options.csv and not (options.log or options.log_to_file):
         sys.exit('Error: Option -c/--csv requires -l/--log')

"requires -l/-L"?

Yes!

Quoted 5 lines

tools/kvm/kvm_stat/kvm_stat Press any other key to refresh statistics immediately. · line 1736
Quoted 3 lines
tools/kvm/kvm_stat/kvm_stat +1 -1
-    if options.skip_zero_records and not options.log:
+    if options.skip_zero_records and not (options.log or options.log_to_file):
         sys.exit('Error: Option -z/--skip-zero-records requires -l/--log')
Likewise.

Of course.

Quoted 21 lines

tools/kvm/kvm_stat/kvm_stat +3 -1

     try:
         # verify that we were passed a valid regex up front

@@ -1758,7 +1813,9 @@ def main():
         sys.stdout.write('  ' + '\n  '.join(sorted(set(event_list))) + '\n')
         sys.exit(0)
 
-    if options.log:
+    if options.log or options.log_to_file:
+        if options.log_to_file:
+            signal.signal(signal.SIGHUP, handle_signal)
         keys = sorted(stats.get().keys())
         if options.csv:
             frmt = CSVFormat(keys, options.skip_zero_records)

tools/kvm/kvm_stat/kvm_stat.txt +0 -0

index 24296dccc00a..de7c4a2497f9 100644
@@ -92,6 +92,11 @@ OPTIONS
 --log::
         run in logging mode (like vmstat)

Quoted 6 lines

tools/kvm/kvm_stat/kvm_stat.txt OPTIONS · line 98
Quoted 4 lines
tools/kvm/kvm_stat/kvm_stat.txt +4 -0
+
+-L::
+--log-to-file::
+        like '--log', but logging to a file
-L<file>:: / --log-to-file=<file>

Argh...

Ciao,
Stefan

Paolo Bonzini

maintainer

2020-04-01 13:24 UTC #8 lore raw

Re: [PATCH 2/3] tools/kvm_stat: Add command line switch '-L' to log to file

Quoted 13 lines · Stefan Raspl

Quoted 12 lines · Paolo Bonzini
Quoted 10 lines · Stefan Raspl
From: Stefan Raspl <[email protected]>
To integrate with logrotate, we have a signal handler that will re-open the logfile.
Assuming we have a systemd unit file with
ExecStart=kvm_stat -dtc -s 10 -L /var/log/kvm_stat.csv ExecReload=/bin/kill -HUP $MAINPID
and a logrotate config featuring
postrotate
/bin/systemctl restart kvm_stat.service

Quoted 3 lines

Quoted 1 line
Does reload work too?
Reload and restart work fine - any specific concerns or areas to look for?

No, I would just put reload in the postrotate script.

Quoted 7 lines

Quoted 4 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+    if opts.log_to_file:
+        f.close()
"if f:"?
I'd argue the former is a lot more telling/easier to read - one of the downsides of using extremely short variable names like 'f'.

Fair enough.

Paolo

Stefan Raspl

author

2020-03-31 20:00 UTC #9 lore raw

PATCH 3/3 tools/kvm_stat: add sample systemd unit file

In reply to Stefan Raspl

From: Stefan Raspl <[email protected]>

Add a sample unit file as a basis for systemd integration of kvm_stat logs.

Signed-off-by Stefan Raspl <[email protected]>

---
 tools/kvm/kvm_stat/kvm_stat.service | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 tools/kvm/kvm_stat/kvm_stat.service

tools/kvm/kvm_stat/kvm_stat.service new file +16 -0

new file mode 100644
index 000000000000..71aabaffe779
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+[Unit]
+Description=Service that logs KVM kernel module trace events
+Before=qemu-kvm.service
+
+[Service]
+Type=simple
+ExecStart=/usr/bin/kvm_stat -dtcz -s 10 -L /var/log/kvm_stat.csv
+ExecReload=/bin/kill -HUP $MAINPID
+Restart=always
+SyslogIdentifier=kvm_stat
+SyslogLevel=debug
+
+[Install]
+WantedBy=multi-user.target

Signature · 1 line

2.17.1