Skip to content
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

Fix KeyError when handling non-numbers in boot entries #391

Merged
merged 1 commit into from
Jan 11, 2022

Conversation

r0x0d
Copy link
Member

@r0x0d r0x0d commented Dec 10, 2021

The error happened because of the lack of a rule for the BootEntries parser,
before it only considered numbers between 0-9. However, the output of efibootmgr
can contain numbers and letters, for example, 000A is a valid output.

Jira reference: https://issues.redhat.com/browse/OAMG-6166
Bugzilla reference (If any): https://bugzilla.redhat.com/show_bug.cgi?id=2027686

@r0x0d r0x0d self-assigned this Dec 10, 2021
@r0x0d
Copy link
Member Author

r0x0d commented Dec 10, 2021

Adding new entries to the efibootmgr

# /dev/vda1 is the name of the vagrant disk
efibootmgr -c -b 000A -d /dev/vda1 -p 1 -l "\EFI\centos\shimx64.efi" -L "CentOS 8"
efibootmgr -c -b 000B -d /dev/vda1 -p 1 -l "\EFI\centos\shimx64.efi" -L "CentOS 8"
efibootmgr -c -b 000C -d /dev/vda1 -p 1 -l "\EFI\centos\shimx64.efi" -L "CentOS 8"
efibootmgr -c -b 00A2 -d /dev/vda1 -p 1 -l "\EFI\centos\shimx64.efi" -L "CentOS 8"
efibootmgr -c -b A2A2 -d /dev/vda1 -p 1 -l "\EFI\centos\shimx64.efi" -L "CentOS 8"
...

Test outputs

First test output: 
BootEntries: {'0000': <convert2rhel.grub.EFIBootLoader object at 0x7f790a839978>,
 '0001': <convert2rhel.grub.EFIBootLoader object at 0x7f790a839828>,
 '0002': <convert2rhel.grub.EFIBootLoader object at 0x7f790a8392b0>,
 '0003': <convert2rhel.grub.EFIBootLoader object at 0x7f790a8397f0>,
 '0004': <convert2rhel.grub.EFIBootLoader object at 0x7f790a839710>,
 '0005': <convert2rhel.grub.EFIBootLoader object at 0x7f790a839470>,
 '0006': <convert2rhel.grub.EFIBootLoader object at 0x7f790a839588>,
 '0007': <convert2rhel.grub.EFIBootLoader object at 0x7f790a839358>,
 '0008': <convert2rhel.grub.EFIBootLoader object at 0x7f790a8391d0>,
 '0009': <convert2rhel.grub.EFIBootLoader object at 0x7f790a839dd8>,
 '000A': <convert2rhel.grub.EFIBootLoader object at 0x7f790a839eb8>,
 '000B': <convert2rhel.grub.EFIBootLoader object at 0x7f790a839fd0>,
 '000C': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b0f0>,
 '000D': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b1d0>,
 '000E': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b2b0>,
 '000F': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b390>,
 '0010': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b470>,
 '0011': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b550>,
 '0012': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b630>,
 '0013': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b710>,
 '0014': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b7f0>,
 '0015': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b8d0>,
 '0016': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86b9b0>,
 '0017': <convert2rhel.grub.EFIBootLoader object at 0x7f790a86ba90>}
BootNum: 0017
BootOrder: ('0017', '0016', '0015', '0014', '0013', '0012', '0011', '0010', '000F', '000E', '000D', '000C', '000B', '000A', '0009', '0008', '0007', '0006', '0005', '0004', '0003', '0001', '0000', '0002')

Second test output:
BootEntries: {'0000': <convert2rhel.grub.EFIBootLoader object at 0x7fdda21879b0>,
 '0001': <convert2rhel.grub.EFIBootLoader object at 0x7fdda2187860>,
 '0002': <convert2rhel.grub.EFIBootLoader object at 0x7fdda21872e8>,
 '0003': <convert2rhel.grub.EFIBootLoader object at 0x7fdda2187828>,
 '000A': <convert2rhel.grub.EFIBootLoader object at 0x7fdda21873c8>,
 '000B': <convert2rhel.grub.EFIBootLoader object at 0x7fdda2187438>,
 '00A2': <convert2rhel.grub.EFIBootLoader object at 0x7fdda21874e0>,
 'A2A2': <convert2rhel.grub.EFIBootLoader object at 0x7fdda21875c0>,
 'AAA2': <convert2rhel.grub.EFIBootLoader object at 0x7fdda2187240>}
BootNum: A2A2
BootOrder: ('A2A2', 'AAA2', '00A2', '000A', '000B', '0003', '0001', '0000', '0002')

abadger
abadger previously approved these changes Dec 15, 2021
Copy link
Member

@abadger abadger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good change to me. +1

Copy link
Member Author

@r0x0d r0x0d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewd

convert2rhel/unit_tests/checks_test.py Outdated Show resolved Hide resolved
convert2rhel/grub.py Show resolved Hide resolved
@r0x0d r0x0d force-pushed the uefi-parser-key-error branch 2 times, most recently from 6fa7681 to 9109be2 Compare December 22, 2021 13:21
convert2rhel/unit_tests/grub_test.py Show resolved Hide resolved
convert2rhel/unit_tests/checks_test.py Outdated Show resolved Hide resolved
The error happened because of the lack of a rule for the BootEntries parser,
before it only considered numbers between 0-9. However, the output of efibootmgr
can contain numbers and letters, for example `000A` is a valid output.

Jira reference: https://issues.redhat.com/browse/OAMG-6166
Bugzilla reference (If any): https://bugzilla.redhat.com/show_bug.cgi?id=2027686
Copy link
Member

@Venefilyn Venefilyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Venefilyn Venefilyn merged commit a46b15a into oamg:main Jan 11, 2022
@r0x0d r0x0d deleted the uefi-parser-key-error branch April 4, 2022 13:04
@Venefilyn Venefilyn mentioned this pull request Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants