Skip to content

Systemd netboot#270611

Merged
lheckemann merged 1 commit intoNixOS:masterfrom
astro:systemd-netboot
Feb 27, 2024
Merged

Systemd netboot#270611
lheckemann merged 1 commit intoNixOS:masterfrom
astro:systemd-netboot

Conversation

@astro
Copy link
Copy Markdown
Contributor

@astro astro commented Nov 28, 2023

Description of changes

Allows the netboot module to work with boot.initrd.systemd.enable

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Priorities

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions 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/` labels Nov 28, 2023
Copy link
Copy Markdown
Member

@NickCao NickCao left a comment

Choose a reason for hiding this comment

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

Please take a look at #233707. Part of the boot failure is due to depends not being honored.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 28, 2023
@astro
Copy link
Copy Markdown
Contributor Author

astro commented Nov 28, 2023

Please take a look at #233707. Part of the boot failure is due to depends not being honored.

Note that this PR here is about netboot.nix, not qemu-vm.nix.

x-systemd.requires-mounts-for= was in my first attempt as well but it seemed optional in the end.

Prepending the overlayfs parameters with /sysroot is identical.

In the other PR I am missing ensuring the existence of the upperdir and workdir. overlayfs won't create them by itself but it insists on them being there.

@ElvishJerricco
Copy link
Copy Markdown
Contributor

ElvishJerricco commented Nov 29, 2023

The point is that you're using depends wrong. It should be a list of stage 2 paths. NixOS is supposed to take care of prepending the stage 1 /sysroot / /mnt-root for you. This is for depends though, not the overlay mount options. What you've done will necessarily depend on /sysroot/sysroot/foo instead, once depends is implemented for systemd; which, I might add, it is not, so this misuse of depends isn't even doing anything.

@astro
Copy link
Copy Markdown
Contributor Author

astro commented Dec 1, 2023

The point is that you're using depends wrong. It should be a list of stage 2 paths. NixOS is supposed to take care of prepending the stage 1 /sysroot / /mnt-root for you. This is for depends though, not the overlay mount options. What you've done will necessarily depend on /sysroot/sysroot/foo instead, once depends is implemented for systemd; which, I might add, it is not, so this misuse of depends isn't even doing anything.

@ElvishJerricco Right, depends doesn't need to be changed. I dropped that.

Copy link
Copy Markdown
Member

@lheckemann lheckemann Dec 6, 2023

Choose a reason for hiding this comment

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

Could we continue testing with the scripted stage-1 (additionally)? I love that we're enabling use of systemd stage-1, but I'd not like if the scripted stage-1 bitrotted before being removed :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, I dropped that change.

If you tell me where I could still add it as a separate test.

Copy link
Copy Markdown
Member

@lheckemann lheckemann Dec 11, 2023

Choose a reason for hiding this comment

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

Maybe renaming extraConfig to extraMachineConfig and adding an extraSystemConfig argument which gets added to the modules list, then adding another test called uefiNetbootSystemd or something.

Or if you're in the mood for bigger refactors, making use of how the test thing is based on the module system nowadays, but I'm definitely fine for that not to happen now.

Copy link
Copy Markdown
Member

@lheckemann lheckemann left a comment

Choose a reason for hiding this comment

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

It's just occurred to me that this would probably break switching configs in the booted system, since /sysroot won't exist in stage-2. We should probably test this, and if my suspicion is correct, work out how to get around it (maybe an initrd-specific mount unit?)

@lheckemann
Copy link
Copy Markdown
Member

#273642 related

@lheckemann
Copy link
Copy Markdown
Member

systemd/systemd#30377 would also eliminate the need for the mkdir service, at least if there's a solution for "what root are the referenced dirs from".

lheckemann added a commit to lheckemann/nixpkgs that referenced this pull request Dec 11, 2023
The mkdir-rw-store service is copied from NixOS#270611.

This needs to tested thoroughly (and maybe made optional? Though I'd
prefer not to) before it can be merged.

Co-Authored-By: Astro <astro@spaceboyz.net>
@lheckemann
Copy link
Copy Markdown
Member

Adrian Vovk has pointed out that if we mount the tmpfs under /run, it will be carried over from the initrd into the final system, which solves the config-switching problem. This sounds like a solid approach to me, and I'll be giving it a shot for my work on the ISO image.

@ElvishJerricco
Copy link
Copy Markdown
Contributor

Why is this only needed for systemd stage 1? Why don't we need /mnt-root in the overlay's mount options for scripted stage 1? Why don't we need any boot.initrd.*Commands to do that mkdir for scripted stage 1?

@lheckemann
Copy link
Copy Markdown
Member

Because stage-1-init.sh has specific support for

Adding the prefix to the overlay component directories

local optionsPrefixed="$( echo "$optionsFiltered" | sed -E 's#\<(lowerdir|upperdir|workdir)=#\1=/mnt-root#g' )"

and for creating the upper and work dirs if they don't exist

# Create backing directories for overlayfs
if [ "$fsType" = overlay ]; then
for i in upper work; do
dir="$( echo "$optionsPrefixed" | grep -o "${i}dir=[^,]*" )"
mkdir -m 0700 -p "${dir##*=}"
done
fi

@ElvishJerricco
Copy link
Copy Markdown
Contributor

Now that #286176 is merged, we should just use that in this PR.

@astro
Copy link
Copy Markdown
Contributor Author

astro commented Feb 27, 2024

Updated to just use #286176 which really works well!

@lheckemann lheckemann merged commit 98684f4 into NixOS:master Feb 27, 2024
@astro astro deleted the systemd-netboot branch February 28, 2024 11:03
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 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

Development

Successfully merging this pull request may close these issues.

4 participants