makeModulesClosure: fixup firmware extraction #96008
Conversation
|
somewhat related to #92081 |
| for i in $(modinfo -b $kernel --set-version "$version" -F firmware $module); do | ||
| # $ modinfo -F firmware unix | ||
| # name: unix | ||
| if test "$i" = "name:"; then break; fi |
There was a problem hiding this comment.
Should this really break, as opposed to skipping the line? Maybe piping modinfo into grep -v ^name: would make more sense?
That said, this seems to me like modinfo is behaving incorrectly, given its manpage — it should only print the value of the field if given a -F option.
There was a problem hiding this comment.
I think the intention was probably to use continue instead of break.
Actually the man page of modinfo does say
modinfo by default lists each attribute of the module in form fieldname : value, for easy reading. The filename is listed the same way (although it's not really an attribute).
but I agree that it should probably not do so if -F is given..
There was a problem hiding this comment.
Hm, it only seems to do this for built-in modules. That confuses me some more.
There was a problem hiding this comment.
The intention is to "continue" to the next module. But because it's two nested loops, I break the inner one, otherwise (using the example provided) we match on name: which we would continue/skip, then we're feeded unix (which is wrong too).
We should break the inner one, and continue on the outer one.
There was a problem hiding this comment.
I somewhat agree modinfo acts weird, piping it to | grep -v actually sounds better. Let me know.
There was a problem hiding this comment.
Yeah, while the kmod fix is my main preference, my preferred workaround is the grep approach, since otherwise we'll fail to load firmware for any built-in modules that require it.
|
Ok, the following patch works for me: 0001-modinfo-dont-print-module-name-when-field-is-given.txt (edit: updated patch) |
This came up in NixOS#96008. Without this patch modinfo always prints the module name for builtin modules, even if an explicit --field is passed.
|
To note: There was also fixes on the kernel modules to read included. If #96123 is merged, we still need those. |
| # $ modinfo -F firmware unix | ||
| # name: unix | ||
| if test "$i" = "name:"; then break; fi |
There was a problem hiding this comment.
Once modinfo patches are merged, we still need this in:
-b $kernel --set-version "$version"
| # $ modinfo -F firmware unix | |
| # name: unix | |
| if test "$i" = "name:"; then break; fi |
This came up in NixOS#96008. Without this patch modinfo always prints the module name for builtin modules, even if an explicit --field is passed.
|
Since the patch for |
After a recent upgrade of modinfo, its output is now incorrect for builtin modules. This commit filters out the output until a fix is made available upstream
baloo
force-pushed
the
baloo/bugfixes/make-modules-closure_firmware
branch
from
ce5edfe
to
e417362
Compare
Before this commit, the firmware information would be loaded from the currently running kernel, not from the kernel to be loaded. This commit ensures the correct kernel version and modules are read.
baloo
force-pushed
the
baloo/bugfixes/make-modules-closure_firmware
branch
from
e417362
to
70bc1a3
Compare
|
Splitted the two commits, and rewrote the fix to use a |
Motivation for this change
When building on a non nixos platform, I would get error trying to lookup the firmware for unix module.
modinfo -F firmware unixoutput looks wrong. This is an attempt to work around that. Also I'm not sure we were loading module information from the to-be-loaded kernel, but rather the currently running one.Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)