Skip to contentSkip to conversation

rteval: Miscellaneous fixes

PATCH In review Punit Agrawal 2 participants 8 messages Sep 9, 2021 – Sep 13, 2021

lore
Share & tools
lore
#0 lore raw

cover rteval: Miscellaneous fixes

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/[email protected]/
[1] https://lore.kernel.org/all/[email protected]/

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(-)
Signature · 1 line
2.32.0
#1 lore raw

PATCH 1/2 rteval: osinfo.py: Fix RT kernel detection

From: Punit Agrawal <[email protected]>

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.

---
 rteval/sysinfo/osinfo.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
rteval/sysinfo/osinfo.py +1 -1
index 98e5b4422cad..7b7bfe9ce4ec 100644
@@ -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")
Signature · 1 line
2.32.0
#2 lore raw

Re: [PATCH 1/2] rteval: osinfo.py: Fix RT kernel detection

Quoted 32 lines · Punit Agrawal

From: Punit Agrawal <[email protected]>

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.

---
 rteval/sysinfo/osinfo.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
rteval/sysinfo/osinfo.py +1 -1
index 98e5b4422cad..7b7bfe9ce4ec 100644
@@ -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")
Signature · 1 line
2.32.0
#3 lore raw

PATCH 2/2 rteval: services.py: Fix incorrect detection of container environment

In reply to Punit Agrawal

From: Punit Agrawal <[email protected]>

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.

---
 rteval/sysinfo/services.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
rteval/sysinfo/services.py +3 -3
index 06ff5ae9cd0c..94857aea6be4 100644
@@ -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()
Signature · 1 line
2.32.0
#4 lore raw

Re: [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment

Quoted 46 lines · Punit Agrawal

From: Punit Agrawal <[email protected]>

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.

---
 rteval/sysinfo/services.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
rteval/sysinfo/services.py +3 -3
index 06ff5ae9cd0c..94857aea6be4 100644
@@ -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()
Signature · 1 line
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

#5 lore raw

Re: [PATCH 2/2] rteval: services.py: Fix incorrect detection of container environment

Quoted 44 lines · John Kacur
Quoted 42 lines · Punit Agrawal

From: Punit Agrawal <[email protected]>

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.

---
 rteval/sysinfo/services.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
rteval/sysinfo/services.py +3 -3
index 06ff5ae9cd0c..94857aea6be4 100644
@@ -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()
Quoted 8 lines
Quoted 4 lines
Signature · 1 line
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.

[...]

#6 lore raw

PATCH v2 1/1 rteval: services.py: Fix incorrect detection of container environment

In reply to John Kacur

From: Punit Agrawal <[email protected]>

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.

---
v1 -> v2:
* Update Popen() calls to return strings for process output

---
 rteval/sysinfo/services.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
rteval/sysinfo/services.py +5 -5
index 06ff5ae9cd0c..283109b9b234 100644
@@ -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")
Signature · 1 line
2.32.0
#7 lore raw

Re: [PATCH v2] rteval: services.py: Fix incorrect detection of container environment

Quoted 66 lines · Punit Agrawal

From: Punit Agrawal <[email protected]>

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.

---
v1 -> v2:
* Update Popen() calls to return strings for process output

---
 rteval/sysinfo/services.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
rteval/sysinfo/services.py +5 -5
index 06ff5ae9cd0c..283109b9b234 100644
@@ -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")
Signature · 1 line
2.32.0