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

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

[Jose Quaresma]

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>
---

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

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

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 50882c31a7..ee14bb6910 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,8 +106,7 @@ 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)
@@ -117,23 +120,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()
         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)
+        appendfiles = " %s %s" % (autoloadpath, modconfpath)
+        d.appendVar('FILES:%s' % pkg, appendfiles)
+        d.appendVar('CONFFILES:%s' % pkg, appendfiles)
 
         if "description" in vals:
             old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
@@ -169,8 +168,7 @@ python split_kernel_module_packages () {
     postrm = d.getVar('pkg_postrm:modules')
 
     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,7 +187,7 @@ 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)]:
+    for dir in ["%s%s" % (dvar, modprobedir), "%s%s" % (dvar, modulesloaddir)]:
         if 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 V2 2/2] 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 V2 1/2] kernel-module-split: make autoload and probeconf distribution specific

[Peter Kjellerstedt]

> -----Original Message-----
> From: Jose Quaresma <quaresma.jose@gmail.com>
> Sent: den 20 juni 2023 01:14
> 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 V2 1/2] kernel-module-split: make autoload and probeconf distribution specific
> 
> 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>
> ---
> 
> V2: use the same location as before on the class and define the new
> location just for systemd.
> 
>  .../kernel-module-split.bbclass               | 30 +++++++++----------
>  .../distro/include/init-manager-systemd.inc   |  4 +++
>  2 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
> index 50882c31a7..ee14bb6910 100644
> --- a/meta/classes-recipe/kernel-module-split.bbclass
> +++ b/meta/classes-recipe/kernel-module-split.bbclass

[cut]

> @@ -169,8 +168,7 @@ python split_kernel_module_packages () {
>      postrm = d.getVar('pkg_postrm:modules')
> 
>      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")))

There is no need for the trailing slashes in FILES:${PN}, so the above 
can be simplified to: 

        d.appendVar('FILES:' + metapkg, '%s %s %s/modules' % (modulesloaddir, modprobedir, d.getVar("nonarch_base_libdir")))

Not sure that warrants a new patch though.

>          d.appendVar('pkg_postinst:%s' % metapkg, postinst)
>          d.prependVar('pkg_postrm:%s' % metapkg, postrm);
>          return

Reviewed-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>

//Peter

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

[Ola x Nilsson]

On Mon, Jun 19 2023, Jose Quaresma wrote:

> 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>
> ---
>
> V2: use the same location as before on the class and define the new
> location just for systemd.
>
>  .../kernel-module-split.bbclass               | 30 +++++++++----------
>  .../distro/include/init-manager-systemd.inc   |  4 +++
>  2 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
> index 50882c31a7..ee14bb6910 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,8 +106,7 @@ 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)

You could take the opportunity to rewrite this section to use context
manager to handle the file object:
  with open(...) as f


> @@ -117,23 +120,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')

Same.

>              f.write("%s\n" % modconf)
>              f.close()
>          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)
> +        appendfiles = " %s %s" % (autoloadpath, modconfpath)
> +        d.appendVar('FILES:%s' % pkg, appendfiles)
> +        d.appendVar('CONFFILES:%s' % pkg, appendfiles)
>  

I think it would be best to use autoloadpath and modconfpath for the
automatic creation of such files, but use all known variants of these
paths for the package splitting.  If the module makefile creates the
files in one of the places the package splitter should find them without
requireing the customization of the recipe. 

>          if "description" in vals:
>              old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
> @@ -169,8 +168,7 @@ python split_kernel_module_packages () {
>      postrm = d.getVar('pkg_postrm:modules')
>  
>      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,7 +187,7 @@ 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)]:
> +    for dir in ["%s%s" % (dvar, modprobedir), "%s%s" % (dvar, modulesloaddir)]:
>          if len(os.listdir(dir)) == 0:
>              os.rmdir(dir)
>  }

Same.

> 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"


-- 
Ola x Nilsson

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

[Luca Ceresoli]

Hello Jose,

On Mon, 19 Jun 2023 23:13:43 +0000
"Jose Quaresma" <quaresma.jose@gmail.com> wrote:

> 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 patchset is triggering some AB failures:

  Exception: NameError: name 'modprobedir' is not defined

Some logs:

https://autobuilder.yoctoproject.org/typhoon/#/builders/104/builds/6229/steps/12/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/23/builds/7658/steps/14/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/120/builds/2935/steps/12/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/89/builds/7369/steps/17/logs/stdio

Luca

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

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

[Jose Quaresma]

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

Luca Ceresoli <luca.ceresoli@bootlin.com> escreveu no dia terça, 20/06/2023
à(s) 14:57:

> Hello Jose,
>
> On Mon, 19 Jun 2023 23:13:43 +0000
> "Jose Quaresma" <quaresma.jose@gmail.com> wrote:
>
> > 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 patchset is triggering some AB failures:
>
>   Exception: NameError: name 'modprobedir' is not defined
>
> Some logs:
>
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/104/builds/6229/steps/12/logs/stdio
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/23/builds/7658/steps/14/logs/stdio
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/120/builds/2935/steps/12/logs/stdio
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/89/builds/7369/steps/17/logs/stdio


Hi Luca,

Thanks for the info.

I missed some parts of the patch when updating to v2.
I am also addressing some comments on v2 and will send a v3
after some more testing.

--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -166,6 +166,8 @@ 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':
         d.appendVar('FILES:' + metapkg, '%s/ %s/ %s/modules/' %
(modulesloaddir, modprobedir, d.getVar("nonarch_base_libdir")))



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


-- 
Best regards,

José Quaresma

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