[PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

[PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

[Richard Purdie]

The current module_autoload_* and module_conf_* variables are error
both ugly and error prone. They aren't registered in the task checksums
so changes to them aren't reflected in the build. This turns out to
be near impossible to fix with the current variable format in any
sensible way :(.

This patch replace module_autoload with the list of variables in
KERNEL_MODULE_AUTOLOAD which is a much simpler and usable API. An
error is printed if an old style variable is encountered. It should
be simple to convert to this.

module_conf_* are harder to deal with since there is data associated
with it, it isn't simply a flag. We need a list of variables that are set
in order to be able to correctly handle the task checksum so we add
KERNEL_MODULE_PROBECONF for this purpose and error if the user hasn't
added a module to it when they should have.

[YOCTO #5786]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>

diff --git a/meta/classes/kernel-module-split.bbclass b/meta/classes/kernel-module-split.bbclass
index d43f743..e38a6f6 100644
--- a/meta/classes/kernel-module-split.bbclass
+++ b/meta/classes/kernel-module-split.bbclass
@@ -130,8 +130,11 @@ python split_kernel_module_packages () {
 
         # If autoloading is requested, output /etc/modules-load.d/<name>.conf and append
         # appropriate modprobe commands to the postinst
+        autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD", True) or "").split()
         autoload = d.getVar('module_autoload_%s' % basename, True)
         if autoload:
+            bb.error("KERNEL_MODULE_AUTOLOAD has replaced module_autoload_%s, please replace it!" % basename)
+        if basename in autoloadlist:
             name = '%s/etc/modules-load.d/%s.conf' % (dvar, basename)
             f = open(name, 'w')
             for m in autoload.split():
@@ -144,12 +147,15 @@ python split_kernel_module_packages () {
             d.setVar('pkg_postinst_%s' % pkg, postinst)
 
         # Write out any modconf fragment
+        modconflist = (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()
         modconf = d.getVar('module_conf_%s' % basename, True)
-        if modconf:
+        if modconf and basename in modconflist:
             name = '%s/etc/modprobe.d/%s.conf' % (dvar, basename)
             f = open(name, 'w')
             f.write("%s\n" % modconf)
             f.close()
+        elif modconf:
+            bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
 
         files = d.getVar('FILES_%s' % pkg, True)
         files = "%s /etc/modules-load.d/%s.conf /etc/modprobe.d/%s.conf" % (files, basename, basename)
@@ -185,3 +191,5 @@ python split_kernel_module_packages () {
         if len(os.listdir(dir)) == 0:
             os.rmdir(dir)
 }
+
+do_package[vardeps] += '${@" ".join(map(lambda s: "module_conf_" + s, (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()))}'

Re: [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

[Bruce Ashfield]

On Fri, Jun 13, 2014 at 11:46 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> The current module_autoload_* and module_conf_* variables are error
> both ugly and error prone. They aren't registered in the task checksums
> so changes to them aren't reflected in the build. This turns out to
> be near impossible to fix with the current variable format in any
> sensible way :(.
>
> This patch replace module_autoload with the list of variables in
> KERNEL_MODULE_AUTOLOAD which is a much simpler and usable API. An
> error is printed if an old style variable is encountered. It should
> be simple to convert to this.
>
> module_conf_* are harder to deal with since there is data associated
> with it, it isn't simply a flag. We need a list of variables that are set
> in order to be able to correctly handle the task checksum so we add
> KERNEL_MODULE_PROBECONF for this purpose and error if the user hasn't
> added a module to it when they should have.

Looks reasonable to me. In my experience, there aren't a lot of users of
the module_* variables, so the transition to the new names and semantics
shouldn't be a big issue.

Cheers,

Bruce

>
> [YOCTO #5786]
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>
> diff --git a/meta/classes/kernel-module-split.bbclass b/meta/classes/kernel-module-split.bbclass
> index d43f743..e38a6f6 100644
> --- a/meta/classes/kernel-module-split.bbclass
> +++ b/meta/classes/kernel-module-split.bbclass
> @@ -130,8 +130,11 @@ python split_kernel_module_packages () {
>
>          # If autoloading is requested, output /etc/modules-load.d/<name>.conf and append
>          # appropriate modprobe commands to the postinst
> +        autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD", True) or "").split()
>          autoload = d.getVar('module_autoload_%s' % basename, True)
>          if autoload:
> +            bb.error("KERNEL_MODULE_AUTOLOAD has replaced module_autoload_%s, please replace it!" % basename)
> +        if basename in autoloadlist:
>              name = '%s/etc/modules-load.d/%s.conf' % (dvar, basename)
>              f = open(name, 'w')
>              for m in autoload.split():
> @@ -144,12 +147,15 @@ python split_kernel_module_packages () {
>              d.setVar('pkg_postinst_%s' % pkg, postinst)
>
>          # Write out any modconf fragment
> +        modconflist = (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()
>          modconf = d.getVar('module_conf_%s' % basename, True)
> -        if modconf:
> +        if modconf and basename in modconflist:
>              name = '%s/etc/modprobe.d/%s.conf' % (dvar, basename)
>              f = open(name, 'w')
>              f.write("%s\n" % modconf)
>              f.close()
> +        elif modconf:
> +            bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
>
>          files = d.getVar('FILES_%s' % pkg, True)
>          files = "%s /etc/modules-load.d/%s.conf /etc/modprobe.d/%s.conf" % (files, basename, basename)
> @@ -185,3 +191,5 @@ python split_kernel_module_packages () {
>          if len(os.listdir(dir)) == 0:
>              os.rmdir(dir)
>  }
> +
> +do_package[vardeps] += '${@" ".join(map(lambda s: "module_conf_" + s, (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()))}'
>
>



-- 
"Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end"

Re: [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

[Denys Dmytriyenko]

On Fri, Jun 13, 2014 at 03:02:05PM -0400, Bruce Ashfield wrote:
> On Fri, Jun 13, 2014 at 11:46 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > The current module_autoload_* and module_conf_* variables are error
> > both ugly and error prone. They aren't registered in the task checksums
> > so changes to them aren't reflected in the build. This turns out to
> > be near impossible to fix with the current variable format in any
> > sensible way :(.
> >
> > This patch replace module_autoload with the list of variables in
> > KERNEL_MODULE_AUTOLOAD which is a much simpler and usable API. An
> > error is printed if an old style variable is encountered. It should
> > be simple to convert to this.
> >
> > module_conf_* are harder to deal with since there is data associated
> > with it, it isn't simply a flag. We need a list of variables that are set
> > in order to be able to correctly handle the task checksum so we add
> > KERNEL_MODULE_PROBECONF for this purpose and error if the user hasn't
> > added a module to it when they should have.
> 
> Looks reasonable to me. In my experience, there aren't a lot of users of
> the module_* variables, so the transition to the new names and semantics
> shouldn't be a big issue.

Right, only BSP/Distro maintainers should care... :)

BTW, how about a documentation patch? :)

-- 
Denys


> > [YOCTO #5786]
> >
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> >
> > diff --git a/meta/classes/kernel-module-split.bbclass b/meta/classes/kernel-module-split.bbclass
> > index d43f743..e38a6f6 100644
> > --- a/meta/classes/kernel-module-split.bbclass
> > +++ b/meta/classes/kernel-module-split.bbclass
> > @@ -130,8 +130,11 @@ python split_kernel_module_packages () {
> >
> >          # If autoloading is requested, output /etc/modules-load.d/<name>.conf and append
> >          # appropriate modprobe commands to the postinst
> > +        autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD", True) or "").split()
> >          autoload = d.getVar('module_autoload_%s' % basename, True)
> >          if autoload:
> > +            bb.error("KERNEL_MODULE_AUTOLOAD has replaced module_autoload_%s, please replace it!" % basename)
> > +        if basename in autoloadlist:
> >              name = '%s/etc/modules-load.d/%s.conf' % (dvar, basename)
> >              f = open(name, 'w')
> >              for m in autoload.split():
> > @@ -144,12 +147,15 @@ python split_kernel_module_packages () {
> >              d.setVar('pkg_postinst_%s' % pkg, postinst)
> >
> >          # Write out any modconf fragment
> > +        modconflist = (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()
> >          modconf = d.getVar('module_conf_%s' % basename, True)
> > -        if modconf:
> > +        if modconf and basename in modconflist:
> >              name = '%s/etc/modprobe.d/%s.conf' % (dvar, basename)
> >              f = open(name, 'w')
> >              f.write("%s\n" % modconf)
> >              f.close()
> > +        elif modconf:
> > +            bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
> >
> >          files = d.getVar('FILES_%s' % pkg, True)
> >          files = "%s /etc/modules-load.d/%s.conf /etc/modprobe.d/%s.conf" % (files, basename, basename)
> > @@ -185,3 +191,5 @@ python split_kernel_module_packages () {
> >          if len(os.listdir(dir)) == 0:
> >              os.rmdir(dir)
> >  }
> > +
> > +do_package[vardeps] += '${@" ".join(map(lambda s: "module_conf_" + s, (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()))}'
> >
> >
> 
> 
> 
> -- 
> "Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end"
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

[Hart, Darren]

On 6/13/14, 12:13, "Denys Dmytriyenko" <denis@denix.org> wrote:

>On Fri, Jun 13, 2014 at 03:02:05PM -0400, Bruce Ashfield wrote:
>> On Fri, Jun 13, 2014 at 11:46 AM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> > The current module_autoload_* and module_conf_* variables are error
>> > both ugly and error prone. They aren't registered in the task
>>checksums
>> > so changes to them aren't reflected in the build. This turns out to
>> > be near impossible to fix with the current variable format in any
>> > sensible way :(.
>> >
>> > This patch replace module_autoload with the list of variables in
>> > KERNEL_MODULE_AUTOLOAD which is a much simpler and usable API. An
>> > error is printed if an old style variable is encountered. It should
>> > be simple to convert to this.
>> >
>> > module_conf_* are harder to deal with since there is data associated
>> > with it, it isn't simply a flag. We need a list of variables that are
>>set
>> > in order to be able to correctly handle the task checksum so we add
>> > KERNEL_MODULE_PROBECONF for this purpose and error if the user hasn't
>> > added a module to it when they should have.
>> 
>> Looks reasonable to me. In my experience, there aren't a lot of users of
>> the module_* variables, so the transition to the new names and semantics
>> shouldn't be a big issue.
>
>Right, only BSP/Distro maintainers should care... :)
>
>BTW, how about a documentation patch? :)

Adding Scott R.

I was just looking into that. It appears the ref-manual.html is the place
to update. The glossary has a module_autoload definition, which I suppose
needs to be replaced with KERNEL_MODULE_AUTOLOAD, which  will have
slightly different semantics.

If I understand this correctly, the old model was:

	module_autoload_foo = "foo"
	module_autoload_bar = "bar"

Although the following line in the docs confuses me:

	module_autoload_<modname> = "modname1 modname2 modname3"


And now, if I interpreted the commit comment correctly, it should look
like:

	KERNEL_MODULE_AUTOLOAD = "foo"
	...
	KERNEL_MODULE_AUTOLOAD += "bar"

I'm not sure how KERNEL_MODULE_PROBECONF is involved, or what value it
brings beyond module_conf. From what I can tell, the changes now require:

	KERNEL_MODULE_PROBECONF = "foo"

	module_conf_foo = "options foo baz=1"

(/me notes the order of operations is non-obvious here "if modconf and
basename in modconflist")

Do I have this correct?

--
Darren

>
>-- 
>Denys
>
>
>> > [YOCTO #5786]
>> >
>> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>> >
>> > diff --git a/meta/classes/kernel-module-split.bbclass
>>b/meta/classes/kernel-module-split.bbclass
>> > index d43f743..e38a6f6 100644
>> > --- a/meta/classes/kernel-module-split.bbclass
>> > +++ b/meta/classes/kernel-module-split.bbclass
>> > @@ -130,8 +130,11 @@ python split_kernel_module_packages () {
>> >
>> >          # If autoloading is requested, output
>>/etc/modules-load.d/<name>.conf and append
>> >          # appropriate modprobe commands to the postinst
>> > +        autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD", True) or
>>"").split()
>> >          autoload = d.getVar('module_autoload_%s' % basename, True)
>> >          if autoload:
>> > +            bb.error("KERNEL_MODULE_AUTOLOAD has replaced
>>module_autoload_%s, please replace it!" % basename)
>> > +        if basename in autoloadlist:
>> >              name = '%s/etc/modules-load.d/%s.conf' % (dvar, basename)
>> >              f = open(name, 'w')
>> >              for m in autoload.split():
>> > @@ -144,12 +147,15 @@ python split_kernel_module_packages () {
>> >              d.setVar('pkg_postinst_%s' % pkg, postinst)
>> >
>> >          # Write out any modconf fragment
>> > +        modconflist = (d.getVar("KERNEL_MODULE_PROBECONF", True) or
>>"").split()
>> >          modconf = d.getVar('module_conf_%s' % basename, True)
>> > -        if modconf:
>> > +        if modconf and basename in modconflist:
>> >              name = '%s/etc/modprobe.d/%s.conf' % (dvar, basename)
>> >              f = open(name, 'w')
>> >              f.write("%s\n" % modconf)
>> >              f.close()
>> > +        elif modconf:
>> > +            bb.error("Please ensure module %s is listed in
>>KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename,
>>basename))
>> >
>> >          files = d.getVar('FILES_%s' % pkg, True)
>> >          files = "%s /etc/modules-load.d/%s.conf
>>/etc/modprobe.d/%s.conf" % (files, basename, basename)
>> > @@ -185,3 +191,5 @@ python split_kernel_module_packages () {
>> >          if len(os.listdir(dir)) == 0:
>> >              os.rmdir(dir)
>> >  }
>> > +
>> > +do_package[vardeps] += '${@" ".join(map(lambda s: "module_conf_" +
>>s, (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()))}'
>> >
>> >
>> 
>> 
>> 
>> -- 
>> "Thou shalt not follow the NULL pointer, for chaos and madness await
>> thee at its end"
>> -- 
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>


-- 
Darren Hart					Open Source Technology Center
darren.hart@intel.com				            Intel Corporation

Re: [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

[Richard Purdie]

On Tue, 2014-06-17 at 14:53 +0000, Hart, Darren wrote:
> Adding Scott R.

I do need to sort out a documentation update.

> I was just looking into that. It appears the ref-manual.html is the place
> to update. The glossary has a module_autoload definition, which I suppose
> needs to be replaced with KERNEL_MODULE_AUTOLOAD, which  will have
> slightly different semantics.
> 
> If I understand this correctly, the old model was:
> 
> 	module_autoload_foo = "foo"
> 	module_autoload_bar = "bar"
> 
> Although the following line in the docs confuses me:
> 
> 	module_autoload_<modname> = "modname1 modname2 modname3"

That is just wrong.

> 
> And now, if I interpreted the commit comment correctly, it should look
> like:
> 
> 	KERNEL_MODULE_AUTOLOAD = "foo"
> 	...
> 	KERNEL_MODULE_AUTOLOAD += "bar"

Correct.

> I'm not sure how KERNEL_MODULE_PROBECONF is involved, or what value it
> brings beyond module_conf. From what I can tell, the changes now require:
> 
> 	KERNEL_MODULE_PROBECONF = "foo"
> 
> 	module_conf_foo = "options foo baz=1"
> 
> (/me notes the order of operations is non-obvious here "if modconf and
> basename in modconflist")

For module_conf, the value is the build system can know which variables
were set and account for them in the task checksums. If it doesn't have
the list, we'd have to iterate the whole data store and that is a
*painfully* slow operation.

module_conf isn't commonly used so maintaining a list isn't too much of
a hardship IMO.

> Do I have this correct?

Yes.

CHeers,

Richard

Re: [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

[Denys Dmytriyenko]

Sorry for the delay...

On Tue, Jun 17, 2014 at 05:46:58PM +0100, Richard Purdie wrote:
> On Tue, 2014-06-17 at 14:53 +0000, Hart, Darren wrote:
> > Adding Scott R.
> 
> I do need to sort out a documentation update.
> 
> > I was just looking into that. It appears the ref-manual.html is the place
> > to update. The glossary has a module_autoload definition, which I suppose
> > needs to be replaced with KERNEL_MODULE_AUTOLOAD, which  will have
> > slightly different semantics.
> > 
> > If I understand this correctly, the old model was:
> > 
> > 	module_autoload_foo = "foo"
> > 	module_autoload_bar = "bar"
> > 
> > Although the following line in the docs confuses me:
> > 
> > 	module_autoload_<modname> = "modname1 modname2 modname3"
> 
> That is just wrong.

Yeah, I think I confused people here... During one of the discussions I tried 
to mention that standard /etc/modules-load.d/ can have a single file with 
multiple module entries in it (and the above line was given as an example). 
Unfortunately, kernel-module-split class couldn't handle that and required 
placing one module entry per file. So the old syntax would look like this:

module_autoload_<modname1> = "modname1"
module_autoload_<modname2> = "modname2"
etc.

Sorry for the confusion.


> > And now, if I interpreted the commit comment correctly, it should look
> > like:
> > 
> > 	KERNEL_MODULE_AUTOLOAD = "foo"
> > 	...
> > 	KERNEL_MODULE_AUTOLOAD += "bar"
> 
> Correct.
> 
> > I'm not sure how KERNEL_MODULE_PROBECONF is involved, or what value it
> > brings beyond module_conf. From what I can tell, the changes now require:
> > 
> > 	KERNEL_MODULE_PROBECONF = "foo"
> > 
> > 	module_conf_foo = "options foo baz=1"
> > 
> > (/me notes the order of operations is non-obvious here "if modconf and
> > basename in modconflist")
> 
> For module_conf, the value is the build system can know which variables
> were set and account for them in the task checksums. If it doesn't have
> the list, we'd have to iterate the whole data store and that is a
> *painfully* slow operation.
> 
> module_conf isn't commonly used so maintaining a list isn't too much of
> a hardship IMO.
> 
> > Do I have this correct?
> 
> Yes.
> 
> CHeers,
> 
> Richard
> 

Re: [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

[Martin Jansa]

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

On Mon, Jun 23, 2014 at 06:54:57PM -0400, Denys Dmytriyenko wrote:
> Sorry for the delay...
> 
> On Tue, Jun 17, 2014 at 05:46:58PM +0100, Richard Purdie wrote:
> > On Tue, 2014-06-17 at 14:53 +0000, Hart, Darren wrote:
> > > Adding Scott R.
> > 
> > I do need to sort out a documentation update.
> > 
> > > I was just looking into that. It appears the ref-manual.html is the place
> > > to update. The glossary has a module_autoload definition, which I suppose
> > > needs to be replaced with KERNEL_MODULE_AUTOLOAD, which  will have
> > > slightly different semantics.
> > > 
> > > If I understand this correctly, the old model was:
> > > 
> > > 	module_autoload_foo = "foo"
> > > 	module_autoload_bar = "bar"
> > > 
> > > Although the following line in the docs confuses me:
> > > 
> > > 	module_autoload_<modname> = "modname1 modname2 modname3"
> > 
> > That is just wrong.
> 
> Yeah, I think I confused people here... During one of the discussions I tried 
> to mention that standard /etc/modules-load.d/ can have a single file with 
> multiple module entries in it (and the above line was given as an example). 
> Unfortunately, kernel-module-split class couldn't handle that and required 
> placing one module entry per file. So the old syntax would look like this:
> 
> module_autoload_<modname1> = "modname1"
> module_autoload_<modname2> = "modname2"
> etc.
> 
> Sorry for the confusion.

Sometimes basename != module name and having multiple entries in one
/etc/modules-load.d/file is sometimes useful, see

http://lists.openembedded.org/pipermail/openembedded-core/2014-July/094645.html

which returns support for both features

> > > And now, if I interpreted the commit comment correctly, it should look
> > > like:
> > > 
> > > 	KERNEL_MODULE_AUTOLOAD = "foo"
> > > 	...
> > > 	KERNEL_MODULE_AUTOLOAD += "bar"
> > 
> > Correct.
> > 
> > > I'm not sure how KERNEL_MODULE_PROBECONF is involved, or what value it
> > > brings beyond module_conf. From what I can tell, the changes now require:
> > > 
> > > 	KERNEL_MODULE_PROBECONF = "foo"
> > > 
> > > 	module_conf_foo = "options foo baz=1"
> > > 
> > > (/me notes the order of operations is non-obvious here "if modconf and
> > > basename in modconflist")
> > 
> > For module_conf, the value is the build system can know which variables
> > were set and account for them in the task checksums. If it doesn't have
> > the list, we'd have to iterate the whole data store and that is a
> > *painfully* slow operation.
> > 
> > module_conf isn't commonly used so maintaining a list isn't too much of
> > a hardship IMO.
> > 
> > > Do I have this correct?
> > 
> > Yes.
> > 
> > CHeers,
> > 
> > Richard
> > 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF

[Martin Jansa]

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

On Fri, Jun 13, 2014 at 03:02:05PM -0400, Bruce Ashfield wrote:
> On Fri, Jun 13, 2014 at 11:46 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > The current module_autoload_* and module_conf_* variables are error
> > both ugly and error prone. They aren't registered in the task checksums

Looks good, there is extra "error" in first sentence.

> > so changes to them aren't reflected in the build. This turns out to
> > be near impossible to fix with the current variable format in any
> > sensible way :(.
> >
> > This patch replace module_autoload with the list of variables in
> > KERNEL_MODULE_AUTOLOAD which is a much simpler and usable API. An
> > error is printed if an old style variable is encountered. It should
> > be simple to convert to this.
> >
> > module_conf_* are harder to deal with since there is data associated
> > with it, it isn't simply a flag. We need a list of variables that are set
> > in order to be able to correctly handle the task checksum so we add
> > KERNEL_MODULE_PROBECONF for this purpose and error if the user hasn't
> > added a module to it when they should have.
> 
> Looks reasonable to me. In my experience, there aren't a lot of users of
> the module_* variables, so the transition to the new names and semantics
> shouldn't be a big issue.
> 
> Cheers,
> 
> Bruce
> 
> >
> > [YOCTO #5786]
> >
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> >
> > diff --git a/meta/classes/kernel-module-split.bbclass b/meta/classes/kernel-module-split.bbclass
> > index d43f743..e38a6f6 100644
> > --- a/meta/classes/kernel-module-split.bbclass
> > +++ b/meta/classes/kernel-module-split.bbclass
> > @@ -130,8 +130,11 @@ python split_kernel_module_packages () {
> >
> >          # If autoloading is requested, output /etc/modules-load.d/<name>.conf and append
> >          # appropriate modprobe commands to the postinst
> > +        autoloadlist = (d.getVar("KERNEL_MODULE_AUTOLOAD", True) or "").split()
> >          autoload = d.getVar('module_autoload_%s' % basename, True)
> >          if autoload:
> > +            bb.error("KERNEL_MODULE_AUTOLOAD has replaced module_autoload_%s, please replace it!" % basename)
> > +        if basename in autoloadlist:
> >              name = '%s/etc/modules-load.d/%s.conf' % (dvar, basename)
> >              f = open(name, 'w')
> >              for m in autoload.split():
> > @@ -144,12 +147,15 @@ python split_kernel_module_packages () {
> >              d.setVar('pkg_postinst_%s' % pkg, postinst)
> >
> >          # Write out any modconf fragment
> > +        modconflist = (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()
> >          modconf = d.getVar('module_conf_%s' % basename, True)
> > -        if modconf:
> > +        if modconf and basename in modconflist:
> >              name = '%s/etc/modprobe.d/%s.conf' % (dvar, basename)
> >              f = open(name, 'w')
> >              f.write("%s\n" % modconf)
> >              f.close()
> > +        elif modconf:
> > +            bb.error("Please ensure module %s is listed in KERNEL_MODULE_PROBECONF since module_conf_%s is set" % (basename, basename))
> >
> >          files = d.getVar('FILES_%s' % pkg, True)
> >          files = "%s /etc/modules-load.d/%s.conf /etc/modprobe.d/%s.conf" % (files, basename, basename)
> > @@ -185,3 +191,5 @@ python split_kernel_module_packages () {
> >          if len(os.listdir(dir)) == 0:
> >              os.rmdir(dir)
> >  }
> > +
> > +do_package[vardeps] += '${@" ".join(map(lambda s: "module_conf_" + s, (d.getVar("KERNEL_MODULE_PROBECONF", True) or "").split()))}'
> >
> >
> 
> 
> 
> -- 
> "Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end"
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 188 bytes --]