nixos/systemd-boot: Add support for an XBOOTLDR partition

[sdht0]

Description of changes

It is possible that the size of the ESP partition is very small (say 100MB), for e.g., when dual booting with Windows. Currently, systemd-boot install entries only to the ESP partition, which results in "Out of disk space" errors when trying to store multiple kernel generations. This PR adds support for an additional XBOOTLDR partition as described in the Boot Loader Specification, which helps sidestep the issue.

The changes in systemd-boot nixos module were straighforward. However, to add tests, I had to make changes to the qemu and disk image modules. Perhaps there is a less invasive way by creating a custom VM only for these specific tests? Atleast the code in the disk image module will need to be replicated though.

Thanks to @colemickens for encouraging me to further expand on his PR #226692.
Cc @ElvishJerricco @RaitoBezarius, can you take a look.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.

[sdht0]

I separated out the changes to different modules and added comments.

I also added a release note for 23.11, although I'm not sure if this will be merged in time for the release.

@ElvishJerricco @RaitoBezarius Can you take a look now? Thanks.

[sdht0]

@RaitoBezarius @ElvishJerricco Not sure if you've seen this yet. Maybe there is still time to get this merged for 23.11?

I think this is an important feature. I'm actually surprised it isn't already added. Perhaps most NixOS users don't dual boot with Windows or somehow work around the limited ESP partition size.

[sdht0]

Thanks @ElvishJerricco. Is there anything else I need to do to get this merged?

[RaitoBezarius]

Just FYI, I opened NixOS/rfcs#165 to discuss an improvement of bootspec, if you feel like XBOOTLDR can fit inside this story, please join the discussion.

[sdht0]

Thanks for the ping! I'll take a look sometime later this week.

[ElvishJerricco]

@danielfullmer are you ok being added as a maintainer of these tests?

[RaitoBezarius]

@danielfullmer are you ok being added as a maintainer of these tests?

I honestly was planning to remove Daniel from maintainers of systemd-boot (tests included) because I don't think he has been very active around them (which is fine).

@JulienMalka told me in private communications that he may be interested in taking over systemd-boot maintenance, at least, for the tests side.

[sdht0]

Ah right I copied the maintainer values too.

Actually, if you want, I can be a (co-)maintainer. I got a good enough understanding of how the systemd-boot installation and tests work and can help out with that atleast.

[RaitoBezarius]

@JulienMalka ping ^

[nixos-discourse]

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

https://discourse.nixos.org/t/boot-partition-and-mount-point-with-dual-boot-windows/38512/4

[JulienMalka]

Friendly ping @ElvishJerricco, your currently standing review is the last blocker for this.

[nyabinary]

Friendly ping @ElvishJerricco, your currently standing review is the last blocker for this.

It also needs a rebase :3

[ElvishJerricco]

@sdht0 I've gone ahead and rebased, resolved conflicts, and cleaned up the git history in my branch: master...ElvishJerricco:nixpkgs:systemd-boot-xbootldr

The only significant conflict was a slight refactor of systemd-boot-builder.py, so I edited the changes to adhere to that refactor. Do you mind if I push that to this PR?

[sdht0]

@ElvishJerricco nit: maybe remove the stray /EFI capitalization if it's lowercase everywhere else, otherwise looks good. Alright. let's do this!

[ElvishJerricco]

maybe remove the stray /EFI capitalization if it's lowercase everywhere else

Fair enough. Though, I would like to have that change eventually, because if you load a driver for a case-sensitive file system, it does actually matter. We can leave it for later though

[ElvishJerricco]

@JulienMalka It looks good to me now. Feel free to double check since I rebased it.

@RaitoBezarius

I'm not a big fan of how intrusive supporting XBOOTLDR is, it seems fragile, and I am not sure any of the active maintainer will be able to support it, so please be certain that you will be around to actively tend to it if we end up merging this.

Are your concerns here sufficiently alleviated?

[colemickens]

@ofborg eval

[colemickens]

Meant to send this comment last night, but I tested this (the latest variable commit) on a new Rock5b install with split efi and (ext)boot and it worked perfectly.

[RaitoBezarius]

Suggested change

	partitionTableType = selectPartitionTableLayout { inherit (cfg) useDefaultFilesystems useEFIBoot; inherit (cfg) useXbootldr; };
	partitionTableType = selectPartitionTableLayout { inherit (cfg) useDefaultFilesystems useEFIBoot useXbootldr; };

[RaitoBezarius]

@JulienMalka It looks good to me now. Feel free to double check since I rebased it.

@RaitoBezarius

I'm not a big fan of how intrusive supporting XBOOTLDR is, it seems fragile, and I am not sure any of the active maintainer will be able to support it, so please be certain that you will be around to actively tend to it if we end up merging this.

Are your concerns here sufficiently alleviated?

No, the QEMU VM is still too much from my personal perspective, but I won't block on that, if we have any issue with this, I will just ping the maintainer at that time and if we cannot get it to work, I will schedule it for removal.

@nikstur and I have been planning a bunch of overhauls for the QEMU VM framework interface, and we would like to keep the module modifications to a minimal until we can get started.

In my ideal world, XBOOTLDR should just be normal operations of ESP + a weird /boot partition and all of that has nothing to do in internals like qemu-vm.nix, I made my point, I unfortunately don't have much time to stick around.

[nikstur]

I'm really quite opposed to the changes to the qemu-vm.nix module and I also believe they are not needed.

I also think anyone should be able to quickly catch on what's happening if they understand the rest of the code.

I do not feel that way. I've read and changed the qemu-vm.nix code many times and its convoluted and confusingly mixes concepts all the time. It is extremely hard to change and even harder to change without breaking things. I feel like your change would pile on to this and make the issue worse.

Additionally, XBOOTLDR is generally not very useful for VMs because you typically can just make the ESP as large as you need in a virtual image. The only real value then lies in testing. However, I don't want to treat qemu-vm as the kitchen sink where we implement all sorts of testing features.

Instead of changing qemu-vm.nix, you should do:

• Create the image with an XBOOTLDR partition in the test that requires it by directly calling make-disk-image
• Tell the test driver to use this image instead by pointing NIX_DISK_IMAGE to your image. You can find an example of how this works here: https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/appliance-repart-image.nix#L100

You could put this in a fixture so that it can be reused by others if that's a concern.

[sdht0]

Instead of changing qemu-vm.nix, you should do:

It'll be a while before I can start looking into the suggested method. If someone can take a stab at it meanwhile, please go for it.

[sdht0]

Deleted the branch by an incorrect push, sorry.

@nikstur @RaitoBezarius After many struggles, I've managed to figure out how to avoid changes in qemu-vm. Can you please take a look at https://github.com/NixOS/nixpkgs/pull/285401/files#diff-f71ea5f910763bce18731f20cfb7fa3c6f31d4ebecf3a37c75cf410858b5bb53.

@JulienMalka @ElvishJerricco the rest of the changes remain the same in the new PR, but please do take another look.