diff mbox series

rteval: kcompile: Tweak default source tarball selection

Message ID 20220419162142.91470-1-vschneid@redhat.com
State New
Headers show
Series rteval: kcompile: Tweak default source tarball selection | expand

Commit Message

Valentin Schneider April 19, 2022, 4:21 p.m. UTC
Kcompile._WorkloadSetup() looks for a "tarball" and "tarfile" entry in
the CfgSection, but I couldn't find a single setter for thoses. The
only way for a user to specify a file is via --kcompile-source, which
doesn't seem to be actually used by the module.

As it stands, this method will look for a tarball matching the
hardcoded kernel_prefix.

Make Kcompile
- actually use --kcompile-source
- use the latest tarball in the loadsource if no source is specified.

Signed-off-by: Valentin Schneider <vschneid@redhat.com>
---
I'm not entirely sure about that one, I tried to dig up the git
history for uses of those "tarball" and "tarfile" options but didn't
really get anywhere, so maybe I'm completely wrong...
---
 rteval/modules/loads/kcompile.py | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

Comments

John Kacur May 2, 2022, 3:17 p.m. UTC | #1
On Tue, 19 Apr 2022, Valentin Schneider wrote:

> Kcompile._WorkloadSetup() looks for a "tarball" and "tarfile" entry in
> the CfgSection, but I couldn't find a single setter for thoses. The
> only way for a user to specify a file is via --kcompile-source, which
> doesn't seem to be actually used by the module.
> 
> As it stands, this method will look for a tarball matching the
> hardcoded kernel_prefix.
> 
> Make Kcompile
> - actually use --kcompile-source
> - use the latest tarball in the loadsource if no source is specified.
> 
> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
> ---
> I'm not entirely sure about that one, I tried to dig up the git
> history for uses of those "tarball" and "tarfile" options but didn't
> really get anywhere, so maybe I'm completely wrong...
> ---
>  rteval/modules/loads/kcompile.py | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/rteval/modules/loads/kcompile.py b/rteval/modules/loads/kcompile.py
> index 367f8dc..2747913 100644
> --- a/rteval/modules/loads/kcompile.py
> +++ b/rteval/modules/loads/kcompile.py
> @@ -28,6 +28,7 @@ import sys
>  import os
>  import os.path
>  import glob
> +import re
>  import subprocess
>  from rteval.modules import rtevalRuntimeError
>  from rteval.modules.loads import CommandLineLoad
> @@ -35,7 +36,7 @@ from rteval.Log import Log
>  from rteval.misc import expand_cpulist, compress_cpulist
>  from rteval.systopology import SysTopology
>  
> -kernel_prefix = "linux-5.13"
> +RE_KERNEL_TAR = re.compile(r"linux-(?P<maj>\d+)\.(?P<min>\d+).*\.tar.*")
>  
>  class KBuildJob:
>      '''Class to manage a build job bound to a particular node'''
> @@ -163,18 +164,27 @@ class Kcompile(CommandLineLoad):
>              return
>  
>          # find our source tarball
> -        if 'tarball' in self._cfg:
> -            tarfile = os.path.join(self.srcdir, self._cfg.tarfile)
> +        if self._cfg.source:
> +            tarfile = os.path.join(self.srcdir, self._cfg.source)
>              if not os.path.exists(tarfile):
>                  raise rtevalRuntimeError(self, " tarfile %s does not exist!" % tarfile)
>              self.source = tarfile
>          else:
> -            tarfiles = glob.glob(os.path.join(self.srcdir, "%s*" % kernel_prefix))
> +            tarfiles = [f for f in os.listdir(self.srcdir)
> +                        if RE_KERNEL_TAR.match(f)]
>              if tarfiles:
> -                self.source = tarfiles[0]
> +                # Use (one of) the most recent kernels
> +                def verkey(f):
> +                    match = RE_KERNEL_TAR.match(f)
> +                    return (match.group("maj"), match.group("min"))
> +
> +                tarfiles.sort(key=verkey)
> +                self.source = tarfiles[-1]
>              else:
>                  raise rtevalRuntimeError(self, " no kernel tarballs found in %s" % self.srcdir)
>  
> +        kernel_prefix = re.search(r"linux-\d\.\d", self.source).group(0)
> +
>          # check for existing directory
>          kdir = None
>          names = os.listdir(self.builddir)
> @@ -322,7 +332,7 @@ class Kcompile(CommandLineLoad):
>  
>  def ModuleParameters():
>      return {"source":   {"descr": "Source tar ball",
> -                         "default": "linux-5.13.2.tar.xz",
> +                         "default": "",
>                           "metavar": "TARBALL"},
>              "jobspercore": {"descr": "Number of working threads per core",
>                              "default": 2,
> -- 
> 2.27.0

I think we wanted to make sure we were doing an apples to apples 
comparison, so we restricted the default kernel to the the one we supply 
in rteval-loads. I am thinking of changing this in the future to using the 
one supplied by the distribution, but still mulling this over.

However, --kcompile-source should work as advertised. Do you think you 
could resend a patch with just the part to make --kcompile-source work 
correctly?

Thanks

John
Valentin Schneider May 3, 2022, 10:27 a.m. UTC | #2
On 02/05/22 11:17, John Kacur wrote:
> On Tue, 19 Apr 2022, Valentin Schneider wrote:
>
>> Kcompile._WorkloadSetup() looks for a "tarball" and "tarfile" entry in
>> the CfgSection, but I couldn't find a single setter for thoses. The
>> only way for a user to specify a file is via --kcompile-source, which
>> doesn't seem to be actually used by the module.
>>
>> As it stands, this method will look for a tarball matching the
>> hardcoded kernel_prefix.
>>
>> Make Kcompile
>> - actually use --kcompile-source
>> - use the latest tarball in the loadsource if no source is specified.
>>
>> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
>> ---
>> I'm not entirely sure about that one, I tried to dig up the git
>> history for uses of those "tarball" and "tarfile" options but didn't
>> really get anywhere, so maybe I'm completely wrong...
>> ---
>>  rteval/modules/loads/kcompile.py | 22 ++++++++++++++++------
>>  1 file changed, 16 insertions(+), 6 deletions(-)
>>

> I think we wanted to make sure we were doing an apples to apples
> comparison, so we restricted the default kernel to the the one we supply
> in rteval-loads. I am thinking of changing this in the future to using the
> one supplied by the distribution, but still mulling this over.
>
> However, --kcompile-source should work as advertised. Do you think you
> could resend a patch with just the part to make --kcompile-source work
> correctly?
>

Sure thing, will do.

> Thanks
>
> John
diff mbox series

Patch

diff --git a/rteval/modules/loads/kcompile.py b/rteval/modules/loads/kcompile.py
index 367f8dc..2747913 100644
--- a/rteval/modules/loads/kcompile.py
+++ b/rteval/modules/loads/kcompile.py
@@ -28,6 +28,7 @@  import sys
 import os
 import os.path
 import glob
+import re
 import subprocess
 from rteval.modules import rtevalRuntimeError
 from rteval.modules.loads import CommandLineLoad
@@ -35,7 +36,7 @@  from rteval.Log import Log
 from rteval.misc import expand_cpulist, compress_cpulist
 from rteval.systopology import SysTopology
 
-kernel_prefix = "linux-5.13"
+RE_KERNEL_TAR = re.compile(r"linux-(?P<maj>\d+)\.(?P<min>\d+).*\.tar.*")
 
 class KBuildJob:
     '''Class to manage a build job bound to a particular node'''
@@ -163,18 +164,27 @@  class Kcompile(CommandLineLoad):
             return
 
         # find our source tarball
-        if 'tarball' in self._cfg:
-            tarfile = os.path.join(self.srcdir, self._cfg.tarfile)
+        if self._cfg.source:
+            tarfile = os.path.join(self.srcdir, self._cfg.source)
             if not os.path.exists(tarfile):
                 raise rtevalRuntimeError(self, " tarfile %s does not exist!" % tarfile)
             self.source = tarfile
         else:
-            tarfiles = glob.glob(os.path.join(self.srcdir, "%s*" % kernel_prefix))
+            tarfiles = [f for f in os.listdir(self.srcdir)
+                        if RE_KERNEL_TAR.match(f)]
             if tarfiles:
-                self.source = tarfiles[0]
+                # Use (one of) the most recent kernels
+                def verkey(f):
+                    match = RE_KERNEL_TAR.match(f)
+                    return (match.group("maj"), match.group("min"))
+
+                tarfiles.sort(key=verkey)
+                self.source = tarfiles[-1]
             else:
                 raise rtevalRuntimeError(self, " no kernel tarballs found in %s" % self.srcdir)
 
+        kernel_prefix = re.search(r"linux-\d\.\d", self.source).group(0)
+
         # check for existing directory
         kdir = None
         names = os.listdir(self.builddir)
@@ -322,7 +332,7 @@  class Kcompile(CommandLineLoad):
 
 def ModuleParameters():
     return {"source":   {"descr": "Source tar ball",
-                         "default": "linux-5.13.2.tar.xz",
+                         "default": "",
                          "metavar": "TARBALL"},
             "jobspercore": {"descr": "Number of working threads per core",
                             "default": 2,