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
initrd: Opt-in bare bones systemd-based initrd #164943
initrd: Opt-in bare bones systemd-based initrd #164943
Conversation
57296ec
to
5253f1c
Compare
lib = import ./systemd-lib.nix { inherit lib config pkgs; }; | ||
unitOptions = import ./systemd-unit-options.nix { inherit lib systemdUtils; }; | ||
types = import ./systemd-types.nix { inherit lib systemdUtils; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lib = import ./systemd-lib.nix { inherit lib config pkgs; }; | |
unitOptions = import ./systemd-unit-options.nix { inherit lib systemdUtils; }; | |
types = import ./systemd-types.nix { inherit lib systemdUtils; }; | |
lib = import ./systemd/lib.nix { inherit lib config pkgs; }; | |
unitOptions = import ./systemd/unit-options.nix { inherit lib systemdUtils; }; | |
types = import ./systemd/types.nix { inherit lib systemdUtils; }; |
Maybe? I am not sure. Just an idea.
d6f3bd9
to
4b4e589
Compare
f97afcd
to
954b88f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started this review yesterday, there's a chance you already touched some of the things I reviewed.
Mostly code quality nitpicks, but more test{s,ing} for [this] core system component would be nice to see.
mapAttrs' (n: v: nameValuePair "${n}.path" (pathToUnit n v)) cfg.paths | ||
// mapAttrs' (n: v: nameValuePair "${n}.service" (initrdServiceToUnit n v)) cfg.services | ||
// mapAttrs' (n: v: nameValuePair "${n}.slice" (sliceToUnit n v)) cfg.slices | ||
// mapAttrs' (n: v: nameValuePair "${n}.socket" (socketToUnit n v)) cfg.sockets | ||
// mapAttrs' (n: v: nameValuePair "${n}.target" (targetToUnit n v)) cfg.targets | ||
// mapAttrs' (n: v: nameValuePair "${n}.timer" (timerToUnit n v)) cfg.timers | ||
// listToAttrs (map | ||
(v: let n = escapeSystemdPath v.where; | ||
in nameValuePair "${n}.mount" (mountToUnit n v)) cfg.mounts) | ||
// listToAttrs (map | ||
(v: let n = escapeSystemdPath v.where; | ||
in nameValuePair "${n}.automount" (automountToUnit n v)) cfg.automounts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels pretty repetitive but it does keep the code simpler vs. a higher level approach. Not sure what to make of that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea it's not the first time this is repeated in nixos. I suppose I could factor it out into a function. Would that be good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that'd be nice! The generic-unit-ification being in what is effectively the consumer of the systemd lib seems weird.
55a6600
to
9d86e36
Compare
5d8aeef
to
89ccc7d
Compare
#164016 has been merged, so I've rebased onto its last commit and removed the WIP label from this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't really looked at the Rust code apart from my one suggestion which helped me in debugging yesterday.
The approach is very promising and apart from the first commit not really splittable anymore. Most things I comment here are so that we nail some details right at the beginning instead of people starting to work on different aspects of the initrd while the core of it is still in flux.
I'm currently discussing stuff with @tfc to make sure we can get better test coverage. While I don't think this test needs more, I'd love future PRs with more features having the ability to add more stage 1 tests.
About the switch to the real system - I don't know if this PR should already contain code to switch to the new systemd directly without the shell script in between. While this approach in this PR already works of course, it's another thing people may make assumptions about ("this worked in systemd stage-1 before!!"). As you know, I'm experimenting with this detail and will provide my findings when they come up.
89ccc7d
to
c730ab0
Compare
We can just use serviceToUnit here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use module composition instead of generated modules.
It keeps the types simple.
1ea78c4
to
1e5261f
Compare
@roberth can you take another look? I added two commits, one that does the type composition as requested and another one that adds type composition to the unit types to get rid of options that are not used |
22ac161
to
0d69a99
Compare
0d69a99
to
c465c8d
Compare
As requested by @roberth, we now have an option similar to environment.etc. There's also extra store paths to copy and a way to suppress store paths to make customizations possible. We also link mount and umount to /bin to make recovery easier when something fails
This is more in line with what dracut does (it appends "Initramfs") and makes it clear where the boot is currently at when it hangs.
@dasJ Those changes look great! |
If there are no more objections and pushes, I'll merge this in the next 24-48h |
Awesome! This unlocks so much potential improvements. Thanks a lot for all the work of everyone! 🥳🥳🥳 |
Tried this out on my laptop, it's almost usable except for two issues: |
Disable systemd as initrd to get impermanence back. Lose TPM unlock in the process. systemd as initrd explicitly does not support `pre*Commands` or `post*Commands` as stated in the PR introducing it: NixOS/nixpkgs#164943 This is evident when trying to make apply changes using those declarations, and resulting in the same generation/outputs. That's because nothing is actually changing. The scripts are never run.
Third time's the charm.
Motivation
This PR adds an option,
boot.initrd.systemd.enable
, that causes NixOS to generate an initrd that usessystemd
for PID 1. This brings several benefits:systemd-ask-password
can be used to request credentials, providing a more streamlined way for early boot tasks to communicate with users.How this compares to my previous attempts
This version of a systemd-based initrd has three major improvements over the previous attempts.
The third improvement is especially important. This PR only implements the core functionality needed to have a systemd-based initrd. There are many things that NixOS's traditional initrd supports that this does not yet. But with this framework in place, we can begin to add those features to the systemd-initrd one by one in separate, parallel PRs.
Notably, this PR does keep the
make-initrd-ng
concept from the previous one. You can check the README included with it in this PR for details, but the short version is that we don't copy full closures into initrd, and we don't patch things. Instead, we try to automate copying only the exactly correct things to their original paths to the best of our ability, and manually add what the automation misses in theboot.initrd.systemd.objects
option. The benefits are outlined in the README.This can be tested with
nix-build ./nixos -A vm
.What this approach will not do
is include a compatibility layer for options like
postDeviceCommands
and the like. Although this is possible in principle with special units that run the concatenated script withBefore=
andAfter=
clauses on the appropriate targets, I think this is a bad idea for several reason.extraUtils
and the*Commands
options alive in this initrd.Next steps
Here are a number of things to do in future PRs, in very loose, decreasing order of how I view their importance.
script
style options inboot.initrd.systemd.services
. Currently they do not copy the necessary scripts in.Most of these should be doable independently of each other, and likely need little more API than the core functionality provided by this PR.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)nixos/doc/manual/md-to-db.sh
to update generated release notes