openstack
/
sushy-tools
Code Issues Proposed changes

Bring 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 solution offered by this patch has been discovered in the
early days of computers and has been used with great success ever
after - we will simply turn the domain off and on again for the
time a change is being done to the domain.

Change-Id: I7b9d4ca23d5eb42b791c9399dbe03f921664569e
This commit is contained in:
Ilya Etingof 2019-11-28 18:48:54 +01:00
parent 723ffd231f
commit 564ae30f32
3 changed files with 129 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
releasenotes/notes/fix-libvirt-statefulness-0a2a7812d79fdd25.yaml Normal file
View File

@ -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.

22
sushy_tools/emulator/resources/systems/libvirtdriver.py
View File

@ -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

114
sushy_tools/tests/unit/emulator/resources/systems/test_libvirt.py
View File

@ -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):