Below is the first swag at a kernel-modules-extra package. It seems to do the proper thing, and move the listed modules to the /lib/modules/`uname -r`/extra/ directory[1], and then package those up into a kernel-<flavor>-modules-extra subpackage. I have a scratch build with this change here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3523738
The macros the define the kernel-modules-extra subpackage and %files sections look nasty, but they are required for when we build multiple kernel flavors (e.g. PAE, debug, etc) so we get the proper per-flavor subpackage.
The mod-extra.list file is the configuration file for which modules should be put into the modules-extra subpackage. This isn't a fully comprehensive list of exactly what will wind up in the subpackage. Instead, it is a somewhat higher level list and the code will go through and also put anything with a dependency[2] on one of these modules into the subpackage as well (e.g. listing wimax.ko will also get wimax/i2400m/i2400m.ko pulled in).
Comments welcome.
josh
[1] The extra/ directory already exists and is set aside for "out of tree" modules, so we're slightly overriding the intention by sticking modules that are built in-tree there. However, we do consider these modules _extra_ and it seemed silly to create yet another directory for this purpose.
[2] The dependency stuff is a poor-man's depmod, which isn't run until after the kernel package is installed. It's a best effort really, not 100% comprehensive but it seems to catch most things. We could run depmod here and then use those results to catch everything but I wasn't sure if that was desired or required.
---
kernel.spec | 69 ++++++++++++++++++++++- mod-extra.list | 170 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 238 insertions(+), 1 deletions(-) create mode 100644 mod-extra.list
diff --git a/kernel.spec b/kernel.spec index b72afb3..9eadbb1 100644 --- a/kernel.spec +++ b/kernel.spec @@ -548,6 +548,7 @@ Source1: compat-wireless-%{cwversion}.tar.bz2 Source11: genkey Source14: find-provides Source15: merge.pl +Source16: mod-extra.list
Source19: Makefile.release Source20: Makefile.config @@ -902,6 +903,24 @@ against the %{?2:%{2} }kernel package.\ %{nil}
# +# This macro creates a kernel-<subpackage>-modules-extra package. +# %%kernel_modules-extra_package <subpackage> <pretty-name> +# +%define kernel_modules-extra_package() \ +%package %{?1:%{1}-}modules-extra\ +Summary: Extra kernel modules to match the %{?2:%{2} }kernel\ +Group: System Environment/Kernel\ +Provides: kernel%{?1:-%{1}}-modules-extra-%{_target_cpu} = %{version}-%{release}\ +Provides: kernel-modules-extra-%{_target_cpu} = %{version}-%{release}%{?1:.%{1}}\ +Provides: kernel-modules-extra = %{version}-%{release}%{?1:.%{1}}\ +Provides: kernel-modules-extra-uname-r = %{KVERREL}%{?1:.%{1}}\ +Requires: kernel%{?1:-%{1}}-%{_target_cpu} = %{version}-%{release}%{?1:.%{1}}\ +AutoReqProv: no\ +%description -n kernel%{?variant}%{?1:-%{1}}-modules-extra\ +This package provides less commonly used kernel modules for the %{?2:%{2} }kernel package.\ +%{nil} + +# # This macro creates a kernel-<subpackage> and its -devel and -debuginfo too. # %%define variant_summary The Linux kernel compiled for <configuration> # %%kernel_variant_package [-n <pretty-name>] <subpackage> @@ -912,12 +931,14 @@ Summary: %{variant_summary}\ Group: System Environment/Kernel\ %kernel_reqprovconf\ %{expand:%%kernel_devel_package %1 %{!?-n:%1}%{?-n:%{-n*}}}\ +%{expand:%%kernel_modules-extra_package %1 %{!?-n:%1}%{?-n:%{-n*}}}\ %{expand:%%kernel_debuginfo_package %1}\ %{nil}
# First the auxiliary packages of the main kernel package. %kernel_devel_package +%kernel_modules-extra_package %kernel_debuginfo_package
@@ -1645,6 +1666,50 @@ BuildKernel() {
rm -f modinfo modnames
+ pushd $RPM_BUILD_ROOT/lib/modules/$KernelVer/ + rm -rf modnames + find . -name "*.ko" -type f > modnames + # Look through all of the modules, and throw any that have a dependency in + # our list into the list as well. + rm -rf dep.list dep2.list + cp %{SOURCE16} . + for dep in `cat modnames` + do + depends=`modinfo $dep | grep depends| cut -f2 -d":" | sed -e 's/^[ \t]*//'` + [ -z "$depends" ] && continue; + for mod in `echo $depends | sed -e 's/,/ /g'` + do + match=`grep "^$mod.ko" mod-extra.list` ||: + if [ -z "$match" ] + then + continue + else + echo $dep >> dep.list + fi + done + done + + for mod in `cat mod-extra.list` + do + # get the path for the module + modpath=`grep /$mod modnames` + [ -z "$modpath" ] && continue; + echo $modpath >> dep.list + done + + sort -u dep.list > dep2.list + + # now move the modules into the extra/ directory + for mod in `cat dep2.list` + do + newpath=`dirname $mod | sed -e 's/kernel//extra//'` + mkdir -p $newpath + mv $mod $newpath + done + + rm modnames mod-extra.list dep.list dep2.list + popd + # remove files that will be auto generated by depmod at rpm -i time for i in alias alias.bin builtin.bin ccwmap dep dep.bin ieee1394map inputmap isapnpmap ofmap pcimap seriomap symbols symbols.bin usbmap do @@ -2061,7 +2126,6 @@ fi /lib/modules/%{KVERREL}%{?2:.%{2}}/kernel\ /lib/modules/%{KVERREL}%{?2:.%{2}}/build\ /lib/modules/%{KVERREL}%{?2:.%{2}}/source\ -/lib/modules/%{KVERREL}%{?2:.%{2}}/extra\ /lib/modules/%{KVERREL}%{?2:.%{2}}/updates\ %if %{with_backports}\ /lib/modules/%{KVERREL}%{?2:.%{2}}/backports\ @@ -2075,6 +2139,9 @@ fi %{expand:%%files %{?2:%{2}-}devel}\ %defattr(-,root,root)\ /usr/src/kernels/%{KVERREL}%{?2:.%{2}}\ +%{expand:%%files %{?2:%{2}-}modules-extra}\ +%defattr(-,root,root)\ +/lib/modules/%{KVERREL}%{?2:.%{2}}/extra\ %if %{with_debuginfo}\ %ifnarch noarch\ %{expand:%%files -f debuginfo%{?2}.list %{?2:%{2}-}debuginfo}\ diff --git a/mod-extra.list b/mod-extra.list new file mode 100644 index 0000000..faec1d8 --- /dev/null +++ b/mod-extra.list @@ -0,0 +1,170 @@ +aer_inject.ko +yenta_socket.ko +tcp_bic.ko +tcp_westwood.ko +tcp_htcp.ko +tcp_highspeed.ko +tcp_hybla.ko +tcp_vegas.ko +tcp_scalable.ko +tcp_lp.ko +tcp_veno.ko +tcp_yeah.ko +tcp_illinois.ko +dccp_diag.ko +dccp_ipv4.ko +dccp_ipv6.ko +dccp.ko +dccp_probe.ko +sctp.ko +sctp_probe.ko +rds.ko +rds_rdma.ko +rds_tcp.ko +atm.ko +br2684.ko +clip.ko +lec.ko +pppoatm.ko +l2tp_core.ko +l2tp_debugfs.ko +l2tp_eth.ko +l2tp_ip.ko +l2tp_netlink.ko +l2tp_ppp.ko +8021q.ko +llc.ko +ipx.ko +appletalk.ko +ipddp.ko +wanrouter.ko +phonet.ko +pn_pep.ko +af_802154.ko +ieee802154.ko +sch_atm.ko +sch_cbq.ko +sch_choke.ko +sch_drr.ko +sch_dsmark.ko +sch_gred.ko +sch_hfsc.ko +sch_htb.ko +sch_ingress.ko +sch_mqprio.ko +sch_multiq.ko +sch_netem.ko +sch_prio.ko +sch_qfq.ko +sch_red.ko +sch_sfb.ko +sch_sfq.ko +sch_tbf.ko +sch_teql.ko +ax25.ko +netrom.ko +rose.ko +6pack.ko +baycom_par.ko +baycom_ser_fdx.ko +baycom_ser_hdx.ko +bpqether.ko +hdlcdrv.ko +mkiss.ko +yam.ko +can.ko +can-raw.ko +can-bcm.ko +can-dev.ko +slcan.ko +vcan.ko +c_can.ko +c_can_platform.ko +ems_pci.ko +kvaser_pci.ko +plx_pci.ko +sja1000.ko +sja1000_platform.ko +softing_cs.ko +softing.ko +ems_usb.ko +esd_usb2.ko +wimax.ko +nfc.ko +nci.ko +mtd.ko +mptbase.ko +mptctl.ko +mptfc.ko +i2400m.ko +hisax.ko +hysdn.ko +isdn.ko +mISDN_core.ko +mISDN_dsp.ko +capi.ko +dss1_divert.ko +bas_gigaset.ko +gigaset.ko +avm_cs.ko +b1.ko +diva_idi.ko +divas.ko +avmfritz.ko +hfcpci.ko +hfcmulti.ko +netjet.ko +w6692.ko +avma1_cs.ko +elsa_cs.ko +hfc4s8s_l1.ko +joydev.ko +a3d.ko +adi.ko +analog.ko +cobra.ko +db9.ko +gamecon.ko +gf2k.ko +grip.ko +grip_mp.ko +guillemot.ko +iforce.ko +interact.ko +joydump.ko +magellan.ko +sidewinder.ko +spaceball.ko +spaceorb.ko +stinger.ko +tmdc.ko +turbografx.ko +twidjoy.ko +walkera0701.ko +warrior.ko +xpad.ko +zhenhua.ko +trancevibrator.ko +umc.ko +uwb.ko +whci.ko +hwa-rc.ko +uio.ko +uio_aec.ko +uio_pci_generic.ko +jfs.ko +gfs2.ko +ocfs2.ko +ocfs2_stackglue.ko +ocfs2_dlm.ko +cuse.ko +affs.ko +befs.ko +jffs2.ko +ubifs.ko +sysv.ko +ufs.ko +ncpfs.ko +coda.ko +9p.ko +9pnet.ko
Hi!
On 18.11.2011 15:52, Josh Boyer wrote:
Below is the first swag at a kernel-modules-extra package.
A little bit of background and reasons for this move would be nice. I assume it was mentioned somewhere already, but seems I missed it.
Anyway, two minor comments:
[...] kernel.spec | 69 ++++++++++++++++++++++- [...] diff --git a/kernel.spec b/kernel.spec index b72afb3..9eadbb1 100644 --- a/kernel.spec +++ b/kernel.spec [...] +# This macro creates a kernel-<subpackage>-modules-extra package. +# %%kernel_modules-extra_package <subpackage> <pretty-name> +# +%define kernel_modules-extra_package() \ +%package %{?1:%{1}-}modules-extra\ +Summary: Extra kernel modules to match the %{?2:%{2} }kernel\ +Group: System Environment/Kernel\ +Provides: kernel%{?1:-%{1}}-modules-extra-%{_target_cpu} = %{version}-%{release}\ +Provides: kernel-modules-extra-%{_target_cpu} = %{version}-%{release}%{?1:.%{1}}\ +Provides: kernel-modules-extra = %{version}-%{release}%{?1:.%{1}}\ +Provides: kernel-modules-extra-uname-r = %{KVERREL}%{?1:.%{1}}\
Do we really need all of those provides? Yes, they are derived from the provides in the main package, but I wonder if we still need all of those there and if it's worth adding similar ones to this new subpackage. Maybe trimming the list of provides in the main package in the long term might be something to think about. In addition:
+Requires: kernel%{?1:-%{1}}-%{_target_cpu} = %{version}-%{release}%{?1:.%{1}}\
Just FYI: Kmod2 packages in some popular add on repo depend on the kernel they were build for with this:
kernel-uname-r = 3.1.1-2.fc16.x86_64
That seems to work well afaics. I think there was a reason back years ago why we settled for this strategy and did not use the requires you chose -- but I can't recall it and maybe it's not relevant any more these days anyway (and maybe my mind comes up with the reason seconds after sending this mail...).
Cu knurd
On Sat, Nov 19, 2011 at 1:33 PM, Thorsten Leemhuis fedora@leemhuis.info wrote:
Hi!
On 18.11.2011 15:52, Josh Boyer wrote:
Below is the first swag at a kernel-modules-extra package.
A little bit of background and reasons for this move would be nice. I assume it was mentioned somewhere already, but seems I missed it.
There are a couple of reasons I can think of.
1) There is a bunch of cruft that is rarely used. We were actually kind of split on whether or not to just disable things like AFFS/BFS, etc or to create a subpackage. The subpackage route won because we didn't want to disable something and then require some kind of popularity context/bugzilla ME TOO-fest for requesting it to be re-enabled. Maybe we were just chicken.
2) Following along with the above, all of that cruft is something of a security exposure. We've had cases where esoteric modules get auto-loaded by the system due to inadvertent errors, and some of these modules seem to have a bad history with security (like sctp). Stuffing them into a subpackage doesn't lessen the individual module security issues, but it reduces the attack surface of a default install by a little.
3) Bloat. Now, you might look at the scratch build and say "the example list only moved 3MB worth of modules and that isn't much". You would be right. But that is just an example build. If we aren't going to disable them (see 1) then moving them to something that isn't installed by default will at least save a bit of disk space. Disk might be cheap, but at least then you could use that extra 3MB to store more Justin Bieber pictures, or KDE configuration settings, or whatever floats your boat.
Anyway, two minor comments:
[...] kernel.spec | 69 ++++++++++++++++++++++- [...] diff --git a/kernel.spec b/kernel.spec index b72afb3..9eadbb1 100644 --- a/kernel.spec +++ b/kernel.spec [...] +# This macro creates a kernel-<subpackage>-modules-extra package. +# %%kernel_modules-extra_package <subpackage> <pretty-name> +# +%define kernel_modules-extra_package() \ +%package %{?1:%{1}-}modules-extra\ +Summary: Extra kernel modules to match the %{?2:%{2} }kernel\ +Group: System Environment/Kernel\ +Provides: kernel%{?1:-%{1}}-modules-extra-%{_target_cpu} = %{version}-%{release}\ +Provides: kernel-modules-extra-%{_target_cpu} = %{version}-%{release}%{?1:.%{1}}\ +Provides: kernel-modules-extra = %{version}-%{release}%{?1:.%{1}}\ +Provides: kernel-modules-extra-uname-r = %{KVERREL}%{?1:.%{1}}\
Do we really need all of those provides? Yes, they are derived from the provides in the main package, but I wonder if we still need all of those there and if it's worth adding similar ones to this new subpackage. Maybe trimming the list of provides in the main package in the long term might be something to think about. In addition:
Good point. Copy and paste was easy, but I agree it might be worth looking at why all of this is listed for Provides.
+Requires: kernel%{?1:-%{1}}-%{_target_cpu} = %{version}-%{release}%{?1:.%{1}}\
Just FYI: Kmod2 packages in some popular add on repo depend on the kernel they were build for with this:
kernel-uname-r = 3.1.1-2.fc16.x86_64
That seems to work well afaics. I think there was a reason back years ago why we settled for this strategy and did not use the requires you chose -- but I can't recall it and maybe it's not relevant any more these days anyway (and maybe my mind comes up with the reason seconds after sending this mail...).
I can add both Requires methods, or switch. I just picked one to make sure it wasn't free-floating for the scratch build. If you remember the reason for the other (which looks like something dkms or kmod would use at install time) then let me know.
Thanks for taking the time to look it over!
josh
Hi!
Sorry for the late reply.
On 19.11.2011 22:14, Josh Boyer wrote:
On Sat, Nov 19, 2011 at 1:33 PM, Thorsten Leemhuis fedora@leemhuis.info wrote:
On 18.11.2011 15:52, Josh Boyer wrote:
Below is the first swag at a kernel-modules-extra package.
A little bit of background and reasons for this move would be nice. I assume it was mentioned somewhere already, but seems I missed it.
There are a couple of reasons I can think of.
[reasons]
Thx. Might be a good idea to add this to some readme or a shorter variant to the %description.
In addition while at it: Would it be worth shipping a few hand selected staging modules in this package as well? Not all of them, only those where there is clearly someone working on improving the code upstream? The gma500 driver for example. And the Hyper-V drivers -- getting those out and tested afaics would even be of interest for Red Hat / RHEL afaics.
[...
+Requires: kernel%{?1:-%{1}}-%{_target_cpu} = %{version}-%{release}%{?1:.%{1}}\
Just FYI: Kmod2 packages in some popular add on repo depend on the kernel they were build for with this: kernel-uname-r = 3.1.1-2.fc16.x86_64 That seems to work well afaics. I think there was a reason back years ago why we settled for this strategy and did not use the requires you chose -- but I can't recall it and maybe it's not relevant any more these days anyway (and maybe my mind comes up with the reason seconds after sending this mail...).
I can add both Requires methods, or switch.
Both sounds dangerous and I still can't think of any reason to prefer one over the other. so I'd leave it as it is..
[...] Thanks for taking the time to look it over!
np
Cu knurd
----- Original Message -----
From: "Thorsten Leemhuis" fedora@leemhuis.info
+Requires: kernel%{?1:-%{1}}-%{_target_cpu} = %{version}-%{release}%{?1:.%{1}}\
Just FYI: Kmod2 packages in some popular add on repo depend on the kernel they were build for with this:
kernel-uname-r = 3.1.1-2.fc16.x86_64
That seems to work well afaics. I think there was a reason back years ago why we settled for this strategy and did not use the requires you chose -- but I can't recall it and maybe it's not relevant any more these days anyway (and maybe my mind comes up with the reason seconds after sending this mail...).
Today's broken deps report highlights why kmod2 uses kernel-uname-r instead. Seems the kernel variant packages (e.g. PAE) don't have a Provides that includes the variant name. I've switched to putting the Requires on kernel-uname-r instead, which does include the variant.
I should have listened to you to begin with :).
josh
kernel@lists.fedoraproject.org