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.

Sign up for GitHub

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

Jump to bottom

nixos: Support systemd-gpt-auto-root #282022

Merged
merged 2 commits into from
Mar 18, 2024
+69 −9
Merged

nixos: Support systemd-gpt-auto-root #282022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1731f2
nixos: Support systemd-gpt-auto-root
ElvishJerricco Jan 19, 2024
3758681
nixos/tests/hibernate: Use EFI
ElvishJerricco Mar 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion nixos/modules/system/boot/stage-1.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ in

config = mkIf config.boot.initrd.enable {
assertions = [
{ assertion = any (fs: fs.mountPoint == "/") fileSystems;
{ assertion = !config.boot.initrd.systemd.enable -> any (fs: fs.mountPoint == "/") fileSystems;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If config.boot.initrd.systemd.enable is set, there is no assertion. If config.boot.initrd.systemd.enable is not set, there is an assertion when no "/" mountpoint is set.

message = "The ‘fileSystems’ option does not specify your root file system.";
}
{ assertion = let inherit (config.boot) resumeDevice; in
Expand Down
29 changes: 25 additions & 4 deletions nixos/modules/system/boot/systemd/initrd.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,19 @@ in {
default = [];
};

root = lib.mkOption {
type = lib.types.enum [ "fstab" "gpt-auto" ];
default = "fstab";
example = "gpt-auto";
description = ''
Controls how systemd will interpret the root FS in initrd. See
{manpage}`kernel-command-line(7)`. NixOS currently does not
allow specifying the root file system itself this
way. Instead, the `fstab` value is used in order to interpret
the root file system specified with the `fileSystems` option.
'';
};

emergencyAccess = mkOption {
type = with types; oneOf [ bool (nullOr (passwdEntry str)) ];
description = lib.mdDoc ''
Expand Down Expand Up @@ -342,7 +355,12 @@ in {
};

config = mkIf (config.boot.initrd.enable && cfg.enable) {
assertions = map (name: {
assertions = [
{
assertion = cfg.root == "fstab" -> any (fs: fs.mountPoint == "/") (builtins.attrValues config.fileSystems);
message = "The ‘fileSystems’ option does not specify your root file system.";
Comment on lines +360 to +361
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, and here's the other assertion.

}
] ++ map (name: {
assertion = lib.attrByPath name (throw "impossible") config.boot.initrd == "";
message = ''
systemd stage 1 does not support 'boot.initrd.${lib.concatStringsSep "." name}'. Please
Expand Down Expand Up @@ -371,7 +389,12 @@ in {
"autofs"
# systemd-cryptenroll
] ++ lib.optional cfg.enableTpm2 "tpm-tis"
++ lib.optional (cfg.enableTpm2 && !(pkgs.stdenv.hostPlatform.isRiscV64 || pkgs.stdenv.hostPlatform.isArmv7)) "tpm-crb";
++ lib.optional (cfg.enableTpm2 && !(pkgs.stdenv.hostPlatform.isRiscV64 || pkgs.stdenv.hostPlatform.isArmv7)) "tpm-crb"
++ lib.optional cfg.package.withEfi "efivarfs";
ElvishJerricco marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be conditional on cfg.root == "gpt-auto"? I don't see why this feature should add kernel modules for users who don't enable it. Adding it is problematic for me because it doesn't exist on armv6l.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lopsided98 efivarfs is useful for a few features, notably including hibernation. I'd rather disable it on platforms that don't support it than require it to be explicitly enabled.

If efivarfs doesn't exist on a platform, then I'm surprised we build systemd with withEfi = true. Maybe that's the change we should make?

Copy link
Contributor

@lopsided98 lopsided98 Mar 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withEfi defaults to stdenv.hostPlatform.isEfi. In practice, isEfi means "platforms where systemd EFI support can be built". There are unlikely to be any armv6l systems that use EFI, so it would be fine to remove it from isEfi, but I'm still not super happy with tying efivarfs in the initrd to systemd EFI support.

There is no way to override boot.initrd.availableKernelModules to remove an item, so we should be careful about what is added unconditionally. If someone wants to disable EFI support in their kernel for whatever reason, they also have to override systemd and rebuild the world. If hibernation needs efivarsfs, it should be automatically enabled in that module as well. I actually have a similar objection to autofs, but it has never caused enough problems for me to try fixing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lopsided98

  1. You don't have to rebuild the world to override systemd. You can just set systemd.package = pkgs.systemd.override { withEfi = false; };
  2. Hibernation isn't really something we have a module for. It's just something systemd is able to do. So simply having systemctl means a user is able to attempt to hibernate.

We could have an option like boot.initrd.systemd.efiSupport with a default of cfg.package.withEfi so that it can be reasonably overridden. I still think this is wrong, however. Ideally, a system that doesn't have EFI should have systemd.package set to a systemd override with EFI disabled.

Alternatively, we could have a new option like boot.initrd.optionalKernelModules which lists modules that should be included if the kernel build has them. This would be helpful in other situations as well. Like the all-hardware.nix profile breaks when you build with the RPi kernel, since it disables many of the modules that all-hardware.nix has in availableKernelModules.

Copy link
Member

@arianvp arianvp Mar 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually have a similar objection to autofs, but it has never caused enough problems for me to try fixing it.

autofs is required for systemd to boot as far as I am aware. as systemd ships with some required .automount units

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually have a similar objection to autofs, but it has never caused enough problems for me to try fixing it.

autofs is required for systemd to boot as far as I am aware. as systemd ships with some required .automount units

From the docs on CONFIG_AUTOFS_FS:

The automounter is a tool to automatically mount remote file systems on demand. This implementation is partially kernel-based to reduce overhead in the already-mounted case; this is unlike the BSD automounter (amd), which is a pure user space daemon.

To use the automounter you need the user-space tools from https://www.kernel.org/pub/linux/daemons/autofs/; you also want to answer Y to "NFS file system support", below.

To compile this support as a module, choose M here: the module will be called autofs.

If you are not a part of a fairly large, distributed network or don't have a laptop which needs to dynamically reconfigure to the local network, you probably do not need an automounter, and can say N here.

I only see one .automount unit that ships with systemd and it looks trivially replaceable.


boot.kernelParams = [
"root=${config.boot.initrd.systemd.root}"
] ++ lib.optional (config.boot.resumeDevice != "") "resume=${config.boot.resumeDevice}";

boot.initrd.systemd = {
initrdBin = [pkgs.bash pkgs.coreutils cfg.package.kmod cfg.package];
Expand Down Expand Up @@ -554,7 +577,5 @@ in {
serviceConfig.Type = "oneshot";
};
};

boot.kernelParams = lib.mkIf (config.boot.resumeDevice != "") [ "resume=${config.boot.resumeDevice}" ];
};
}
2 changes: 1 addition & 1 deletion nixos/tests/hibernate.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ makeTest {
virtualisation.useNixStoreImage = true;

swapDevices = lib.mkOverride 0 [ { device = "/dev/vdc"; options = [ "x-systemd.makefs" ]; } ];
boot.resumeDevice = "/dev/vdc";
boot.initrd.systemd.enable = systemdStage1;
virtualisation.useEFIBoot = true;
};
};

Expand Down
1 change: 1 addition & 0 deletions nixos/tests/installer-systemd-stage-1.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
clevisLuksFallback
clevisZfs
clevisZfsFallback
gptAutoRoot
;

}
42 changes: 40 additions & 2 deletions nixos/tests/installer.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ let
testScriptFun = { bootLoader, createPartitions, grubDevice, grubUseEfi, grubIdentifier
, postInstallCommands, preBootCommands, postBootCommands, extraConfig
, testSpecialisationConfig, testFlakeSwitch, clevisTest, clevisFallbackTest
, disableFileSystems
}:
let
qemu-common = import ../lib/qemu-common.nix { inherit (pkgs) lib pkgs; };
Expand Down Expand Up @@ -163,7 +164,7 @@ let
${createPartitions}
with subtest("Create the NixOS configuration"):
machine.succeed("nixos-generate-config --root /mnt")
machine.succeed("nixos-generate-config ${optionalString disableFileSystems "--no-filesystems"} --root /mnt")
machine.succeed("cat /mnt/etc/nixos/hardware-configuration.nix >&2")
machine.copy_from_host(
"${ makeConfig {
Expand Down Expand Up @@ -433,6 +434,7 @@ let
, testFlakeSwitch ? false
, clevisTest ? false
, clevisFallbackTest ? false
, disableFileSystems ? false
}:
makeTest {
inherit enableOCR;
Expand Down Expand Up @@ -541,7 +543,8 @@ let
testScript = testScriptFun {
inherit bootLoader createPartitions postInstallCommands preBootCommands postBootCommands
grubDevice grubIdentifier grubUseEfi extraConfig
testSpecialisationConfig testFlakeSwitch clevisTest clevisFallbackTest;
testSpecialisationConfig testFlakeSwitch clevisTest clevisFallbackTest
disableFileSystems;
};
};

Expand Down Expand Up @@ -1414,4 +1417,39 @@ in {
};
};
};

gptAutoRoot = let
rootPartType = {
ia32 = "44479540-F297-41B2-9AF7-D131D5F0458A";
x64 = "4F68BCE3-E8CD-4DB1-96E7-FBCAF984B709";
arm = "69DAD710-2CE4-4E3C-B16C-21A1D49ABED3";
aa64 = "B921B045-1DF0-41C3-AF44-4C6F280D3FAE";
}.${pkgs.stdenv.hostPlatform.efiArch};
in makeInstallerTest "gptAutoRoot" {
disableFileSystems = true;
createPartitions = ''
machine.succeed(
"sgdisk --zap-all /dev/vda",
"sgdisk --new=1:0:+100M --typecode=0:ef00 /dev/vda", # /boot
"sgdisk --new=2:0:+1G --typecode=0:8200 /dev/vda", # swap
"sgdisk --new=3:0:+5G --typecode=0:${rootPartType} /dev/vda", # /
"udevadm settle",
"mkfs.vfat /dev/vda1",
"mkswap /dev/vda2 -L swap",
"swapon -L swap",
"mkfs.ext4 -L root /dev/vda3",
"udevadm settle",
"mount /dev/vda3 /mnt",
"mkdir -p /mnt/boot",
"mount /dev/vda1 /mnt/boot"
)
'';
bootLoader = "systemd-boot";
extraConfig = ''
boot.initrd.systemd.root = "gpt-auto";
boot.initrd.supportedFilesystems = ["ext4"];
'';
};
}
2 changes: 1 addition & 1 deletion pkgs/os-specific/linux/systemd/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ stdenv.mkDerivation (finalAttrs: {
# needed - and therefore `interfaceVersion` should be incremented.
interfaceVersion = 2;

inherit withBootloader withCryptsetup withHostnamed withImportd withKmod
inherit withBootloader withCryptsetup withEfi withHostnamed withImportd withKmod
withLocaled withMachined withPortabled withTimedated withUtmp util-linux kmod kbd;

tests = {
Expand Down
Loading