diff --git a/sushy_tools/emulator/drivers/libvirtdriver.py b/sushy_tools/emulator/drivers/libvirtdriver.py index f0b84bd7..880fbf2c 100644 --- a/sushy_tools/emulator/drivers/libvirtdriver.py +++ b/sushy_tools/emulator/drivers/libvirtdriver.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import logging +import uuid import xml.etree.ElementTree as ET from collections import namedtuple @@ -28,6 +30,10 @@ except ImportError: is_loaded = bool(libvirt) + +logger = logging.getLogger(__name__) + + BiosProcessResult = namedtuple('BiosProcessResult', ['tree', 'attributes_written', @@ -99,6 +105,27 @@ class LibvirtDriver(AbstractDriver): def __init__(self, uri=None): self._uri = uri or self.LIBVIRT_URI + def _get_domain(self, identity, readonly=False): + with libvirt_open(self._uri, readonly=readonly) as conn: + try: + return conn.lookupByName(identity) + + except libvirt.libvirtError as ex: + try: + uu_identity = uuid.UUID(identity) + + return conn.lookupByUUID(uu_identity.bytes) + + except (ValueError, libvirt.libvirtError): + msg = ('Error finding domain by name/UUID "%(identity)s" ' + 'at libvirt URI %(uri)s": %(err)s' % + {'identity': identity, + 'uri': self._uri, 'err': ex}) + + logger.debug(msg) + + raise FishyError(msg) + @property def driver(self): """Return human-friendly driver information @@ -122,13 +149,12 @@ class LibvirtDriver(AbstractDriver): The universal unique identifier (UUID) for this system. Can be used in place of system name if there are duplicates. - :param identity: libvirt domain name or ID + :param identity: libvirt domain name or UUID :returns: computer system UUID """ - with libvirt_open(self._uri, readonly=True) as conn: - domain = conn.lookupByName(identity) - return domain.UUIDString() + domain = self._get_domain(identity, readonly=True) + return domain.UUIDString() def get_power_state(self, identity): """Get computer system power state @@ -138,10 +164,8 @@ class LibvirtDriver(AbstractDriver): :returns: current power state as *On* or *Off* `str` or `None` if power state can't be determined """ - with libvirt_open(self._uri, readonly=True) as conn: - - domain = conn.lookupByName(identity) - return 'On' if domain.isActive() else 'Off' + domain = self._get_domain(identity, readonly=True) + return 'On' if domain.isActive() else 'Off' def set_power_state(self, identity, state): """Set computer system power state @@ -154,35 +178,33 @@ class LibvirtDriver(AbstractDriver): :raises: `FishyError` if power state can't be set """ - with libvirt_open(self._uri) as conn: + domain = self._get_domain(identity) - domain = conn.lookupByName(identity) + try: + if state in ('On', 'ForceOn'): + if not domain.isActive(): + domain.create() + elif state == 'ForceOff': + if domain.isActive(): + domain.destroy() + elif state == 'GracefulShutdown': + if domain.isActive(): + domain.shutdown() + elif state == 'GracefulRestart': + if domain.isActive(): + domain.reboot() + elif state == 'ForceRestart': + if domain.isActive(): + domain.reset() + elif state == 'Nmi': + if domain.isActive(): + domain.injectNMI() - try: - if state in ('On', 'ForceOn'): - if not domain.isActive(): - domain.create() - elif state == 'ForceOff': - if domain.isActive(): - domain.destroy() - elif state == 'GracefulShutdown': - if domain.isActive(): - domain.shutdown() - elif state == 'GracefulRestart': - if domain.isActive(): - domain.reboot() - elif state == 'ForceRestart': - if domain.isActive(): - domain.reset() - elif state == 'Nmi': - if domain.isActive(): - domain.injectNMI() + except libvirt.libvirtError as e: + msg = ('Error changing power state at libvirt URI "%(uri)s": ' + '%(error)s' % {'uri': self._uri, 'error': e}) - except libvirt.libvirtError as e: - msg = ('Error changing power state at libvirt URI "%(uri)s": ' - '%(error)s' % {'uri': self._uri, 'error': e}) - - raise FishyError(msg) + raise FishyError(msg) def get_boot_device(self, identity): """Get computer system boot device name @@ -192,19 +214,17 @@ class LibvirtDriver(AbstractDriver): :returns: boot device name as `str` or `None` if device name can't be determined """ - with libvirt_open(self._uri, readonly=True) as conn: + domain = self._get_domain(identity, readonly=True) - domain = conn.lookupByName(identity) + tree = ET.fromstring(domain.XMLDesc()) - tree = ET.fromstring(domain.XMLDesc()) + boot_element = tree.find('.//boot') + if boot_element is not None: + boot_source_target = ( + self.BOOT_DEVICE_MAP_REV.get(boot_element.get('dev')) + ) - boot_element = tree.find('.//boot') - if boot_element is not None: - boot_source_target = ( - self.BOOT_DEVICE_MAP_REV.get(boot_element.get('dev')) - ) - - return boot_source_target + return boot_source_target def set_boot_device(self, identity, boot_source): """Get/Set computer system boot device name @@ -216,39 +236,38 @@ class LibvirtDriver(AbstractDriver): :raises: `FishyError` if boot device can't be set """ - with libvirt_open(self._uri) as conn: + domain = self._get_domain(identity) - domain = conn.lookupByName(identity) + # XML schema: https://libvirt.org/formatdomain.html#elementsOSBIOS + tree = ET.fromstring(domain.XMLDesc()) - # XML schema: https://libvirt.org/formatdomain.html#elementsOSBIOS - tree = ET.fromstring(domain.XMLDesc()) + try: + target = self.BOOT_DEVICE_MAP[boot_source] - try: - target = self.BOOT_DEVICE_MAP[boot_source] + except KeyError: + msg = ('Unknown power state requested: ' + '%(boot_source)s' % {'boot_source': boot_source}) - except KeyError: - msg = ('Unknown power state requested: ' - '%(boot_source)s' % {'boot_source': boot_source}) + raise FishyError(msg) - raise FishyError(msg) + for os_element in tree.findall('os'): + # Remove all "boot" elements + for boot_element in os_element.findall('boot'): + os_element.remove(boot_element) - for os_element in tree.findall('os'): - # Remove all "boot" elements - for boot_element in os_element.findall('boot'): - os_element.remove(boot_element) + # Add a new boot element with the request boot device + boot_element = ET.SubElement(os_element, 'boot') + boot_element.set('dev', target) - # Add a new boot element with the request boot device - boot_element = ET.SubElement(os_element, 'boot') - boot_element.set('dev', target) - - try: + try: + with libvirt_open(self._uri) as conn: conn.defineXML(ET.tostring(tree).decode('utf-8')) - except libvirt.libvirtError as e: - msg = ('Error changing boot device at libvirt URI "%(uri)s": ' - '%(error)s' % {'uri': self._uri, 'error': e}) + except libvirt.libvirtError as e: + msg = ('Error changing boot device at libvirt URI "%(uri)s": ' + '%(error)s' % {'uri': self._uri, 'error': e}) - raise FishyError(msg) + raise FishyError(msg) def get_boot_mode(self, identity): """Get computer system boot mode. @@ -256,21 +275,19 @@ class LibvirtDriver(AbstractDriver): :returns: either *Uefi* or *Legacy* as `str` or `None` if current boot mode can't be determined """ - with libvirt_open(self._uri, readonly=True) as conn: + domain = self._get_domain(identity, readonly=True) - domain = conn.lookupByName(identity) + # XML schema: https://libvirt.org/formatdomain.html#elementsOSBIOS + tree = ET.fromstring(domain.XMLDesc()) - # XML schema: https://libvirt.org/formatdomain.html#elementsOSBIOS - tree = ET.fromstring(domain.XMLDesc()) + loader_element = tree.find('.//loader') - loader_element = tree.find('.//loader') + if loader_element is not None: + boot_mode = ( + self.BOOT_MODE_MAP_REV.get(loader_element.get('type')) + ) - if loader_element is not None: - boot_mode = ( - self.BOOT_MODE_MAP_REV.get(loader_element.get('type')) - ) - - return boot_mode + return boot_mode def set_boot_mode(self, identity, boot_mode): """Set computer system boot mode. @@ -281,54 +298,55 @@ class LibvirtDriver(AbstractDriver): :raises: `FishyError` if boot mode can't be set """ - with libvirt_open(self._uri) as conn: + domain = self._get_domain(identity, readonly=True) - domain = conn.lookupByName(identity) + # XML schema: https://libvirt.org/formatdomain.html#elementsOSBIOS + tree = ET.fromstring(domain.XMLDesc()) - # XML schema: https://libvirt.org/formatdomain.html#elementsOSBIOS - tree = ET.fromstring(domain.XMLDesc()) + try: + loader_type = self.BOOT_MODE_MAP[boot_mode] + + except KeyError: + msg = ('Unknown boot mode requested: ' + '%(boot_mode)s' % {'boot_mode': boot_mode}) + + raise FishyError(msg) + + for os_element in tree.findall('os'): + type_element = os_element.find('type') + if type_element is None: + os_arch = None + else: + os_arch = type_element.get('arch') try: - loader_type = self.BOOT_MODE_MAP[boot_mode] + loader_path = self.BOOT_LOADER_MAP[boot_mode][os_arch] except KeyError: - msg = ('Unknown boot mode requested: ' - '%(boot_mode)s' % {'boot_mode': boot_mode}) + # NOTE(etingof): assume no specific boot loader + loader_path = '' - raise FishyError(msg) + # Update all "loader" elements + for loader_element in os_element.findall('loader'): + loader_element.set('type', loader_type) + # NOTE(etingof): here we override previous boot loader for + # for the domain. If it's different than what we have + # hardcoded in the BOOT_LOADER_MAP, we won't be able to + # revert back to the original boor loader should we change + # domain boot mode. + loader_element.text = loader_path - for os_element in tree.findall('os'): - type_element = os_element.find('type') - if type_element is None: - os_arch = None - else: - os_arch = type_element.get('arch') + with libvirt_open(self._uri) as conn: try: - loader_path = self.BOOT_LOADER_MAP[boot_mode][os_arch] + conn.defineXML(ET.tostring(tree).decode('utf-8')) - except KeyError: - # NOTE(etingof): assume no specific boot loader - loader_path = '' + except libvirt.libvirtError as e: + msg = ('Error changing boot mode at libvirt URI ' + '"%(uri)s": %(error)s' % {'uri': self._uri, + 'error': e}) - # Update all "loader" elements - for loader_element in os_element.findall('loader'): - loader_element.set('type', loader_type) - # NOTE(etingof): here we override previous boot loader for - # for the domain. If it's different than what we have - # hardcoded in the BOOT_LOADER_MAP, we won't be able to - # revert back to the original boor loader should we change - # domain boot mode. - loader_element.text = loader_path - - try: - conn.defineXML(ET.tostring(tree).decode('utf-8')) - - except libvirt.libvirtError as e: - msg = ('Error changing boot mode at libvirt URI "%(uri)s": ' - '%(error)s' % {'uri': self._uri, 'error': e}) - - raise FishyError(msg) + raise FishyError(msg) def get_total_memory(self, identity): """Get computer system total memory @@ -338,9 +356,8 @@ class LibvirtDriver(AbstractDriver): :returns: available RAM in GiB as `int` or `None` if total memory count can't be determined """ - with libvirt_open(self._uri, readonly=True) as conn: - domain = conn.lookupByName(identity) - return int(domain.maxMemory() / 1024 / 1024) + domain = self._get_domain(identity, readonly=True) + return int(domain.maxMemory() / 1024 / 1024) def get_total_cpus(self, identity): """Get computer system total count of available CPUs @@ -350,25 +367,23 @@ class LibvirtDriver(AbstractDriver): :returns: available CPU count as `int` or `None` if CPU count can't be determined """ - with libvirt_open(self._uri, readonly=True) as conn: + domain = self._get_domain(identity, readonly=True) - domain = conn.lookupByName(identity) + tree = ET.fromstring(domain.XMLDesc()) - tree = ET.fromstring(domain.XMLDesc()) + total_cpus = 0 - total_cpus = 0 + if domain.isActive(): + total_cpus = domain.maxVcpus() - if domain.isActive(): - total_cpus = domain.maxVcpus() + # If we can't get it from maxVcpus() try to find it by + # inspecting the domain XML + if total_cpus <= 0: + vcpu_element = tree.find('.//vcpu') + if vcpu_element is not None: + total_cpus = int(vcpu_element.text) - # If we can't get it from maxVcpus() try to find it by - # inspecting the domain XML - if total_cpus <= 0: - vcpu_element = tree.find('.//vcpu') - if vcpu_element is not None: - total_cpus = int(vcpu_element.text) - - return total_cpus or None + return total_cpus or None def _process_bios_attributes(self, domain_xml, @@ -451,21 +466,24 @@ class LibvirtDriver(AbstractDriver): :raises: `FishyError` if BIOS attributes cannot be saved """ - with libvirt_open(self._uri) as conn: - libvirt_domain = conn.lookupByName(identity) - result = self._process_bios_attributes(libvirt_domain.XMLDesc(), - bios_attributes, - update_existing_attributes) + domain = self._get_domain(identity) - if result.attributes_written: - try: + result = self._process_bios_attributes(domain.XMLDesc(), + bios_attributes, + 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 BIOS attributes' - ' at libvirt URI "%(uri)s": ' - '%(error)s' % {'uri': self._uri, 'error': e}) - raise FishyError(msg) - return result.bios_attributes + + except libvirt.libvirtError as e: + msg = ('Error updating BIOS attributes' + ' at libvirt URI "%(uri)s": ' + '%(error)s' % {'uri': self._uri, 'error': e}) + raise FishyError(msg) + + return result.bios_attributes def get_bios(self, identity): """Get BIOS section @@ -516,9 +534,8 @@ class LibvirtDriver(AbstractDriver): :returns: list of network interfaces dict with their attributes """ - with libvirt_open(self._uri, readonly=True) as conn: - domain = conn.lookupByName(identity) - tree = ET.fromstring(domain.XMLDesc()) - return [{'id': iface.get('address'), 'mac': iface.get('address')} - for iface in tree.findall( - ".//devices/interface[@type='network']/mac")] + domain = self._get_domain(identity, readonly=True) + tree = ET.fromstring(domain.XMLDesc()) + return [{'id': iface.get('address'), 'mac': iface.get('address')} + for iface in tree.findall( + ".//devices/interface[@type='network']/mac")] diff --git a/sushy_tools/tests/unit/emulator/test_libvirt.py b/sushy_tools/tests/unit/emulator/test_libvirt.py index 43ec7f5d..8c3f4578 100644 --- a/sushy_tools/tests/unit/emulator/test_libvirt.py +++ b/sushy_tools/tests/unit/emulator/test_libvirt.py @@ -11,6 +11,8 @@ # under the License. import libvirt +import uuid + from oslotest import base from six.moves import mock @@ -24,6 +26,40 @@ class LibvirtDriverTestCase(base.BaseTestCase): self.test_driver = LibvirtDriver() super(LibvirtDriverTestCase, self).setUp() + @mock.patch('libvirt.open', autospec=True) + def test__get_domain_by_name(self, libvirt_mock): + domain_id = 'name' + domain_info = 'domain' + + conn_mock = libvirt_mock.return_value + lookupByName_mock = conn_mock.lookupByName + lookupByName_mock.return_value = domain_info + lookupByUUID_mock = conn_mock.lookupByUUID + + domain = self.test_driver._get_domain(domain_id) + + self.assertEqual(domain_info, domain) + lookupByName_mock.assert_called_once_with(domain_id) + self.assertFalse(lookupByUUID_mock.called) + + @mock.patch('libvirt.open', autospec=True) + def test__get_domain_by_uuid(self, libvirt_mock): + domain_id = uuid.UUID('b9fbc4f5-2c81-4c80-97ea-272621fb7360') + domain_info = 'domain' + + conn_mock = libvirt_mock.return_value + lookupByName_mock = conn_mock.lookupByName + lookupByName_mock.side_effect = libvirt.libvirtError( + 'domain not found') + lookupByUUID_mock = conn_mock.lookupByUUID + lookupByUUID_mock.return_value = domain_info + + domain = self.test_driver._get_domain(str(domain_id)) + + self.assertEqual(domain_info, domain) + lookupByName_mock.assert_called_once_with(str(domain_id)) + lookupByUUID_mock.assert_called_once_with(domain_id.bytes) + @mock.patch('libvirt.openReadOnly', autospec=True) def test_uuid(self, libvirt_mock): conn_mock = libvirt_mock.return_value @@ -169,7 +205,8 @@ class LibvirtDriverTestCase(base.BaseTestCase): self.assertEqual('Legacy', boot_mode) @mock.patch('libvirt.open', autospec=True) - def test_set_boot_mode(self, libvirt_mock): + @mock.patch('libvirt.openReadOnly', autospec=True) + def test_set_boot_mode(self, libvirt_mock, libvirt_rw_mock): with open('sushy_tools/tests/unit/emulator/domain.xml', 'r') as f: data = f.read() @@ -179,6 +216,7 @@ class LibvirtDriverTestCase(base.BaseTestCase): self.test_driver.set_boot_mode('zzzz-yyyy-xxxx', 'Uefi') + conn_mock = libvirt_rw_mock.return_value conn_mock.defineXML.assert_called_once_with(mock.ANY) @mock.patch('libvirt.openReadOnly', autospec=True)