openstack/ironic-python-agent
Code Issues Proposed changes

Do not try to guess EFI partition path by its number

Browse Source 
The logic of adding a partition number to the device path does not work
for devicemapper devices (e.g. a multipath storage device).

Conflicts:
	ironic_python_agent/efi_utils.py
	ironic_python_agent/extensions/image.py
	ironic_python_agent/tests/unit/extensions/test_image.py
	ironic_python_agent/tests/unit/test_efi_utils.py

Change-Id: I9a445e847d282c50adfa4bad5e7136776861005d
(cherry picked from commit f09f6c9f1a)
(cherry picked from commit 12e0369887)
This commit is contained in:
Dmitry Tantsur
2022-05-04 14:45:33 +02:00
committed by Riccardo Pittau
parent cfc30696fe
commit 9704b819c9
4 changed files with 112 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

66
ironic_python_agent/extensions/image.py
View File

@@ -313,12 +313,43 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
# Update the nvram using efibootmgr
# https://linux.die.net/man/8/efibootmgr
utils.execute('efibootmgr', '-v', '-c', '-d', device,
'-p', efi_partition, '-w', '-L', label,
'-p', str(efi_partition), '-w', '-L', label,
'-l', v_efi_bl_path)
# Increment the ID in case the loop runs again.
label_id += 1
def get_partition_path_by_number(device, part_num):
"""Get partition path (/dev/something) by a partition number on device.
Only works for GPT partition table.
"""
uuid = None
partinfo, _ = utils.execute('sgdisk', '-i', str(part_num), device,
use_standard_locale=True)
for line in partinfo.splitlines():
if not line.strip():
continue
try:
field, value = line.rsplit(':', 1)
except ValueError:
LOG.warning('Invalid sgdisk line: %s', line)
continue
if 'partition unique guid' in field.lower():
uuid = value.strip().lower()
LOG.debug('GPT partition number %s on device %s has UUID %s',
part_num, device, uuid)
break
if uuid is not None:
return _get_partition(device, uuid)
else:
LOG.warning('No UUID information provided in sgdisk output for '
'partition %s on device %s', part_num, device)
def _manage_uefi(device, efi_system_part_uuid=None):
"""Manage the device looking for valid efi bootloaders to update the nvram.
@@ -334,24 +365,35 @@ def _manage_uefi(device, efi_system_part_uuid=None):
"""
efi_partition_mount_point = None
efi_mounted = False
LOG.debug('Attempting UEFI loader autodetection and NVRAM record setup.')
efi_partition = None
try:
# Force UEFI to rescan the device.
_rescan_device(device)
local_path = tempfile.mkdtemp()
# Trust the contents on the disk in the event of a whole disk image.
efi_partition = utils.get_efi_part_on_device(device)
efi_part_num = utils.get_efi_part_on_device(device)
if efi_part_num is not None:
efi_partition = get_partition_path_by_number(device, efi_part_num)
if not efi_partition and efi_system_part_uuid:
# _get_partition returns <device>+<partition> and we only need the
# partition number
partition = _get_partition(device, uuid=efi_system_part_uuid)
efi_partition = _get_partition(device, uuid=efi_system_part_uuid)
try:
efi_partition = int(partition.replace(device, ""))
efi_part_num = int(efi_partition.replace(device, ""))
except ValueError:
# NVMe Devices get a partitioning scheme that is different from
# traditional block devices like SCSI/SATA
efi_partition = int(partition.replace(device + 'p', ""))
try:
efi_part_num = int(efi_partition.replace(device + 'p', ""))
except ValueError as exc:
# At least provide a reasonable error message if the device
# does not follow this procedure.
raise errors.DeviceNotFound(
"Cannot detect the partition number of the device "
"%s: %s" % (device, exc))
if not efi_partition:
# NOTE(dtantsur): we cannot have a valid EFI deployment without an
@@ -368,20 +410,12 @@ def _manage_uefi(device, efi_system_part_uuid=None):
if not os.path.exists(efi_partition_mount_point):
os.makedirs(efi_partition_mount_point)
# The mount needs the device with the partition, in case the
# device ends with a digit we add a `p` and the partition number we
# found, otherwise we just join the device and the partition number
if device[-1].isdigit():
efi_device_part = '{}p{}'.format(device, efi_partition)
utils.execute('mount', efi_device_part, efi_partition_mount_point)
else:
efi_device_part = '{}{}'.format(device, efi_partition)
utils.execute('mount', efi_device_part, efi_partition_mount_point)
utils.execute('mount', efi_partition, efi_partition_mount_point)
efi_mounted = True
valid_efi_bootloaders = _get_efi_bootloaders(efi_partition_mount_point)
if valid_efi_bootloaders:
_run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
_run_efibootmgr(valid_efi_bootloaders, device, efi_part_num,
efi_partition_mount_point)
return True
else:

77
ironic_python_agent/tests/unit/extensions/test_image.py
View File

@@ -25,6 +25,7 @@ from ironic_python_agent import errors
from ironic_python_agent.extensions import image
from ironic_python_agent import hardware
from ironic_python_agent.tests.unit import base
from ironic_python_agent.tests.unit.samples import hardware_samples
from ironic_python_agent import utils
@@ -216,14 +217,14 @@ class TestImageExtension(base.IronicAgentTest):
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=False)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__uefi_bootloader_given_partition(
self, mkdir_mock, mock_utils_efi_part, mock_partition,
self, mkdir_mock, mock_utils_efi_part, mock_get_partition,
mock_efi_bl, mock_execute, mock_dispatch):
mock_dispatch.side_effect = [
self.fake_dev, hardware.BootInfo(current_boot_mode='uefi')
]
mock_partition.side_effect = [self.fake_dev, self.fake_efi_system_part]
mock_get_partition.return_value = self.fake_efi_system_part
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
mock_utils_efi_part.return_value = '1'
mock_utils_efi_part.return_value = None
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
@@ -260,16 +261,16 @@ class TestImageExtension(base.IronicAgentTest):
@mock.patch.object(hardware, 'is_md_device', lambda *_: False)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(image, 'get_partition_path_by_number', autospec=True)
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__uefi_bootloader_find_partition(
self, mkdir_mock, mock_utils_efi_part, mock_partition,
self, mkdir_mock, mock_utils_efi_part, mock_get_part_path,
mock_efi_bl, mock_execute, mock_dispatch):
mock_dispatch.side_effect = [
self.fake_dev, hardware.BootInfo(current_boot_mode='uefi')
]
mock_partition.return_value = self.fake_dev
mock_get_part_path.return_value = self.fake_efi_system_part
mock_utils_efi_part.return_value = '1'
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
mock_execute.side_effect = iter([('', ''), ('', ''),
@@ -307,16 +308,16 @@ class TestImageExtension(base.IronicAgentTest):
@mock.patch.object(hardware, 'is_md_device', lambda *_: False)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(image, 'get_partition_path_by_number', autospec=True)
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__uefi_bootloader_with_entry_removal(
self, mkdir_mock, mock_utils_efi_part, mock_partition,
self, mkdir_mock, mock_utils_efi_part, mock_get_part_path,
mock_efi_bl, mock_execute, mock_dispatch):
mock_dispatch.side_effect = [
self.fake_dev, hardware.BootInfo(current_boot_mode='uefi')
]
mock_partition.return_value = self.fake_dev
mock_get_part_path.return_value = self.fake_efi_system_part
mock_utils_efi_part.return_value = '1'
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
stdout_msg = """
@@ -364,16 +365,16 @@ Boot0002 VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
@mock.patch.object(hardware, 'is_md_device', lambda *_: False)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(image, 'get_partition_path_by_number', autospec=True)
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__uefi_bootloader_with_entry_removal_lenovo(
self, mkdir_mock, mock_utils_efi_part, mock_partition,
self, mkdir_mock, mock_utils_efi_part, mock_get_part_path,
mock_efi_bl, mock_execute, mock_dispatch):
mock_dispatch.side_effect = [
self.fake_dev, hardware.BootInfo(current_boot_mode='uefi')
]
mock_partition.return_value = self.fake_dev
mock_get_part_path.return_value = self.fake_efi_system_part
mock_utils_efi_part.return_value = '1'
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
# NOTE(TheJulia): This test string was derived from a lenovo SR650
@@ -426,16 +427,16 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
@mock.patch.object(hardware, 'is_md_device', lambda *_: False)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(image, 'get_partition_path_by_number', autospec=True)
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__add_multi_bootloaders(
self, mkdir_mock, mock_utils_efi_part, mock_partition,
self, mkdir_mock, mock_utils_efi_part, mock_get_part_path,
mock_efi_bl, mock_execute, mock_dispatch):
mock_dispatch.side_effect = [
self.fake_dev, hardware.BootInfo(current_boot_mode='uefi')
]
mock_partition.return_value = self.fake_dev
mock_get_part_path.return_value = self.fake_efi_system_part
mock_utils_efi_part.return_value = '1'
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI',
'WINDOWS/system32/winload.efi']
@@ -2309,15 +2310,14 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(image, 'get_partition_path_by_number', autospec=True)
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__manage_uefi(self, mkdir_mock, mock_utils_efi_part,
mock_get_part_uuid, mock_efi_bl, mock_execute,
mock_get_part_path, mock_efi_bl, mock_execute,
mock_dispatch):
mock_utils_efi_part.return_value = '1'
mock_get_part_uuid.return_value = self.fake_dev
mock_get_part_path.return_value = self.fake_efi_system_part
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
mock_execute.side_effect = iter([('', ''), ('', ''),
@@ -2348,14 +2348,14 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(image, 'get_partition_path_by_number', autospec=True)
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__manage_uefi_found_csv(self, mkdir_mock, mock_utils_efi_part,
mock_get_part_uuid, mock_efi_bl,
mock_get_part_path, mock_efi_bl,
mock_execute, mock_dispatch):
mock_utils_efi_part.return_value = '1'
mock_get_part_uuid.return_value = self.fake_dev
mock_get_part_path.return_value = self.fake_efi_system_part
mock_efi_bl.return_value = ['EFI/vendor/BOOTX64.CSV']
# Format is <file>,<entry_name>,<options>,humanfriendlytextnotused
@@ -2406,15 +2406,14 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(image, 'get_partition_path_by_number', autospec=True)
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__manage_uefi_nvme_device(self, mkdir_mock, mock_utils_efi_part,
mock_get_part_uuid, mock_efi_bl,
mock_get_part_path, mock_efi_bl,
mock_execute, mock_dispatch):
mock_utils_efi_part.return_value = '1'
mock_get_part_uuid.return_value = '/dev/fakenvme0p1'
mock_get_part_path.return_value = '/dev/fakenvme0p1'
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
mock_execute.side_effect = iter([('', ''), ('', ''),
@@ -2445,16 +2444,14 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(image, 'get_partition_path_by_number', autospec=True)
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
def test__manage_uefi_wholedisk(
self, mkdir_mock, mock_utils_efi_part,
mock_get_part_uuid, mock_efi_bl, mock_execute,
mock_dispatch):
mock_get_part_path, mock_efi_bl, mock_execute, mock_dispatch):
mock_utils_efi_part.return_value = '1'
mock_get_part_uuid.side_effect = Exception
mock_get_part_path.return_value = self.fake_efi_system_part
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
mock_execute.side_effect = iter([('', ''), ('', ''),
@@ -2590,3 +2587,21 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
mock_open.side_effect = OSError('boom')
image._append_uefi_to_fstab(
self.fake_dir, 'abcd-efgh')
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
class TestGetPartitionPathByNumber(base.IronicAgentTest):
def test_ok(self, mock_execute, mock_get_partition):
mock_execute.return_value = (hardware_samples.SGDISK_INFO_TEMPLATE, '')
mock_get_partition.return_value = '/dev/fake1'
result = image.get_partition_path_by_number('/dev/fake', 1)
self.assertEqual('/dev/fake1', result)
mock_execute.assert_called_once_with('sgdisk', '-i', '1', '/dev/fake',
use_standard_locale=True)
def test_broken(self, mock_execute, mock_get_partition):
mock_execute.return_value = ('I am a teaport', '')
self.assertIsNone(
image.get_partition_path_by_number('/dev/fake', 1))
mock_get_partition.assert_not_called()

11
ironic_python_agent/tests/unit/samples/hardware_samples.py
View File

@@ -1500,3 +1500,14 @@ NVME_CLI_INFO_TEMPLATE_FORMAT_UNSUPPORTED = ("""
]
}
""")
SGDISK_INFO_TEMPLATE = ("""
Partition GUID code: C12A7328-F81F-11D2-BA4B-00A0C93EC93B (EFI system partition)
Partition unique GUID: FAED7408-6D92-4FC6-883B-9069E2274ECA
First sector: 2048 (at 1024.0 KiB)
Last sector: 1050623 (at 513.0 MiB)
Partition size: 1048576 sectors (512.0 MiB)
Attribute flags: 0000000000000000
Partition name: 'EFI System Partition'
""") # noqa

5
releasenotes/notes/efi-partuuid-5fe933a462eeede1.yaml Normal file
View File

@@ -0,0 +1,5 @@
---
fixes:
- |
Fixes configuring UEFI boot when the EFI partition is located on a
devicemapper device.