[OS-BUILD PATCH] Keep sctp and l2tp modules in modules-extra

[Don Zickus (via Email Bridge)]

From: Don Zickus

Keep sctp and l2tp modules in modules-extra

There are two modules: sctp_diag and l2tp_ip6 that are in kernel-modules
that accidentally dragged the rest of the sctp and l2tp mdoules in
there.  Instead all those modules should stay in modules-extra.

Move the offending modules into modules-extra.

Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Signed-off-by: Don Zickus

diff a/redhat/rhel_files/mod-extra.list.rhel b/redhat/rhel_files/mod-extra.list.rhel
--- a/redhat/rhel_files/mod-extra.list.rhel
+++ b/redhat/rhel_files/mod-extra.list.rhel
@@ -79,6 +79,7 @@ l2tp_core.ko
 l2tp_debugfs.ko
 l2tp_eth.ko
 l2tp_ip.ko
+l2tp_ip6.ko
 l2tp_netlink.ko
 l2tp_ppp.ko
 lec.ko
@@ -132,6 +133,7 @@ sch_red.ko
 sch_sfb.ko
 sch_teql.ko
 sctp.ko
+sctp_diag.ko
 sctp_probe.ko
 sidewinder.ko
 sja1000.ko

--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117

[Paul Bolle]

Don Zickus (via Email Bridge) schreef op do 06-05-2021 om 22:15 [+0000]:
> There are two modules: sctp_diag and l2tp_ip6 that are in kernel-modules
> that accidentally dragged the rest of the sctp and l2tp mdoules in
> there.  Instead all those modules should stay in modules-extra.
>
> Move the offending modules into modules-extra.

In the explanation of MR !1041 ("mod-extra.list.fedora: remove 72 unused
modules") I noted: "It would be nice to warn about unused modules in this list
automagically but I have no idea how that could be done."

("Unused" turned out to be ambiguous. "Ignored" would have been better.)

Has anyone any suggestions for a way to make sure problems like this do not
stay unnoticed for _years_.

Thanks,

Paul Bolle

[Justin Forbes (via Email Bridge)]

From: Justin Forbes on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5692979...

There is something coming that will help with that.  When rpminspect is
used for autoqa in bodhi, we should see a notification when files move.

[Marcelo Ricardo Leitner]

On Thu, May 06, 2021 at 10:15:12PM -0000, Don Zickus (via Email Bridge) wrote:
> From: Don Zickus
>
> Keep sctp and l2tp modules in modules-extra
>
> There are two modules: sctp_diag and l2tp_ip6 that are in kernel-modules
> that accidentally dragged the rest of the sctp and l2tp mdoules in
> there.  Instead all those modules should stay in modules-extra.
>
> Move the offending modules into modules-extra.
>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> Signed-off-by: Don Zickus

Good catch.
The email is missing the --- delimiter, btw.

Acked-by: Marcelo Ricardo Leitner

[Don Zickus (via Email Bridge)]

From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5701816...

So I hacked up a script that can catch this.  The trick is where to run
this.  I think a self-test area is perfect, but the current self-tests
is a little heavy and only makes sense as a post-commit cron job.  Maybe
it should be called a 'garbage collection' cron job.

Anyway, the commands I used are:
time find . -name "Makefile*" -exec grep '.o' {} ; > /tmp/don.list (3
seconds)
for i in `cat redhat/rhel_files/*.list.rhel
redhat/fedora_files/*.list.fedora`;do mod="$(echo $i | sed 's/ko$/o/')";
grep -q $mod /tmp/don.list || echo "Bad module: $i"; done|wc -l

which found 78 modules.  39 for rhel and 39 for fedora.

That would accomplish your goal.  Just need to figure out where to plug
it in.

[Justin Forbes (via Email Bridge)]

From: Justin Forbes on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5702139...
> Anyway, the commands I used are: time find . -name "Makefile*" -exec

grep '.o' {} ; > /tmp/don.list (3 seconds) for i in `cat
redhat/rhel_files/*.list.rhel redhat/fedora_files/*.list.fedora`;do
mod="$(echo $i | sed 's/ko$/o/')"; grep -q $mod /tmp/don.list || echo
"Bad module: $i"; done|wc -l

I don't think that addresses the issue, I think the issue is when a new
module comes along with a dep on a module that was in extras, and moves
the extras module back to kernel-modules.   The only real way to do that
is to diff the file lists of module every time and list anything missing
that was in the previous verison.

[Marcelo Ricardo Leitner]

On Fri, May 07, 2021 at 07:02:27PM -0000, Justin Forbes (via Email
Bridge) wrote:
> From: Justin Forbes on gitlab.com
> https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5702139...
> > Anyway, the commands I used are: time find . -name "Makefile*" -exec
>
> grep '.o' {} ; > /tmp/don.list (3 seconds) for i in `cat
> redhat/rhel_files/*.list.rhel redhat/fedora_files/*.list.fedora`;do
> mod="$(echo $i | sed 's/ko$/o/')"; grep -q $mod /tmp/don.list || echo
> "Bad module: $i"; done|wc -l
>
> I don't think that addresses the issue, I think the issue is when a new
> module comes along with a dep on a module that was in extras, and moves
> the extras module back to kernel-modules.   The only real way to do that
> is to diff the file lists of module every time and list anything missing
> that was in the previous verison.

Or treat mod-extra.list (and others) as mandatory. If it's there, and
the module is not listed in the final file list, as in:
%{expand:%%files -f %{name}-%{?3:%{3}-}modules-extra.list
%{?3:%{3}-}modules-extra}\
it's a bug.

That said, the best place to handle it would be while building the package.

[Paul Bolle]

Marcelo Ricardo Leitner schreef op vr 07-05-2021 om 12:11 [-0700]:
> Or treat mod-extra.list (and others) as mandatory. If it's there, and
> the module is not listed in the final file list, as in:
> %{expand:%%files -f %{name}-%{?3:%{3}-}modules-extra.list
> %{?3:%{3}-}modules-extra}\
> it's a bug.

Mid-air collision!

I just commented on gitlab.com about that same line. Though I thought it's the
other way round: if the file is on the list but doesn't end up in the package
it should be a bug.

kernel.spec is _really_ hard to grok, so Marcelo's view could very well be the
better approach.

Thanks,

Paul Bolle

[Paul Bolle (via Email Bridge)]

From: Paul Bolle on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5702661...

Somewhere deep in a horrific rpmmacro I noticed:
    %{expand:%%files -f kernel-%{?3:%{3}-}modules-extra.list
%{?3:%{3}-}modules-extra}

My guess is that "%files -f <file>" is liberal, i.e. it happily ignores
filenames in <file> that do not actually exist. (Readable and current
rpmbuild documentation comes at a premium. So I never found it.) If my
guess is correct then it would be nice to lobby for a strict version of
"%files -f <file>".

Please not "guess" above.

[Justin Forbes (via Email Bridge)]

From: Justin Forbes on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5702898...

We do want it to ignore files that do not actually exist, othrewise we
would have to change the extras list to be per arch, and frankly that
sounds like more than I want to deal with. I can very easily add
something to the kerneltest scripts that run on x86_64 at least after
every build to diff modules-extra with the previous build. It would be a
start, though I think we could do something similar with rpminspect
output in bodhi QA.  All rawhide and eln builds get filed in bodhi.

[Don Zickus (via Email Bridge)]

From: Don Zickus on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5703112...
> CKI Gitlab commented:
>
> Marcelo Ricardo Leitner  commented via email:
> > https://gitlab.com/cki-project/kernel-

ark/-/merge_requests/1117#note_570213935
> > > Anyway, the commands I used are: time find . -name "Makefile*"

-exec
> > grep '.o' {} ; > /tmp/don.list (3 seconds) for i in `cat
> > redhat/rhel_files/*.list.rhel redhat/fedora_files/*.list.fedora`;do
> > mod="$(echo $i | sed 's/ko$/o/')"; grep -q $mod /tmp/don.list ||

echo
> > "Bad module: $i"; done|wc -l
> >
> > I don't think that addresses the issue, I think the issue is when a

new
> > module comes along with a dep on a module that was in extras, and

moves
> > the extras module back to kernel-modules.   The only real way to do

that
> > is to diff the file lists of module every time and list anything

missing
> > that was in the previous verison.
>
> Or treat mod-extra.list (and others) as mandatory. If it's there, and
> the module is not listed in the final file list, as in:
> %{expand:%%files -f %{name}-%{?3:%{3}-}modules-extra.list
> %{?3:%{3}-}modules-extra}\
> it's a bug.
>
> That said, the best place to handle it would be while building the

package.

That was my first thought until I wondered if every module in
modules-extra.list is applicable to every arch.  Is it possible that
some of
those modules are not built for an arch or its variant?

[Paul Bolle]

Don Zickus (via Email Bridge) schreef op vr 07-05-2021 om 19:46 [+0000]:
> That was my first thought until I wondered if every module in
> modules-extra.list is applicable to every arch.  Is it possible that
> some of those modules are not built for an arch or its variant?

We seem to be discussing one issue both through e-mail and on gitlab.com. I'm
not that flexible. Where should I focus my attention?

[Justin Forbes (via Email Bridge)]

From: Justin Forbes on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5703342...

It should work both ways, the email bridge seems a big laggy today
though. instead of instant, it is on a delay.  Email comments end up on
gitlab and gitlab comments should end up in email though.

[Paul Bolle (via Email Bridge)]

From: Paul Bolle on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5704401...

filter-modules.sh and friends are arch-aware. (I hacked them into being
noisy about bogus modules and that should work on all arches.)

I realize that your plate is full already, but it does seems odd that
mod-extra.list is global. Would cobbling together filter-extra-
modules.sh be worth our time?

[Justin Forbes (via Email Bridge)]

From: Justin Forbes on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1117#note_5704473...

I don't know that it would be worthwhile, I think it might be easier to
write something to check it after the fact, or even the script to verify
against sources.  Honestly, if we stop building a module and the entry
is just abandoned in the extra list for a bit, that is unfortunate, but
not really problematic.  Modules moving from extras to kernel-modules
due to deps is a real issue, and there are a number of ways to monitor
that.