[PATCH V4 1/3] kernel-module-split: make autoload and probeconf distribution specific

[PATCH V4 1/3] kernel-module-split: make autoload and probeconf distribution specific

[Jose Quaresma]

Also take the oportunity to only add configuration files to FILES
and CONFFILES when they exist and are used.

The modules-load.d [1] - Configure kernel modules to load at boot
should install their configuration files in /usr/lib/modules-load.d.

The modprobe.d [2] - Configuration directory for modprobe
should install their configuration files in /lib/modprobe.d

[1] https://www.freedesktop.org/software/systemd/man/modules-load.d.html
[2] https://www.man7.org/linux/man-pages//man5/modprobe.d.5.html

[YOCTO #12212] https://bugzilla.yoctoproject.org/show_bug.cgi?id=12212

CC: Ola x Nilsson <Ola.x.Nilsson@axis.com>
CC: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
---

v4: also remove the sysconfdir when systemd is not used as it causes
QA Issue: linux-yocto: Files/directories were installed but not shipped in any package

v3: fix v2 build issue and only add configuration files to FILES
and CONFFILES when they exist and are used.

v2: use the same location as before on the class and define the new
location just for systemd.

 .../kernel-module-split.bbclass               | 38 ++++++++++---------
 .../distro/include/init-manager-systemd.inc   |  4 ++
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 50882c31a7..1d5e39b44d 100644
--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -30,8 +30,11 @@ fi
 
 PACKAGE_WRITE_DEPS += "kmod-native depmodwrapper-cross"
 
+modulesloaddir ??= "${sysconfdir}/modules-load.d"
+modprobedir ??= "${sysconfdir}/modprobe.d"
+
 do_install:append() {
-	install -d ${D}${sysconfdir}/modules-load.d/ ${D}${sysconfdir}/modprobe.d/
+	install -d ${D}${modulesloaddir} ${D}${modprobedir}
 }
 
 KERNEL_SPLIT_MODULES ?= "1"
@@ -93,8 +96,9 @@ python split_kernel_module_packages () {
 
         dvar = d.getVar('PKGD')
 
-        # If autoloading is requested, output /etc/modules-load.d/<name>.conf and append
+        # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
         # appropriate modprobe commands to the postinst
+        autoloadpath = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
         autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
         autoload = d.getVar('module_autoload_%s' % basename)
         if autoload and autoload == basename:
@@ -102,14 +106,16 @@ python split_kernel_module_packages () {
         if autoload and basename not in autoloadlist:
             bb.warn("module_autoload_%s is defined but '%s' isn't included in KERNEL_MODULE_AUTOLOAD, please add it there" % (basename, basename))
         if basename in autoloadlist:
-            name = '%s/etc/modules-load.d/%s.conf' % (dvar, basename)
-            f = open(name, 'w')
+            f = open("%s%s" % (dvar, autoloadpath), 'w')
             if autoload:
                 for m in autoload.split():
                     f.write('%s\n' % m)
             else:
                 f.write('%s\n' % basename)
             f.close()
+            autoloadpath2append = ' %s' % autoloadpath
+            d.appendVar('FILES:%s' % pkg, autoloadpath2append)
+            d.appendVar('CONFFILES:%s' % pkg, autoloadpath2append)
             postinst = d.getVar('pkg_postinst:%s' % pkg)
             if not postinst:
                 bb.fatal("pkg_postinst:%s not defined" % pkg)
@@ -117,24 +123,19 @@ python split_kernel_module_packages () {
             d.setVar('pkg_postinst:%s' % pkg, postinst)
 
         # Write out any modconf fragment
+        modconfpath = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
         modconflist = (d.getVar("KERNEL_MODULE_PROBECONF") or "").split()
         modconf = d.getVar('module_conf_%s' % basename)
         if modconf and basename in modconflist:
-            name = '%s/etc/modprobe.d/%s.conf' % (dvar, basename)
-            f = open(name, 'w')
+            f = open("%s%s" % (dvar, modconfpath), 'w')
             f.write("%s\n" % modconf)
             f.close()
+            modconfpath2append = ' %s' % modconfpath
+            d.appendVar('FILES:%s' % pkg, modconfpath2append)
+            d.appendVar('CONFFILES:%s' % pkg, modconfpath2append)
         elif modconf:
             bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
 
-        files = d.getVar('FILES:%s' % pkg)
-        files = "%s /etc/modules-load.d/%s.conf /etc/modprobe.d/%s.conf" % (files, basename, basename)
-        d.setVar('FILES:%s' % pkg, files)
-
-        conffiles = d.getVar('CONFFILES:%s' % pkg)
-        conffiles = "%s /etc/modules-load.d/%s.conf /etc/modprobe.d/%s.conf" % (conffiles, basename, basename)
-        d.setVar('CONFFILES:%s' % pkg, conffiles)
-
         if "description" in vals:
             old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
             d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
@@ -167,10 +168,11 @@ python split_kernel_module_packages () {
     splitmods = d.getVar('KERNEL_SPLIT_MODULES')
     postinst = d.getVar('pkg_postinst:modules')
     postrm = d.getVar('pkg_postrm:modules')
+    modulesloaddir = d.getVar('modulesloaddir')
+    modprobedir = d.getVar('modprobedir')
 
     if splitmods != '1':
-        etcdir = d.getVar('sysconfdir')
-        d.appendVar('FILES:' + metapkg, '%s/modules-load.d/ %s/modprobe.d/ %s/modules/' % (etcdir, etcdir, d.getVar("nonarch_base_libdir")))
+        d.appendVar('FILES:' + metapkg, '%s %s %s/modules' % (modulesloaddir, modprobedir, d.getVar("nonarch_base_libdir")))
         d.appendVar('pkg_postinst:%s' % metapkg, postinst)
         d.prependVar('pkg_postrm:%s' % metapkg, postrm);
         return
@@ -189,8 +191,8 @@ python split_kernel_module_packages () {
     # avoid warnings. removedirs only raises an OSError if an empty
     # directory cannot be removed.
     dvar = d.getVar('PKGD')
-    for dir in ["%s/etc/modprobe.d" % (dvar), "%s/etc/modules-load.d" % (dvar), "%s/etc" % (dvar)]:
-        if len(os.listdir(dir)) == 0:
+    for dir in ["%s%s" % (dvar, modprobedir), "%s%s" % (dvar, modulesloaddir), "%s%s" % (dvar, d.getVar('sysconfdir'))]:
+        if os.path.isdir(dir) and len(os.listdir(dir)) == 0:
             os.rmdir(dir)
 }
 
diff --git a/meta/conf/distro/include/init-manager-systemd.inc b/meta/conf/distro/include/init-manager-systemd.inc
index 7867d90028..fc13089764 100644
--- a/meta/conf/distro/include/init-manager-systemd.inc
+++ b/meta/conf/distro/include/init-manager-systemd.inc
@@ -5,3 +5,7 @@ VIRTUAL-RUNTIME_init_manager ??= "systemd"
 VIRTUAL-RUNTIME_initscripts ??= "systemd-compat-units"
 VIRTUAL-RUNTIME_login_manager ??= "shadow-base"
 VIRTUAL-RUNTIME_dev_manager ??= "systemd"
+
+# use autoload and probeconf distribution specific
+modulesloaddir ?= "${libdir}/modules-load.d"
+modprobedir ?= "${nonarch_base_libdir}/modprobe.d"
-- 
2.34.1

[PATCH V4 2/3] kernel-module-split: use context manager to open files

[Jose Quaresma]

Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
---
 .../kernel-module-split.bbclass               | 23 ++++++++-----------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 1d5e39b44d..e19ad8e3c5 100644
--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -76,9 +76,8 @@ python split_kernel_module_packages () {
             cmd = "%sobjcopy -j .modinfo -O binary %s %s" % (d.getVar("HOST_PREFIX") or "", file, tmpfile)
         subprocess.check_call(cmd, shell=True)
         # errors='replace': Some old kernel versions contain invalid utf-8 characters in mod descriptions (like 0xf6, 'ö')
-        f = open(tmpfile, errors='replace')
-        l = f.read().split("\000")
-        f.close()
+        with open(tmpfile, errors='replace') as f:
+            l = f.read().split("\000")
         os.close(tf[0])
         os.unlink(tmpfile)
         if compressed:
@@ -106,13 +105,12 @@ python split_kernel_module_packages () {
         if autoload and basename not in autoloadlist:
             bb.warn("module_autoload_%s is defined but '%s' isn't included in KERNEL_MODULE_AUTOLOAD, please add it there" % (basename, basename))
         if basename in autoloadlist:
-            f = open("%s%s" % (dvar, autoloadpath), 'w')
-            if autoload:
-                for m in autoload.split():
-                    f.write('%s\n' % m)
-            else:
-                f.write('%s\n' % basename)
-            f.close()
+            with open("%s%s" % (dvar, autoloadpath), 'w') as f:
+                if autoload:
+                    for m in autoload.split():
+                        f.write('%s\n' % m)
+                else:
+                    f.write('%s\n' % basename)
             autoloadpath2append = ' %s' % autoloadpath
             d.appendVar('FILES:%s' % pkg, autoloadpath2append)
             d.appendVar('CONFFILES:%s' % pkg, autoloadpath2append)
@@ -127,9 +125,8 @@ python split_kernel_module_packages () {
         modconflist = (d.getVar("KERNEL_MODULE_PROBECONF") or "").split()
         modconf = d.getVar('module_conf_%s' % basename)
         if modconf and basename in modconflist:
-            f = open("%s%s" % (dvar, modconfpath), 'w')
-            f.write("%s\n" % modconf)
-            f.close()
+            with open("%s%s" % (dvar, modconfpath), 'w') as f:
+                f.write("%s\n" % modconf)
             modconfpath2append = ' %s' % modconfpath
             d.appendVar('FILES:%s' % pkg, modconfpath2append)
             d.appendVar('CONFFILES:%s' % pkg, modconfpath2append)
-- 
2.34.1

[PATCH V4 3/3] kernel: autoload and probeconf are handled by kernel-module-split

[Jose Quaresma]

Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
---
 meta/classes-recipe/kernel.bbclass | 2 --
 1 file changed, 2 deletions(-)

diff --git a/meta/classes-recipe/kernel.bbclass b/meta/classes-recipe/kernel.bbclass
index e82b696d1a..855c784149 100644
--- a/meta/classes-recipe/kernel.bbclass
+++ b/meta/classes-recipe/kernel.bbclass
@@ -483,8 +483,6 @@ kernel_do_install() {
 	install -m 0644 .config ${D}/${KERNEL_IMAGEDEST}/config-${KERNEL_VERSION}
 	install -m 0644 vmlinux ${D}/${KERNEL_IMAGEDEST}/vmlinux-${KERNEL_VERSION}
 	[ -e Module.symvers ] && install -m 0644 Module.symvers ${D}/${KERNEL_IMAGEDEST}/Module.symvers-${KERNEL_VERSION}
-	install -d ${D}${sysconfdir}/modules-load.d
-	install -d ${D}${sysconfdir}/modprobe.d
 }
 
 # Must be ran no earlier than after do_kernel_checkout or else Makefile won't be in ${S}/Makefile
-- 
2.34.1

RE: [PATCH V4 1/3] kernel-module-split: make autoload and probeconf distribution specific

[Peter Kjellerstedt]

> -----Original Message-----
> From: Jose Quaresma <quaresma.jose@gmail.com>
> Sent: den 22 juni 2023 10:45
> To: openembedded-core@lists.openembedded.org
> Cc: Jose Quaresma <jose.quaresma@foundries.io>; Ola x Nilsson <Ola.x.Nilsson@axis.com>; Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Subject: [PATCH V4 1/3] kernel-module-split: make autoload and probeconf distribution specific
> 
> Also take the oportunity to only add configuration files to FILES
> and CONFFILES when they exist and are used.
> 
> The modules-load.d [1] - Configure kernel modules to load at boot
> should install their configuration files in /usr/lib/modules-load.d.
> 
> The modprobe.d [2] - Configuration directory for modprobe
> should install their configuration files in /lib/modprobe.d
> 
> [1] https://www.freedesktop.org/software/systemd/man/modules-load.d.html
> [2] https://www.man7.org/linux/man-pages//man5/modprobe.d.5.html
> 
> [YOCTO #12212] https://bugzilla.yoctoproject.org/show_bug.cgi?id=12212
> 
> CC: Ola x Nilsson <Ola.x.Nilsson@axis.com>
> CC: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> ---
> 
> v4: also remove the sysconfdir when systemd is not used as it causes
> QA Issue: linux-yocto: Files/directories were installed but not shipped in
> any package
> 
> v3: fix v2 build issue and only add configuration files to FILES
> and CONFFILES when they exist and are used.
> 
> v2: use the same location as before on the class and define the new
> location just for systemd.
> 
>  .../kernel-module-split.bbclass               | 38 ++++++++++---------
>  .../distro/include/init-manager-systemd.inc   |  4 ++
>  2 files changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
> index 50882c31a7..1d5e39b44d 100644
> --- a/meta/classes-recipe/kernel-module-split.bbclass
> +++ b/meta/classes-recipe/kernel-module-split.bbclass

[cut]

> @@ -189,8 +191,8 @@ python split_kernel_module_packages () {
>      # avoid warnings. removedirs only raises an OSError if an empty
>      # directory cannot be removed.
>      dvar = d.getVar('PKGD')
> -    for dir in ["%s/etc/modprobe.d" % (dvar), "%s/etc/modules-load.d" % (dvar), "%s/etc" % (dvar)]:
> -        if len(os.listdir(dir)) == 0:
> +    for dir in ["%s%s" % (dvar, modprobedir), "%s%s" % (dvar, modulesloaddir), "%s%s" % (dvar, d.getVar('sysconfdir'))]:
> +        if os.path.isdir(dir) and len(os.listdir(dir)) == 0:
>              os.rmdir(dir)
>  }
>

But if modprobedir and modulesloaddir are /usr/lib-based, why should this code 
try to remove /etc? Don't you really want something corresponding to:

rmdir -p --ignore-fail-on-non-empty ${PKGD}${modprobedir} ${PKGD}${modulesloaddir}

e.g.:

    for dir in ["%s%s" % (dvar, modprobedir), "%s%s" % (dvar, modulesloaddir)]:
        while dir != "/" and os.path.isdir(dir) and len(os.listdir(dir)) == 0:
            os.rmdir(dir)
            dir = os.path.dirname(dir)

//Peter

Re: [PATCH V4 1/3] kernel-module-split: make autoload and probeconf distribution specific

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 3695 bytes --]

Peter Kjellerstedt <peter.kjellerstedt@axis.com> escreveu no dia quinta,
22/06/2023 à(s) 10:49:

> > -----Original Message-----
> > From: Jose Quaresma <quaresma.jose@gmail.com>
> > Sent: den 22 juni 2023 10:45
> > To: openembedded-core@lists.openembedded.org
> > Cc: Jose Quaresma <jose.quaresma@foundries.io>; Ola x Nilsson <
> Ola.x.Nilsson@axis.com>; Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > Subject: [PATCH V4 1/3] kernel-module-split: make autoload and probeconf
> distribution specific
> >
> > Also take the oportunity to only add configuration files to FILES
> > and CONFFILES when they exist and are used.
> >
> > The modules-load.d [1] - Configure kernel modules to load at boot
> > should install their configuration files in /usr/lib/modules-load.d.
> >
> > The modprobe.d [2] - Configuration directory for modprobe
> > should install their configuration files in /lib/modprobe.d
> >
> > [1] https://www.freedesktop.org/software/systemd/man/modules-load.d.html
> > [2] https://www.man7.org/linux/man-pages//man5/modprobe.d.5.html
> >
> > [YOCTO #12212] https://bugzilla.yoctoproject.org/show_bug.cgi?id=12212
> >
> > CC: Ola x Nilsson <Ola.x.Nilsson@axis.com>
> > CC: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> > ---
> >
> > v4: also remove the sysconfdir when systemd is not used as it causes
> > QA Issue: linux-yocto: Files/directories were installed but not shipped
> in
> > any package
> >
> > v3: fix v2 build issue and only add configuration files to FILES
> > and CONFFILES when they exist and are used.
> >
> > v2: use the same location as before on the class and define the new
> > location just for systemd.
> >
> >  .../kernel-module-split.bbclass               | 38 ++++++++++---------
> >  .../distro/include/init-manager-systemd.inc   |  4 ++
> >  2 files changed, 24 insertions(+), 18 deletions(-)
> >
> > diff --git a/meta/classes-recipe/kernel-module-split.bbclass
> b/meta/classes-recipe/kernel-module-split.bbclass
> > index 50882c31a7..1d5e39b44d 100644
> > --- a/meta/classes-recipe/kernel-module-split.bbclass
> > +++ b/meta/classes-recipe/kernel-module-split.bbclass
>
> [cut]
>
> > @@ -189,8 +191,8 @@ python split_kernel_module_packages () {
> >      # avoid warnings. removedirs only raises an OSError if an empty
> >      # directory cannot be removed.
> >      dvar = d.getVar('PKGD')
> > -    for dir in ["%s/etc/modprobe.d" % (dvar), "%s/etc/modules-load.d" %
> (dvar), "%s/etc" % (dvar)]:
> > -        if len(os.listdir(dir)) == 0:
> > +    for dir in ["%s%s" % (dvar, modprobedir), "%s%s" % (dvar,
> modulesloaddir), "%s%s" % (dvar, d.getVar('sysconfdir'))]:
> > +        if os.path.isdir(dir) and len(os.listdir(dir)) == 0:
> >              os.rmdir(dir)
> >  }
> >
>
> But if modprobedir and modulesloaddir are /usr/lib-based, why should this
> code
> try to remove /etc? Don't you really want something corresponding to:
>
> rmdir -p --ignore-fail-on-non-empty ${PKGD}${modprobedir}
> ${PKGD}${modulesloaddir}
>
> e.g.:
>
>     for dir in ["%s%s" % (dvar, modprobedir), "%s%s" % (dvar,
> modulesloaddir)]:
>         while dir != "/" and os.path.isdir(dir) and len(os.listdir(dir))
> == 0:
>             os.rmdir(dir)
>             dir = os.path.dirname(dir)
>
> //Peter
>
>
I have another patch to remove this block internally where we only create
the directories
when needed (if we have mod config files) and with that we do not need to
remove the end.
I will submit after this series passing the tests on the autobuilder.

Jose

-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 5523 bytes --]

Re: [OE-core] [PATCH V4 1/3] kernel-module-split: make autoload and probeconf distribution specific

[Luca Ceresoli]

Hi Jose,

On Thu, 22 Jun 2023 08:44:51 +0000
"Jose Quaresma" <quaresma.jose@gmail.com> wrote:

> Also take the oportunity to only add configuration files to FILES
> and CONFFILES when they exist and are used.
> 
> The modules-load.d [1] - Configure kernel modules to load at boot
> should install their configuration files in /usr/lib/modules-load.d.
> 
> The modprobe.d [2] - Configuration directory for modprobe
> should install their configuration files in /lib/modprobe.d
> 
> [1] https://www.freedesktop.org/software/systemd/man/modules-load.d.html
> [2] https://www.man7.org/linux/man-pages//man5/modprobe.d.5.html
> 
> [YOCTO #12212] https://bugzilla.yoctoproject.org/show_bug.cgi?id=12212
> 
> CC: Ola x Nilsson <Ola.x.Nilsson@axis.com>
> CC: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>

This has now turned to:

ERROR: linux-yocto-6.1.33+gitAUTOINC+db25e51e20_947f660bde-r0 do_package: QA Issue: linux-yocto: Files/directories were installed but not shipped in any package:
  /usr/lib

https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/5410/steps/14/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/5383/steps/15/logs/stdio

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [OE-core] [PATCH V4 1/3] kernel-module-split: make autoload and probeconf distribution specific

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 1955 bytes --]

Hi Luca,

Luca Ceresoli <luca.ceresoli@bootlin.com> escreveu no dia quinta,
22/06/2023 à(s) 17:36:

> Hi Jose,
>
> On Thu, 22 Jun 2023 08:44:51 +0000
> "Jose Quaresma" <quaresma.jose@gmail.com> wrote:
>
> > Also take the oportunity to only add configuration files to FILES
> > and CONFFILES when they exist and are used.
> >
> > The modules-load.d [1] - Configure kernel modules to load at boot
> > should install their configuration files in /usr/lib/modules-load.d.
> >
> > The modprobe.d [2] - Configuration directory for modprobe
> > should install their configuration files in /lib/modprobe.d
> >
> > [1] https://www.freedesktop.org/software/systemd/man/modules-load.d.html
> > [2] https://www.man7.org/linux/man-pages//man5/modprobe.d.5.html
> >
> > [YOCTO #12212] https://bugzilla.yoctoproject.org/show_bug.cgi?id=12212
> >
> > CC: Ola x Nilsson <Ola.x.Nilsson@axis.com>
> > CC: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
>
> This has now turned to:
>
> ERROR: linux-yocto-6.1.33+gitAUTOINC+db25e51e20_947f660bde-r0 do_package:
> QA Issue: linux-yocto: Files/directories were installed but not shipped in
> any package:
>   /usr/lib
>
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/5410/steps/14/logs/stdio
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/5383/steps/15/logs/stdio


I have another patch where we only create the directories when needed (if
we have mod config files)
and with that we do not need to remove them at the end.
However this one changes the logic a bit, so I'd like to send this one
separately
but I can see that it won't work so I'll have to insert it on V5.

Once again thanks for testing.

Jose


>
> Luca
>
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>


-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 3796 bytes --]