Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/stage-1: Do not allow missing kernel modules in initrd #78430

Merged
merged 1 commit into from
Jan 11, 2021

Conversation

puckipedia
Copy link
Contributor

Motivation for this change

I, and others I've talked to, have repeatedly hit the issue where we accidentally typo a name of a kernel module in boot.initrd.availableKernelModules, and wonder why e.g. the disk won't mount. Turns out, the code explicitly allows this to happen! This fixes that.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 24, 2020
@xaverdh
Copy link
Contributor

xaverdh commented Jan 24, 2020

Does setting allowMissing actually work for you?
I guess it shouldn't due to #69511

@puckipedia
Copy link
Contributor Author

interesting, setting it to false worked as expected for me..

@stale

This comment has been minimized.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 23, 2020
@puckipedia
Copy link
Contributor Author

one-line diff, still valid, tyvm

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 23, 2020
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/431

@fabianhjr
Copy link
Member

Tagging @edolstra for review since they where the last person to set that value.

@edolstra edolstra merged commit 187af93 into NixOS:master Jan 11, 2021
@edolstra
Copy link
Member

It was probably set to true for the benefit of people running non-standard kernels or kernel configurations.

@xaverdh
Copy link
Contributor

xaverdh commented Jan 11, 2021

Actually this creates an issue, because xhci-pci.ko.xz needs renesas_usb_fw.mem (which is not present).
We should probably still ignore missing firmware by default, since its such a common case..

@xaverdh
Copy link
Contributor

xaverdh commented Jan 11, 2021

opened #109030

@delroth
Copy link
Contributor

delroth commented Jan 13, 2021

This makes it really annoying to use NixOS with a custom built kernel. There is a whole bunch of modules that get added to the initrd "by default" in nixos/modules/system/boot/kernel.nix and can't easily be removed:

        boot.initrd.availableKernelModules =
          [ # Note: most of these (especially the SATA/PATA modules)
            # shouldn't be included by default since nixos-generate-config
            # detects them, but I'm keeping them for now for backwards
            # compatibility.

            # Some SATA/PATA stuff.
            "ahci"
            "sata_nv"
            "sata_via"
            "sata_sis"
            "sata_uli"
            "ata_piix"
            "pata_marvell"

            # Standard SCSI stuff.
            "sd_mod"
            "sr_mod"

            # SD cards and internal eMMC drives.
            "mmc_block"

            # Support USB keyboards, in case the boot fails and we only have
            # a USB keyboard, or for LUKS passphrase prompt.
            "uhci_hcd"
            "ehci_hcd"
            "ehci_pci"
            "ohci_hcd"
            "ohci_pci"
            "xhci_hcd"
            "xhci_pci"
            "usbhid"
            "hid_generic" "hid_lenovo" "hid_apple" "hid_roccat"
            "hid_logitech_hidpp" "hid_logitech_dj" "hid_microsoft"

          ] ++ optionals (pkgs.stdenv.isi686 || pkgs.stdenv.isx86_64) [
            # Misc. x86 keyboard stuff.
            "pcips2" "atkbd" "i8042"

            # x86 RTC needed by the stage 2 init script.
            "rtc_cmos"
          ];

My kernel doesn't build some of these modules, and now that's breaking the initrd build.

Maybe I've missed an obvious way to get rid of these defaults without also breaking the multiple places that merge into this list in other NixOS modules?

@vcunat
Copy link
Member

vcunat commented Jan 13, 2021

This commit also broke our aarch64 ISO build and thus is blocking nixos-unstable channel.

modprobe: FATAL: Module vmw_balloon not found in directory /nix/store/jz2zc29g4lw24jhcb452ph2ji4jp70mj-linux-5.4.88-modules/lib/modules/5.4.88

@xaverdh
Copy link
Contributor

xaverdh commented Jan 13, 2021

This makes it really annoying to use NixOS with a custom built kernel. There is a whole bunch of modules that get added to the initrd "by default" in nixos/modules/system/boot/kernel.nix and can't easily be removed:

Maybe I've missed an obvious way to get rid of these defaults without also breaking the multiple places that merge into this list in other NixOS modules?

Does boot.initrd.availableKernelModules = mkForce [...] not work here?

@xaverdh
Copy link
Contributor

xaverdh commented Jan 13, 2021

This commit also broke our aarch64 ISO build and thus is blocking nixos-unstable channel.

modprobe: FATAL: Module vmw_balloon not found in directory /nix/store/jz2zc29g4lw24jhcb452ph2ji4jp70mj-linux-5.4.88-modules/lib/modules/5.4.88

I can't find any reference of vmw_balloon in nixpkgs, yet its listed as a root module in the logs. Does the builder have vmw_balloon in its kernel cmdline by any chance?

@vcunat
Copy link
Member

vcunat commented Jan 13, 2021

$ git grep vmw_balloon | cat
nixos/modules/profiles/all-hardware.nix:      "mptspi" "vmw_balloon" "vmwgfx" "vmw_vmci" "vmw_vsock_vmci_transport" "vmxnet3" "vsock"

@vcunat
Copy link
Member

vcunat commented Jan 13, 2021

So we make such parts conditional, I guess. linux/latest/source/drivers/misc/Kconfig says that this particular module only makes sense on x86.

depends on VMWARE_VMCI && X86 && HYPERVISOR_GUEST

@xaverdh
Copy link
Contributor

xaverdh commented Jan 13, 2021

$ git grep vmw_balloon | cat
nixos/modules/profiles/all-hardware.nix:      "mptspi" "vmw_balloon" "vmwgfx" "vmw_vmci" "vmw_vsock_vmci_transport" "vmxnet3" "vsock"

Strange, I didn't find it, maybe was in the wrong directory.

So we make such parts conditional, I guess. linux/latest/source/drivers/misc/Kconfig says that this particular module only makes sense on x86.

depends on VMWARE_VMCI && X86 && HYPERVISOR_GUEST

Yes this module should only be added conditional on the target arch then.

Actually according to the last succeeding build all of these probably don't work on aarch:
"vmw_balloon" "vmwgfx" "vmw_vmci" "vmw_vsock_vmci_transport" "hv_storvsc"

@primeos
Copy link
Member

primeos commented Jan 13, 2021

Looking at the comment from @delroth and the nixos.iso_minimal.aarch64-linux failure that blocks nixos-unstable (#109252) I suggest that we revert this change until we have at least fixed the defaults in Nixpkgs that are architecture dependent (and we should think about the issues that @delroth pointed out too. i.e. personal configurations and other stuff outside of Nixpkgs).

@vcunat
Copy link
Member

vcunat commented Jan 13, 2021

I didn't want to spend time looking deeper, so I just unblocked the channel in 8ca3383. Feel free to work out a more proper solution (or revert some of these).

@primeos
Copy link
Member

primeos commented Jan 13, 2021

I've opened #109280 to discuss some of the implications resulting from this breaking change (see "Open/unresolved issues to consider" and "Potential improvements"). I'm not sure how many users this will affect but just in case I started to collect some ideas how we could try to help with the transition (updating configuration.nix) or make it easier to keep using custom kernel configurations. Feel free to add your thoughts.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/failed-to-build-kernel-with-nix-flake/11971/3

@samueldr
Copy link
Member

samueldr commented Aug 15, 2021

@etu
Copy link
Contributor

etu commented Aug 15, 2021

It would be great if this option was configurable...

It's a bit cumbersome to have to maintain a nixpkgs fork with only this option change to build images for non-supported platforms.

@delroth
Copy link
Contributor

delroth commented Aug 15, 2021

boot.initrd.includeDefaultModules = false; helps a lot, in my case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants