Re: [Qemu-devel] [PULL 14/49] scripts/kvm/kvm_stat: Improve debugfs access checking

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 01/26/2016 02:46 PM, Paolo Bonzini wrote:
> From: Janosch Frank <frankja@linux.vnet.ibm.com>
> 
> Access checking with F_OK was replaced with the better readable
> os.path.exists().
> 
> On Linux exists() returns False when the user doesn't have sufficient
> permissions for statting the directory. Therefore the error message
> now states that sufficient rights are needed when the check fails.
> 
> Also added check for /sys/kernel/debug/tracing/.
> 
> Signed-off-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> Message-Id: <1452525484-32309-9-git-send-email-frankja@linux.vnet.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/kvm/kvm_stat | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
> index 5ca09f4..6f0692d 100755
> --- a/scripts/kvm/kvm_stat
> +++ b/scripts/kvm/kvm_stat
> @@ -473,12 +473,18 @@ class Stats:
>                  self.values[key] = (newval, newdelta)
>          return self.values
> 
> -if not os.access('/sys/kernel/debug', os.F_OK):
> -    print 'Please enable CONFIG_DEBUG_FS in your kernel'
> +if not os.path.exists('/sys/kernel/debug'):
> +    sys.stderr.write('Please enable CONFIG_DEBUG_FS in your kernel.')
>      sys.exit(1)
> -if not os.access(PATH_DEBUGFS_KVM, os.F_OK):
> -    print "Please mount debugfs ('mount -t debugfs debugfs /sys/kernel/debug')"
> -    print "and ensure the kvm modules are loaded"
> +if not os.path.exists(PATH_DEBUGFS_KVM):
> +    sys.stderr.write("Please make sure, that debugfs is mounted and "
> +                     "readable by the current user:\n"
> +                     "('mount -t debugfs debugfs /sys/kernel/debug')\n"
> +                     "Also ensure, that the kvm modules are loaded.\n")
> +    sys.exit(1)
> +if not os.path.exists(PATH_DEBUGFS_TRACING):
> +    sys.stderr.write("Please make {0} readable by the current user.\n"
> +                     .format(PATH_DEBUGFS_TRACING))
>      sys.exit(1)


Hmmm, this now bails out on kernels without CONFIG_TRACING. It could still fall
back to the normal kvm_stat values. What about just removing the last sys.exit(1)
and make this a warning that stays for a second or so?