* [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
* 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
* [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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).