Skip to content

Replace EFI boot entry during conversion #1466

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.

Sign up for GitHub

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

Jump to bottom
Open
matejmatuska wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Replace EFI boot entry during conversion #1466

matejmatuska wants to merge 6 commits into from

Conversation

@matejmatuska
Copy link
Member

@matejmatuska matejmatuska commented Dec 10, 2025 •
edited
Loading

After an upgrade+conversion we want the EFI boot entry to have the name of the target OS (e.g. Red Hat Enterprise Linux, when converting from CS->RHEL). The source distro boot entry and also EFI directory (e.g. /boot/efi/EFI/centos/) should be removed.

This PR:

  • adds a new library - common.firmware - contains is_efi and is_bios
  • splits out the functions in common.grub library that are not related to grub to 2 new libraries:
    • common.firmware.efi - functions related to EFI, StopActorExecutionErrors changed to EFIError
    • common.partitons (name suggestions are) welcome - operations on disks/parititions - this is used by both efi and grub lib
  • refactor existing efi functions in the addarmbootloaderworkaround and removeupgradebootentry actors into the new efi lib
  • adds a new actor that:
    • Removes the EFI dir of the source distro (e.g. /boot/efi/EFI/centos/).
    • Removes the EFI boot entry of the source distro.
    • Adds a new EFI boot entry for the target distro.
      • The contents of the target distro EFI dir are brought in by the grub-efi-x64 and shim-x64 packages.
  • adds another actor that inhibits conversions on systems with secure boot enabled
    • At least for now, I am going with an inhibitor as this current solution doesn't work with secureboot enabled. The problem probably is, in my understanding, that the shim and grub only contain keys for verifying signatures of the source distro. Our upgrade boot entry uses the target distros kernel, which is signed by the target distro. And so the shim/grub of the source distro rejects it.
    • A possible solution IMO is to create an EFI boot entry for the upgrade pointing to the target distro's shim, similarly to how it's done in the addarmbootloaderworkaround actor.

Jira: RHEL-133549

TODO

  • ARM - should work
  • secure boot - For now inhibited, see explanation above.
  • secure boot inhibitor - Need to get a better link for the report.

@github-actions
Copy link

github-actions bot commented Dec 10, 2025

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the leapp-repository contribution and development guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

  • review please @oamg/developers to notify leapp developers of the review request
  • /packit copr-build to submit a public copr build using packit

Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build.
However, here are additional useful commands for packit:

  • /packit test to re-run manually the default tests
  • /packit retest-failed to re-run failed tests manually
  • /packit test oamg/leapp#42 to run tests with leapp builds for the leapp PR#42 (default is latest upstream - main - build)

Note that first time contributors cannot run tests automatically - they need to be started by a reviewer.

It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported, beaker-minimal and kernel-rt, both can be used to be run on all upgrade paths or just a couple of specific ones.
To launch on-demand tests with packit:

  • /packit test --labels kernel-rt to schedule kernel-rt tests set for all upgrade paths
  • /packit test --labels beaker-minimal-8.10to9.4,kernel-rt-8.10to9.4 to schedule kernel-rt and beaker-minimal test sets for 8.10->9.4 upgrade path

See other labels for particular jobs defined in the .packit.yaml file.

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra.

@matejmatuska
Copy link
Member Author

matejmatuska commented Dec 15, 2025 •
edited
Loading

I've found an interesting setup when testing CS9->RHEL10.1 on particular VM instance (ping me for details). The CS entry is already called Red Hat Enterprise Linux on the source system despite it pointing to the CS efidir: \EFI\centos\shimaa64.efi. efibootmgr also warned about this when creating our entry even though it doesn't seem to be a technical problem:
efibootmgr: ** Warning ** : Boot0002 has same label Red Hat Enterprise Linux

The entry was correctly removed by the actor, because we removed it based on BootCurrent boot entry number. Just noting.

@pirat89 pirat89 added this to the 8.10/9.8 milestone Dec 17, 2025
@matejmatuska matejmatuska force-pushed the convert-efi-entry branch 2 times, most recently from 9bae012 to 2d04536 Compare December 18, 2025 16:24
@matejmatuska
Split grub lib into multiple dedicated libs
1968c5f 
The grub library has been used for all things related to booting,
including also utilities related to EFI and "partitions".

This patch splits the unrelated information grub library into multiple
dedicated libraries:
- grub - now only grub related utilities
- firmware - contains the is_efi and is_bios helpers
- firmware/efi - utilities related to EFI
- partitions - contains utilities related to partitions and devices, used by
               both grub and efi
@matejmatuska
squash! Split grub lib into multiple dedicated libs
9d4e52a 
Functions in efi library now raise efi.EFIError instead of
StopActorExecutionError.
@matejmatuska
lib/efi: Refactor EFI related functions into the efi shared lib
9d6cd92
@matejmatuska
Update EFI entry and directory during conversion
b91aecd 
After an upgrade+conversion, the upgraded system might end up with an
EFI boot loader entry from the source system pointing the shim or grub
EFI binary from the source system. E.g. Centos Stream pointing to
\EFI\centos\shimx64.efi instead of Red Hat Enterprise Linux pointing to
\EFI\redhat\shimx64.efi.

This patch introduces a new convert/updateefi actor that adds a new EFI
boot entry for the target system with the correct EFI binary, removes
the source system directory on ESP, e.g /boot/efi/EFI/centos, and
finally removes the source system EFI boot entry.

This is handled for both x86_64 and aarch64 EFI systems.

This currently does not work with SecureBoot enabled. The problem
probably is, in my understanding, that the shim and grub only contain
keys for verifying signatures of the source distro. Our upgrade boot
entry uses the target distros kernel, which is signed by the target
distro. And so the shim/grub of the source distro rejects it.

Jira: RHEL-133549
@matejmatuska
convert/updateefi: Prevent efi_finalization_fix from setting BootNext
a00d647
@matejmatuska
Inhibit conversions with secure boot enabled
8ae0a4d 
The EFI bootloader entry replacement during conversion, handled by the
convert/updateefi actor, does currently not handle secure boot. This
patch adds the convert/securebootinbihit actor that inhibits secureboot
during conversion, regular upgrade is not affected.

Jira: RHEL-133549
@matejmatuska
Copy link
Member Author

matejmatuska commented Dec 22, 2025

On python3.6 linter complains if I don't add pylint: disable no-self-use and on the other tested version it complains that the check has been moved to pylint extensions. Not sure if we have a way to resolve that.

I will maybe need to move the test out of the class, but then the fixture name needs to be uglier.

@matejmatuska matejmatuska added the enhancement New feature or request label Dec 22, 2025
@matejmatuska
Copy link
Member Author

matejmatuska commented Dec 22, 2025 •
edited
Loading

I now consider this ready for full review, there are just some minor things I will change (reports and the linter problem mostly).

@matejmatuska matejmatuska marked this pull request as ready for review December 22, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

8.10/9.8

Development

Successfully merging this pull request may close these issues.

2 participants

@matejmatuska @pirat89