Openembedded Core Discussions
 help / color / mirror / Atom feed
From: Martin Jansa <martin.jansa@gmail.com>
To: Bruce Ashfield <bruce.ashfield@gmail.com>
Cc: "Hart, Darren" <darren.hart@intel.com>,
	openembedded-core <openembedded-core@lists.openembedded.org>
Subject: Re: [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF
Date: Fri, 13 Jun 2014 23:03:02 +0200	[thread overview]
Message-ID: <20140613210302.GK2428@jama> (raw)
In-Reply-To: <CADkTA4MtGT145NMXzwfL9z6HDnrLzbvcDdeGeCjC5AJuN-Ad+g@mail.gmail.com>

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

      parent reply	other threads:[~2014-06-13 21:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-13 15:46 [PATCH] kernel-module-split: Add support for KERNEL_MODULE_AUTOLOAD and KERNEL_MODULE_PROBECONF Richard Purdie
2014-06-13 19:02 ` Bruce Ashfield
2014-06-13 19:13   ` Denys Dmytriyenko
2014-06-17 14:53     ` Hart, Darren
2014-06-17 16:46       ` Richard Purdie
2014-06-23 22:54         ` Denys Dmytriyenko
2014-07-22 11:17           ` Martin Jansa
2014-06-13 21:03   ` Martin Jansa [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140613210302.GK2428@jama \
    --to=martin.jansa@gmail.com \
    --cc=bruce.ashfield@gmail.com \
    --cc=darren.hart@intel.com \
    --cc=openembedded-core@lists.openembedded.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox