-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
systemd-boot: allow splitting entries to xbootldr part #226692
Conversation
I'm partly getting this out there, possibly still too late, just to make sure it's a known use-case regarding bootspec... I'm trying to get caught back up to NixOS-world... |
I believe XBOOTLDR is already supported by bootspec. |
I will likely abandon this whenever this is resolved on lanzaboot: nix-community/lanzaboote#173 |
Hi, what is needed to get this PR over the finish line? My EFI partition is only around 100MB thanks to Windows, and in a virtual machine, I can see that NixOS already needs 90MB from two kernels. |
@sdht0 as I noted in the linked lanzaboote issue you can use this to workaround for now:
|
Ah neat, thanks. |
I think I'd prefer to have naming closer to the spec stuff; like |
09a67b1
to
035eeaa
Compare
@ElvishJerricco rebased, and updated per your suggestion. It's just a rename but I can end-to-end test this. My only machine with xboot part is now using lanzaboote. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
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 did not manage to get the bind mount trick working (entries were not appearing in the bootloader), so I used an override instead by copying the two files from nixpkgs:
disabledModules = [ "system/boot/loader/systemd-boot/systemd-boot.nix" ];
imports = [ ./systemd-boot.nix ];
@@ -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, "@entriesMountPoint@%s" % (efi_file_path)) |
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.
s/entriesMountPoint/xbootMountPoint/ everywhere no?
|
||
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("@entriesMountPoint@", os.O_RDONLY)) |
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.
Needs a second syncfs command:
if "@xbootMountPoint@" != "@efiSysMountPoint@":
rc = libc.syncfs(os.open("@efiSysMountPoint@", os.O_RDONLY))
if rc != 0:
print("could not sync @efiSysMountPoint@: {}".format(os.strerror(rc)), file=sys.stderr)
@@ -286,8 +286,8 @@ def main() -> None: | |||
print("updating systemd-boot from %s to %s" % (installed_version, available_version)) | |||
subprocess.check_call(["@systemd@/bin/bootctl", "--esp-path=@efiSysMountPoint@"] + bootctl_flags + ["update"]) |
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.
Add an explicit boot-path
?
if "@xbootMountPoint@" != "@efiSysMountPoint@":
bootctl_flags.append("--boot-path=@xbootMountPoint@")
@sdht0 do you want to take this over maybe? You left a good review and I clearly didn't test this when I pushed it last. It might be good for you to take it over, if you want. If you want, just open a new one and CC me on it and I'll close this one in favor of it. |
I don't mind. I'll prepare the PR in the next couple of weeks, after I've added the ability to cleanup entries on a config change. Is there anything else I need to do to get it actually merged? This PR has been open for a while. |
I think I'd want another review from @ElvishJerricco, but I'm not really aware of anything else that should block it. Also, if @RaitoBezarius can weigh in on whether or not XBOOTLDR is officially supported by bootspec, or not, and whether or not that should affect the usefulness or mergability of this PR. I'm not sure I can speak to that. |
I guess one other noteworthy thing is that As a future a note, a good followup PR would be one that enables copying drivers to the |
Per se, XBOOTLDR is not formalized inside bootspec in its version 1, but you can always add it as an extension and we can document it. I don't think anything except the builder needs the information about the entries mountpoint. Drivers will also be necessary if the target entry mountpoint is not a supported FS for UEFI. |
I also think it's extremely important for that PR to have tests. |
New PR is at #260241 |
Description of changes
This allows
systemd-boot
to be installed to the ESP, while NixOS loader entries and files can exist on the extended boot loader partition.This is particularly useful on dual boot systems, where Windows is allowed to install first and establishes the ESP partition. Often times it chooses a very small size that is prohibitive with regards to NixOS's boot files.
This PR adds an option that allows the user to redirect the loader entries/files to (what is hopefully their) extended boot loader partition.
Note, I use this PR on systems where this extra partition is in use, AND systems where it is not, so I am not worried about regression(s).
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/
)