[Qemu-devel] [PATCH] scripts/kvm/kvm_stat: Fix tracefs access checking

[Qemu-devel] [PATCH] scripts/kvm/kvm_stat: Fix tracefs access checking

[Janosch Frank]

The following patch fixes the access checking behavior changed in
7aa4ee5. Kvm_stat will now fall back to debugfs statistics if the
tracefs ones were chosen but are not available. It will then print a
warning and wait five seconds until it continues.

Five seconds are not long, but I do not want to annoy users with
longer delays and the message is still visible after closing
kvm_stat.

Janosch Frank (1):
  scripts/kvm/kvm_stat: Fix tracefs access checking

 scripts/kvm/kvm_stat | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

-- 
2.3.0

[Qemu-devel] [PATCH] scripts/kvm/kvm_stat: Fix tracefs access checking

[Janosch Frank]

On kernels build without CONFIG_TRACING kvm_stat will bail out even
when traces are not used. This is not very helpful, especially if the
user can't install a new kernel. Instead, we should warn the user and
fall back to debugfs statistics.

These changes check if trace statistics were selected without kernel
support, warn with a small timeout, set the debugfs statistics option
to True and the tracefs one to False.

Fixes: 7aa4ee5 ('scripts/kvm/kvm_stat: Improve debugfs access checking')
Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
---
 scripts/kvm/kvm_stat | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index d43e8f3..bab34bb 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -22,6 +22,7 @@ import resource
 import struct
 import re
 from collections import defaultdict
+from time import sleep
 
 VMX_EXIT_REASONS = {
     'EXCEPTION_NMI':        0,
@@ -778,7 +779,7 @@ def get_providers(options):
 
     return providers
 
-def check_access():
+def check_access(options):
     if not os.path.exists('/sys/kernel/debug'):
         sys.stderr.write('Please enable CONFIG_DEBUG_FS in your kernel.')
         sys.exit(1)
@@ -790,14 +791,23 @@ def check_access():
                          "Also ensure, that the kvm modules are loaded.\n")
         sys.exit(1)
 
-    if not os.path.exists(PATH_DEBUGFS_TRACING):
-        sys.stderr.write("Please make {0} readable by the current user.\n"
+    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 "
+                         "current user.\n"
+                         "Falling back to debugfs statistics!\n"
                          .format(PATH_DEBUGFS_TRACING))
-        sys.exit(1)
+        options.debugfs = True
+        options.tracepoints = False
+        sleep(5)
+
+    return options
 
 def main():
-    check_access()
     options = get_options()
+    options = check_access(options)
     providers = get_providers(options)
     stats = Stats(providers, fields=options.fields)
 
-- 
2.3.0

Re: [Qemu-devel] [PATCH] scripts/kvm/kvm_stat: Fix tracefs access checking

[Paolo Bonzini]

On 03/02/2016 08:41, Janosch Frank wrote:
> The following patch fixes the access checking behavior changed in
> 7aa4ee5. Kvm_stat will now fall back to debugfs statistics if the
> tracefs ones were chosen but are not available. It will then print a
> warning and wait five seconds until it continues.
> 
> Five seconds are not long, but I do not want to annoy users with
> longer delays and the message is still visible after closing
> kvm_stat.
> 
> Janosch Frank (1):
>   scripts/kvm/kvm_stat: Fix tracefs access checking
> 
>  scripts/kvm/kvm_stat | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 

I think exiting is better if the -t flag is explicitly passed:

diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
index bab34bb..3cf1181 100755
--- a/scripts/kvm/kvm_stat
+++ b/scripts/kvm/kvm_stat
@@ -796,11 +796,12 @@ def check_access(options):
         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 "
-                         "current user.\n"
-                         "Falling back to debugfs statistics!\n"
-                         .format(PATH_DEBUGFS_TRACING))
+                         "current user.\n")
+        if options.tracepoints:
+            sys.exit(1)
+
+        sys.stderr.write("Falling back to debugfs statistics!\n"
         options.debugfs = True
-        options.tracepoints = False
         sleep(5)

     return options

Otherwise okay.

Paolo