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/systemd-boot: Add mirroredBoots #246897

Closed
wants to merge 2 commits into from
Closed

nixos/systemd-boot: Add mirroredBoots #246897

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4418de4
nixos/systemd-boot: Add mirroredBoots
Gerg-L Aug 3, 2023
b37413a
nixosTest testing
Gerg-L Aug 6, 2023
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
44 changes: 22 additions & 22 deletions nixos/modules/system/boot/loader/systemd-boot/systemd-boot-builder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ def generation_conf_filename(profile: Optional[str], generation: int, specialisa


def write_loader_conf(profile: Optional[str], generation: int, specialisation: Optional[str]) -> None:
with open("@efiSysMountPoint@/loader/loader.conf.tmp", 'w') as f:
with open("@mountPoint@/loader/loader.conf.tmp", 'w') as f:
if "@timeout@" != "":
f.write("timeout @timeout@\n")
f.write("default %s\n" % generation_conf_filename(profile, generation, specialisation))
if not @editor@:
f.write("editor 0\n");
f.write("console-mode @consoleMode@\n");
os.rename("@efiSysMountPoint@/loader/loader.conf.tmp", "@efiSysMountPoint@/loader/loader.conf")
os.rename("@mountPoint@/loader/loader.conf.tmp", "@mountPoint@/loader/loader.conf")


def profile_path(profile: Optional[str], generation: int, specialisation: Optional[str], name: str) -> str:
Expand All @@ -81,7 +81,7 @@ def copy_from_profile(profile: Optional[str], generation: int, specialisation: O
store_dir = os.path.basename(os.path.dirname(store_file_path))
efi_file_path = "/efi/nixos/%s-%s.efi" % (store_dir, suffix)
if not dry_run:
copy_if_not_exists(store_file_path, "@efiSysMountPoint@%s" % (efi_file_path))
copy_if_not_exists(store_file_path, "@mountPoint@%s" % (efi_file_path))
return efi_file_path


Expand Down Expand Up @@ -117,7 +117,7 @@ def write_entry(profile: Optional[str], generation: int, specialisation: Optiona

try:
append_initrd_secrets = profile_path(profile, generation, specialisation, "append-initrd-secrets")
subprocess.check_call([append_initrd_secrets, "@efiSysMountPoint@%s" % (initrd)])
subprocess.check_call([append_initrd_secrets, "@mountPoint@%s" % (initrd)])
except FileNotFoundError:
pass
except subprocess.CalledProcessError:
Expand All @@ -129,7 +129,7 @@ def write_entry(profile: Optional[str], generation: int, specialisation: Optiona
f'for "{title} - Configuration {generation}", an older generation', file=sys.stderr)
print("note: this is normal after having removed "
"or renamed a file in `boot.initrd.secrets`", file=sys.stderr)
entry_file = "@efiSysMountPoint@/loader/entries/%s" % (
entry_file = "@mountPoint@/loader/entries/%s" % (
generation_conf_filename(profile, generation, specialisation))
tmp_path = "%s.tmp" % (entry_file)
kernel_params = "init=%s " % profile_path(profile, generation, specialisation, "init")
Expand Down Expand Up @@ -188,13 +188,13 @@ def get_specialisations(profile: Optional[str], generation: int, _: Optional[str


def remove_old_entries(gens: List[SystemIdentifier]) -> None:
rex_profile = re.compile("^@efiSysMountPoint@/loader/entries/nixos-(.*)-generation-.*\.conf$")
rex_generation = re.compile("^@efiSysMountPoint@/loader/entries/nixos.*-generation-([0-9]+)(-specialisation-.*)?\.conf$")
rex_profile = re.compile("^@mountPoint@/loader/entries/nixos-(.*)-generation-.*\.conf$")
rex_generation = re.compile("^@mountPoint@/loader/entries/nixos.*-generation-([0-9]+)(-specialisation-.*)?\.conf$")
known_paths = []
for gen in gens:
known_paths.append(copy_from_profile(*gen, "kernel", True))
known_paths.append(copy_from_profile(*gen, "initrd", True))
for path in glob.iglob("@efiSysMountPoint@/loader/entries/nixos*-generation-[1-9]*.conf"):
for path in glob.iglob("@mountPoint@/loader/entries/nixos*-generation-[1-9]*.conf"):
if rex_profile.match(path):
prof = rex_profile.sub(r"\1", path)
else:
Expand All @@ -205,7 +205,7 @@ def remove_old_entries(gens: List[SystemIdentifier]) -> None:
continue
if not (prof, gen_number, None) in gens:
os.unlink(path)
for path in glob.iglob("@efiSysMountPoint@/efi/nixos/*"):
for path in glob.iglob("@mountPoint@/efi/nixos/*"):
if not path in known_paths and not os.path.isdir(path):
os.unlink(path)

Expand Down Expand Up @@ -252,14 +252,14 @@ def main() -> None:

if os.getenv("NIXOS_INSTALL_BOOTLOADER") == "1":
# bootctl uses fopen() with modes "wxe" and fails if the file exists.
if os.path.exists("@efiSysMountPoint@/loader/loader.conf"):
os.unlink("@efiSysMountPoint@/loader/loader.conf")
if os.path.exists("@mountPoint@/loader/loader.conf"):
os.unlink("@mountPoint@/loader/loader.conf")

subprocess.check_call(["@systemd@/bin/bootctl", "--esp-path=@efiSysMountPoint@"] + bootctl_flags + ["install"])
subprocess.check_call(["@systemd@/bin/bootctl", "--esp-path=@mountPoint@"] + bootctl_flags + ["install"])
else:
# Update bootloader to latest if needed
available_out = subprocess.check_output(["@systemd@/bin/bootctl", "--version"], universal_newlines=True).split()[2]
installed_out = subprocess.check_output(["@systemd@/bin/bootctl", "--esp-path=@efiSysMountPoint@", "status"], universal_newlines=True)
installed_out = subprocess.check_output(["@systemd@/bin/bootctl", "--esp-path=@mountPoint@", "status"], universal_newlines=True)

# See status_binaries() in systemd bootctl.c for code which generates this
installed_match = re.search(r"^\W+File:.*/EFI/(?:BOOT|systemd)/.*\.efi \(systemd-boot ([\d.]+[^)]*)\)$",
Expand All @@ -284,10 +284,10 @@ def main() -> None:
print("skipping systemd-boot update to %s because of known regression" % available_version)
else:
print("updating systemd-boot from %s to %s" % (installed_version, available_version))
subprocess.check_call(["@systemd@/bin/bootctl", "--esp-path=@efiSysMountPoint@"] + bootctl_flags + ["update"])
subprocess.check_call(["@systemd@/bin/bootctl", "--esp-path=@mountPoint@"] + bootctl_flags + ["update"])

mkdir_p("@efiSysMountPoint@/efi/nixos")
mkdir_p("@efiSysMountPoint@/loader/entries")
mkdir_p("@mountPoint@/efi/nixos")
mkdir_p("@mountPoint@/loader/entries")

gens = get_generations()
for profile in get_profiles():
Expand All @@ -309,9 +309,9 @@ def main() -> None:
else:
raise e

for root, _, files in os.walk('@efiSysMountPoint@/efi/nixos/.extra-files', topdown=False):
relative_root = root.removeprefix("@efiSysMountPoint@/efi/nixos/.extra-files").removeprefix("/")
actual_root = os.path.join("@efiSysMountPoint@", relative_root)
for root, _, files in os.walk('@mountPoint@/efi/nixos/.extra-files', topdown=False):
relative_root = root.removeprefix("@mountPoint@/efi/nixos/.extra-files").removeprefix("/")
actual_root = os.path.join("@mountPoint@", relative_root)

for file in files:
actual_file = os.path.join(actual_root, file)
Expand All @@ -324,17 +324,17 @@ def main() -> None:
os.rmdir(actual_root)
os.rmdir(root)

mkdir_p("@efiSysMountPoint@/efi/nixos/.extra-files")
mkdir_p("@mountPoint@/efi/nixos/.extra-files")

subprocess.check_call("@copyExtraFiles@")

# Since fat32 provides little recovery facilities after a crash,
# it can leave the system in an unbootable state, when a crash/outage
# happens shortly after an update. To decrease the likelihood of this
# event sync the efi filesystem after each update.
rc = libc.syncfs(os.open("@efiSysMountPoint@", os.O_RDONLY))
rc = libc.syncfs(os.open("@mountPoint@", os.O_RDONLY))
if rc != 0:
print("could not sync @efiSysMountPoint@: {}".format(os.strerror(rc)), file=sys.stderr)
print("could not sync @mountPoint@: {}".format(os.strerror(rc)), file=sys.stderr)


if __name__ == '__main__':
Expand Down
44 changes: 31 additions & 13 deletions nixos/modules/system/boot/loader/systemd-boot/systemd-boot.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ let

python3 = pkgs.python3.withPackages (ps: [ ps.packaging ]);

systemdBootBuilder = pkgs.substituteAll {
systemdBootBuilder = mountPoint: pkgs.substituteAll {
src = ./systemd-boot-builder.py;

isExecutable = true;
Expand All @@ -28,7 +28,9 @@ let

inherit (cfg) consoleMode graceful;

inherit (efi) efiSysMountPoint canTouchEfiVariables;
inherit (efi) canTouchEfiVariables;

inherit mountPoint;

inherit (config.system.nixos) distroName;

Expand All @@ -40,33 +42,38 @@ let
empty_file=$(${pkgs.coreutils}/bin/mktemp)

${concatStrings (mapAttrsToList (n: v: ''
${pkgs.coreutils}/bin/install -Dp "${v}" "${efi.efiSysMountPoint}/"${escapeShellArg n}
${pkgs.coreutils}/bin/install -D $empty_file "${efi.efiSysMountPoint}/efi/nixos/.extra-files/"${escapeShellArg n}
${pkgs.coreutils}/bin/install -Dp "${v}" "${mountPoint}/"${escapeShellArg n}
${pkgs.coreutils}/bin/install -D $empty_file "${mountPoint}/efi/nixos/.extra-files/"${escapeShellArg n}
'') cfg.extraFiles)}

${concatStrings (mapAttrsToList (n: v: ''
${pkgs.coreutils}/bin/install -Dp "${pkgs.writeText n v}" "${efi.efiSysMountPoint}/loader/entries/"${escapeShellArg n}
${pkgs.coreutils}/bin/install -D $empty_file "${efi.efiSysMountPoint}/efi/nixos/.extra-files/loader/entries/"${escapeShellArg n}
${pkgs.coreutils}/bin/install -Dp "${pkgs.writeText n v}" "${mountPoint}/loader/entries/"${escapeShellArg n}
${pkgs.coreutils}/bin/install -D $empty_file "${mountPoint}/efi/nixos/.extra-files/loader/entries/"${escapeShellArg n}
'') cfg.extraEntries)}
'';
};

checkedSystemdBootBuilder = pkgs.runCommand "systemd-boot" {
checkedSystemdBootBuilder = mountPoint: pkgs.runCommand "systemd-boot" {
nativeBuildInputs = [ pkgs.mypy python3 ];
} ''
install -m755 ${systemdBootBuilder} $out
install -m755 ${systemdBootBuilder mountPoint} $out
mypy \
--no-implicit-optional \
--disallow-untyped-calls \
--disallow-untyped-defs \
$out
'';

finalSystemdBootBuilder = pkgs.writeScript "install-systemd-boot.sh" ''
#!${pkgs.runtimeShell}
${checkedSystemdBootBuilder} "$@"
${cfg.extraInstallCommands}
'';
finalSystemdBootBuilder = let
installDirs =
if cfg.mirroredBoots != []
then cfg.mirroredBoots
else [efi.efiSysMountPoint];
Comment on lines +68 to +71
Copy link
Contributor

Choose a reason for hiding this comment

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

My first thought was that mirroredBoots should be additional mountpoints, but seeing this I guess the implementation is only these mountpoints (the "main" ESP mountpoint gets ignored)? Am I the only one that find that surprising?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, and AFAIU the grub.nix case, mirroredBoots are additionals mountpoints there:

nixpkgs/nixos/modules/system/boot/loader/grub/grub.nix

Lines 708 to 710 in ec6cc9c

boot.loader.grub.mirroredBoots = optionals (cfg.devices != [ ]) [
{ path = "/boot"; inherit (cfg) devices; inherit (efi) efiSysMountPoint; }
];

in
pkgs.writeShellScript "install-systemd-boot.sh"
(lib.concatMapStrings (x: "${checkedSystemdBootBuilder x} \"$@\"\n") installDirs)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that, like in grub.nix, the usual error detection set -e should be activated, and maybe also set -u.

Why keeping on passing $@? I don't see any argument neither used, nor passed (so far).

Nitpick: here each mountpoint generates a new derivation, the mountpoint could instead be passed as an envvar mountpoint=${escapeShellArg x} ${checkedSystemdBootBuilder}. Again that's likely just a few derivations in practice, so just a nitpick.

+ cfg.extraInstallCommands;

in {

imports =
Expand Down Expand Up @@ -238,6 +245,17 @@ in {
'';
};

mirroredBoots = lib.mkOption {
type = lib.types.listOf lib.types.str;
Copy link
Contributor

Choose a reason for hiding this comment

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

grub.nix's mirroredBoots' type is more sophisticated, maybe it would be more correct to use the same or a subset.

default = [];
example = ''
[ "/boot1" "/boot2" ]
'';
description = lib.mdDoc ''
Mirror the boot configuration to multiple locations.
'';
};

};

config = mkIf cfg.enable {
Expand Down