Fix libvirt driver to handle domains by UUID

The libvirt driver does not support domain identifiers in form
of UUID despite the abstract API requirement. This change
fixes that and effectively unifies libvirt driver API with
the nova one.

Story: 2004306
Task: 27868
Change-Id: I3098146e8f7d79234870b68f090678857c666475

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,12 +149,11 @@ 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 = self._get_domain(identity, readonly=True)
-            domain = conn.lookupByName(identity)
         return domain.UUIDString()
 
     def get_power_state(self, identity):
@@ -138,9 +164,7 @@ 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 = self._get_domain(identity, readonly=True)
-
-            domain = conn.lookupByName(identity)
         return 'On' if domain.isActive() else 'Off'
 
     def set_power_state(self, identity, state):
@@ -154,9 +178,7 @@ 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'):
@@ -192,9 +214,7 @@ 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())
 
@@ -216,9 +236,7 @@ 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())
@@ -242,6 +260,7 @@ class LibvirtDriver(AbstractDriver):
             boot_element.set('dev', target)
 
         try:
+            with libvirt_open(self._uri) as conn:
                 conn.defineXML(ET.tostring(tree).decode('utf-8'))
 
         except libvirt.libvirtError as e:
@@ -256,9 +275,7 @@ 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())
@@ -281,9 +298,7 @@ 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())
@@ -321,12 +336,15 @@ class LibvirtDriver(AbstractDriver):
                 # domain boot mode.
                 loader_element.text = loader_path
 
+            with libvirt_open(self._uri) as conn:
+
                 try:
                     conn.defineXML(ET.tostring(tree).decode('utf-8'))
 
                 except libvirt.libvirtError as e:
-                msg = ('Error changing boot mode at libvirt URI "%(uri)s": '
+                    msg = ('Error changing boot mode at libvirt URI '
-                       '%(error)s' % {'uri': self._uri, 'error': e})
+                           '"%(uri)s": %(error)s' % {'uri': self._uri,
+                                                     'error': e})
 
                     raise FishyError(msg)
 
@@ -338,8 +356,7 @@ 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 = self._get_domain(identity, readonly=True)
-            domain = conn.lookupByName(identity)
         return int(domain.maxMemory() / 1024 / 1024)
 
     def get_total_cpus(self, identity):
@@ -350,9 +367,7 @@ 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())
 
@@ -451,20 +466,23 @@ class LibvirtDriver(AbstractDriver):
 
         :raises: `FishyError` if BIOS attributes cannot be saved
         """
-        with libvirt_open(self._uri) as conn:
+        domain = self._get_domain(identity)
-            libvirt_domain = conn.lookupByName(identity)
+
-            result = self._process_bios_attributes(libvirt_domain.XMLDesc(),
+        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
 
     def get_bios(self, identity):
@@ -516,8 +534,7 @@ class LibvirtDriver(AbstractDriver):
 
         :returns: list of network interfaces dict with their attributes
         """
-        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())
         return [{'id': iface.get('address'), 'mac': iface.get('address')}
                 for iface in tree.findall(

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)