All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [OE-core] [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0
  2025-03-16  8:21 ` [PATCH 1/1] " Dixit Parmar
@ 2025-03-20 19:22   ` Bruce Ashfield
  0 siblings, 0 replies; 5+ messages in thread
From: Bruce Ashfield @ 2025-03-20 19:22 UTC (permalink / raw)
  To: dixitparmar19; +Cc: openembedded-core, George Thopas

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0
@ 2025-06-05 13:12 Dixit Parmar
  2025-06-05 13:16 ` [OE-core] " Bruce Ashfield
  2025-06-06  8:02 ` Mathieu Dubois-Briand
  0 siblings, 2 replies; 5+ messages in thread
From: Dixit Parmar @ 2025-06-05 13:12 UTC (permalink / raw)
  To: openembedded-core; +Cc: 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



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [OE-core] [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0
  2025-06-05 13:12 [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0 Dixit Parmar
@ 2025-06-05 13:16 ` Bruce Ashfield
  2025-06-06  8:02 ` Mathieu Dubois-Briand
  1 sibling, 0 replies; 5+ messages in thread
From: Bruce Ashfield @ 2025-06-05 13:16 UTC (permalink / raw)
  To: dixitparmar19; +Cc: openembedded-core

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

On Thu, Jun 5, 2025 at 9:12 AM Dixit Parmar via lists.openembedded.org
<dixitparmar19=gmail.com@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>
>
> ---
> Previous version of the patch:
> https://lists.openembedded.org/g/openembedded-core/topic/111815220
> oe-selftest
> <https://lists.openembedded.org/g/openembedded-core/topic/111815220oe-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)
>

With the tests, this version looks good to me!

Bruce



> ---
>  .../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
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#218016):
> https://lists.openembedded.org/g/openembedded-core/message/218016
> Mute This Topic: https://lists.openembedded.org/mt/113484744/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: 16837 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [OE-core] [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0
  2025-06-05 13:12 [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0 Dixit Parmar
  2025-06-05 13:16 ` [OE-core] " Bruce Ashfield
@ 2025-06-06  8:02 ` Mathieu Dubois-Briand
  2025-06-07 10:28   ` Dixit Parmar
  1 sibling, 1 reply; 5+ messages in thread
From: Mathieu Dubois-Briand @ 2025-06-06  8:02 UTC (permalink / raw)
  To: dixitparmar19, openembedded-core

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0
  2025-06-06  8:02 ` Mathieu Dubois-Briand
@ 2025-06-07 10:28   ` Dixit Parmar
  0 siblings, 0 replies; 5+ messages in thread
From: Dixit Parmar @ 2025-06-07 10:28 UTC (permalink / raw)
  To: openembedded-core

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-06-07 10:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-05 13:12 [PATCH 1/1] kernel-module-split: fix conf file generation when KERNEL_SPLIT_MODULES=0 Dixit Parmar
2025-06-05 13:16 ` [OE-core] " Bruce Ashfield
2025-06-06  8:02 ` Mathieu Dubois-Briand
2025-06-07 10:28   ` Dixit Parmar
  -- strict thread matches above, loose matches on Subject: below --
2025-02-22  7:11 [PATCH] " Dixit Parmar
2025-03-16  8:21 ` [PATCH 1/1] " Dixit Parmar
2025-03-20 19:22   ` [OE-core] " Bruce Ashfield

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.