From bab6feb3a0bc785e2f0a53fcdfaccf14cb26e050 Mon Sep 17 00:00:00 2001 From: Jacob Anders Date: Tue, 20 Feb 2024 11:07:29 +1000 Subject: [PATCH] Replace hardcoded BiosVersion with an updatable field Currently, BiosVersion value returned in respose to GET /System/ comes from a value hardcoded in template. This change replaces the hardcoded value with a variable stored in sushy:bios metadata in the libvirt xml definition of the VM. Change-Id: Iaefc695c6cb177bc232112515c7b33c7b9af8b68 --- ...le-firmware-versions-b1f947339b2e6b94.yaml | 9 + sushy_tools/emulator/main.py | 10 + .../emulator/resources/systems/base.py | 25 +++ .../resources/systems/libvirtdriver.py | 173 +++++++++++++++++- sushy_tools/emulator/templates/system.json | 2 +- .../tests/unit/emulator/domain_versions.xml | 36 ++++ .../resources/systems/test_libvirt.py | 136 ++++++++++++++ 7 files changed, 387 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/updatable-firmware-versions-b1f947339b2e6b94.yaml create mode 100644 sushy_tools/tests/unit/emulator/domain_versions.xml diff --git a/releasenotes/notes/updatable-firmware-versions-b1f947339b2e6b94.yaml b/releasenotes/notes/updatable-firmware-versions-b1f947339b2e6b94.yaml new file mode 100644 index 00000000..1324c3bb --- /dev/null +++ b/releasenotes/notes/updatable-firmware-versions-b1f947339b2e6b94.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Replaces hardcoded BIOS version string with updatable field backed by a + custom section in libvirt metadata (similar to prior art on BIOS + settings). This can be used for BIOS version information as well as + storing firmware version information of other components. NOTE: this + enhancement is meant to facilitate testing BIOS/firmware upgrade codepaths + and NOT performing actual BIOS/firmware upgrades. diff --git a/sushy_tools/emulator/main.py b/sushy_tools/emulator/main.py index 320fadbd..3b578a92 100755 --- a/sushy_tools/emulator/main.py +++ b/sushy_tools/emulator/main.py @@ -412,6 +412,15 @@ def system_collection_resource(): @api_utils.returns_json def system_resource(identity): uuid = app.systems.uuid(identity) + try: + versions = app.systems.get_versions(identity) + except error.NotSupportedError: + app.logger.debug('Fetching BIOS version information not supported ' + 'for system "%s"', identity) + versions = {} + + bios_version = versions.get('BiosVersion') + if flask.request.method == 'GET': app.logger.debug('Serving resources for system "%s"', identity) @@ -429,6 +438,7 @@ def system_resource(identity): uuid=app.systems.uuid(identity), power_state=app.systems.get_power_state(identity), total_memory_gb=try_get(app.systems.get_total_memory), + bios_version=bios_version, total_cpus=try_get(app.systems.get_total_cpus), boot_source_target=app.systems.get_boot_device(identity), boot_source_mode=try_get(app.systems.get_boot_mode), diff --git a/sushy_tools/emulator/resources/systems/base.py b/sushy_tools/emulator/resources/systems/base.py index 446c72b4..dd82f816 100644 --- a/sushy_tools/emulator/resources/systems/base.py +++ b/sushy_tools/emulator/resources/systems/base.py @@ -167,6 +167,15 @@ class AbstractSystemsDriver(metaclass=abc.ABCMeta): """ raise error.NotSupportedError('Not implemented') + def get_versions(self, identity): + """Get firmware version information for the system + + :returns: key-value pairs of firmware versions + + :raises: `FishyError` if firmware versions cannot be processed + """ + raise error.NotSupportedError('Not implemented') + def set_bios(self, identity, attributes): """Update BIOS attributes @@ -176,6 +185,15 @@ class AbstractSystemsDriver(metaclass=abc.ABCMeta): """ raise error.NotSupportedError('Not implemented') + def set_versions(self, identity, firmware_versions): + """Update firmware versions + + :param firmware_versions: key-value pairs of versions to update + + :raises: `FishyError` if firmware versions cannot be processed + """ + raise error.NotSupportedError('Not implemented') + def reset_bios(self, identity): """Reset BIOS attributes to default @@ -183,6 +201,13 @@ class AbstractSystemsDriver(metaclass=abc.ABCMeta): """ raise error.NotSupportedError('Not implemented') + def reset_versions(self, identity): + """Reset firmware versions to default + + :raises: `FishyError` if firmware versions cannot be processed + """ + raise error.NotSupportedError('Not implemented') + def get_nics(self, identity): """Get list of NICs and their attributes diff --git a/sushy_tools/emulator/resources/systems/libvirtdriver.py b/sushy_tools/emulator/resources/systems/libvirtdriver.py index 9efea48e..ba13dd16 100644 --- a/sushy_tools/emulator/resources/systems/libvirtdriver.py +++ b/sushy_tools/emulator/resources/systems/libvirtdriver.py @@ -38,6 +38,11 @@ BiosProcessResult = namedtuple('BiosProcessResult', 'attributes_written', 'bios_attributes']) +FirmwareProcessResult = namedtuple('FirmwareProcessResult', + ['tree', + 'attributes_written', + 'firmware_versions']) + class libvirt_open(object): @@ -127,6 +132,8 @@ class LibvirtDriver(AbstractSystemsDriver): constants.DEVICE_TYPE_CD: ('hdc', 'ide'), } + DEFAULT_FIRMWARE_VERSIONS = {"BiosVersion": "1.0.0"} + DEFAULT_BIOS_ATTRIBUTES = {"BootMode": "Uefi", "EmbeddedSata": "Raid", "L2Cache": "10x256 KB", @@ -785,11 +792,14 @@ class LibvirtDriver(AbstractSystemsDriver): bios = metadata.find('sushy:bios', ns) attributes_written = False - if bios is not None and update_existing_attributes: - metadata.remove(bios) - bios = None if bios is None: bios = ET.SubElement(metadata, '{%s}bios' % (namespace)) + + attributes = bios.find('sushy:attributes', ns) + if attributes is not None and update_existing_attributes: + bios.remove(attributes) + attributes = None + if attributes is None: attributes = ET.SubElement(bios, '{%s}attributes' % (namespace)) for key, value in sorted(bios_attributes.items()): if not isinstance(value, str): @@ -805,6 +815,84 @@ class LibvirtDriver(AbstractSystemsDriver): return BiosProcessResult(tree, attributes_written, bios_attributes) + def _process_versions_attributes( + self, + domain_xml, + firmware_versions=DEFAULT_FIRMWARE_VERSIONS, + update_existing_attributes=False): + """Process Libvirt domain XML for firmware version attributes + + This method supports adding default firmware version information, + retrieving existing version attributes and + updating existing version attributes. + + This method is introduced to make XML testable otherwise have to + compare XML strings to test if XML saved to libvirt is as expected. + + Sample of custom XML (attributes section retained for context + although this code doesn't manage attributes, only versions: + + [...] + + + + + + + + + + + + + + [...] + + :param domain_xml: Libvirt domain XML to process + :param firmware_versions: firmware version information for updates or + default values if not specified + :param update_existing_attributes: Update existing firmware version + attributes + + :returns: namedtuple of tree: processed XML element tree, + attributes_written: if changes were made to XML, + versions: dict of firmware versions + """ + namespace = 'http://openstack.org/xmlns/libvirt/sushy' + ET.register_namespace('sushy', namespace) + ns = {'sushy': namespace} + + tree = ET.fromstring(domain_xml) + metadata = tree.find('metadata') + + if metadata is None: + metadata = ET.SubElement(tree, 'metadata') + bios = metadata.find('sushy:bios', ns) + + attributes_written = False + if bios is None: + bios = ET.SubElement(metadata, '{%s}bios' % (namespace)) + versions = bios.find('sushy:versions', ns) + if versions is not None and update_existing_attributes: + bios.remove(versions) + versions = None + if versions is None: + versions = ET.SubElement(bios, '{%s}versions' % (namespace)) + for key, value in sorted(firmware_versions.items()): + if not isinstance(value, str): + value = str(value) + ET.SubElement(versions, + '{%s}version' % (namespace), + name=key, + value=value) + attributes_written = True + + firmware_versions = {ver.attrib['name']: ver.attrib['value'] + for ver in tree.find('.//sushy:versions', ns)} + + return FirmwareProcessResult(tree, attributes_written, + firmware_versions) + def _process_bios(self, identity, bios_attributes=DEFAULT_BIOS_ATTRIBUTES, update_existing_attributes=False): @@ -844,6 +932,44 @@ class LibvirtDriver(AbstractSystemsDriver): return result.bios_attributes + def _process_versions(self, identity, + firmware_versions=DEFAULT_FIRMWARE_VERSIONS, + update_existing_attributes=False): + """Process Libvirt domain XML for firmware versions + + Process Libvirt domain XML for firmware versions and update it if + necessary + + :param identity: libvirt domain name or ID + :param firmware_versions: Full list of firmware versions to use if + they are missing or update necessary + :param update_existing_attributes: Update existing firmware versions + + :returns: New or existing dict of firmware versions + + :raises: `error.FishyError` if firmware versions cannot be saved + """ + + domain = self._get_domain(identity) + + result = self._process_versions_attributes( + domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE), + firmware_versions, + update_existing_attributes) + + if result.attributes_written: + + try: + with libvirt_open(self._uri) as conn: + conn.defineXML(ET.tostring(result.tree).decode('utf-8')) + + except libvirt.libvirtError as e: + msg = ('Error updating firmware versions' + ' at libvirt URI "%(uri)s": ' + '%(error)s' % {'uri': self._uri, 'error': e}) + raise error.FishyError(msg) + return result.firmware_versions + def get_bios(self, identity): """Get BIOS section @@ -854,6 +980,17 @@ class LibvirtDriver(AbstractSystemsDriver): """ return self._process_bios(identity) + def get_versions(self, identity): + """Get firmware versions section + + If there are no firmware version attributes, domain is updated with + default values. + + :param identity: libvirt domain name or ID + :returns: dict of firmware version attributes + """ + return self._process_versions(identity) + def set_bios(self, identity, attributes): """Update BIOS attributes @@ -876,6 +1013,28 @@ class LibvirtDriver(AbstractSystemsDriver): self._process_bios(identity, bios_attributes, update_existing_attributes=True) + def set_versions(self, identity, firmware_versions): + """Update firmware versions + + These values do not have any effect on VM. This is a workaround + because there is no libvirt API to manage firmware versions. + By storing fake firmware versions they are attached to VM and are + persisted through VM lifecycle. + + Updates to versions are immediate unlike in real firmware that + would require system reboot. + + :param identity: libvirt domain name or ID + :param firmware_versions: dict of firmware versions to update. + Can pass only versions that need update, not all + """ + versions = self.get_versions(identity) + + versions.update(firmware_versions) + + self._process_versions(identity, firmware_versions, + update_existing_attributes=True) + def reset_bios(self, identity): """Reset BIOS attributes to default @@ -884,6 +1043,14 @@ class LibvirtDriver(AbstractSystemsDriver): self._process_bios(identity, self.DEFAULT_BIOS_ATTRIBUTES, update_existing_attributes=True) + def reset_versions(self, identity): + """Reset firmware versions to default + + :param identity: libvirt domain name or ID + """ + self._process_versions(identity, self.DEFAULT_FIRMWARE_VERSIONS, + update_existing_attributes=True) + def get_nics(self, identity): """Get list of network interfaces and their MAC addresses diff --git a/sushy_tools/emulator/templates/system.json b/sushy_tools/emulator/templates/system.json index e7ad142a..c019b99f 100644 --- a/sushy_tools/emulator/templates/system.json +++ b/sushy_tools/emulator/templates/system.json @@ -66,7 +66,7 @@ "Bios": { "@odata.id": {{ "/redfish/v1/Systems/%s/BIOS"|format(identity)|tojson }} }, - "BiosVersion": "1.0.0", + "BiosVersion": {{ bios_version|string|tojson }}, "Processors": { "@odata.id": {{ "/redfish/v1/Systems/%s/Processors"|format(identity)|tojson }} }, diff --git a/sushy_tools/tests/unit/emulator/domain_versions.xml b/sushy_tools/tests/unit/emulator/domain_versions.xml new file mode 100644 index 00000000..85385d58 --- /dev/null +++ b/sushy_tools/tests/unit/emulator/domain_versions.xml @@ -0,0 +1,36 @@ + + 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..9c77bd93 100644 --- a/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py +++ b/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py @@ -944,6 +944,12 @@ class LibvirtDriverTestCase(base.BaseTestCase): .find('sushy:bios', ns) .find('sushy:attributes', ns)) + def _assert_versions_xml(self, tree): + ns = {'sushy': 'http://openstack.org/xmlns/libvirt/sushy'} + self.assertIsNotNone(tree.find('metadata') + .find('sushy:bios', ns) + .find('sushy:versions', ns)) + @mock.patch('libvirt.open', autospec=True) def test__process_bios_error(self, libvirt_mock): with open('sushy_tools/tests/unit/emulator/domain.xml') as f: @@ -961,6 +967,136 @@ class LibvirtDriverTestCase(base.BaseTestCase): {"BootMode": "Uefi", "ProcTurboMode": "Enabled"}) + @mock.patch('libvirt.open', autospec=True) + def test_get_versions(self, libvirt_mock): + with open('sushy_tools/tests/unit/emulator/domain.xml') as f: + domain_xml = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = domain_xml + + firmware_versions = self.test_driver.get_versions(self.uuid) + self.assertEqual(LibvirtDriver.DEFAULT_FIRMWARE_VERSIONS, + firmware_versions) + conn_mock.defineXML.assert_called_once_with(mock.ANY) + + @mock.patch('libvirt.open', autospec=True) + def test_get_versions_existing(self, libvirt_mock): + with open('sushy_tools/tests/unit/emulator/domain_versions.xml') as f: + domain_xml = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = domain_xml + + versions = self.test_driver.get_versions(self.uuid) + self.assertEqual({"BiosVersion": "1.0.0"}, + versions) + conn_mock.defineXML.assert_not_called() + + @mock.patch('libvirt.open', autospec=True) + def test_set_versions(self, libvirt_mock): + with open('sushy_tools/tests/unit/emulator/domain_versions.xml') as f: + domain_xml = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = domain_xml + + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.set_versions( + self.uuid, {"BiosVersion": "1.1.0"}) + + conn_mock.defineXML.assert_called_once_with(mock.ANY) + + @mock.patch('libvirt.open', autospec=True) + def test_reset_versions(self, libvirt_mock): + with open('sushy_tools/tests/unit/emulator/domain_versions.xml') as f: + domain_xml = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = domain_xml + + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.reset_versions(self.uuid) + + conn_mock.defineXML.assert_called_once_with(mock.ANY) + + def test__process_versions_attributes_get_default(self): + with open('sushy_tools/tests/unit/emulator/domain.xml') as f: + domain_xml = f.read() + + result = self.test_driver._process_versions_attributes(domain_xml) + self.assertTrue(result.attributes_written) + self.assertEqual(LibvirtDriver.DEFAULT_FIRMWARE_VERSIONS, + result.firmware_versions) + self._assert_versions_xml(result.tree) + + def test__process_versions_attributes_get_default_metadata_exists(self): + with open('sushy_tools/tests/unit/emulator/' + 'domain_metadata.xml') as f: + domain_xml = f.read() + + result = self.test_driver._process_versions_attributes(domain_xml) + self.assertTrue(result.attributes_written) + self.assertEqual(LibvirtDriver.DEFAULT_FIRMWARE_VERSIONS, + result.firmware_versions) + self._assert_versions_xml(result.tree) + + def test__process_versions_attributes_get_existing(self): + with open('sushy_tools/tests/unit/emulator/domain_versions.xml') as f: + domain_xml = f.read() + + result = self.test_driver._process_versions_attributes(domain_xml) + self.assertFalse(result.attributes_written) + self.assertEqual({"BiosVersion": "1.0.0"}, + result.firmware_versions) + self._assert_versions_xml(result.tree) + + def test__process_versions_attributes_update(self): + with open('sushy_tools/tests/unit/emulator/domain_versions.xml') as f: + domain_xml = f.read() + result = self.test_driver._process_versions_attributes( + domain_xml, + {"BiosVersion": "2.0.0"}, + True) + self.assertTrue(result.attributes_written) + self.assertEqual({"BiosVersion": "2.0.0"}, + result.firmware_versions) + self._assert_versions_xml(result.tree) + + def test__process_versions_attributes_update_non_string(self): + with open('sushy_tools/tests/unit/emulator/domain_versions.xml') as f: + domain_xml = f.read() + result = self.test_driver._process_versions_attributes( + domain_xml, + {"BiosVersion": 42}, + True) + self.assertTrue(result.attributes_written) + self.assertEqual({"BiosVersion": "42"}, + result.firmware_versions) + self._assert_versions_xml(result.tree) + + @mock.patch('libvirt.open', autospec=True) + def test__process_versions_error(self, libvirt_mock): + with open('sushy_tools/tests/unit/emulator/domain.xml') as f: + domain_xml = f.read() + + conn_mock = libvirt_mock.return_value + domain_mock = conn_mock.lookupByUUID.return_value + domain_mock.XMLDesc.return_value = domain_xml + conn_mock.defineXML.side_effect = libvirt.libvirtError( + 'because I can') + + self.assertRaises(error.FishyError, + self.test_driver._process_bios, + 'xxx-yyy-zzz', + {"BiosVersion" "1.0.0"}) + @mock.patch('libvirt.openReadOnly', autospec=True) def test_get_nics(self, libvirt_mock): with open('sushy_tools/tests/unit/emulator/domain_nics.xml') as f: