diff --git a/releasenotes/notes/fix-libvirt-statefulness-0a2a7812d79fdd25.yaml b/releasenotes/notes/fix-libvirt-statefulness-0a2a7812d79fdd25.yaml new file mode 100644 index 00000000..efa6eec4 --- /dev/null +++ b/releasenotes/notes/fix-libvirt-statefulness-0a2a7812d79fdd25.yaml @@ -0,0 +1,16 @@ +--- +fixes: + - | + Brings libvirt domain down prior to any change. When Redfish emulator + is running against libvirt virtualization backend, any changes being + done to domain are not applied for as long as the domain is up. This + leads to two nuisances: + + + REST API is not really REST-ful meaning that successfully + applied change is not reflected in the document tree + + Multiple changes done to live domain XML tree may override one + another because N-1 change done to a domain is not visible + to N's change + + The fix is to bring running domain down briefly while the change is + applied. diff --git a/sushy_tools/emulator/resources/systems/libvirtdriver.py b/sushy_tools/emulator/resources/systems/libvirtdriver.py index 455084a4..1870ab6e 100644 --- a/sushy_tools/emulator/resources/systems/libvirtdriver.py +++ b/sushy_tools/emulator/resources/systems/libvirtdriver.py @@ -67,6 +67,23 @@ class libvirt_open(object): self._conn.close() +def power_cycle(wrapped): + def wrapper(self, identity, *args, **kwargs): + power_state = self.get_power_state(identity) + + if power_state == 'On': + self.set_power_state(identity, 'ForceOff') + + try: + return wrapped(self, identity, *args, **kwargs) + + finally: + if power_state == 'On': + self.set_power_state(identity, power_state) + + return wrapper + + class LibvirtDriver(AbstractSystemsDriver): """Libvirt driver""" @@ -349,6 +366,7 @@ class LibvirtDriver(AbstractSystemsDriver): return boot_source_target + @power_cycle def set_boot_device(self, identity, boot_source): """Get/Set computer system boot device name @@ -458,6 +476,7 @@ class LibvirtDriver(AbstractSystemsDriver): return boot_mode + @power_cycle def set_boot_mode(self, identity, boot_mode): """Set computer system boot mode. @@ -702,6 +721,7 @@ class LibvirtDriver(AbstractSystemsDriver): """ return self._process_bios(identity) + @power_cycle def set_bios(self, identity, attributes): """Update BIOS attributes @@ -724,6 +744,7 @@ class LibvirtDriver(AbstractSystemsDriver): self._process_bios(identity, bios_attributes, update_existing_attributes=True) + @power_cycle def reset_bios(self, identity): """Reset BIOS attributes to default @@ -949,6 +970,7 @@ class LibvirtDriver(AbstractSystemsDriver): if dev_type == lv_device: device_element.remove(disk_element) + @power_cycle def set_boot_image(self, identity, device, boot_image=None, write_protected=True): """Set backend VM boot image 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 29c44250..0243ddf8 100644 --- a/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py +++ b/sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py @@ -161,6 +161,46 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock.injectNMI.assert_called_once_with() + @mock.patch('libvirt.open', autospec=True) + def test_power_cycle_when_off(self, libvirt_mock): + with open('sushy_tools/tests/unit/emulator/' + 'domain_boot_os.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 + + with mock.patch.object( + self.test_driver, 'get_power_state') as gps_mock: + with mock.patch.object( + self.test_driver, 'set_power_state') as sps_mock: + gps_mock.return_value = 'Off' + self.test_driver.set_boot_device(self.uuid, 'Cd') + + self.assertTrue(gps_mock.called) + self.assertFalse(sps_mock.called) + + @mock.patch('libvirt.open', autospec=True) + def test_power_cycle_when_on(self, libvirt_mock): + with open('sushy_tools/tests/unit/emulator/' + 'domain_boot_os.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 + + with mock.patch.object( + self.test_driver, 'get_power_state') as gps_mock: + with mock.patch.object( + self.test_driver, 'set_power_state') as sps_mock: + gps_mock.return_value = 'On' + self.test_driver.set_boot_device(self.uuid, 'Cd') + + self.assertTrue(gps_mock.called) + self.assertTrue(sps_mock.called) + @mock.patch('libvirt.openReadOnly', autospec=True) def test_get_boot_device_os(self, libvirt_mock): with open('sushy_tools/tests/unit/emulator/' @@ -185,7 +225,9 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = data - self.test_driver.set_boot_device(self.uuid, 'Hdd') + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.set_boot_device(self.uuid, 'Hdd') conn_mock.defineXML.assert_called_once_with(mock.ANY) @@ -213,7 +255,9 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = data - self.test_driver.set_boot_device(self.uuid, 'Hdd') + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.set_boot_device(self.uuid, 'Hdd') conn_mock.defineXML.assert_called_once_with(mock.ANY) @@ -247,7 +291,9 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = data - self.test_driver.set_boot_device(self.uuid, 'Pxe') + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.set_boot_device(self.uuid, 'Pxe') conn_mock.defineXML.assert_called_once_with(mock.ANY) @@ -282,7 +328,9 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = data - self.test_driver.set_boot_mode(self.uuid, 'UEFI') + 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 conn_mock.defineXML.assert_called_once_with(mock.ANY) @@ -297,7 +345,9 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = data - self.test_driver.set_boot_mode(self.uuid, 'UEFI') + 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] @@ -323,9 +373,11 @@ class LibvirtDriverTestCase(base.BaseTestCase): with mock.patch.dict( self.test_driver.KNOWN_BOOT_LOADERS, {}, clear=True): - self.assertRaises( - error.FishyError, self.test_driver.set_boot_mode, - self.uuid, 'Uefi') + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.assertRaises( + error.FishyError, self.test_driver.set_boot_mode, + self.uuid, 'Uefi') @mock.patch('libvirt.open', autospec=True) @mock.patch('libvirt.openReadOnly', autospec=True) @@ -343,9 +395,11 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = data - self.assertRaises( - error.FishyError, self.test_driver.set_boot_mode, - self.uuid, 'Uefi') + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.assertRaises( + error.FishyError, self.test_driver.set_boot_mode, + self.uuid, 'Uefi') @mock.patch('libvirt.open', autospec=True) @mock.patch('libvirt.openReadOnly', autospec=True) @@ -363,9 +417,11 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = data - self.assertRaises( - error.FishyError, self.test_driver.set_boot_mode, - self.uuid, 'Uefi') + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.assertRaises( + error.FishyError, self.test_driver.set_boot_mode, + self.uuid, 'Uefi') @mock.patch('libvirt.open', autospec=True) @mock.patch('libvirt.openReadOnly', autospec=True) @@ -383,9 +439,11 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = data - self.assertRaises( - error.FishyError, self.test_driver.set_boot_mode, - self.uuid, 'Uefi') + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.assertRaises( + error.FishyError, self.test_driver.set_boot_mode, + self.uuid, 'Uefi') @mock.patch('libvirt.open', autospec=True) @mock.patch('libvirt.openReadOnly', autospec=True) @@ -404,7 +462,9 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = data - self.test_driver.set_boot_mode(self.uuid, 'UEFI') + 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] @@ -450,7 +510,9 @@ class LibvirtDriverTestCase(base.BaseTestCase): pool_mock.XMLDesc.return_value = data - self.test_driver.set_boot_image(self.uuid, 'Cd', '/tmp/image.iso') + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.set_boot_image(self.uuid, 'Cd', '/tmp/image.iso') conn_mock = libvirt_rw_mock.return_value pool_mock.listAllVolumes.assert_called_once_with() @@ -524,9 +586,12 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = domain_xml - self.test_driver.set_bios(self.uuid, - {"BootMode": "Uefi", - "ProcTurboMode": "Enabled"}) + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.set_bios( + self.uuid, {"BootMode": "Uefi", + "ProcTurboMode": "Enabled"}) + conn_mock.defineXML.assert_called_once_with(mock.ANY) @mock.patch('libvirt.open', autospec=True) @@ -538,7 +603,10 @@ class LibvirtDriverTestCase(base.BaseTestCase): domain_mock = conn_mock.lookupByUUID.return_value domain_mock.XMLDesc.return_value = domain_xml - self.test_driver.reset_bios(self.uuid) + with mock.patch.object( + self.test_driver, 'get_power_state', return_value='Off'): + self.test_driver.reset_bios(self.uuid) + conn_mock.defineXML.assert_called_once_with(mock.ANY) def test__process_bios_attributes_get_default(self):