[PATCH] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[PATCH] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

When KERNEL_SPLIT_MODULES=0 modprob and autload conf files are not getting
generated for the kernel modules.

Separated out conf file handling mechanism as handle_conf_files() function
from hook frob_metadata() function. handle_conf_files() gets re-used from the
existing hook function and add_conf_files function which got introduced for
splitmods=0 flow in this patch.

[YOCTO #15145]

Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
Cc: George Thopas <george.thopas@gmail.com>
---
 .../kernel-module-split.bbclass               | 82 +++++++++++++++----
 1 file changed, 67 insertions(+), 15 deletions(-)

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 9487365eb7..2fb3ab967f 100644
--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -86,11 +86,7 @@ python split_kernel_module_packages () {
             vals[m.group(1)] = m.group(2)
         return vals
 
-    def frob_metadata(file, pkg, pattern, format, basename):
-        vals = extract_modinfo(file)
-
-        dvar = d.getVar('PKGD')
-
+    def handle_conf_files(d, basename, pkg):
         # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
         # appropriate modprobe commands to the postinst
         autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
@@ -101,7 +97,7 @@ python split_kernel_module_packages () {
             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:
             conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 if autoload:
@@ -114,7 +110,7 @@ python split_kernel_module_packages () {
             d.appendVar('CONFFILES:%s' % pkg, conf2append)
             postinst = d.getVar('pkg_postinst:%s' % pkg)
             if not postinst:
-                bb.fatal("pkg_postinst:%s not defined" % pkg)
+                postinst = d.getVar('pkg_postinst:modules')
             postinst += d.getVar('autoload_postinst_fragment') % (autoload or basename)
             d.setVar('pkg_postinst:%s' % pkg, postinst)
 
@@ -123,7 +119,7 @@ python split_kernel_module_packages () {
         modconf = d.getVar('module_conf_%s' % basename)
         if modconf and basename in modconflist:
             conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 f.write("%s\n" % modconf)
@@ -134,6 +130,62 @@ python split_kernel_module_packages () {
         elif modconf:
             bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
 
+    def add_conf_files(d, root, file_regex, output_pattern):
+        """
+        Arguments:
+        root           -- the path in which to search
+        file_regex     -- regular expression to match searched files. Use
+                          parentheses () to mark the part of this expression
+                          that should be used to derive the module name (to be
+                          substituted where %s is used in other function
+                          arguments as noted below)
+        output_pattern -- pattern to use for the package names. Must include %s.
+        """
+
+        dvar = d.getVar('PKGD')
+        root = d.expand(root)
+        output_pattern = d.expand(output_pattern)
+
+        # if the root directory doesn't exist, don't error out later but silently do
+        # no splitting.
+        if not os.path.exists(dvar + root):
+            return []
+
+        # get list of modules
+        objs = []
+        for walkroot, dirs, files in os.walk(dvar + root):
+            for file in files:
+                relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
+                if relpath:
+                    objs.append(relpath)
+
+        for o in sorted(objs):
+            import re, stat
+            if False:
+                m = re.match(file_regex, o)
+            else:
+                m = re.match(file_regex, os.path.basename(o))
+
+            if not m:
+                continue
+
+            file = os.path.join(dvar + root, o)
+            mode = os.lstat(file).st_mode
+            if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or (allow_dirs and stat.S_ISDIR(mode))):
+                continue
+
+            on = legitimize_package_name(m.group(1))
+            pkg = output_pattern % on
+
+            basename = m.group(1)
+            handle_conf_files(d, basename, pkg)
+
+    def frob_metadata(file, pkg, pattern, format, basename):
+        vals = extract_modinfo(file)
+        dvar = d.getVar('PKGD')
+
+        handle_conf_files(d, basename, pkg)
+
         if "description" in vals:
             old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
             d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
@@ -167,19 +219,19 @@ python split_kernel_module_packages () {
     postinst = d.getVar('pkg_postinst:modules')
     postrm = d.getVar('pkg_postrm:modules')
 
+    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
+    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
+    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
+    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
+
     if splitmods != '1':
         d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
             (d.getVar('modulesloaddir'), d.getVar('modprobedir'), d.getVar("nonarch_base_libdir")))
         d.appendVar('pkg_postinst:%s' % metapkg, postinst)
-        d.prependVar('pkg_postrm:%s' % metapkg, postrm);
+        d.prependVar('pkg_postrm:%s' % metapkg, postrm)
+        add_conf_files(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern)
         return
 
-    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
-
-    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
-    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
-    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
-
     modules = do_split_packages(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern, description='%s kernel module', postinst=postinst, postrm=postrm, recursive=True, hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name, kernel_version))
     if modules:
         d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
-- 
2.43.0

Re: [OE-core] [PATCH] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Bruce Ashfield]

In message: [OE-core] [PATCH] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0
on 22/02/2025 Dixit Parmar via lists.openembedded.org wrote:

> When KERNEL_SPLIT_MODULES=0 modprob and autload conf files are not getting

s/modprob/modprobe/
s/autload/autoload/

> generated for the kernel modules.
> 
> Separated out conf file handling mechanism as handle_conf_files() function
> from hook frob_metadata() function. handle_conf_files() gets re-used from the
> existing hook function and add_conf_files function which got introduced for
> splitmods=0 flow in this patch.

Can you show the conf files for the same kernel configuration
with and without the kernel_split_modules enabled ? That way
we know there's no change in existing behaviour.

We also should make a test for this in the OE selftests. We
are adding conditional code paths, so they should be tested
to ensure no regressions in either in the future.

> 
> [YOCTO #15145]
> 
> Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
> Cc: George Thopas <george.thopas@gmail.com>
> ---
>  .../kernel-module-split.bbclass               | 82 +++++++++++++++----
>  1 file changed, 67 insertions(+), 15 deletions(-)
> 
> diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
> index 9487365eb7..2fb3ab967f 100644
> --- a/meta/classes-recipe/kernel-module-split.bbclass
> +++ b/meta/classes-recipe/kernel-module-split.bbclass
> @@ -86,11 +86,7 @@ python split_kernel_module_packages () {
>              vals[m.group(1)] = m.group(2)
>          return vals
>  
> -    def frob_metadata(file, pkg, pattern, format, basename):
> -        vals = extract_modinfo(file)
> -
> -        dvar = d.getVar('PKGD')
> -
> +    def handle_conf_files(d, basename, pkg):
>          # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
>          # appropriate modprobe commands to the postinst
>          autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
> @@ -101,7 +97,7 @@ python split_kernel_module_packages () {
>              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:
>              conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
> -            name = '%s%s' % (dvar, conf)
> +            name = '%s%s' % (d.getVar('PKGD'), conf)
>              os.makedirs(os.path.dirname(name), exist_ok=True)
>              with open(name, 'w') as f:
>                  if autoload:
> @@ -114,7 +110,7 @@ python split_kernel_module_packages () {
>              d.appendVar('CONFFILES:%s' % pkg, conf2append)
>              postinst = d.getVar('pkg_postinst:%s' % pkg)
>              if not postinst:
> -                bb.fatal("pkg_postinst:%s not defined" % pkg)
> +                postinst = d.getVar('pkg_postinst:modules')

I'm curious about the above line. It is unclear to me why we'd
only have this postinst be relevant if none was previously set.

>              postinst += d.getVar('autoload_postinst_fragment') % (autoload or basename)
>              d.setVar('pkg_postinst:%s' % pkg, postinst)
>  
> @@ -123,7 +119,7 @@ python split_kernel_module_packages () {
>          modconf = d.getVar('module_conf_%s' % basename)
>          if modconf and basename in modconflist:
>              conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
> -            name = '%s%s' % (dvar, conf)
> +            name = '%s%s' % (d.getVar('PKGD'), conf)
>              os.makedirs(os.path.dirname(name), exist_ok=True)
>              with open(name, 'w') as f:
>                  f.write("%s\n" % modconf)
> @@ -134,6 +130,62 @@ python split_kernel_module_packages () {
>          elif modconf:
>              bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
>  
> +    def add_conf_files(d, root, file_regex, output_pattern):
> +        """
> +        Arguments:
> +        root           -- the path in which to search
> +        file_regex     -- regular expression to match searched files. Use
> +                          parentheses () to mark the part of this expression
> +                          that should be used to derive the module name (to be
> +                          substituted where %s is used in other function
> +                          arguments as noted below)
> +        output_pattern -- pattern to use for the package names. Must include %s.
> +        """
> +
> +        dvar = d.getVar('PKGD')
> +        root = d.expand(root)
> +        output_pattern = d.expand(output_pattern)
> +
> +        # if the root directory doesn't exist, don't error out later but silently do
> +        # no splitting.
> +        if not os.path.exists(dvar + root):
> +            return []

Is there really a scenario where the directory won't exist ? Isn't
this just running in our own install phase ? So all prerequisites
and directories should be in place.

> +
> +        # get list of modules
> +        objs = []
> +        for walkroot, dirs, files in os.walk(dvar + root):
> +            for file in files:
> +                relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
> +                if relpath:
> +                    objs.append(relpath)
> +
> +        for o in sorted(objs):
> +            import re, stat
> +            if False:
> +                m = re.match(file_regex, o)
> +            else:
> +                m = re.match(file_regex, os.path.basename(o))
> +
> +            if not m:
> +                continue
> +
> +            file = os.path.join(dvar + root, o)
> +            mode = os.lstat(file).st_mode
> +            if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or (allow_dirs and stat.S_ISDIR(mode))):
> +                continue
> +
> +            on = legitimize_package_name(m.group(1))
> +            pkg = output_pattern % on
> +
> +            basename = m.group(1)
> +            handle_conf_files(d, basename, pkg)

The walking and sorting seems quite heavy.  Isn't this called from do_split_packages indirectly ?
Do we really need to walk and gather the information ? Is this mainly for the case of no-split
on the kernel modules ? If that is the case, isn't there a way to short circuit the processing
on the split-package case ?

Bruce

> +
> +    def frob_metadata(file, pkg, pattern, format, basename):
> +        vals = extract_modinfo(file)
> +        dvar = d.getVar('PKGD')
> +
> +        handle_conf_files(d, basename, pkg)
> +
>          if "description" in vals:
>              old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
>              d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
> @@ -167,19 +219,19 @@ python split_kernel_module_packages () {
>      postinst = d.getVar('pkg_postinst:modules')
>      postrm = d.getVar('pkg_postrm:modules')
>  
> +    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
> +    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
> +    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
> +    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
> +
>      if splitmods != '1':
>          d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
>              (d.getVar('modulesloaddir'), d.getVar('modprobedir'), d.getVar("nonarch_base_libdir")))
>          d.appendVar('pkg_postinst:%s' % metapkg, postinst)
> -        d.prependVar('pkg_postrm:%s' % metapkg, postrm);
> +        d.prependVar('pkg_postrm:%s' % metapkg, postrm)
> +        add_conf_files(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern)
>          return
>  
> -    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
> -
> -    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
> -    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
> -    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
> -
>      modules = do_split_packages(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern, description='%s kernel module', postinst=postinst, postrm=postrm, recursive=True, hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name, kernel_version))
>      if modules:
>          d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
> -- 
> 2.43.0
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#211817): https://lists.openembedded.org/g/openembedded-core/message/211817
> Mute This Topic: https://lists.openembedded.org/mt/111322503/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

Re: [PATCH] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

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

Can you show the conf files for the same kernel configuration
with and without the kernel_split_modules enabled ? That way
we know there's no change in existing behaviour.
> I have confirmed that in my testing. Can you suggest how I can
share that information here?
We also should make a test for this in the OE selftests. We
are adding conditional code paths, so they should be tested
to ensure no regressions in either in the future.
> Never done that before. May be I can do it given some direction
as separate patch.
I'm curious about the above line. It is unclear to me why we'd
only have this postinst be relevant if none was previously set.
> Reverted.
Is there really a scenario where the directory won't exist ? Isn't
this just running in our own install phase ? So all prerequisites
and directories should be in place.
> Ideally no, we kept it for safer side, I have added log warning.
The walking and sorting seems quite heavy.  Isn't this called from do_split_packages indirectly ?
Do we really need to walk and gather the information ? Is this mainly for the case of no-split
on the kernel modules ? If that is the case, isn't there a way to short circuit the processing
on the split-package case ?
> Litterally I could not think of anything else here and not sure of there
are any short-circuit options. I have limited knowledge in this. I am open to suggestions
if this is not the best solution at the moment.

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

Re: [PATCH] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

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

Submitted V2 patch: https://lists.openembedded.org/g/openembedded-core/message/212995

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

Re: [OE-core] [PATCH] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Trevor Woerner]

On Sun 2025-03-16 @ 01:16:47 AM, Dixit Parmar via lists.openembedded.org wrote:
> Can you show the conf files for the same kernel configuration
> with and without the kernel_split_modules enabled ? That way
> we know there's no change in existing behaviour.
> > I have confirmed that in my testing. Can you suggest how I can
> share that information here?

What data are you expecting to find, and where are you expecting to find
it, that prompted this fix? I assume that in one case there will be
information/files populated in the image's /etc/modprobe.d and in the other
case (the buggy case) that isn't happening? Or is there something else?

Can you provide complete instructions and/or your configuration demonstrating
the problem occurring before your fix, and the problem being solved by your
fix? Simply enabling KERNEL_SPLIT_MODULES by itself won't show anything unless
the user is trying to enable some module(s) too.

I found that I had to go read the bugzilla report to better understand the
issue and potentially how to show it occurring. Keep the reference to the
bugzilla, but ideally the commit message would have all the necessary
information to understand and reproduce the issue.

> We also should make a test for this in the OE selftests. We
> are adding conditional code paths, so they should be tested
> to ensure no regressions in either in the future.
> > Never done that before. May be I can do it given some direction
> as separate patch.
> I'm curious about the above line. It is unclear to me why we'd
> only have this postinst be relevant if none was previously set.
> > Reverted.
> Is there really a scenario where the directory won't exist ? Isn't
> this just running in our own install phase ? So all prerequisites
> and directories should be in place.
> > Ideally no, we kept it for safer side, I have added log warning.
> The walking and sorting seems quite heavy.  Isn't this called from do_split_packages indirectly ?
> Do we really need to walk and gather the information ? Is this mainly for the case of no-split
> on the kernel modules ? If that is the case, isn't there a way to short circuit the processing
> on the split-package case ?
> > Litterally I could not think of anything else here and not sure of there
> are any short-circuit options. I have limited knowledge in this. I am open to suggestions
> if this is not the best solution at the moment.

It is customary that quoted text have the start-of-line ">" symbols (the more
symbols, the further back in history the comment) and that replies not have
any start-of-line character, as I'm doing here. Your email program should have
an option to do this automatically.

Re: [PATCH] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

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

On Mon, Mar 17, 2025 at 07:54 PM, Trevor Woerner wrote:

> 
> On Sun 2025-03-16 @ 01:16:47 AM, Dixit Parmar via lists.openembedded.org
> wrote:
> 
>> Can you show the conf files for the same kernel configuration
>> with and without the kernel_split_modules enabled ? That way
>> we know there's no change in existing behaviour.
>> 
>>> I have confirmed that in my testing. Can you suggest how I can
>> 
>> share that information here?
> 
> What data are you expecting to find, and where are you expecting to find
> it, that prompted this fix? I assume that in one case there will be
> information/files populated in the image's /etc/modprobe.d and in the
> other
> case (the buggy case) that isn't happening? Or is there something else?
> 
> Can you provide complete instructions and/or your configuration
> demonstrating
> the problem occurring before your fix, and the problem being solved by
> your
> fix? Simply enabling KERNEL_SPLIT_MODULES by itself won't show anything
> unless
> the user is trying to enable some module(s) too.
> 
> I found that I had to go read the bugzilla report to better understand the
> 
> issue and potentially how to show it occurring. Keep the reference to the
> bugzilla, but ideally the commit message would have all the necessary
> information to understand and reproduce the issue.

Yeah. I shall add the issue reproduction configuration, and overview of the fix in the next patchset v2. Do you see any other code changes that are required? so I can take it in the next set of changes.

> 
> 
>> We also should make a test for this in the OE selftests. We
>> are adding conditional code paths, so they should be tested
>> to ensure no regressions in either in the future.
>> 
>>> Never done that before. May be I can do it given some direction
>> 
>> as separate patch.
>> I'm curious about the above line. It is unclear to me why we'd
>> only have this postinst be relevant if none was previously set.
>> 
>>> Reverted.
>> 
>> Is there really a scenario where the directory won't exist ? Isn't
>> this just running in our own install phase ? So all prerequisites
>> and directories should be in place.
>> 
>>> Ideally no, we kept it for safer side, I have added log warning.
>> 
>> The walking and sorting seems quite heavy.  Isn't this called from
>> do_split_packages indirectly ?
>> Do we really need to walk and gather the information ? Is this mainly for
>> the case of no-split
>> on the kernel modules ? If that is the case, isn't there a way to short
>> circuit the processing
>> on the split-package case ?
>> 
>>> Litterally I could not think of anything else here and not sure of there
>> 
>> are any short-circuit options. I have limited knowledge in this. I am open
>> to suggestions
>> if this is not the best solution at the moment.
> 
> It is customary that quoted text have the start-of-line ">" symbols (the
> more
> symbols, the further back in history the comment) and that replies not
> have
> any start-of-line character, as I'm doing here. Your email program should
> have
> an option to do this automatically.

As you might have rightly assumed, I am a newbie so figuring out this stuff, I hope you understand.

~Dixit

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

Re: [OE-core] [PATCH] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Bruce Ashfield]

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

On Wed, Mar 19, 2025 at 3:40 AM Dixit Parmar via lists.openembedded.org
<dixitparmar19=gmail.com@lists.openembedded.org> wrote:

> On Mon, Mar 17, 2025 at 07:54 PM, Trevor Woerner wrote:
>
> On Sun 2025-03-16 @ 01:16:47 AM, Dixit Parmar via lists.openembedded.org
> wrote:
>
> Can you show the conf files for the same kernel configuration
> with and without the kernel_split_modules enabled ? That way
> we know there's no change in existing behaviour.
>
> I have confirmed that in my testing. Can you suggest how I can
>
> share that information here?
>
> What data are you expecting to find, and where are you expecting to find
> it, that prompted this fix? I assume that in one case there will be
> information/files populated in the image's /etc/modprobe.d and in the other
> case (the buggy case) that isn't happening? Or is there something else?
>
> Can you provide complete instructions and/or your configuration
> demonstrating
> the problem occurring before your fix, and the problem being solved by your
> fix? Simply enabling KERNEL_SPLIT_MODULES by itself won't show anything
> unless
> the user is trying to enable some module(s) too.
>
> I found that I had to go read the bugzilla report to better understand the
> issue and potentially how to show it occurring. Keep the reference to the
> bugzilla, but ideally the commit message would have all the necessary
> information to understand and reproduce the issue.
>
> Yeah. I shall add the issue reproduction configuration, and overview of
> the fix in the next patchset v2. Do you see any other code changes that are
> required? so I can take it in the next set of changes.
>
>
Hold on the next version for a day or so.

I've been traveling and haven't had time to reply to your other questions,
but will
get time either today or tomorrow.

Bruce



> We also should make a test for this in the OE selftests. We
> are adding conditional code paths, so they should be tested
> to ensure no regressions in either in the future.
>
> Never done that before. May be I can do it given some direction
>
> as separate patch.
> I'm curious about the above line. It is unclear to me why we'd
> only have this postinst be relevant if none was previously set.
>
> Reverted.
>
> Is there really a scenario where the directory won't exist ? Isn't
> this just running in our own install phase ? So all prerequisites
> and directories should be in place.
>
> Ideally no, we kept it for safer side, I have added log warning.
>
> The walking and sorting seems quite heavy.  Isn't this called from
> do_split_packages indirectly ?
> Do we really need to walk and gather the information ? Is this mainly for
> the case of no-split
> on the kernel modules ? If that is the case, isn't there a way to short
> circuit the processing
> on the split-package case ?
>
> Litterally I could not think of anything else here and not sure of there
>
> are any short-circuit options. I have limited knowledge in this. I am open
> to suggestions
> if this is not the best solution at the moment.
>
> It is customary that quoted text have the start-of-line ">" symbols (the
> more
> symbols, the further back in history the comment) and that replies not have
> any start-of-line character, as I'm doing here. Your email program should
> have
> an option to do this automatically.
>
> As you might have rightly assumed, I am a newbie so figuring out this
> stuff, I hope you understand.
>
> ~Dixit
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#213266):
> https://lists.openembedded.org/g/openembedded-core/message/213266
> Mute This Topic: https://lists.openembedded.org/mt/111322503/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end
- "Use the force Harry" - Gandalf, Star Trek II

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

[PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

When KERNEL_SPLIT_MODULES=0 modprobe and autoload conf files are not getting
generated for the kernel modules.

Separated out conf file handling mechanism as handle_conf_files() function
from hook frob_metadata() function. handle_conf_files() gets re-used from the
existing hook function and add_conf_files function which got introduced for
splitmods=0 flow in this patch.

[YOCTO #15145]

Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
Cc: George Thopas <george.thopas@gmail.com>
--
Can you show the conf files for the same kernel configuration
with and without the kernel_split_modules enabled ? That way
we know there's no change in existing behaviour.
> I have confirmed that in my testing. Can you suggest how I can
share that information here?

We also should make a test for this in the OE selftests. We
are adding conditional code paths, so they should be tested
to ensure no regressions in either in the future.
> Never done that before. May be I can do it given some direction 
as separate patch.

I'm curious about the above line. It is unclear to me why we'd
only have this postinst be relevant if none was previously set.
> Reverted.

Is there really a scenario where the directory won't exist ? Isn't
this just running in our own install phase ? So all prerequisites
and directories should be in place.
> Ideally no, we kept it for safer side, I have added log warning.

The walking and sorting seems quite heavy.  Isn't this called from do_split_packages indirectly ?
Do we really need to walk and gather the information ? Is this mainly for the case of no-split
on the kernel modules ? If that is the case, isn't there a way to short circuit the processing
on the split-package case ?
> Litterally I could not think of anything else here and not sure of there
are any short-circuit options. I have limited knowledge in this. I am open to suggestions
if this is not the best solution at the moment.
---
 .../kernel-module-split.bbclass               | 81 +++++++++++++++----
 1 file changed, 67 insertions(+), 14 deletions(-)

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 9487365eb7..b7ee3a8f9e 100644
--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -86,11 +86,7 @@ python split_kernel_module_packages () {
             vals[m.group(1)] = m.group(2)
         return vals
 
-    def frob_metadata(file, pkg, pattern, format, basename):
-        vals = extract_modinfo(file)
-
-        dvar = d.getVar('PKGD')
-
+    def handle_conf_files(d, basename, pkg):
         # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
         # appropriate modprobe commands to the postinst
         autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
@@ -101,7 +97,7 @@ python split_kernel_module_packages () {
             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:
             conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 if autoload:
@@ -123,7 +119,7 @@ python split_kernel_module_packages () {
         modconf = d.getVar('module_conf_%s' % basename)
         if modconf and basename in modconflist:
             conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 f.write("%s\n" % modconf)
@@ -134,6 +130,63 @@ python split_kernel_module_packages () {
         elif modconf:
             bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
 
+    def add_conf_files(d, root, file_regex, output_pattern):
+        """
+        Arguments:
+        root           -- the path in which to search
+        file_regex     -- regular expression to match searched files. Use
+                          parentheses () to mark the part of this expression
+                          that should be used to derive the module name (to be
+                          substituted where %s is used in other function
+                          arguments as noted below)
+        output_pattern -- pattern to use for the package names. Must include %s.
+        """
+
+        dvar = d.getVar('PKGD')
+        root = d.expand(root)
+        output_pattern = d.expand(output_pattern)
+
+        # check if the root directory doesn't exist for safe side, don't error out later but silently do
+        # no splitting.
+        if not os.path.exists(dvar + root):
+            bb.warn("kernel module root directory path does not exist")
+            return []
+
+        # get list of modules
+        objs = []
+        for walkroot, dirs, files in os.walk(dvar + root):
+            for file in files:
+                relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
+                if relpath:
+                    objs.append(relpath)
+
+        for o in sorted(objs):
+            import re, stat
+            if False:
+                m = re.match(file_regex, o)
+            else:
+                m = re.match(file_regex, os.path.basename(o))
+
+            if not m:
+                continue
+
+            file = os.path.join(dvar + root, o)
+            mode = os.lstat(file).st_mode
+            if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or (allow_dirs and stat.S_ISDIR(mode))):
+                continue
+
+            on = legitimize_package_name(m.group(1))
+            pkg = output_pattern % on
+
+            basename = m.group(1)
+            handle_conf_files(d, basename, pkg)
+
+    def frob_metadata(file, pkg, pattern, format, basename):
+        vals = extract_modinfo(file)
+        dvar = d.getVar('PKGD')
+
+        handle_conf_files(d, basename, pkg)
+
         if "description" in vals:
             old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
             d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
@@ -167,19 +220,19 @@ python split_kernel_module_packages () {
     postinst = d.getVar('pkg_postinst:modules')
     postrm = d.getVar('pkg_postrm:modules')
 
+    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
+    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
+    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
+    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
+
     if splitmods != '1':
         d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
             (d.getVar('modulesloaddir'), d.getVar('modprobedir'), d.getVar("nonarch_base_libdir")))
         d.appendVar('pkg_postinst:%s' % metapkg, postinst)
-        d.prependVar('pkg_postrm:%s' % metapkg, postrm);
+        d.prependVar('pkg_postrm:%s' % metapkg, postrm)
+        add_conf_files(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern)
         return
 
-    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
-
-    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
-    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
-    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
-
     modules = do_split_packages(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern, description='%s kernel module', postinst=postinst, postrm=postrm, recursive=True, hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name, kernel_version))
     if modules:
         d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
-- 
2.43.0

Re: [OE-core] [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Bruce Ashfield]

In message: [OE-core] [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0
on 16/03/2025 Dixit Parmar via lists.openembedded.org wrote:

> When KERNEL_SPLIT_MODULES=0 modprobe and autoload conf files are not getting
> generated for the kernel modules.
> 
> Separated out conf file handling mechanism as handle_conf_files() function
> from hook frob_metadata() function. handle_conf_files() gets re-used from the
> existing hook function and add_conf_files function which got introduced for
> splitmods=0 flow in this patch.

Trevor's advice on the commit message was excellent, I won't repeat
it here. I'll just simply add that the commit message should give us
all the information we need about the issue (and what its symptoms
are) and how this fixes it (not explaining the code, but describing
the strategy)

> 
> [YOCTO #15145]
> 
> Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
> Cc: George Thopas <george.thopas@gmail.com>
> --
> Can you show the conf files for the same kernel configuration
> with and without the kernel_split_modules enabled ? That way
> we know there's no change in existing behaviour.
> > I have confirmed that in my testing. Can you suggest how I can
> share that information here?

A good way would simply to put it in the commit message that you've
built and booted with both modes and that the generated module
conf files are the same in both cases. Showing a diff command
that you ran, or how you determined they were the same would be enough
as well.

> 
> We also should make a test for this in the OE selftests. We
> are adding conditional code paths, so they should be tested
> to ensure no regressions in either in the future.
> > Never done that before. May be I can do it given some direction 
> as separate patch.

Any selftests should be in a separate patch, but they do need
to come along with any new verions of this. Saying that "it will
be send later" won't get the patches merged. Since everyone gets
busy and these sorts of things tend to not happen.

If you search on the mailing list(s) you'll find examples of
other self tests being submitted. Just follow what they are
doing. I'd suggest that a self test needs to run both modes
and do a check that the generated .conf files are correct
(basically what you'd be showing in the commit message). Even
better if a boot and showing that they are (auto)loaded.

That's the only way we can be sure this won't break in the
future and be comfortable with the complexty being added.

> 
> I'm curious about the above line. It is unclear to me why we'd
> only have this postinst be relevant if none was previously set.
> > Reverted.
> 
> Is there really a scenario where the directory won't exist ? Isn't
> this just running in our own install phase ? So all prerequisites
> and directories should be in place.
> > Ideally no, we kept it for safer side, I have added log warning.
>

As Trevor mentioned in his reply, it is best to configure your
mail client to reply inline. I had made that comment near the
line of code in question, but with the time between my review,
your reply and now this reply .. the context of what was being
discussed is very difficult to remember.

If that directory doesn't exist, just error out and do not
continue. This isn't something recoverable, and the build should
be stopped.


> The walking and sorting seems quite heavy.  Isn't this called from do_split_packages indirectly ?
> Do we really need to walk and gather the information ? Is this mainly for the case of no-split
> on the kernel modules ? If that is the case, isn't there a way to short circuit the processing
> on the split-package case ?
> > Litterally I could not think of anything else here and not sure of there
> are any short-circuit options. I have limited knowledge in this. I am open to suggestions
> if this is not the best solution at the moment.

Describe the approach in your commit message, this is the type of
information that we need. To be sure that it isn't adding a lot of
time to the build, add some timing information with and without
your changes. This is the type of information we are looking for
in a review.

See a few more comments below.

> ---
>  .../kernel-module-split.bbclass               | 81 +++++++++++++++----
>  1 file changed, 67 insertions(+), 14 deletions(-)
> 
> diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
> index 9487365eb7..b7ee3a8f9e 100644
> --- a/meta/classes-recipe/kernel-module-split.bbclass
> +++ b/meta/classes-recipe/kernel-module-split.bbclass
> @@ -86,11 +86,7 @@ python split_kernel_module_packages () {
>              vals[m.group(1)] = m.group(2)
>          return vals
>  
> -    def frob_metadata(file, pkg, pattern, format, basename):
> -        vals = extract_modinfo(file)
> -
> -        dvar = d.getVar('PKGD')
> -
> +    def handle_conf_files(d, basename, pkg):
>          # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
>          # appropriate modprobe commands to the postinst
>          autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
> @@ -101,7 +97,7 @@ python split_kernel_module_packages () {
>              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:
>              conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
> -            name = '%s%s' % (dvar, conf)
> +            name = '%s%s' % (d.getVar('PKGD'), conf)
>              os.makedirs(os.path.dirname(name), exist_ok=True)
>              with open(name, 'w') as f:
>                  if autoload:
> @@ -123,7 +119,7 @@ python split_kernel_module_packages () {
>          modconf = d.getVar('module_conf_%s' % basename)
>          if modconf and basename in modconflist:
>              conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
> -            name = '%s%s' % (dvar, conf)
> +            name = '%s%s' % (d.getVar('PKGD'), conf)
>              os.makedirs(os.path.dirname(name), exist_ok=True)
>              with open(name, 'w') as f:
>                  f.write("%s\n" % modconf)
> @@ -134,6 +130,63 @@ python split_kernel_module_packages () {
>          elif modconf:
>              bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
>  
> +    def add_conf_files(d, root, file_regex, output_pattern):
> +        """
> +        Arguments:
> +        root           -- the path in which to search
> +        file_regex     -- regular expression to match searched files. Use
> +                          parentheses () to mark the part of this expression
> +                          that should be used to derive the module name (to be
> +                          substituted where %s is used in other function
> +                          arguments as noted below)
> +        output_pattern -- pattern to use for the package names. Must include %s.
> +        """
> +
> +        dvar = d.getVar('PKGD')
> +        root = d.expand(root)

root is a passed parameter, you shouldn't need to call d.expand()
on it. Are you expecting there to be something that can be expanded ?

> +        output_pattern = d.expand(output_pattern)

Likewise with output_pattern

> +
> +        # check if the root directory doesn't exist for safe side, don't error out later but silently do
> +        # no splitting.
> +        if not os.path.exists(dvar + root):
> +            bb.warn("kernel module root directory path does not exist")
> +            return []

as I mentioned above, I see this as a fatal error. Something has gone
terribly wrong if this doesn't exist and we should error and exit.

> +
> +        # get list of modules
> +        objs = []
> +        for walkroot, dirs, files in os.walk(dvar + root):
> +            for file in files:
> +                relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
> +                if relpath:
> +                    objs.append(relpath)

The above block of code needs a comment. What exactly is it doing ?
My quick read of it makes me think it is checking our staged / rootfs / sysroot
for the modules, and then removing that part of the path so we'll presumably
have the path as they will appear on the target when it boots ?

We need explanations in the code about the logic, otherwise, it is going
to be unmaintainable.

> +
> +        for o in sorted(objs):
> +            import re, stat
> +            if False:
> +                m = re.match(file_regex, o)
> +            else:
> +                m = re.match(file_regex, os.path.basename(o))
> +
> +            if not m:
> +                continue

Why isn't this test just done when walking the module files ? I don't
see the point in walking once, and not testing to see if they are
modules during that walk.

> +
> +            file = os.path.join(dvar + root, o)
> +            mode = os.lstat(file).st_mode
> +            if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or (allow_dirs and stat.S_ISDIR(mode))):
> +                continue
> +
> +            on = legitimize_package_name(m.group(1))
> +            pkg = output_pattern % on
> +
> +            basename = m.group(1)
> +            handle_conf_files(d, basename, pkg)
> +
> +    def frob_metadata(file, pkg, pattern, format, basename):
> +        vals = extract_modinfo(file)
> +        dvar = d.getVar('PKGD')
> +
> +        handle_conf_files(d, basename, pkg)
> +
>          if "description" in vals:
>              old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
>              d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
> @@ -167,19 +220,19 @@ python split_kernel_module_packages () {
>      postinst = d.getVar('pkg_postinst:modules')
>      postrm = d.getVar('pkg_postrm:modules')
>  
> +    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
> +    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
> +    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
> +    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
> +
>      if splitmods != '1':
>          d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
>              (d.getVar('modulesloaddir'), d.getVar('modprobedir'), d.getVar("nonarch_base_libdir")))
>          d.appendVar('pkg_postinst:%s' % metapkg, postinst)
> -        d.prependVar('pkg_postrm:%s' % metapkg, postrm);
> +        d.prependVar('pkg_postrm:%s' % metapkg, postrm)

Why did you remove the semicolon above ? I admit it is strange to see
it in the python routine, but it doesn't look related to your changes.

> +        add_conf_files(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern)

I think this would be better called "create_conf_files" or "generate_conf_files".

It was the semicolon and the name of the routine that had me asking about the
routine being called from do_split_packages(). But I see that it in fact is
not called during package splitting, but only called once (when splitting is
not enabled) .. amd I ready that correctly now ?

Bruce

>          return
>  
> -    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
> -
> -    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
> -    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
> -    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
> -
>      modules = do_split_packages(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern, description='%s kernel module', postinst=postinst, postrm=postrm, recursive=True, hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name, kernel_version))
>      if modules:
>          d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
> -- 
> 2.43.0
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#212994): https://lists.openembedded.org/g/openembedded-core/message/212994
> Mute This Topic: https://lists.openembedded.org/mt/111728665/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

Re: [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

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

Hi Bruce, Thank you for your constructive feedback. Please find my inline replies.
FYI, I had submitted this v2 change before your comment on the previous version, where you suggested to wait :| Thank you for considering this.

On Fri, Mar 21, 2025 at 12:52 AM, Bruce Ashfield wrote:

> 
> In message: [OE-core] [PATCH 1/1] kernel-module-split: fix conf file
> generation when KERNEL_SPLIT_MODULES=0
> on 16/03/2025 Dixit Parmar via lists.openembedded.org wrote:
> 
> 
>> When KERNEL_SPLIT_MODULES=0 modprobe and autoload conf files are not
>> getting
>> generated for the kernel modules.
>> 
>> Separated out conf file handling mechanism as handle_conf_files() function
>> 
>> from hook frob_metadata() function. handle_conf_files() gets re-used from
>> the
>> existing hook function and add_conf_files function which got introduced
>> for
>> splitmods=0 flow in this patch.
> 
> Trevor's advice on the commit message was excellent, I won't repeat
> it here. I'll just simply add that the commit message should give us
> all the information we need about the issue (and what its symptoms
> are) and how this fixes it (not explaining the code, but describing
> the strategy)

Understood. I will update the commit message with all these details.

> 
> 
>> [YOCTO #15145]
>> 
>> Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
>> Cc: George Thopas <george.thopas@gmail.com>
>> --
>> Can you show the conf files for the same kernel configuration
>> with and without the kernel_split_modules enabled ? That way
>> we know there's no change in existing behaviour.
>> 
>>> I have confirmed that in my testing. Can you suggest how I can
>> 
>> share that information here?
> 
> A good way would simply to put it in the commit message that you've
> built and booted with both modes and that the generated module
> conf files are the same in both cases. Showing a diff command
> that you ran, or how you determined they were the same would be enough
> as well.

Agreed.

> 
> 
>> We also should make a test for this in the OE selftests. We
>> are adding conditional code paths, so they should be tested
>> to ensure no regressions in either in the future.
>> 
>>> Never done that before. May be I can do it given some direction
>> 
>> as separate patch.
> 
> Any selftests should be in a separate patch, but they do need
> to come along with any new verions of this. Saying that "it will
> be send later" won't get the patches merged. Since everyone gets
> busy and these sorts of things tend to not happen.
> 
> If you search on the mailing list(s) you'll find examples of
> other self tests being submitted. Just follow what they are
> doing. I'd suggest that a self test needs to run both modes
> and do a check that the generated .conf files are correct
> (basically what you'd be showing in the commit message). Even
> better if a boot and showing that they are (auto)loaded.
> 
> That's the only way we can be sure this won't break in the
> future and be comfortable with the complexty being added.

Agree. I will look at the self-test implementations and develop the test for this too.

> 
> 
>> I'm curious about the above line. It is unclear to me why we'd
>> only have this postinst be relevant if none was previously set.
>> 
>>> Reverted.
>> 
>> Is there really a scenario where the directory won't exist ? Isn't
>> this just running in our own install phase ? So all prerequisites
>> and directories should be in place.
>> 
>>> Ideally no, we kept it for safer side, I have added log warning.
>> 
>> 
> 
> As Trevor mentioned in his reply, it is best to configure your
> mail client to reply inline. I had made that comment near the
> line of code in question, but with the time between my review,
> your reply and now this reply .. the context of what was being
> discussed is very difficult to remember.
> 
> If that directory doesn't exist, just error out and do not
> continue. This isn't something recoverable, and the build should
> be stopped.

Okay. Will handle it that way.

> 
> 
>> The walking and sorting seems quite heavy. Isn't this called from
>> do_split_packages indirectly ?
>> Do we really need to walk and gather the information ? Is this mainly for
>> the case of no-split
>> on the kernel modules ? If that is the case, isn't there a way to short
>> circuit the processing
>> on the split-package case ?
>> 
>>> Litterally I could not think of anything else here and not sure of there
>> 
>> are any short-circuit options. I have limited knowledge in this. I am open
>> to suggestions
>> if this is not the best solution at the moment.
> 
> Describe the approach in your commit message, this is the type of
> information that we need. To be sure that it isn't adding a lot of
> time to the build, add some timing information with and without
> your changes. This is the type of information we are looking for
> in a review.

Understood.

> 
> See a few more comments below.
> 
> 
>> ---
>> .../kernel-module-split.bbclass | 81 +++++++++++++++----
>> 1 file changed, 67 insertions(+), 14 deletions(-)
>> 
>> diff --git a/meta/classes-recipe/kernel-module-split.bbclass
>> b/meta/classes-recipe/kernel-module-split.bbclass
>> index 9487365eb7..b7ee3a8f9e 100644
>> --- a/meta/classes-recipe/kernel-module-split.bbclass
>> +++ b/meta/classes-recipe/kernel-module-split.bbclass
>> @@ -86,11 +86,7 @@ python split_kernel_module_packages () {
>> vals[m.group(1)] = m.group(2)
>> return vals
>> 
>> - def frob_metadata(file, pkg, pattern, format, basename):
>> - vals = extract_modinfo(file)
>> -
>> - dvar = d.getVar('PKGD')
>> -
>> + def handle_conf_files(d, basename, pkg):
>> # If autoloading is requested, output ${modulesloaddir}/<name>.conf and
>> append
>> # appropriate modprobe commands to the postinst
>> autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
>> @@ -101,7 +97,7 @@ python split_kernel_module_packages () {
>> 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:
>> conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
>> - name = '%s%s' % (dvar, conf)
>> + name = '%s%s' % (d.getVar('PKGD'), conf)
>> os.makedirs(os.path.dirname(name), exist_ok=True)
>> with open(name, 'w') as f:
>> if autoload:
>> @@ -123,7 +119,7 @@ python split_kernel_module_packages () {
>> modconf = d.getVar('module_conf_%s' % basename)
>> if modconf and basename in modconflist:
>> conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
>> - name = '%s%s' % (dvar, conf)
>> + name = '%s%s' % (d.getVar('PKGD'), conf)
>> os.makedirs(os.path.dirname(name), exist_ok=True)
>> with open(name, 'w') as f:
>> f.write("%s\n" % modconf)
>> @@ -134,6 +130,63 @@ python split_kernel_module_packages () {
>> elif modconf:
>> bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF
>> since module_conf_%s is set" % (basename, basename))
>> 
>> + def add_conf_files(d, root, file_regex, output_pattern):
>> + """
>> + Arguments:
>> + root -- the path in which to search
>> + file_regex -- regular expression to match searched files. Use
>> + parentheses () to mark the part of this expression
>> + that should be used to derive the module name (to be
>> + substituted where %s is used in other function
>> + arguments as noted below)
>> + output_pattern -- pattern to use for the package names. Must include %s.
>> 
>> + """
>> +
>> + dvar = d.getVar('PKGD')
>> + root = d.expand(root)
> 
> root is a passed parameter, you shouldn't need to call d.expand()
> on it. Are you expecting there to be something that can be expanded ?

> 
> 
>> + output_pattern = d.expand(output_pattern)
> 
> Likewise with output_pattern

Correct. It's not needed as I don't expect it to be expanded. I missed it.

> 
> 
>> +
>> + # check if the root directory doesn't exist for safe side, don't error
>> out later but silently do
>> + # no splitting.
>> + if not os.path.exists(dvar + root):
>> + bb.warn("kernel module root directory path does not exist")
>> + return []
> 
> as I mentioned above, I see this as a fatal error. Something has gone
> terribly wrong if this doesn't exist and we should error and exit.
> 
> 
>> +
>> + # get list of modules
>> + objs = []
>> + for walkroot, dirs, files in os.walk(dvar + root):
>> + for file in files:
>> + relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
>> 
>> + if relpath:
>> + objs.append(relpath)
> 
> The above block of code needs a comment. What exactly is it doing ?
> My quick read of it makes me think it is checking our staged / rootfs /
> sysroot
> for the modules, and then removing that part of the path so we'll
> presumably
> have the path as they will appear on the target when it boots ?
> 
> We need explanations in the code about the logic, otherwise, it is going
> to be unmaintainable.

Will add a comment.

> 
> 
>> +
>> + for o in sorted(objs):
>> + import re, stat
>> + if False:
>> + m = re.match(file_regex, o)
>> + else:
>> + m = re.match(file_regex, os.path.basename(o))
>> +
>> + if not m:
>> + continue
> 
> Why isn't this test just done when walking the module files ? I don't
> see the point in walking once, and not testing to see if they are
> modules during that walk.

Probably, I had planned to optimize it later and then missed it. will revise this.

> 
> 
>> +
>> + file = os.path.join(dvar + root, o)
>> + mode = os.lstat(file).st_mode
>> + if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or
>> (allow_dirs and stat.S_ISDIR(mode))):
>> + continue
>> +
>> + on = legitimize_package_name(m.group(1))
>> + pkg = output_pattern % on
>> +
>> + basename = m.group(1)
>> + handle_conf_files(d, basename, pkg)
>> +
>> + def frob_metadata(file, pkg, pattern, format, basename):
>> + vals = extract_modinfo(file)
>> + dvar = d.getVar('PKGD')
>> +
>> + handle_conf_files(d, basename, pkg)
>> +
>> if "description" in vals:
>> old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
>> d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
>> @@ -167,19 +220,19 @@ python split_kernel_module_packages () {
>> postinst = d.getVar('pkg_postinst:modules')
>> postrm = d.getVar('pkg_postrm:modules')
>> 
>> + module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
>> + module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
>> + module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
>> + module_pattern = module_pattern_prefix + kernel_package_name +
>> '-module-%s' + module_pattern_suffix
>> +
>> if splitmods != '1':
>> d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
>> (d.getVar('modulesloaddir'), d.getVar('modprobedir'),
>> d.getVar("nonarch_base_libdir")))
>> d.appendVar('pkg_postinst:%s' % metapkg, postinst)
>> - d.prependVar('pkg_postrm:%s' % metapkg, postrm);
>> + d.prependVar('pkg_postrm:%s' % metapkg, postrm)
> 
> Why did you remove the semicolon above ? I admit it is strange to see
> it in the python routine, but it doesn't look related to your changes.

I thought the same way, I found it unusual in python and it did not have an impact so I thought to follow the standard and do the cleanup.

> 
> 
>> + add_conf_files(d, root='${nonarch_base_libdir}/modules',
>> file_regex=module_regex, output_pattern=module_pattern)
> 
> I think this would be better called "create_conf_files" or
> "generate_conf_files".
> 
> It was the semicolon and the name of the routine that had me asking about
> the
> routine being called from do_split_packages(). But I see that it in fact
> is
> not called during package splitting, but only called once (when splitting
> is
> not enabled) .. amd I ready that correctly now ?

Correct. Here there are two branches of execution based on whether the split is enabled or not. the execution reaches at do_split_package only in case when split is enabled, otherwise, for split disabled case we will directly call the conf file API.

Thanks,
Dixit

> 
> Bruce
> 
> 
>> return
>> 
>> - module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
>> -
>> - module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
>> - module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
>> - module_pattern = module_pattern_prefix + kernel_package_name +
>> '-module-%s' + module_pattern_suffix
>> -
>> modules = do_split_packages(d, root='${nonarch_base_libdir}/modules',
>> file_regex=module_regex, output_pattern=module_pattern, description='%s
>> kernel module', postinst=postinst, postrm=postrm, recursive=True,
>> hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name,
>> kernel_version))
>> if modules:
>> d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
>> --
>> 2.43.0
> 
>

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

[PATCH V2] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

When KERNEL_SPLIT_MODULES=0 modprobe and autoload conf files are not getting
generated for the kernel modules.

Separated out conf file handling mechanism as handle_conf_files() function
from hook frob_metadata() function. handle_conf_files() gets re-used from the
existing hook function and add_conf_files function which got introduced for
splitmods=0 flow in this patch.

[YOCTO #15145]

Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
Cc: George Thopas <george.thopas@gmail.com>
---
 .../kernel-module-split.bbclass               | 81 +++++++++++++++----
 1 file changed, 67 insertions(+), 14 deletions(-)

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 9487365eb7..b7ee3a8f9e 100644
--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -86,11 +86,7 @@ python split_kernel_module_packages () {
             vals[m.group(1)] = m.group(2)
         return vals
 
-    def frob_metadata(file, pkg, pattern, format, basename):
-        vals = extract_modinfo(file)
-
-        dvar = d.getVar('PKGD')
-
+    def handle_conf_files(d, basename, pkg):
         # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
         # appropriate modprobe commands to the postinst
         autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
@@ -101,7 +97,7 @@ python split_kernel_module_packages () {
             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:
             conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 if autoload:
@@ -123,7 +119,7 @@ python split_kernel_module_packages () {
         modconf = d.getVar('module_conf_%s' % basename)
         if modconf and basename in modconflist:
             conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 f.write("%s\n" % modconf)
@@ -134,6 +130,63 @@ python split_kernel_module_packages () {
         elif modconf:
             bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
 
+    def add_conf_files(d, root, file_regex, output_pattern):
+        """
+        Arguments:
+        root           -- the path in which to search
+        file_regex     -- regular expression to match searched files. Use
+                          parentheses () to mark the part of this expression
+                          that should be used to derive the module name (to be
+                          substituted where %s is used in other function
+                          arguments as noted below)
+        output_pattern -- pattern to use for the package names. Must include %s.
+        """
+
+        dvar = d.getVar('PKGD')
+        root = d.expand(root)
+        output_pattern = d.expand(output_pattern)
+
+        # check if the root directory doesn't exist for safe side, don't error out later but silently do
+        # no splitting.
+        if not os.path.exists(dvar + root):
+            bb.warn("kernel module root directory path does not exist")
+            return []
+
+        # get list of modules
+        objs = []
+        for walkroot, dirs, files in os.walk(dvar + root):
+            for file in files:
+                relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
+                if relpath:
+                    objs.append(relpath)
+
+        for o in sorted(objs):
+            import re, stat
+            if False:
+                m = re.match(file_regex, o)
+            else:
+                m = re.match(file_regex, os.path.basename(o))
+
+            if not m:
+                continue
+
+            file = os.path.join(dvar + root, o)
+            mode = os.lstat(file).st_mode
+            if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or (allow_dirs and stat.S_ISDIR(mode))):
+                continue
+
+            on = legitimize_package_name(m.group(1))
+            pkg = output_pattern % on
+
+            basename = m.group(1)
+            handle_conf_files(d, basename, pkg)
+
+    def frob_metadata(file, pkg, pattern, format, basename):
+        vals = extract_modinfo(file)
+        dvar = d.getVar('PKGD')
+
+        handle_conf_files(d, basename, pkg)
+
         if "description" in vals:
             old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
             d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
@@ -167,19 +220,19 @@ python split_kernel_module_packages () {
     postinst = d.getVar('pkg_postinst:modules')
     postrm = d.getVar('pkg_postrm:modules')
 
+    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
+    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
+    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
+    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
+
     if splitmods != '1':
         d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
             (d.getVar('modulesloaddir'), d.getVar('modprobedir'), d.getVar("nonarch_base_libdir")))
         d.appendVar('pkg_postinst:%s' % metapkg, postinst)
-        d.prependVar('pkg_postrm:%s' % metapkg, postrm);
+        d.prependVar('pkg_postrm:%s' % metapkg, postrm)
+        add_conf_files(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern)
         return
 
-    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
-
-    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
-    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
-    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
-
     modules = do_split_packages(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern, description='%s kernel module', postinst=postinst, postrm=postrm, recursive=True, hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name, kernel_version))
     if modules:
         d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
-- 
2.43.0

[PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

When KERNEL_SPLIT_MODULES=0 modprobe and autoload conf files are not getting
generated for the kernel modules.

Separated out conf file handling mechanism as handle_conf_files() function
from hook frob_metadata() function. handle_conf_files() gets re-used from the
existing hook function and add_conf_files function which got introduced for
splitmods=0 flow in this patch.

[YOCTO #15145]

Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
Cc: George Thopas <george.thopas@gmail.com>
---
 .../kernel-module-split.bbclass               | 81 +++++++++++++++----
 1 file changed, 67 insertions(+), 14 deletions(-)

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 9487365eb7..b7ee3a8f9e 100644
--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -86,11 +86,7 @@ python split_kernel_module_packages () {
             vals[m.group(1)] = m.group(2)
         return vals
 
-    def frob_metadata(file, pkg, pattern, format, basename):
-        vals = extract_modinfo(file)
-
-        dvar = d.getVar('PKGD')
-
+    def handle_conf_files(d, basename, pkg):
         # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
         # appropriate modprobe commands to the postinst
         autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
@@ -101,7 +97,7 @@ python split_kernel_module_packages () {
             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:
             conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 if autoload:
@@ -123,7 +119,7 @@ python split_kernel_module_packages () {
         modconf = d.getVar('module_conf_%s' % basename)
         if modconf and basename in modconflist:
             conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 f.write("%s\n" % modconf)
@@ -134,6 +130,63 @@ python split_kernel_module_packages () {
         elif modconf:
             bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
 
+    def add_conf_files(d, root, file_regex, output_pattern):
+        """
+        Arguments:
+        root           -- the path in which to search
+        file_regex     -- regular expression to match searched files. Use
+                          parentheses () to mark the part of this expression
+                          that should be used to derive the module name (to be
+                          substituted where %s is used in other function
+                          arguments as noted below)
+        output_pattern -- pattern to use for the package names. Must include %s.
+        """
+
+        dvar = d.getVar('PKGD')
+        root = d.expand(root)
+        output_pattern = d.expand(output_pattern)
+
+        # check if the root directory doesn't exist for safe side, don't error out later but silently do
+        # no splitting.
+        if not os.path.exists(dvar + root):
+            bb.warn("kernel module root directory path does not exist")
+            return []
+
+        # get list of modules
+        objs = []
+        for walkroot, dirs, files in os.walk(dvar + root):
+            for file in files:
+                relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
+                if relpath:
+                    objs.append(relpath)
+
+        for o in sorted(objs):
+            import re, stat
+            if False:
+                m = re.match(file_regex, o)
+            else:
+                m = re.match(file_regex, os.path.basename(o))
+
+            if not m:
+                continue
+
+            file = os.path.join(dvar + root, o)
+            mode = os.lstat(file).st_mode
+            if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or (allow_dirs and stat.S_ISDIR(mode))):
+                continue
+
+            on = legitimize_package_name(m.group(1))
+            pkg = output_pattern % on
+
+            basename = m.group(1)
+            handle_conf_files(d, basename, pkg)
+
+    def frob_metadata(file, pkg, pattern, format, basename):
+        vals = extract_modinfo(file)
+        dvar = d.getVar('PKGD')
+
+        handle_conf_files(d, basename, pkg)
+
         if "description" in vals:
             old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
             d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
@@ -167,19 +220,19 @@ python split_kernel_module_packages () {
     postinst = d.getVar('pkg_postinst:modules')
     postrm = d.getVar('pkg_postrm:modules')
 
+    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
+    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
+    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
+    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
+
     if splitmods != '1':
         d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
             (d.getVar('modulesloaddir'), d.getVar('modprobedir'), d.getVar("nonarch_base_libdir")))
         d.appendVar('pkg_postinst:%s' % metapkg, postinst)
-        d.prependVar('pkg_postrm:%s' % metapkg, postrm);
+        d.prependVar('pkg_postrm:%s' % metapkg, postrm)
+        add_conf_files(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern)
         return
 
-    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
-
-    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
-    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
-    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
-
     modules = do_split_packages(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern, description='%s kernel module', postinst=postinst, postrm=postrm, recursive=True, hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name, kernel_version))
     if modules:
         d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
-- 
2.43.0

[PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

When KERNEL_SPLIT_MODULES=0 modprobe and autoload conf files are not getting
generated for the kernel modules.

Separated out conf file handling mechanism as handle_conf_files() function
from hook frob_metadata() function. handle_conf_files() gets re-used from the
existing hook function and add_conf_files function which got introduced for
splitmods=0 flow in this patch.

[YOCTO #15145]

Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
Cc: George Thopas <george.thopas@gmail.com>
---
 .../kernel-module-split.bbclass               | 81 +++++++++++++++----
 1 file changed, 67 insertions(+), 14 deletions(-)

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 9487365eb7..b7ee3a8f9e 100644
--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -86,11 +86,7 @@ python split_kernel_module_packages () {
             vals[m.group(1)] = m.group(2)
         return vals
 
-    def frob_metadata(file, pkg, pattern, format, basename):
-        vals = extract_modinfo(file)
-
-        dvar = d.getVar('PKGD')
-
+    def handle_conf_files(d, basename, pkg):
         # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
         # appropriate modprobe commands to the postinst
         autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
@@ -101,7 +97,7 @@ python split_kernel_module_packages () {
             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:
             conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 if autoload:
@@ -123,7 +119,7 @@ python split_kernel_module_packages () {
         modconf = d.getVar('module_conf_%s' % basename)
         if modconf and basename in modconflist:
             conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 f.write("%s\n" % modconf)
@@ -134,6 +130,63 @@ python split_kernel_module_packages () {
         elif modconf:
             bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
 
+    def add_conf_files(d, root, file_regex, output_pattern):
+        """
+        Arguments:
+        root           -- the path in which to search
+        file_regex     -- regular expression to match searched files. Use
+                          parentheses () to mark the part of this expression
+                          that should be used to derive the module name (to be
+                          substituted where %s is used in other function
+                          arguments as noted below)
+        output_pattern -- pattern to use for the package names. Must include %s.
+        """
+
+        dvar = d.getVar('PKGD')
+        root = d.expand(root)
+        output_pattern = d.expand(output_pattern)
+
+        # check if the root directory doesn't exist for safe side, don't error out later but silently do
+        # no splitting.
+        if not os.path.exists(dvar + root):
+            bb.warn("kernel module root directory path does not exist")
+            return []
+
+        # get list of modules
+        objs = []
+        for walkroot, dirs, files in os.walk(dvar + root):
+            for file in files:
+                relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
+                if relpath:
+                    objs.append(relpath)
+
+        for o in sorted(objs):
+            import re, stat
+            if False:
+                m = re.match(file_regex, o)
+            else:
+                m = re.match(file_regex, os.path.basename(o))
+
+            if not m:
+                continue
+
+            file = os.path.join(dvar + root, o)
+            mode = os.lstat(file).st_mode
+            if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or (allow_dirs and stat.S_ISDIR(mode))):
+                continue
+
+            on = legitimize_package_name(m.group(1))
+            pkg = output_pattern % on
+
+            basename = m.group(1)
+            handle_conf_files(d, basename, pkg)
+
+    def frob_metadata(file, pkg, pattern, format, basename):
+        vals = extract_modinfo(file)
+        dvar = d.getVar('PKGD')
+
+        handle_conf_files(d, basename, pkg)
+
         if "description" in vals:
             old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
             d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
@@ -167,19 +220,19 @@ python split_kernel_module_packages () {
     postinst = d.getVar('pkg_postinst:modules')
     postrm = d.getVar('pkg_postrm:modules')
 
+    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
+    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
+    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
+    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
+
     if splitmods != '1':
         d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
             (d.getVar('modulesloaddir'), d.getVar('modprobedir'), d.getVar("nonarch_base_libdir")))
         d.appendVar('pkg_postinst:%s' % metapkg, postinst)
-        d.prependVar('pkg_postrm:%s' % metapkg, postrm);
+        d.prependVar('pkg_postrm:%s' % metapkg, postrm)
+        add_conf_files(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern)
         return
 
-    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
-
-    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
-    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
-    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
-
     modules = do_split_packages(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern, description='%s kernel module', postinst=postinst, postrm=postrm, recursive=True, hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name, kernel_version))
     if modules:
         d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
-- 
2.43.0

[PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

When KERNEL_SPLIT_MODULES=0 modprobe and autoload conf files are not getting
generated for the kernel modules.

Separated out conf file handling mechanism as handle_conf_files() function
from hook frob_metadata() function. handle_conf_files() gets re-used from the
existing hook function and add_conf_files function which got introduced for
splitmods=0 flow in this patch.

[YOCTO #15145]

Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
Cc: George Thopas <george.thopas@gmail.com>
--
Can you show the conf files for the same kernel configuration
with and without the kernel_split_modules enabled ? That way
we know there's no change in existing behaviour.
> I have confirmed that in my testing. Can you suggest how I can
share that information here?

We also should make a test for this in the OE selftests. We
are adding conditional code paths, so they should be tested
to ensure no regressions in either in the future.
> Never done that before. May be I can do it given some direction 
as separate patch.

I'm curious about the above line. It is unclear to me why we'd
only have this postinst be relevant if none was previously set.
> Reverted.

Is there really a scenario where the directory won't exist ? Isn't
this just running in our own install phase ? So all prerequisites
and directories should be in place.
> Ideally no, we kept it for safer side, I have added log warning.

The walking and sorting seems quite heavy.  Isn't this called from do_split_packages indirectly ?
Do we really need to walk and gather the information ? Is this mainly for the case of no-split
on the kernel modules ? If that is the case, isn't there a way to short circuit the processing
on the split-package case ?
> Litterally I could not think of anything else here and not sure of there
are any short-circuit options. I have limited knowledge in this. I am open to suggestions
if this is not the best solution at the moment.
---
 .../kernel-module-split.bbclass               | 81 +++++++++++++++----
 1 file changed, 67 insertions(+), 14 deletions(-)

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 9487365eb7..b7ee3a8f9e 100644
--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -86,11 +86,7 @@ python split_kernel_module_packages () {
             vals[m.group(1)] = m.group(2)
         return vals
 
-    def frob_metadata(file, pkg, pattern, format, basename):
-        vals = extract_modinfo(file)
-
-        dvar = d.getVar('PKGD')
-
+    def handle_conf_files(d, basename, pkg):
         # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
         # appropriate modprobe commands to the postinst
         autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
@@ -101,7 +97,7 @@ python split_kernel_module_packages () {
             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:
             conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 if autoload:
@@ -123,7 +119,7 @@ python split_kernel_module_packages () {
         modconf = d.getVar('module_conf_%s' % basename)
         if modconf and basename in modconflist:
             conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 f.write("%s\n" % modconf)
@@ -134,6 +130,63 @@ python split_kernel_module_packages () {
         elif modconf:
             bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
 
+    def add_conf_files(d, root, file_regex, output_pattern):
+        """
+        Arguments:
+        root           -- the path in which to search
+        file_regex     -- regular expression to match searched files. Use
+                          parentheses () to mark the part of this expression
+                          that should be used to derive the module name (to be
+                          substituted where %s is used in other function
+                          arguments as noted below)
+        output_pattern -- pattern to use for the package names. Must include %s.
+        """
+
+        dvar = d.getVar('PKGD')
+        root = d.expand(root)
+        output_pattern = d.expand(output_pattern)
+
+        # check if the root directory doesn't exist for safe side, don't error out later but silently do
+        # no splitting.
+        if not os.path.exists(dvar + root):
+            bb.warn("kernel module root directory path does not exist")
+            return []
+
+        # get list of modules
+        objs = []
+        for walkroot, dirs, files in os.walk(dvar + root):
+            for file in files:
+                relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
+                if relpath:
+                    objs.append(relpath)
+
+        for o in sorted(objs):
+            import re, stat
+            if False:
+                m = re.match(file_regex, o)
+            else:
+                m = re.match(file_regex, os.path.basename(o))
+
+            if not m:
+                continue
+
+            file = os.path.join(dvar + root, o)
+            mode = os.lstat(file).st_mode
+            if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or (allow_dirs and stat.S_ISDIR(mode))):
+                continue
+
+            on = legitimize_package_name(m.group(1))
+            pkg = output_pattern % on
+
+            basename = m.group(1)
+            handle_conf_files(d, basename, pkg)
+
+    def frob_metadata(file, pkg, pattern, format, basename):
+        vals = extract_modinfo(file)
+        dvar = d.getVar('PKGD')
+
+        handle_conf_files(d, basename, pkg)
+
         if "description" in vals:
             old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
             d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
@@ -167,19 +220,19 @@ python split_kernel_module_packages () {
     postinst = d.getVar('pkg_postinst:modules')
     postrm = d.getVar('pkg_postrm:modules')
 
+    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
+    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
+    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
+    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
+
     if splitmods != '1':
         d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
             (d.getVar('modulesloaddir'), d.getVar('modprobedir'), d.getVar("nonarch_base_libdir")))
         d.appendVar('pkg_postinst:%s' % metapkg, postinst)
-        d.prependVar('pkg_postrm:%s' % metapkg, postrm);
+        d.prependVar('pkg_postrm:%s' % metapkg, postrm)
+        add_conf_files(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern)
         return
 
-    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
-
-    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
-    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
-    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
-
     modules = do_split_packages(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern, description='%s kernel module', postinst=postinst, postrm=postrm, recursive=True, hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name, kernel_version))
     if modules:
         d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
-- 
2.43.0

[PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

KERNEL_MODULE_AUTOLOAD defines the list of the kernel modules to be autoloaded
on boot. kernel-module-split.bbclass generates the required modules.load.d and
conf files for each kernel module. This conf files inturn read by system service
to perform module loading and configuration. When a kernel module is added to
KERNEL_MODULE_AUTOLOAD the conf files must be generated in all cases.
When KERNEL_SPLIT_MODULES=0 modprobe and autoload conf files are not
getting generated for the kernel modules.
To fix that enhanced the class implementation by separating out conf
file handling mechanism in two functions, generate_conf_files() and
frob_metadata(). generate_conf_files() handles no-split case where as
frob_metadata() keeps handling the existing case for spliting the modules.
Splitted common handling/generation of conf files stuff in to handle_conf_files()
function which gets invoked by both frob_metadata() and generate_conf_files()
on top of the scenario specific handling done in respective functions.
This implementation covers generation of the conf files for in-tree kernel
modules as well as standalone kernel module built as seperate package/recipe.

[YOCTO #15145]

Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>

---
Previous version of the patch: https://lists.openembedded.org/g/openembedded-core/topic/111815220
oe-selftest patch: https://lists.openembedded.org/g/openembedded-core/message/218013

Validated these changes by the oe-selftest:
1. With the existing implementation the kernel_module_split.KernelSplitAutoloadTest.test_split_modules_disabled
test case fails because the autoload/modconf .conf files are not getting generated for each modules.
2. With this fix both enabled & disabled testcases are passed. Below are test case running results.

$ oe-selftest -r kernel_module_split.KernelSplitAutoloadTest.test_split_modules_disabled
2025-05-29 16:17:29,377 - oe-selftest - INFO - Adding layer libraries:
2025-05-29 16:17:29,377 - oe-selftest - INFO -  /data/home/diparmar/workspace/test/yocto-contri/poky/meta/lib
2025-05-29 16:17:29,377 - oe-selftest - INFO -  /data/home/diparmar/workspace/test/yocto-contri/poky/meta-yocto-bsp/lib
2025-05-29 16:17:29,377 - oe-selftest - INFO -  /data/home/diparmar/workspace/test/yocto-contri/poky/meta-selftest/lib
2025-05-29 16:17:29,378 - oe-selftest - INFO - Checking base configuration is valid/parsable
NOTE: Starting bitbake server...
2025-05-29 16:17:30,926 - oe-selftest - INFO - Adding: "include selftest.inc" in /data/home/diparmar/workspace/test/yocto-contri/poky/build-st/conf/local.conf
2025-05-29 16:17:30,927 - oe-selftest - INFO - Adding: "include bblayers.inc" in bblayers.conf
2025-05-29 16:17:30,927 - oe-selftest - INFO - test_split_modules_disabled (kernel_module_split.KernelSplitAutoloadTest)
2025-05-29 16:19:51,512 - oe-selftest - INFO -  ... ok
2025-05-29 16:20:00,367 - oe-selftest - INFO - ----------------------------------------------------------------------
2025-05-29 16:20:00,368 - oe-selftest - INFO - Ran 1 test in 149.890s
2025-05-29 16:20:00,368 - oe-selftest - INFO - OK
2025-05-29 16:20:09,019 - oe-selftest - INFO - RESULTS:
2025-05-29 16:20:09,019 - oe-selftest - INFO - RESULTS - kernel_module_split.KernelSplitAutoloadTest.test_split_modules_disabled: PASSED (140.59s)
2025-05-29 16:20:09,036 - oe-selftest - INFO - SUMMARY:
2025-05-29 16:20:09,037 - oe-selftest - INFO - oe-selftest () - Ran 1 test in 149.891s
2025-05-29 16:20:09,037 - oe-selftest - INFO - oe-selftest - OK - All required tests passed (successes=1, skipped=0, failures=0, errors=0)

===

$ oe-selftest -r kernel_module_split.KernelSplitAutoloadTest.test_split_modules_enabled
2025-05-29 16:20:53,538 - oe-selftest - INFO - Adding layer libraries:
2025-05-29 16:20:53,538 - oe-selftest - INFO -  /data/home/diparmar/workspace/test/yocto-contri/poky/meta/lib
2025-05-29 16:20:53,538 - oe-selftest - INFO -  /data/home/diparmar/workspace/test/yocto-contri/poky/meta-yocto-bsp/lib
2025-05-29 16:20:53,538 - oe-selftest - INFO -  /data/home/diparmar/workspace/test/yocto-contri/poky/meta-selftest/lib
2025-05-29 16:20:53,540 - oe-selftest - INFO - Checking base configuration is valid/parsable
NOTE: Starting bitbake server...
2025-05-29 16:20:55,000 - oe-selftest - INFO - Adding: "include selftest.inc" in /data/home/diparmar/workspace/test/yocto-contri/poky/build-st/conf/local.conf
2025-05-29 16:20:55,001 - oe-selftest - INFO - Adding: "include bblayers.inc" in bblayers.conf
2025-05-29 16:20:55,001 - oe-selftest - INFO - test_split_modules_enabled (kernel_module_split.KernelSplitAutoloadTest)
2025-05-29 16:23:08,930 - oe-selftest - INFO -  ... ok
2025-05-29 16:23:19,735 - oe-selftest - INFO - ----------------------------------------------------------------------
2025-05-29 16:23:19,735 - oe-selftest - INFO - Ran 1 test in 145.057s
2025-05-29 16:23:19,736 - oe-selftest - INFO - OK
2025-05-29 16:23:28,312 - oe-selftest - INFO - RESULTS:
2025-05-29 16:23:28,313 - oe-selftest - INFO - RESULTS - kernel_module_split.KernelSplitAutoloadTest.test_split_modules_enabled: PASSED (133.93s)
2025-05-29 16:23:28,321 - oe-selftest - INFO - SUMMARY:
2025-05-29 16:23:28,322 - oe-selftest - INFO - oe-selftest () - Ran 1 test in 145.058s
2025-05-29 16:23:28,322 - oe-selftest - INFO - oe-selftest - OK - All required tests passed (successes=1, skipped=0, failures=0, errors=0)
---
 .../kernel-module-split.bbclass               | 74 +++++++++++++++----
 1 file changed, 59 insertions(+), 15 deletions(-)

diff --git a/meta/classes-recipe/kernel-module-split.bbclass b/meta/classes-recipe/kernel-module-split.bbclass
index 9487365eb7..843f301f99 100644
--- a/meta/classes-recipe/kernel-module-split.bbclass
+++ b/meta/classes-recipe/kernel-module-split.bbclass
@@ -86,11 +86,7 @@ python split_kernel_module_packages () {
             vals[m.group(1)] = m.group(2)
         return vals
 
-    def frob_metadata(file, pkg, pattern, format, basename):
-        vals = extract_modinfo(file)
-
-        dvar = d.getVar('PKGD')
-
+    def handle_conf_files(d, basename, pkg):
         # If autoloading is requested, output ${modulesloaddir}/<name>.conf and append
         # appropriate modprobe commands to the postinst
         autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD") or "").split()
@@ -101,7 +97,7 @@ python split_kernel_module_packages () {
             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:
             conf = '%s/%s.conf' % (d.getVar('modulesloaddir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 if autoload:
@@ -114,7 +110,7 @@ python split_kernel_module_packages () {
             d.appendVar('CONFFILES:%s' % pkg, conf2append)
             postinst = d.getVar('pkg_postinst:%s' % pkg)
             if not postinst:
-                bb.fatal("pkg_postinst:%s not defined" % pkg)
+                postinst = d.getVar('pkg_postinst:modules')
             postinst += d.getVar('autoload_postinst_fragment') % (autoload or basename)
             d.setVar('pkg_postinst:%s' % pkg, postinst)
 
@@ -123,7 +119,7 @@ python split_kernel_module_packages () {
         modconf = d.getVar('module_conf_%s' % basename)
         if modconf and basename in modconflist:
             conf = '%s/%s.conf' % (d.getVar('modprobedir'), basename)
-            name = '%s%s' % (dvar, conf)
+            name = '%s%s' % (d.getVar('PKGD'), conf)
             os.makedirs(os.path.dirname(name), exist_ok=True)
             with open(name, 'w') as f:
                 f.write("%s\n" % modconf)
@@ -134,6 +130,54 @@ python split_kernel_module_packages () {
         elif modconf:
             bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
 
+    def generate_conf_files(d, root, file_regex, output_pattern):
+        """
+        Arguments:
+        root           -- the path in which to search. Contains system lib path
+                          so needs expansion.
+        file_regex     -- regular expression to match searched files. Use
+                          parentheses () to mark the part of this expression
+                          that should be used to derive the module name (to be
+                          substituted where %s is used in other function
+                          arguments as noted below)
+        output_pattern -- pattern to use for the package names. Must include %s.
+        """
+        import re, stat
+
+        dvar = d.getVar('PKGD')
+        root = d.expand(root)
+
+        # if the root directory doesn't exist, it's fatal - exit from the current execution.
+        if not os.path.exists(dvar + root):
+            bb.fatal("kernel module root directory path does not exist")
+
+        # walk through kernel module directory. for each entry in the directory, check if it
+        # matches the desired regex pattern and file type. if it fullfills, process it to generate
+        # it's conf file based on its package name.
+        for walkroot, dirs, files in os.walk(dvar + root):
+            for file in files:
+                relpath = os.path.join(walkroot, file).replace(dvar + root + '/', '', 1)
+                if not relpath:
+                    continue
+                m = re.match(file_regex, os.path.basename(relpath))
+                if not m:
+                    continue
+                file_f = os.path.join(dvar + root, relpath)
+                mode = os.lstat(file_f).st_mode
+                if not (stat.S_ISREG(mode) or (allow_links and stat.S_ISLNK(mode)) or (allow_dirs and stat.S_ISDIR(mode))):
+                    continue
+
+                basename = m.group(1)
+                on = legitimize_package_name(basename)
+                pkg = output_pattern % on
+                handle_conf_files(d, basename, pkg)
+
+    def frob_metadata(file, pkg, pattern, format, basename):
+        vals = extract_modinfo(file)
+        dvar = d.getVar('PKGD')
+
+        handle_conf_files(d, basename, pkg)
+
         if "description" in vals:
             old_desc = d.getVar('DESCRIPTION:' + pkg) or ""
             d.setVar('DESCRIPTION:' + pkg, old_desc + "; " + vals["description"])
@@ -167,19 +211,19 @@ python split_kernel_module_packages () {
     postinst = d.getVar('pkg_postinst:modules')
     postrm = d.getVar('pkg_postrm:modules')
 
+    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
+    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
+    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
+    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
+
     if splitmods != '1':
         d.appendVar('FILES:' + metapkg, '%s %s %s/modules' %
             (d.getVar('modulesloaddir'), d.getVar('modprobedir'), d.getVar("nonarch_base_libdir")))
         d.appendVar('pkg_postinst:%s' % metapkg, postinst)
-        d.prependVar('pkg_postrm:%s' % metapkg, postrm);
+        d.prependVar('pkg_postrm:%s' % metapkg, postrm)
+        generate_conf_files(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern)
         return
 
-    module_regex = r'^(.*)\.k?o(?:\.(gz|xz|zst))?$'
-
-    module_pattern_prefix = d.getVar('KERNEL_MODULE_PACKAGE_PREFIX')
-    module_pattern_suffix = d.getVar('KERNEL_MODULE_PACKAGE_SUFFIX')
-    module_pattern = module_pattern_prefix + kernel_package_name + '-module-%s' + module_pattern_suffix
-
     modules = do_split_packages(d, root='${nonarch_base_libdir}/modules', file_regex=module_regex, output_pattern=module_pattern, description='%s kernel module', postinst=postinst, postrm=postrm, recursive=True, hook=frob_metadata, extra_depends='%s-%s' % (kernel_package_name, kernel_version))
     if modules:
         d.appendVar('RDEPENDS:' + metapkg, ' '+' '.join(modules))
-- 
2.43.0

Re: [OE-core] [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Mathieu Dubois-Briand]

On Thu Jun 5, 2025 at 3:12 PM CEST, Dixit Parmar via lists.openembedded.org wrote:
> KERNEL_MODULE_AUTOLOAD defines the list of the kernel modules to be autoloaded
> on boot. kernel-module-split.bbclass generates the required modules.load.d and
> conf files for each kernel module. This conf files inturn read by system service
> to perform module loading and configuration. When a kernel module is added to
> KERNEL_MODULE_AUTOLOAD the conf files must be generated in all cases.
> When KERNEL_SPLIT_MODULES=0 modprobe and autoload conf files are not
> getting generated for the kernel modules.
> To fix that enhanced the class implementation by separating out conf
> file handling mechanism in two functions, generate_conf_files() and
> frob_metadata(). generate_conf_files() handles no-split case where as
> frob_metadata() keeps handling the existing case for spliting the modules.
> Splitted common handling/generation of conf files stuff in to handle_conf_files()
> function which gets invoked by both frob_metadata() and generate_conf_files()
> on top of the scenario specific handling done in respective functions.
> This implementation covers generation of the conf files for in-tree kernel
> modules as well as standalone kernel module built as seperate package/recipe.
>
> [YOCTO #15145]
>
> Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
>
> ---

Hi Dixit,

I have some conflicts while trying to apply this on master branch. Do
you confirm this is targeting master?

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0

[Dixit Parmar]

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

Hi Mathieu,
There seem to be a patch merged in-between. I will rebase this patch and send new version. Thanks for pointing out.

Thanks,
Dixit

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