Skip to content

Commit 47ac40a

Browse files
juliakregerjuliakreger
committed
Delete EFI boot entry duplicate labels first
Some firmware seems to take an objection with EFI nvram entries being deleted after one is added, resulting in the entire entry table being reset to the last known good state. This is problematic, as ultimately deployments can time out if we previously booted with Networking, and the machine, while commanded to do other wise, reboots back to networking regardless. We will now delete entries first, before proceeding. Additionally, for general use, this pattern may serve the community better by avoiding cases where we would have previously just relied upon efibootmgr[0] to warn us of duplicate entries. [0]: https://github.com/rhboot/efibootmgr/blob/103aa22ece98f09fe3ea2a0c83988f0ee2d0e5a8/src/efibootmgr.c#L228 Change-Id: Ib61a7100a059e79a8b0901fd8f46b9bc41d657dc Story: 2009649 Task: 43808 (cherry picked from commit 67eddfa) (cherry picked from commit 33b3970) (cherry picked from commit 8fca145)
1 parent 750934a commit 47ac40a

3 files changed

Lines changed: 51 additions & 24 deletions

File tree

‎ironic_python_agent/extensions/image.py‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -270,12 +270,10 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
270270

271271
# Before updating let's get information about the bootorder
272272
LOG.debug("Getting information about boot order.")
273-
utils.execute('efibootmgr', '-v')
274-
# NOTE(iurygregory): regex used to identify the Warning in the stderr after
275-
# we add the new entry. Example:
276-
# "efibootmgr: ** Warning ** : Boot0004 has same label ironic"
277-
duplicated_label = re.compile(r'^.*:\s\*\*.*\*\*\s:\s.*'
278-
r'Boot([0-9a-f-A-F]+)\s.*$')
273+
original_efi_output = utils.execute('efibootmgr', '-v')
274+
# NOTE(TheJulia): regex used to identify entries in the efibootmgr
275+
# output on stdout.
276+
entry_label = re.compile(r'Boot([0-9a-f-A-F]+):\s(.*).*$')
279277
label_id = 1
280278
for v_bl in valid_efi_bootloaders:
281279
if 'csv' in v_bl.lower():
@@ -292,20 +290,26 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
292290
v_efi_bl_path = '\\' + v_bl.replace('/', '\\')
293291
label = 'ironic' + str(label_id)
294292

293+
# Iterate through standard out, and look for duplicates
294+
for line in original_efi_output[0].split('\n'):
295+
match = entry_label.match(line)
296+
# Look for the base label in the string if a line match
297+
# occurs, so we can identify if we need to eliminate the
298+
# entry.
299+
if match and label in match.group(2):
300+
boot_num = match.group(1)
301+
LOG.debug("Found bootnum %s matching label", boot_num)
302+
utils.execute('efibootmgr', '-b', boot_num, '-B')
303+
295304
LOG.debug("Adding loader %(path)s on partition %(part)s of device "
296305
" %(dev)s", {'path': v_efi_bl_path, 'part': efi_partition,
297306
'dev': device})
298307
# Update the nvram using efibootmgr
299308
# https://linux.die.net/man/8/efibootmgr
300-
cmd = utils.execute('efibootmgr', '-v', '-c', '-d', device,
301-
'-p', efi_partition, '-w', '-L', label,
302-
'-l', v_efi_bl_path)
303-
for line in cmd[1].split('\n'):
304-
match = duplicated_label.match(line)
305-
if match:
306-
boot_num = match.group(1)
307-
LOG.debug("Found bootnum %s matching label", boot_num)
308-
utils.execute('efibootmgr', '-b', boot_num, '-B')
309+
utils.execute('efibootmgr', '-v', '-c', '-d', device,
310+
'-p', efi_partition, '-w', '-L', label,
311+
'-l', v_efi_bl_path)
312+
# Increment the ID in case the loop runs again.
309313
label_id += 1
310314

311315

‎ironic_python_agent/tests/unit/extensions/test_image.py‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -344,13 +344,17 @@ def test__uefi_bootloader_with_entry_removal(
344344
mock_partition.return_value = self.fake_dev
345345
mock_utils_efi_part.return_value = '1'
346346
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
347-
stdeer_msg = """
348-
efibootmgr: ** Warning ** : Boot0004 has same label ironic1\n
349-
efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
350-
"""
347+
stdout_msg = """
348+
BootCurrent: 0001
349+
Timeout: 0 seconds
350+
BootOrder: 0000,00001
351+
Boot0000: ironic1 HD(1, GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI)
352+
Boot0001: ironic2 HD(1, GPT,4f3c6294-bf9b-4208-9808-111111111112)File(\EFI\Boot\BOOTX64.EFI)
353+
Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
354+
""" # noqa This is a giant literal string for testing.
351355
mock_execute.side_effect = iter([('', ''), ('', ''),
352356
('', ''), ('', ''),
353-
('', ''), ('', stdeer_msg),
357+
(stdout_msg, ''), ('', ''),
354358
('', ''), ('', ''),
355359
('', ''), ('', '')])
356360

@@ -361,12 +365,11 @@ def test__uefi_bootloader_with_entry_removal(
361365
mock.call('mount', self.fake_efi_system_part,
362366
self.fake_dir + '/boot/efi'),
363367
mock.call('efibootmgr', '-v'),
368+
mock.call('efibootmgr', '-b', '0000', '-B'),
364369
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
365370
'-p', '1', '-w',
366371
'-L', 'ironic1', '-l',
367372
'\\EFI\\BOOT\\BOOTX64.EFI'),
368-
mock.call('efibootmgr', '-b', '0004', '-B'),
369-
mock.call('efibootmgr', '-b', '0005', '-B'),
370373
mock.call('umount', self.fake_dir + '/boot/efi',
371374
attempts=3, delay_on_retry=True),
372375
mock.call('sync')]
@@ -381,7 +384,7 @@ def test__uefi_bootloader_with_entry_removal(
381384
mock_efi_bl.assert_called_once_with(self.fake_dir + '/boot/efi')
382385
mock_execute.assert_has_calls(expected)
383386
mock_utils_efi_part.assert_called_once_with(self.fake_dev)
384-
self.assertEqual(10, mock_execute.call_count)
387+
self.assertEqual(9, mock_execute.call_count)
385388

386389
@mock.patch.object(hardware, 'is_md_device', lambda *_: False)
387390
@mock.patch.object(os.path, 'exists', lambda *_: False)
@@ -2150,8 +2153,19 @@ def test__manage_uefi_found_csv(self, mkdir_mock, mock_utils_efi_part,
21502153
# Mild difference, Ubuntu ships a file without a 0xFEFF delimiter
21512154
# at the start of the file, where as Red Hat *does*
21522155
csv_file_data = u'shimx64.efi,Vendor String,,Grub2MadeUSDoThis\n'
2156+
# This test also handles deleting a pre-existing matching vendor
2157+
# string in advance.
2158+
dupe_entry = """
2159+
BootCurrent: 0001
2160+
Timeout: 0 seconds
2161+
BootOrder: 0000,00001
2162+
Boot0000: Vendor String HD(1, GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI)
2163+
Boot0001: Vendor String HD(2, GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI)
2164+
Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
2165+
""" # noqa This is a giant literal string for testing.
21532166

21542167
mock_execute.side_effect = iter([('', ''), ('', ''),
2168+
('', ''), (dupe_entry, ''),
21552169
('', ''), ('', ''),
21562170
('', ''), ('', ''),
21572171
('', '')])
@@ -2162,6 +2176,8 @@ def test__manage_uefi_found_csv(self, mkdir_mock, mock_utils_efi_part,
21622176
mock.call('mount', self.fake_efi_system_part,
21632177
self.fake_dir + '/boot/efi'),
21642178
mock.call('efibootmgr', '-v'),
2179+
mock.call('efibootmgr', '-b', '0000', '-B'),
2180+
mock.call('efibootmgr', '-b', '0001', '-B'),
21652181
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
21662182
'-p', '1', '-w',
21672183
'-L', 'Vendor String', '-l',
@@ -2176,7 +2192,7 @@ def test__manage_uefi_found_csv(self, mkdir_mock, mock_utils_efi_part,
21762192
mkdir_mock.assert_called_once_with(self.fake_dir + '/boot/efi')
21772193
mock_efi_bl.assert_called_once_with(self.fake_dir + '/boot/efi')
21782194
mock_execute.assert_has_calls(expected)
2179-
self.assertEqual(7, mock_execute.call_count)
2195+
self.assertEqual(9, mock_execute.call_count)
21802196

21812197
@mock.patch.object(os.path, 'exists', lambda *_: False)
21822198
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@@ -2344,6 +2360,7 @@ def test__run_efibootmgr_no_bootloaders(self, mock_execute, mock_dispatch):
23442360
mock_execute.assert_has_calls(expected)
23452361

23462362
def test__run_efibootmgr(self, mock_execute, mock_dispatch):
2363+
mock_execute.return_value = ('', '')
23472364
result = image._run_efibootmgr(['EFI/BOOT/BOOTX64.EFI'],
23482365
self.fake_dev,
23492366
self.fake_efi_system_part,

‎releasenotes/notes/de-duplicate-by-label-baa090c5b1bff992.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
fixes:
3+
- |
4+
Fixes cases where duplicates may not be found in the UEFI
5+
firmware NVRAM boot entry table by explicitly looking for, and deleting
6+
for matching labels in advance of creating the EFI boot loader entry.

0 commit comments

Comments
 (0)