diff mbox

[v2] scripts/kvm/kvm_stat: Fix missing right parantheses and ".format(...)"

Message ID 1455848416-13177-1-git-send-email-famz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fam Zheng Feb. 19, 2016, 2:20 a.m. UTC
They seem to have snuck in when applying Janosch Frank
<frankja@linux.vnet.ibm.com>'s previous patch.

Signed-off-by: Fam Zheng <famz@redhat.com>

---
v2: Also fix .format. [Janosch]
---
 scripts/kvm/kvm_stat | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Janosch Frank Feb. 19, 2016, 7:23 a.m. UTC | #1
On 02/19/2016 03:20 AM, Fam Zheng wrote:
> They seem to have snuck in when applying Janosch Frank
> <frankja@linux.vnet.ibm.com>'s previous patch.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> ---
> v2: Also fix .format. [Janosch]
> ---
>  scripts/kvm/kvm_stat | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
> index 3cf1181..769d884 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")
> +                         "current user.\n"
> +                         .format(PATH_DEBUGFS_TRACING))
>          if options.tracepoints:
>              sys.exit(1)
> 
> -        sys.stderr.write("Falling back to debugfs statistics!\n"
> +        sys.stderr.write("Falling back to debugfs statistics!\n")
>          options.debugfs = True
>          sleep(5)
> 
Reviewed-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Tested-by: Janosch Frank <frankja@linux.vnet.ibm.com>

Thanks
Stefan Hajnoczi Feb. 22, 2016, 5:29 p.m. UTC | #2
On Fri, Feb 19, 2016 at 10:20:16AM +0800, Fam Zheng wrote:
> They seem to have snuck in when applying Janosch Frank
> <frankja@linux.vnet.ibm.com>'s previous patch.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> ---
> v2: Also fix .format. [Janosch]
> ---
>  scripts/kvm/kvm_stat | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
> index 3cf1181..769d884 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")
> +                         "current user.\n"
> +                         .format(PATH_DEBUGFS_TRACING))

According to ./configure the minimum Python version is 2.4.

str.format() is a Python 2.6+ feature.

Is ./configure wrong (i.e. you are sure that the current crop of stable
Debian, Red Hat, and SLES distros have 2.6+)?  If so, please fix
./configure.

Otherwise please use traditional Python string formatting instead.

Stefan
Paolo Bonzini Feb. 22, 2016, 5:40 p.m. UTC | #3
On 19/02/2016 03:20, Fam Zheng wrote:
> They seem to have snuck in when applying Janosch Frank
> <frankja@linux.vnet.ibm.com>'s previous patch.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> 
> ---
> v2: Also fix .format. [Janosch]
> ---
>  scripts/kvm/kvm_stat | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kvm/kvm_stat b/scripts/kvm/kvm_stat
> index 3cf1181..769d884 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")
> +                         "current user.\n"
> +                         .format(PATH_DEBUGFS_TRACING))
>          if options.tracepoints:
>              sys.exit(1)
>  
> -        sys.stderr.write("Falling back to debugfs statistics!\n"
> +        sys.stderr.write("Falling back to debugfs statistics!\n")
>          options.debugfs = True
>          sleep(5)
>  
> 

Thanks Fam, I'll send a pull request soon.

Paolo
Paolo Bonzini Feb. 22, 2016, 5:40 p.m. UTC | #4
On 22/02/2016 18:29, Stefan Hajnoczi wrote:
>>> +                         "current user.\n" +
>>> .format(PATH_DEBUGFS_TRACING))
> According to ./configure the minimum Python version is 2.4.
> 
> str.format() is a Python 2.6+ feature.

This was changed in September 2015:

commit fec21036ff516d20721abc01ae7be99ae5bb0c7b
Author: Markus Armbruster <armbru@redhat.com>
Date:   Fri Sep 4 21:53:03 2015 +0200

    configure: Require Python 2.6

    RHEL-6 and SLES-11 provide Python 2.6.  It'll also work on OS X back
    to 10.6.

    Signed-off-by: Markus Armbruster <armbru@redhat.com>
    Message-Id: <1441396383-17304-1-git-send-email-armbru@redhat.com>
    Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Are you looking at an older version of the configure script?

Paolo
Stefan Hajnoczi Feb. 25, 2016, 11:22 a.m. UTC | #5
On Mon, Feb 22, 2016 at 06:40:50PM +0100, Paolo Bonzini wrote:
> 
> 
> On 22/02/2016 18:29, Stefan Hajnoczi wrote:
> >>> +                         "current user.\n" +
> >>> .format(PATH_DEBUGFS_TRACING))
> > According to ./configure the minimum Python version is 2.4.
> > 
> > str.format() is a Python 2.6+ feature.
> 
> This was changed in September 2015:
> 
> commit fec21036ff516d20721abc01ae7be99ae5bb0c7b
> Author: Markus Armbruster <armbru@redhat.com>
> Date:   Fri Sep 4 21:53:03 2015 +0200
> 
>     configure: Require Python 2.6
> 
>     RHEL-6 and SLES-11 provide Python 2.6.  It'll also work on OS X back
>     to 10.6.
> 
>     Signed-off-by: Markus Armbruster <armbru@redhat.com>
>     Message-Id: <1441396383-17304-1-git-send-email-armbru@redhat.com>
>     Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>     Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Are you looking at an older version of the configure script?

Yes, I must have been.  Thanks for pointing out the change.

Stefan
diff mbox

Patch

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