Provide a command line option to install systemd-boot rather than grub2 on x86_64 and arm64 #4368
Conversation
|
Oh, I should add that if anyone gets far enough to try this out with libvirt/virtmanager/etc that x86 UEFI machines need to have secure boot disabled otherwise it will just silently fail to execute systemd-boot/etc. |
| # write the options above, and run a script which will merge the | ||
| # boot cmdline (after stripping inst. and BOOT_) with the anaconda settings | ||
| rc = util.execWithRedirect( | ||
| "/usr/sbin/updateloaderentries.sh", |
There was a problem hiding this comment.
I see that the ZIPL class has an update_bls_args() function that does something similar. Maybe there could be a common helper instead of having a separate script?
There was a problem hiding this comment.
This is the one piece I looked at for a half day or so, but haven't managed to merge together. I think in general you are right, both are doing something quite similar (updating existing BLS entries) but there are a few subtle differences that will need to be merged around how the command line itself is generated. Worse I suspect both have a fundamental error, in that they aren't filtering for newly installed entries which could be a problem.
I think at this point, this is probably the largest remaining issue.
|
|
||
| rc = util.execWithRedirect("bootctl", [ "install", "--esp-path=/boot/efi", | ||
| "--efi-boot-option-description=fedora", | ||
| "--loader-path=/EFI/fedora/" ], |
There was a problem hiding this comment.
I've got a local change for this at the moment, but its not clear what the upstream change will be, so I will likely drop the loader-path parts from v2, under the assumption it can be added back when upstream systemd accepts a patch.
There was a problem hiding this comment.
That discussion seems to be here: systemd/systemd#24828 now that i've removed the arm-specific kernel-install bits, which should be redundant with the compressed EFI/kernel images.
poncovka
added
manual testing required
jlinton
force-pushed
the
allow_systemd_boot_as_bootloader
branch
from
2920492
to
98d1614
Compare
|
I've updated all bits pointed out above to my own satisfaction, except for the updateloaderentries.sh bit. So, I would take the RFC note off, depending on whether we think the updateloaderentries.sh bit needs to be fixed before merging. (I'm on the fence here). |
Thanks a lot for doing a respin and addressing all the issues I pointed out! It looks good to me and in my opinion we should just merge this. It's great to have this support and since isn't the default, people using it should know what they are doing anyways.
I honestly don't think this is a big issue. It can be cleaned up later and the need for this script dropped. At the end of the day, is just an implementation detail. I've approved the changes but I'm not an Anaconda developer, just an occasional contributor. @jkonecny12 @poncovka could you folks please take a look to these patches? |
jlinton
force-pushed
the
allow_systemd_boot_as_bootloader
branch
from
98d1614
to
442cbaf
Compare
|
Please, fix the pylint issues: Also, the kickstart support should be implemented here: https://github.com/pykickstart/pykickstart |
|
Hi all, if I have correct information (which might be wrong) this will not work on SilverBlue systems. Could you please disable this in |
|
This is closely related to fedora-silverblue/issue-tracker#120 and I'm interested in making that work on Silverblue/Kinoite. |
|
I'm @jkonecny12 curious about where the problem with silverblue is? If anything, this should help with the immutable rootfs/etc because the entire config/boot process is contained on the ESP, and AFAIK loader updates can/would be via bootctl. But before anyone gets excited, this commit doesn't replace grub (or any other bootloader, it just provides the framework for testing alternate paths). |
The issue that Jiri is mentioning is that For context, take a look to ostreedev/ostree#1719 that's the RFE to support But the problem was that vfat didn't have support for https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=019a0c9e377c9f7bd477a0742706d93cdddaee4d |
|
The tests pass with the following changes: poncovka@36f73f7 |
jlinton
force-pushed
the
allow_systemd_boot_as_bootloader
branch
from
031d21c
to
1fe7309
Compare
|
I've applied the suggested changes as well as picked up test tweaks noted above, as well as adding systemd-boot to the dependencies list. I also bumped the pykickstart version in anaconda.spec.in in anticipation of the next version, but its probably causing additional failures above because the testing bot can't find a package with that version. |
|
|
||
| Systemd-boot is dead simple, it basically provides a second boot menu | ||
| and injects the kernel parms into an efi stubbed kernel, which in turn | ||
| (optionally) loads it's initrd. As such there aren't any filesystem or |
There was a problem hiding this comment.
"its"
Hmm, so this description doesn't sound right. "second boot menu": I guess you're thinking about the firmware menu, but generally it wouldn't be shown, so for normal use it's just "boot menu". Also, the kerel doesn't load the initrd, it happens earlier. Kernels with an efi stub also usually contain their own commandline, so injection of the parameters is an optional step… So this text would require some editing.
But the bigger issue is that a description of systemd-boot features shouldn't be done here. Please instead refer to some docs (https://www.freedesktop.org/software/systemd/man/systemd-stub.html, https://www.freedesktop.org/software/systemd/man/systemd-boot.html ?). The feature set and details of operation of sd-stub/boot can change over time.
I think a one-sentence description of systemd-boot would be enough.
There was a problem hiding this comment.
Yah I can move it around, but to be pedantic, the kernel-provided stub has this parameter.
https://elixir.bootlin.com/linux/v6.2-rc6/source/Documentation/admin-guide/kernel-parameters.txt#L2031
which is one of the ways to load the initrd with systemd-boot.
| ESP. This requires a larger than normal ESP, but for now we assume that | ||
| this linux installer is creating the partitions, so where the space | ||
| is allocated doesn't matter. |
There was a problem hiding this comment.
"for now we assume" — the installer is in full control, so there is no need hedge like this. Instead: "When the Linux installer is creating the partitions, it can create the ESP as large as needed."
| rc = util.execWithRedirect( | ||
| "/usr/sbin/updateloaderentries", | ||
| [" "], | ||
| root=conf.target.system_root | ||
| ) | ||
| if rc: |
There was a problem hiding this comment.
updateloaderentries is from https://bugzilla.redhat.com/show_bug.cgi?id=2134972.
As I wrote the ticket, please let's not do this at all. updateloaderentries is modifying some existing on-disk files. It would be nicer to just write the with the right contents from the start. But if we can't do that, then a bit of python should good enough:
def clean_up_loader_entry(filename, args):
with open(filename, 'r+') as f:
lines = []
for line in f:
if line.startswith('options'):
filtered = (opt for opt in line.split()[1:]
if not opt.startswith('inst.') and not opt.startswith('BOOT_IMAGE'))
lines += [' '.join((f'options {args}', *filtered)) + '\n']
else:
lines += [line]
f.seek(0)
f.write(''.join(lines))
f.truncate()There was a problem hiding this comment.
Except that bit of python doesn't replace the ability to edit the BLS entries after the machine has been booted (per grubby behavior), nor does it solve many of the other things you commented about in the above defect.
There was a problem hiding this comment.
And I don't think your code is doing what the update script does anyway, which is merge and filter the current boot command line with the existing BLS entries.
| rc = util.execWithRedirect("bootctl", [ "install", "--esp-path=/boot/efi", | ||
| "--efi-boot-option-description=" + productName.split("-")[0] ], | ||
| root=conf.target.system_root, | ||
| env_prune=['MALLOC_PERTURB_']) |
There was a problem hiding this comment.
env_prune=['MALLOC_PERTURB_']
Why is this needed?
There was a problem hiding this comment.
Hmm, now that is a good one. Its a copy paste job from grub2 install where it noted the dev/mapper gets garbage from the bios device names (see commit: 022b8b6) if MALLOC_PERTURB is set. I think I saw this, but with one of early revisions where the BLS rootfs was garbaged up when using /dev/mapper entires. So that code has been replaced now, its probably worth retesting and removing even though its probably harmless to pull out the environment variable.
| if self.get_fw_platform_size() == '32': | ||
| # not supported try a different bootloader | ||
| log.error("efi.py: systemd-boot is not supported on 32-bit platforms") | ||
| raise BootLoaderError(_("Systemd-boot is not supported on this platform")) |
There was a problem hiding this comment.
Strictly speaking, sd-boot has a 32-bit variant. I think it isn't tested much, and it's fine not to even try it from Anaconda, but it exists.
| @@ -0,0 +1,20 @@ | |||
| :Type: Kickstart Installation | |||
| :Summary: Install an image using systemd-boot rather than grub (#2135531) | |||
There was a problem hiding this comment.
The pull request meets all formal requirements, but the functionality requires a package that is not available in Fedora repositories at this moment (see the bug 2134972), so it cannot be fully tested and merged. I will mark it as blocked for now. Since the Beta Freeze of Fedora 38 started yesterday, I am affraid that this feature will have to be postponed to Fedora 39.
- Unit tests and pylint pass.
- Kickstart smoke tests pass.
- Installations with EFIGRUB are bootable.
-
inst.sdbootcannot be tested yet. -
bootloader --sdbootcannot be tested yet.
poncovka
added
blocked
Folks, can we please merge this if only for rawhide in the meantime since this is really an experimental feature and @jlinton has been working on it for months already. I'ts not expected to be used by default so IMO resolving https://bugzilla.redhat.com/show_bug.cgi?id=2134972 shouldn't https://bugzilla.redhat.com/show_bug.cgi?id=2134972 for this PR. Otherwise, the changes could bit rot and cause merge conflicts with would be more work for everyone than just merging as is now. |
|
@martinezjavier @jlinton I talked to the team and they agreed with merging it as an experimental feature, so I will start to work on that. The code will need some small changes to get it ready for F39, but I will handle it. |
poncovka
removed
manual testing required
Perfect, thanks a lot!! |
systemd-boot places all the files need for boot on the ESP, removing the need for a dedicated stage2 boot partition to hold the initrd/etc. So, lets add an option to skip the stage2 validation. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Systemd-boot is a lightweight boot selector that only runs in EFI enviroments. Compared to shim+grub its incredibly simple as it utilizes efi boot services. Lets add a subclass that anaconda can use to install it rather than grub. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
EFIGRUB is checking fw_platform_size to choose which grub, 32 or 64-bit, needs to be installed. Lets hoist that check because systemd-boot will use it to verify that the platform firmware is 64-bits. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Now that we have a systemd bootloader class lets add an efi class that can call it to install the bootloader. Then create Aarch64EFISystemBoot class that can utilize it. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Now, that we have all the infrastructure in place, lets add and document a 'inst.systemd' option that can be used on the boot commadline or placed in kickstart files. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Place a release note for the next verison of anaconda which notes that it is now possible given the right set of packages/etc to install a system utilizing systemd-boot. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
The systemd-boot install feature is for the moment largly expermental, but adding the ability to create a clean install without grub drippings is important for testing. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
poncovka
force-pushed
the
allow_systemd_boot_as_bootloader
branch
from
1fe7309
to
a9e0559
Compare
|
Updated and rebased. We should have a functional CI tomorrow, but the tests pass locally. |
|
/kickstart-test --testtype smoke |
| @@ -197,13 +203,70 @@ def write_config(self): | |||
| super().write_config() | |||
|
|
|||
|
|
|||
| class EFISystemdBoot(EFIBase, SystemdBoot): | |||
| """EFI Systemd-boot""" | |||
| _packages_common = ["efibootmgr", "systemd-udev", "systemd-boot", "sdubby"] | |||
There was a problem hiding this comment.
@jlinton should we remove the sdubby package or is this something special?
There was a problem hiding this comment.
The package should be mandatory, but it is still in review (https://bugzilla.redhat.com/show_bug.cgi?id=2134972). See the comments above.
There was a problem hiding this comment.
https://bugzilla.redhat.com/show_bug.cgi?id=2134972
It is under review
There was a problem hiding this comment.
Formally it is under review. But I think that if the review is supposed to be more than just a rubber-stamp procedure, then IMO this particular package should not pass it. It provides a concentrated dose of bad technical choices and super super brittle workarounds for shortcomings in other packages. The good thing is that all of those other packages are under our control, so we can fix things: anaconda, grub2, kernel-install, kernel scriptlets, systemd-boot. We don't need a combination of rpm packaging, bash, python, and awk to serve as glue between those other packages.
systemd-boot is a lightweight bootloader utility that understands the Linux loader syntax and populates a menu of installed boot options. It then utilizes UEFI-provided system services to run the given image. In the case of x86, arm64, riscv and others the linux kernel can be (is on fedora/RH distros) a standalone EFI executable image. With the initrd= option, the kernel loads its own initrd as well.
This simplifies the boot process significantly as UEFI already provides a fairly heavyweight environment and signing systemd-boot with platform-enrolled keys avoids much of the complexity of shim+grub loading linux filesystem drivers, console and keyboard mgmt, etc.
This set of patches has been tested on x86_64 and arm64, but it requires either an updated set of systemd packages (to name the bootloader, a stubby (aka systemd grubby) package which provides dnf ownership of some of the /boot/efi/ files, and removal of all the grub packages in the -comps.xml files. Once the system has been installed it should behave mostly as one would expect with 'dnf upgrade' and the like.
It's sorta RFC quality at the moment, as the root= options need some further tweaking, and there remains some stubby work for kdump and the like). I've also not included anything that modifies the default partitioning setups, but in the future, picking this option should remove the need for a dedicated /boot partition.
In theory, systemd-boot can then be used along with the systemd-stub to create unified signed images (as is being done at some large companies today!), but I've not tested anything in that path, and the short term goal is simply to clean up all the bits and pieces so that its possible in the F38/39 timeframe to clean install a fedora system without grub on x86 and arm while maintaining grub as the default bootloader.