-
Notifications
You must be signed in to change notification settings - Fork 306
Fix kernel boot on UEFI systems #2117
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
base: main
Are you sure you want to change the base?
Conversation
deepssin
commented
Dec 18, 2025
- Add _update_uefi_grub_config() to sync UEFI GRUB config
- Fixes issue where systems reboot into old kernel on UEFI
|
How does this work for non-UEFI systems? Should we maybe detect EFI vs BIOS and only run those if EFI=true? |
|
I have seen suggestions that /etc/grub.cfg is always a symlink to the "right" grub.cfg. if that's true that would be better. Also, does this code run for BLS systems? If BLS is used we should not be touching grub.cfg at all |
teuthology/task/kernel.py
Outdated
| efi_vendor = remote.sh('ls -d /boot/efi/EFI/*/ 2>/dev/null | head -1 | xargs basename || echo ""').strip() | ||
| if efi_vendor: | ||
| result = remote.run(args=['test', '-f', '/boot/efi/EFI/{}/grub.cfg'.format(efi_vendor)], check_status=False) | ||
| if result.exitstatus == 0: | ||
| remote.run(args=['sudo', 'cp', grubconfig, '/boot/efi/EFI/{}/grub.cfg'.format(efi_vendor)]) | ||
| log.info("Updated UEFI GRUB config at /boot/efi/EFI/{}/grub.cfg".format(efi_vendor)) |
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.
| efi_vendor = remote.sh('ls -d /boot/efi/EFI/*/ 2>/dev/null | head -1 | xargs basename || echo ""').strip() | |
| if efi_vendor: | |
| result = remote.run(args=['test', '-f', '/boot/efi/EFI/{}/grub.cfg'.format(efi_vendor)], check_status=False) | |
| if result.exitstatus == 0: | |
| remote.run(args=['sudo', 'cp', grubconfig, '/boot/efi/EFI/{}/grub.cfg'.format(efi_vendor)]) | |
| log.info("Updated UEFI GRUB config at /boot/efi/EFI/{}/grub.cfg".format(efi_vendor)) | |
| efi_vendor = remote.sh('find /boot/efi/EFI/ -depth -mindepth 1 -print -quit') | |
| if efi_vendor: | |
| result = remote.run(args=['test', '-f', '{}/grub.cfg'.format(efi_vendor)], check_status=False) | |
| if result.exitstatus == 0: | |
| remote.run(args=['sudo', 'cp', grubconfig, '{}/grub.cfg'.format(efi_vendor)]) | |
| log.info("Updated UEFI GRUB config at {}/grub.cfg".format(efi_vendor)) |
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.
Switched to find instead of ls -d as suggested. Initially it only found the first directory (/boot/efi/EFI/BOOT) because the command used -print -quit, which stops after the first match. The system actually boots from /boot/efi/EFI/centos/grub.cfg, so only updating BOOT didn't work.
- So have removed -quit to find all EFI vendor directories
- Correcting the option order: placing global options (-mindepth, -maxdepth) before -type
Have added check https://github.com/deepssin/teuthology/blob/0408de85a4d6eca6e538f650f0963db7f75d15d3/teuthology/task/kernel.py#L983 |
I verified this on my system /etc/grub.cfg doesn't exist:
What exists is /etc/grub2.cfg, which is a symlink to /boot/grub2/grub.cfg (not a UEFI location):
So the symlink check isn't reliable across systems is what I understand:
This avoids relying on symlink configurations that vary. The code now always updates all EFI vendor directories when on UEFI systems, ensuring the correct one is updated regardless of which vendor directory the firmware uses. |
- Add _update_uefi_grub_config() to sync UEFI GRUB config - Fixes issue where systems reboot into old kernel on UEFI Signed-off-by: deepssin <deepssin@redhat.com>
|
I'm still confused. I am no coder but if I'm understanding what I'm reading correctly, this does not handle BLS at all. Is that right? This commit does work but, again, I don't know how sane or valid or copacetic it is: 8ce0b02 |
|
You are correct, @djgalloway, and I don't understand why we're ignoring BLS either |