From cc738adf260816f4e1920f07812eb16132181700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harald=20Jens=C3=A5s?= Date: Wed, 27 Mar 2024 03:46:11 +0100 Subject: [PATCH] =?UTF-8?q?[Libvirt]=C2=A0Support=20firmware=20auto-select?= =?UTF-8?q?ion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Firmware auto-slection in Libvirt uses the QEMU firmware description files to ease the burden on users. This change adds support for get/set boot mode and secure boot for domains created with firmware auto-selection. Prior to this change operations on fw auto-selection domains with errors such as: libvirt: Domain Config error : loader attribute 'readonly' cannot be specified when firmware autoselection is enabled Change-Id: I533edb7a2a296026bb98977f7dd0de2acf553b7e --- ...t-firware-autoselect-b743737cb1cf9c5e.yaml | 6 + .../resources/systems/libvirtdriver.py | 117 ++++++++++++- .../unit/emulator/domain-q35_fw_auto_uefi.xml | 30 ++++ .../domain-q35_fw_auto_uefi_secure.xml | 30 ++++ .../tests/unit/emulator/domain_fw_auto.xml | 27 +++ .../resources/systems/test_libvirt.py | 159 ++++++++++++++++++ 6 files changed, 365 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/add-libvirt-firware-autoselect-b743737cb1cf9c5e.yaml create mode 100644 sushy_tools/tests/unit/emulator/domain-q35_fw_auto_uefi.xml create mode 100644 sushy_tools/tests/unit/emulator/domain-q35_fw_auto_uefi_secure.xml create mode 100644 sushy_tools/tests/unit/emulator/domain_fw_auto.xml diff --git a/releasenotes/notes/add-libvirt-firware-autoselect-b743737cb1cf9c5e.yaml b/releasenotes/notes/add-libvirt-firware-autoselect-b743737cb1cf9c5e.yaml new file mode 100644 index 00000000..92735eaa --- /dev/null +++ b/releasenotes/notes/add-libvirt-firware-autoselect-b743737cb1cf9c5e.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Support for domains utilizing firmware auto-selection has been added to + the libvirt driver. + diff --git a/sushy_tools/emulator/resources/systems/libvirtdriver.py b/sushy_tools/emulator/resources/systems/libvirtdriver.py index 9efea48e..4d0de35c 100644 --- a/sushy_tools/emulator/resources/systems/libvirtdriver.py +++ b/sushy_tools/emulator/resources/systems/libvirtdriver.py @@ -92,6 +92,14 @@ class LibvirtDriver(AbstractSystemsDriver): LIBVIRT_URI = 'qemu:///system' + BOOT_MODE_AUTO_FW_MAP = { + 'UEFI': 'efi', + 'Legacy': 'bios' + } + + BOOT_MODE_AUTO_FW_MAP_REV = {v: k for k, v + in BOOT_MODE_AUTO_FW_MAP.items()} + BOOT_MODE_MAP = { 'Legacy': 'rom', 'UEFI': 'pflash' @@ -503,6 +511,18 @@ class LibvirtDriver(AbstractSystemsDriver): self._defineDomain(tree) + def _is_firmware_autoselection(self, tree): + """Get libvirt firmware autoselection mode + + :param tree: libvirt domain XML tree + + :returns: True if firmware autoselection is enabled + """ + + os_element = tree.find('.//os') + + return True if os_element.get('firmware') else False + def get_boot_mode(self, identity): """Get computer system boot mode. @@ -516,8 +536,15 @@ class LibvirtDriver(AbstractSystemsDriver): # XML schema: https://libvirt.org/formatdomain.html#elementsOSBIOS tree = ET.fromstring(domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)) - loader_element = tree.find('.//loader') + if self._is_firmware_autoselection(tree): + os_element = tree.find('.//os') + boot_mode = ( + self.BOOT_MODE_AUTO_FW_MAP_REV.get(os_element.get('firmware')) + ) + return boot_mode + + loader_element = tree.find('.//loader') if loader_element is not None: boot_mode = ( self.BOOT_MODE_MAP_REV.get(loader_element.get('type')) @@ -558,9 +585,6 @@ class LibvirtDriver(AbstractSystemsDriver): def _build_os_element(self, identity, tree, boot_mode, secure=None): """Set the boot mode and secure boot on the os element - This also converts from the previous manual layout to the automatic - approach. - :raises: `error.FishyError` if boot mode can't be set """ try: @@ -581,6 +605,54 @@ class LibvirtDriver(AbstractSystemsDriver): os_element = os_elements[0] + if self._is_firmware_autoselection(tree): + self._build_os_element_fw_autoselection(boot_mode, secure, + os_element) + else: + self._build_os_element_fw_manualselection(boot_mode, secure, + os_element, loader_type) + + def _build_os_element_fw_autoselection(self, boot_mode, secure, + os_element): + """Set the boot mode and secure boot (auto-selection) + + :raises: `error.FishyError` if boot mode can't be set + """ + os_element.set('firmware', self.BOOT_MODE_AUTO_FW_MAP[boot_mode]) + + # Delete the secure-boot feature element + try: + firmware_element = os_element.findall('firmware').pop() + for e in firmware_element.findall('.feature' + '[@name="secure-boot"]'): + firmware_element.remove(e) + except IndexError: + firmware_element = None + + if boot_mode != 'UEFI': + return + + if firmware_element is None: + firmware_element = ET.SubElement(os_element, 'firmware') + + if secure: + secure_boot_element = ET.SubElement(firmware_element, 'feature') + secure_boot_element.set('name', 'secure-boot') + secure_boot_element.set('enabled', 'yes') + else: + secure_boot_element = ET.SubElement(firmware_element, 'feature') + secure_boot_element.set('name', 'secure-boot') + secure_boot_element.set('enabled', 'no') + + def _build_os_element_fw_manualselection(self, boot_mode, secure, + os_element, loader_type): + """Set the boot mode and secure boot (manual-selection) + + This also converts from the previous manual layout to the automatic + approach. + + :raises: `error.FishyError` if boot mode can't be set + """ type_element = os_element.find('type') if type_element is None: os_arch = None @@ -656,6 +728,43 @@ class LibvirtDriver(AbstractSystemsDriver): # https://libvirt.org/formatdomain.html#operating-system-booting tree = ET.fromstring(domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)) + if self._is_firmware_autoselection(tree): + return self._get_secureboot_fw_auto_selection(identity, tree) + else: + return self._get_secureboot_fw_manual_selection(identity, tree) + + def _get_secureboot_fw_auto_selection(self, identity, tree): + os_element = tree.find('os') + + firmware_element = os_element.findall('firmware') + + if len(firmware_element) == 0: + msg = ('Can\'t get secure boot state because "firmware" element ' + 'is not present in domain "%(identity)s" configuration' + % {'identity': identity}) + raise error.FishyError(msg) + + if len(firmware_element) > 1: + msg = ('Can\'t get secure boot state because "firmware" element ' + 'must be present exactly once in domain "%(identity)s" ' + 'configuration' % {'identity': identity}) + raise error.FishyError(msg) + + feature_secure_boot = os_element.findall('./firmware/feature' + '[@name="secure-boot"]') + if len(feature_secure_boot) > 1: + msg = ('Can\'t get secure boot state because the "firmware" ' + 'element contains multiple "feature" elements with the ' + '"secure-boot" name attribute. "secure-boot" feature ' + 'should be present exactly once in domain %(identity)s" ' + 'configuration' % {'identity': identity}) + raise error.FishyError(msg) + + enabled = feature_secure_boot[0].get('enabled', "no") + + return True if enabled == "yes" else False + + def _get_secureboot_fw_manual_selection(self, identity, tree): os_element = tree.find('os') nvram = os_element.findall('nvram') diff --git a/sushy_tools/tests/unit/emulator/domain-q35_fw_auto_uefi.xml b/sushy_tools/tests/unit/emulator/domain-q35_fw_auto_uefi.xml new file mode 100644 index 00000000..5be26443 --- /dev/null +++ b/sushy_tools/tests/unit/emulator/domain-q35_fw_auto_uefi.xml @@ -0,0 +1,30 @@ + + QEmu-fedora-i686 + c7a5fdbd-cdaf-9455-926a-d65c16db1809 + 219200 + 219200 + 2 + + hvm + + + + + + + /usr/bin/qemu-system-x86_64 + + + + + + + + + + + + + + + diff --git a/sushy_tools/tests/unit/emulator/domain-q35_fw_auto_uefi_secure.xml b/sushy_tools/tests/unit/emulator/domain-q35_fw_auto_uefi_secure.xml new file mode 100644 index 00000000..48538e9e --- /dev/null +++ b/sushy_tools/tests/unit/emulator/domain-q35_fw_auto_uefi_secure.xml @@ -0,0 +1,30 @@ + + QEmu-fedora-i686 + c7a5fdbd-cdaf-9455-926a-d65c16db1809 + 219200 + 219200 + 2 + + hvm + + + + + + + /usr/bin/qemu-system-x86_64 + + + + + + + + + + + + + + + diff --git a/sushy_tools/tests/unit/emulator/domain_fw_auto.xml b/sushy_tools/tests/unit/emulator/domain_fw_auto.xml new file mode 100644 index 00000000..fcba865a --- /dev/null +++ b/sushy_tools/tests/unit/emulator/domain_fw_auto.xml @@ -0,0 +1,27 @@ + + QEmu-fedora-i686 + c7a5fdbd-cdaf-9455-926a-d65c16db1809 + 219200 + 219200 + 2 + + hvm + + + + /usr/bin/qemu-system-x86_64 + + + + + + + + + + + + + + + diff --git a/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py b/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py index 3c16db11..8db2c0ac 100644 --- a/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py +++ b/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py @@ -387,6 +387,27 @@ class LibvirtDriverTestCase(base.BaseTestCase): '\n \n' self.assertIn(expected, conn_mock.defineXML.call_args[0][0]) + def test__is_firmware_autoselection_disabled(self): + with open('sushy_tools/tests/unit/emulator/domain.xml', 'r') as f: + domain = f.read() + + tree = ET.fromstring(domain) + + fw_auto = self.test_driver._is_firmware_autoselection(tree) + + self.assertEqual(False, fw_auto) + + def test__is_firmware_autoselection_enabled(self): + with open(('sushy_tools/tests/unit/emulator/' + 'domain-q35_fw_auto_uefi.xml'), 'r') as f: + domain = f.read() + + tree = ET.fromstring(domain) + + fw_auto = self.test_driver._is_firmware_autoselection(tree) + + self.assertEqual(True, fw_auto) + @mock.patch('libvirt.openReadOnly', autospec=True) def test_get_boot_mode_legacy(self, libvirt_mock): with open('sushy_tools/tests/unit/emulator/domain.xml', 'r') as f: @@ -414,6 +435,20 @@ class LibvirtDriverTestCase(base.BaseTestCase): self.assertEqual('UEFI', boot_mode) + @mock.patch('libvirt.openReadOnly', autospec=True) + def test_get_boot_mode_fw_auto_uefi(self, libvirt_mock): + with open(('sushy_tools/tests/unit/emulator/' + 'domain-q35_fw_auto_uefi.xml'), 'r') as f: + data = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = data + + boot_mode = self.test_driver.get_boot_mode(self.uuid) + + self.assertEqual('UEFI', boot_mode) + @mock.patch('libvirt.open', autospec=True) @mock.patch('libvirt.openReadOnly', autospec=True) def test_set_boot_mode(self, libvirt_mock, libvirt_rw_mock): @@ -431,6 +466,58 @@ class LibvirtDriverTestCase(base.BaseTestCase): conn_mock = libvirt_rw_mock.return_value conn_mock.defineXML.assert_called_once_with(mock.ANY) + @mock.patch('libvirt.open', autospec=True) + @mock.patch('libvirt.openReadOnly', autospec=True) + def test_set_boot_mode_auto_fw_uefi(self, libvirt_mock, libvirt_rw_mock): + with open('sushy_tools/tests/unit/emulator/' + 'domain_fw_auto.xml', 'r') as f: + data = f.read() + + conn_mock = libvirt_rw_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = data + + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.set_boot_mode(self.uuid, 'UEFI') + + conn_mock = libvirt_rw_mock.return_value + xml_document = conn_mock.defineXML.call_args[0][0] + tree = ET.fromstring(xml_document) + os_element = tree.find('os') + self.assertEqual('efi', os_element.get('firmware')) + secure_boot = os_element.findall( + './firmware/feature[@name="secure-boot"]') + self.assertEqual('secure-boot', secure_boot[0].get('name')) + self.assertEqual('no', secure_boot[0].get('enabled')) + conn_mock.defineXML.assert_called_once_with(mock.ANY) + + @mock.patch('libvirt.open', autospec=True) + @mock.patch('libvirt.openReadOnly', autospec=True) + def test_set_boot_mode_auto_fw_legacy(self, libvirt_mock, libvirt_rw_mock): + with open('sushy_tools/tests/unit/emulator/' + 'domain-q35_fw_auto_uefi.xml', 'r') as f: + data = f.read() + + conn_mock = libvirt_rw_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = data + + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.set_boot_mode(self.uuid, 'Legacy') + + conn_mock = libvirt_rw_mock.return_value + xml_document = conn_mock.defineXML.call_args[0][0] + tree = ET.fromstring(xml_document) + os_element = tree.find('os') + self.assertEqual('bios', os_element.get('firmware')) + # There should be no secure-boot feature element + secure_boot = os_element.findall( + './firmware/feature[@name="secure-boot"]') + self.assertEqual([], secure_boot) + conn_mock.defineXML.assert_called_once_with(mock.ANY) + @mock.patch('libvirt.open', autospec=True) @mock.patch('libvirt.openReadOnly', autospec=True) def test_set_boot_mode_legacy(self, libvirt_mock, libvirt_rw_mock): @@ -1155,6 +1242,18 @@ class LibvirtDriverTestCase(base.BaseTestCase): self.assertFalse(self.test_driver.get_secure_boot(self.uuid)) + @mock.patch('libvirt.openReadOnly', autospec=True) + def test_get_secure_boot_fw_auto_off(self, libvirt_mock): + with open('sushy_tools/tests/unit/emulator/' + 'domain-q35_fw_auto_uefi.xml', 'r') as f: + data = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = data + + self.assertFalse(self.test_driver.get_secure_boot(self.uuid)) + @mock.patch('libvirt.openReadOnly', autospec=True) def test_get_secure_boot_on(self, libvirt_mock): with open('sushy_tools/tests/unit/emulator/domain-q35_uefi_secure.xml', @@ -1167,6 +1266,18 @@ class LibvirtDriverTestCase(base.BaseTestCase): self.assertTrue(self.test_driver.get_secure_boot(self.uuid)) + @mock.patch('libvirt.openReadOnly', autospec=True) + def test_get_secure_boot_fw_auto_on(self, libvirt_mock): + with open('sushy_tools/tests/unit/emulator/' + 'domain-q35_fw_auto_uefi_secure.xml', 'r') as f: + data = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = data + + self.assertTrue(self.test_driver.get_secure_boot(self.uuid)) + @mock.patch('libvirt.openReadOnly', autospec=True) def test_get_secure_boot_not_uefi(self, libvirt_mock): with open('sushy_tools/tests/unit/emulator/domain-q35.xml', 'r') as f: @@ -1259,3 +1370,51 @@ class LibvirtDriverTestCase(base.BaseTestCase): uri = 'http://host.path/meow' self.test_driver.set_http_boot_uri(uri) self.assertEqual(uri, self.test_driver.get_http_boot_uri(None)) + + @mock.patch('libvirt.open', autospec=True) + @mock.patch('libvirt.openReadOnly', autospec=True) + def test_set_secure_boot_on_auto_fw(self, libvirt_mock, libvirt_rw_mock): + with open('sushy_tools/tests/unit/emulator/' + 'domain-q35_fw_auto_uefi.xml', 'r') as f: + data = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = data + + self.test_driver.set_secure_boot(self.uuid, True) + + conn_mock = libvirt_rw_mock.return_value + xml_document = conn_mock.defineXML.call_args[0][0] + tree = ET.fromstring(xml_document) + os_element = tree.find('os') + self.assertEqual('efi', os_element.get('firmware')) + secure_boot = os_element.findall( + './firmware/feature[@name="secure-boot"]') + self.assertEqual('secure-boot', secure_boot[0].get('name')) + self.assertEqual('yes', secure_boot[0].get('enabled')) + conn_mock.defineXML.assert_called_once_with(mock.ANY) + + @mock.patch('libvirt.open', autospec=True) + @mock.patch('libvirt.openReadOnly', autospec=True) + def test_set_secure_boot_off_auto_fw(self, libvirt_mock, libvirt_rw_mock): + with open('sushy_tools/tests/unit/emulator/' + 'domain-q35_fw_auto_uefi_secure.xml', 'r') as f: + data = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = data + + self.test_driver.set_secure_boot(self.uuid, False) + + conn_mock = libvirt_rw_mock.return_value + xml_document = conn_mock.defineXML.call_args[0][0] + tree = ET.fromstring(xml_document) + os_element = tree.find('os') + self.assertEqual('efi', os_element.get('firmware')) + secure_boot = os_element.findall( + './firmware/feature[@name="secure-boot"]') + self.assertEqual('secure-boot', secure_boot[0].get('name')) + self.assertEqual('no', secure_boot[0].get('enabled')) + conn_mock.defineXML.assert_called_once_with(mock.ANY)