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
2024-02-20 11:07:29 +10:00 · 2024-02-20 11:07:29 +10:00 · bab6feb3a0
commit bab6feb3a0
parent ec49ec035c
7 changed files with 387 additions and 4 deletions
--- a/releasenotes/notes/updatable-firmware-versions-b1f947339b2e6b94.yaml
+++ 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.
--- 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),
--- 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

--- 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:
+        <domain type="kvm">
+        [...]
+          <metadata xmlns:sushy="http://openstack.org/xmlns/libvirt/sushy">
+            <sushy:bios>
+              <sushy:attributes>
+                <sushy:attribute name="ProcTurboMode" value="Enabled"/>
+                <sushy:attribute name="BootMode" value="Uefi"/>
+                <sushy:attribute name="NicBoot1" value="NetworkBoot"/>
+                <sushy:attribute name="EmbeddedSata" value="Raid"/>
+              </sushy:attributes>
+              <sushy:versions>
+                <sushy:version name="BiosVersion" value="1.1.0"/>
+              </sushy:versions>
+            </sushy:bios>
+          </metadata>
+        [...]
+
+        :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

--- 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 }}
    },
--- a/sushy_tools/tests/unit/emulator/domain_versions.xml
+++ b/sushy_tools/tests/unit/emulator/domain_versions.xml
@ -0,0 +1,36 @@
+<domain type='qemu'
+  xmlns:sushy="http://openstack.org/xmlns/libvirt/sushy">
+  <name>QEmu-fedora-i686</name>
+  <uuid>c7a5fdbd-cdaf-9455-926a-d65c16db1809</uuid>
+  <memory>219200</memory>
+  <currentMemory>219200</currentMemory>
+  <vcpu>2</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='cdrom'/>
+    <loader type='rom'/>
+  </os>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk type='file' device='cdrom'>
+      <source file='/home/user/boot.iso'/>
+      <target dev='hdc'/>
+      <readonly/>
+    </disk>
+    <disk type='file' device='disk'>
+      <source file='/home/user/fedora.img'/>
+      <target dev='hda'/>
+    </disk>
+    <interface type='network'>
+      <source network='default'/>
+    </interface>
+    <graphics type='vnc' port='-1'/>
+  </devices>
+    <metadata>
+      <sushy:bios>
+        <sushy:versions>
+          <sushy:version name="BiosVersion" value="1.0.0"/>
+        </sushy:versions>
+      </sushy:bios>
+    </metadata>
+</domain>
--- 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: