All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] rteval: Miscellaneous fixes
@ 2021-09-09  8:05 Punit Agrawal
  2021-09-09  8:05 ` [PATCH 1/2] rteval: osinfo.py: Fix RT kernel detection Punit Agrawal
  2021-09-09  8:05 ` [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment Punit Agrawal
  0 siblings, 2 replies; 8+ messages in thread
From: Punit Agrawal @ 2021-09-09  8:05 UTC (permalink / raw)
  To: jkacur; +Cc: Punit Agrawal, linux-rt-users, punit1.agrawal

Hi,

Since the previous two patchsets fixing blockers[0], [1], I
encountered a couple of more benign issues - noticed when staring at
the generated report - summary.xml.

The first patch fixes a problem with incorrect detection of RT
kernels. I've tried to retain the old behaviour in case somebody is
still using older kernels. The change can be made more robust if
that's no longer needed.

The second patch fixes an issue with incorrect detection of init
system and container environment. This is a result of change in how
strings are treated in python3 (compared to python2).

As usual, all feedback welcome.

Thanks,
Punit

[0] https://lore.kernel.org/all/20210901080816.721731-1-punitagrawal@gmail.com/
[1] https://lore.kernel.org/all/20210902092452.726905-1-punitagrawal@gmail.com/

Punit Agrawal (2):
  rteval: osinfo.py: Fix RT kernel detection
  rteval: services.py: Fix incorrect detection of container environment

 rteval/sysinfo/osinfo.py   | 2 +-
 rteval/sysinfo/services.py | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] rteval: osinfo.py: Fix RT kernel detection
  2021-09-09  8:05 [PATCH 0/2] rteval: Miscellaneous fixes Punit Agrawal
@ 2021-09-09  8:05 ` Punit Agrawal
  2021-09-10 16:30   ` John Kacur
  2021-09-09  8:05 ` [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment Punit Agrawal
  1 sibling, 1 reply; 8+ messages in thread
From: Punit Agrawal @ 2021-09-09  8:05 UTC (permalink / raw)
  To: jkacur; +Cc: Punit Agrawal, linux-rt-users, punit1.agrawal

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

Since the osinfo.py module was developed, RT kernels have moved on
from identifying as "RT" to "PREEMPT_RT". This change in the kernel
identifier causes osinfo.py to incorrectly detect RT kernels as
non-RT.

Update the check for detecting RT kernel to be able to handle both the
variants.

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 rteval/sysinfo/osinfo.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rteval/sysinfo/osinfo.py b/rteval/sysinfo/osinfo.py
index 98e5b4422cad..7b7bfe9ce4ec 100644
--- a/rteval/sysinfo/osinfo.py
+++ b/rteval/sysinfo/osinfo.py
@@ -91,7 +91,7 @@ class OSInfo:
 
         (sys, node, release, ver, machine) = os.uname()
         isrt = 1
-        if ver.find(' RT ') == -1:
+        if 'RT ' not in ver:
             isrt = 0
 
         node_n = libxml2.newNode("node")
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment
  2021-09-09  8:05 [PATCH 0/2] rteval: Miscellaneous fixes Punit Agrawal
  2021-09-09  8:05 ` [PATCH 1/2] rteval: osinfo.py: Fix RT kernel detection Punit Agrawal
@ 2021-09-09  8:05 ` Punit Agrawal
  2021-09-12 14:59   ` John Kacur
  1 sibling, 1 reply; 8+ messages in thread
From: Punit Agrawal @ 2021-09-09  8:05 UTC (permalink / raw)
  To: jkacur; +Cc: Punit Agrawal, linux-rt-users, punit1.agrawal

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

rteval mistakenly detects that it is running inside a container even
though it is running directly on the host. On further investigation
this was found to be due to change in behaviour around byte strings
and strings when going from python2 to python3.

In python3 byte strings are not equivalent to strings, i.e., b'' == ''
is False. The string comparison functions in services.py are still
relying on the old behaviour in python2 where they were equivalent.

Update the byte string processing by converting them to string.

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
 rteval/sysinfo/services.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py
index 06ff5ae9cd0c..94857aea6be4 100644
--- a/rteval/sysinfo/services.py
+++ b/rteval/sysinfo/services.py
@@ -83,8 +83,8 @@ class SystemServices:
         self.__log(Log.DEBUG, "cmd: %s" % cmd)
         c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         for p in c.stdout:
-            # p are lines like "servicename.service status"
-            v = p.strip().split()
+            # p are lines like b'servicename.service status'
+            v = p.decode().strip().split()
             ret_services[v[0].split('.')[0]] = v[1]
         return ret_services
 
@@ -92,7 +92,7 @@ class SystemServices:
     def services_get(self):
         cmd = [getcmdpath('ps'), '-ocomm=', '1']
         c = subprocess.Popen(cmd, stdout=subprocess.PIPE)
-        self.__init = c.stdout.read().strip()
+        self.__init = c.stdout.read().decode().strip()
         if self.__init == 'systemd':
             self.__log(Log.DEBUG, "Using systemd to get services status")
             return self.__get_services_systemd()
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] rteval: osinfo.py: Fix RT kernel detection
  2021-09-09  8:05 ` [PATCH 1/2] rteval: osinfo.py: Fix RT kernel detection Punit Agrawal
@ 2021-09-10 16:30   ` John Kacur
  0 siblings, 0 replies; 8+ messages in thread
From: John Kacur @ 2021-09-10 16:30 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: linux-rt-users, punit1.agrawal



On Thu, 9 Sep 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> Since the osinfo.py module was developed, RT kernels have moved on
> from identifying as "RT" to "PREEMPT_RT". This change in the kernel
> identifier causes osinfo.py to incorrectly detect RT kernels as
> non-RT.
> 
> Update the check for detecting RT kernel to be able to handle both the
> variants.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  rteval/sysinfo/osinfo.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/rteval/sysinfo/osinfo.py b/rteval/sysinfo/osinfo.py
> index 98e5b4422cad..7b7bfe9ce4ec 100644
> --- a/rteval/sysinfo/osinfo.py
> +++ b/rteval/sysinfo/osinfo.py
> @@ -91,7 +91,7 @@ class OSInfo:
>  
>          (sys, node, release, ver, machine) = os.uname()
>          isrt = 1
> -        if ver.find(' RT ') == -1:
> +        if 'RT ' not in ver:
>              isrt = 0
>  
>          node_n = libxml2.newNode("node")
> -- 
> 2.32.0
> 
> 

Signed-off-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment
  2021-09-09  8:05 ` [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment Punit Agrawal
@ 2021-09-12 14:59   ` John Kacur
  2021-09-13  7:47     ` Punit Agrawal
  2021-09-13  8:06     ` [PATCH v2] " Punit Agrawal
  0 siblings, 2 replies; 8+ messages in thread
From: John Kacur @ 2021-09-12 14:59 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: linux-rt-users, punit1.agrawal



On Thu, 9 Sep 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> rteval mistakenly detects that it is running inside a container even
> though it is running directly on the host. On further investigation
> this was found to be due to change in behaviour around byte strings
> and strings when going from python2 to python3.
> 
> In python3 byte strings are not equivalent to strings, i.e., b'' == ''
> is False. The string comparison functions in services.py are still
> relying on the old behaviour in python2 where they were equivalent.
> 
> Update the byte string processing by converting them to string.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
>  rteval/sysinfo/services.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py
> index 06ff5ae9cd0c..94857aea6be4 100644
> --- a/rteval/sysinfo/services.py
> +++ b/rteval/sysinfo/services.py
> @@ -83,8 +83,8 @@ class SystemServices:
>          self.__log(Log.DEBUG, "cmd: %s" % cmd)
>          c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>          for p in c.stdout:
> -            # p are lines like "servicename.service status"
> -            v = p.strip().split()
> +            # p are lines like b'servicename.service status'
> +            v = p.decode().strip().split()
>              ret_services[v[0].split('.')[0]] = v[1]
>          return ret_services
>  
> @@ -92,7 +92,7 @@ class SystemServices:
>      def services_get(self):
>          cmd = [getcmdpath('ps'), '-ocomm=', '1']
>          c = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> -        self.__init = c.stdout.read().strip()
> +        self.__init = c.stdout.read().decode().strip()
>          if self.__init == 'systemd':
>              self.__log(Log.DEBUG, "Using systemd to get services status")
>              return self.__get_services_systemd()
> -- 
> 2.32.0
> 
> 

Thanks, this looks good, I'm wondering however if the same result could be 
achieved by appending text=True to the subprocess command in each of those
methods? Would you like to test that and send me a new patch?

Thanks

John 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment
  2021-09-12 14:59   ` John Kacur
@ 2021-09-13  7:47     ` Punit Agrawal
  2021-09-13  8:06     ` [PATCH v2] " Punit Agrawal
  1 sibling, 0 replies; 8+ messages in thread
From: Punit Agrawal @ 2021-09-13  7:47 UTC (permalink / raw)
  To: John Kacur; +Cc: linux-rt-users, punit1.agrawal

John Kacur <jkacur@redhat.com> writes:

> On Thu, 9 Sep 2021, Punit Agrawal wrote:
>
>> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> 
>> rteval mistakenly detects that it is running inside a container even
>> though it is running directly on the host. On further investigation
>> this was found to be due to change in behaviour around byte strings
>> and strings when going from python2 to python3.
>> 
>> In python3 byte strings are not equivalent to strings, i.e., b'' == ''
>> is False. The string comparison functions in services.py are still
>> relying on the old behaviour in python2 where they were equivalent.
>> 
>> Update the byte string processing by converting them to string.
>> 
>> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
>> ---
>>  rteval/sysinfo/services.py | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py
>> index 06ff5ae9cd0c..94857aea6be4 100644
>> --- a/rteval/sysinfo/services.py
>> +++ b/rteval/sysinfo/services.py
>> @@ -83,8 +83,8 @@ class SystemServices:
>>          self.__log(Log.DEBUG, "cmd: %s" % cmd)
>>          c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>>          for p in c.stdout:
>> -            # p are lines like "servicename.service status"
>> -            v = p.strip().split()
>> +            # p are lines like b'servicename.service status'
>> +            v = p.decode().strip().split()
>>              ret_services[v[0].split('.')[0]] = v[1]
>>          return ret_services
>>  
>> @@ -92,7 +92,7 @@ class SystemServices:
>>      def services_get(self):
>>          cmd = [getcmdpath('ps'), '-ocomm=', '1']
>>          c = subprocess.Popen(cmd, stdout=subprocess.PIPE)
>> -        self.__init = c.stdout.read().strip()
>> +        self.__init = c.stdout.read().decode().strip()
>>          if self.__init == 'systemd':
>>              self.__log(Log.DEBUG, "Using systemd to get services status")
>>              return self.__get_services_systemd()
>> -- 
>> 2.32.0
>> 
>> 
>
> Thanks, this looks good, I'm wondering however if the same result could be 
> achieved by appending text=True to the subprocess command in each of those
> methods? Would you like to test that and send me a new patch?

Thanks for the suggestion - I missed the "text=True" in the Popen() when
going through the documentation.

I will send a new version converting all Popen() sites in the file.

[...]


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] rteval: services.py: Fix incorrect detection of container environment
  2021-09-12 14:59   ` John Kacur
  2021-09-13  7:47     ` Punit Agrawal
@ 2021-09-13  8:06     ` Punit Agrawal
  2021-09-13 13:33       ` John Kacur
  1 sibling, 1 reply; 8+ messages in thread
From: Punit Agrawal @ 2021-09-13  8:06 UTC (permalink / raw)
  To: jkacur; +Cc: linux-rt-users, Punit Agrawal

From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>

rteval mistakenly detects that it is running inside a container even
though it is running directly on the host. On further investigation
this was found to be due to change in behaviour around byte strings
and strings when going from python2 to python3.

In python3 process output (stdout, stderr) for processes executed via
Popen() is retuned as byte strings and byte strings are not equivalent
to strings, i.e., b'' == '' is False.

Update all call sites for Popen() to include "text=True"
argument - this ensures that the process output is returned as a
string.

Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
---
v1 -> v2:
* Update Popen() calls to return strings for process output
---
 rteval/sysinfo/services.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py
index 06ff5ae9cd0c..283109b9b234 100644
--- a/rteval/sysinfo/services.py
+++ b/rteval/sysinfo/services.py
@@ -62,11 +62,11 @@ class SystemServices:
             if not [1 for p in reject if fnmatch.fnmatch(servicename, p)] \
                     and os.access(service, os.X_OK):
                 cmd = '%s -qs "\(^\|\W\)status)" %s' % (getcmdpath('grep'), service)
-                c = subprocess.Popen(cmd, shell=True)
+                c = subprocess.Popen(cmd, shell=True, text=True)
                 c.wait()
                 if c.returncode == 0:
                     cmd = ['env', '-i', 'LANG="%s"' % os.environ['LANG'], 'PATH="%s"' % os.environ['PATH'], 'TERM="%s"' % os.environ['TERM'], service, 'status']
-                    c = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+                    c = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
                     c.wait()
                     if c.returncode == 0 and (c.stdout.read() or c.stderr.read()):
                         ret_services[servicename] = 'running'
@@ -81,9 +81,9 @@ class SystemServices:
         ret_services = {}
         cmd = '%s list-unit-files -t service --no-legend' % getcmdpath('systemctl')
         self.__log(Log.DEBUG, "cmd: %s" % cmd)
-        c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
         for p in c.stdout:
-            # p are lines like "servicename.service status"
+            # p are lines like b'servicename.service status'
             v = p.strip().split()
             ret_services[v[0].split('.')[0]] = v[1]
         return ret_services
@@ -91,7 +91,7 @@ class SystemServices:
 
     def services_get(self):
         cmd = [getcmdpath('ps'), '-ocomm=', '1']
-        c = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+        c = subprocess.Popen(cmd, stdout=subprocess.PIPE, text=True)
         self.__init = c.stdout.read().strip()
         if self.__init == 'systemd':
             self.__log(Log.DEBUG, "Using systemd to get services status")
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] rteval: services.py: Fix incorrect detection of container environment
  2021-09-13  8:06     ` [PATCH v2] " Punit Agrawal
@ 2021-09-13 13:33       ` John Kacur
  0 siblings, 0 replies; 8+ messages in thread
From: John Kacur @ 2021-09-13 13:33 UTC (permalink / raw)
  To: Punit Agrawal; +Cc: linux-rt-users, Punit Agrawal



On Mon, 13 Sep 2021, Punit Agrawal wrote:

> From: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> 
> rteval mistakenly detects that it is running inside a container even
> though it is running directly on the host. On further investigation
> this was found to be due to change in behaviour around byte strings
> and strings when going from python2 to python3.
> 
> In python3 process output (stdout, stderr) for processes executed via
> Popen() is retuned as byte strings and byte strings are not equivalent
> to strings, i.e., b'' == '' is False.
> 
> Update all call sites for Popen() to include "text=True"
> argument - this ensures that the process output is returned as a
> string.
> 
> Signed-off-by: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
> ---
> v1 -> v2:
> * Update Popen() calls to return strings for process output
> ---
>  rteval/sysinfo/services.py | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/rteval/sysinfo/services.py b/rteval/sysinfo/services.py
> index 06ff5ae9cd0c..283109b9b234 100644
> --- a/rteval/sysinfo/services.py
> +++ b/rteval/sysinfo/services.py
> @@ -62,11 +62,11 @@ class SystemServices:
>              if not [1 for p in reject if fnmatch.fnmatch(servicename, p)] \
>                      and os.access(service, os.X_OK):
>                  cmd = '%s -qs "\(^\|\W\)status)" %s' % (getcmdpath('grep'), service)
> -                c = subprocess.Popen(cmd, shell=True)
> +                c = subprocess.Popen(cmd, shell=True, text=True)
>                  c.wait()
>                  if c.returncode == 0:
>                      cmd = ['env', '-i', 'LANG="%s"' % os.environ['LANG'], 'PATH="%s"' % os.environ['PATH'], 'TERM="%s"' % os.environ['TERM'], service, 'status']
> -                    c = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> +                    c = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
>                      c.wait()
>                      if c.returncode == 0 and (c.stdout.read() or c.stderr.read()):
>                          ret_services[servicename] = 'running'
> @@ -81,9 +81,9 @@ class SystemServices:
>          ret_services = {}
>          cmd = '%s list-unit-files -t service --no-legend' % getcmdpath('systemctl')
>          self.__log(Log.DEBUG, "cmd: %s" % cmd)
> -        c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> +        c = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
>          for p in c.stdout:
> -            # p are lines like "servicename.service status"
> +            # p are lines like b'servicename.service status'
>              v = p.strip().split()
>              ret_services[v[0].split('.')[0]] = v[1]
>          return ret_services
> @@ -91,7 +91,7 @@ class SystemServices:
>  
>      def services_get(self):
>          cmd = [getcmdpath('ps'), '-ocomm=', '1']
> -        c = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> +        c = subprocess.Popen(cmd, stdout=subprocess.PIPE, text=True)
>          self.__init = c.stdout.read().strip()
>          if self.__init == 'systemd':
>              self.__log(Log.DEBUG, "Using systemd to get services status")
> -- 
> 2.32.0
> 
> 

Signed-off-by: John Kacur <jkacur@redhat.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-09-13 13:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-09  8:05 [PATCH 0/2] rteval: Miscellaneous fixes Punit Agrawal
2021-09-09  8:05 ` [PATCH 1/2] rteval: osinfo.py: Fix RT kernel detection Punit Agrawal
2021-09-10 16:30   ` John Kacur
2021-09-09  8:05 ` [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment Punit Agrawal
2021-09-12 14:59   ` John Kacur
2021-09-13  7:47     ` Punit Agrawal
2021-09-13  8:06     ` [PATCH v2] " Punit Agrawal
2021-09-13 13:33       ` John Kacur

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.