Merge "libvirt: Fix service-wide pauses caused by un-proxied libvirt calls"

nova/tests/unit/virt/libvirt/fakelibvirt.py

@@ -1213,6 +1213,13 @@ class DomainSnapshot(object):
         del self._domain._snapshots[self._name]
 
 
+class Secret(object):
+    """A stub Secret class. Not currently returned by any test, but required to
+    exist for introspection.
+    """
+    pass
+
+
 class Connection(object):
     def __init__(self, uri=None, readonly=False, version=FAKE_LIBVIRT_VERSION,
                  hv_version=FAKE_QEMU_VERSION, host_info=None, pci_info=None,
@@ -1634,6 +1641,8 @@ virDomain = Domain
 virNodeDevice = NodeDevice
 
 virConnect = Connection
+virSecret = Secret
+virNWFilter = NWFilter
 
 
 class FakeLibvirtFixture(fixtures.Fixture):

nova/tests/unit/virt/libvirt/test_host.py

@@ -18,6 +18,7 @@ import os
 
 import eventlet
 from eventlet import greenthread
+from eventlet import tpool
 import mock
 from oslo_utils.fixture import uuidsentinel as uuids
 from oslo_utils import uuidutils
@@ -1227,3 +1228,56 @@ class TestLibvirtSEVSupported(TestLibvirtSEV):
                        new=vc._domain_capability_features_with_SEV)
     def test_supported_with_feature(self, fake_exists):
         self.assertTrue(self.host.supports_amd_sev)
+
+
+class LibvirtTpoolProxyTestCase(test.NoDBTestCase):
+    def setUp(self):
+        super(LibvirtTpoolProxyTestCase, self).setUp()
+
+        self.useFixture(fakelibvirt.FakeLibvirtFixture())
+        self.host = host.Host("qemu:///system")
+
+        def _stub_xml(uuid):
+            return ("<domain>"
+                    "  <uuid>" + uuid + "</uuid>"
+                    "  <name>" + uuid + "</name>"
+                    "</domain>")
+
+        self.conn = self.host.get_connection()
+        self.conn.defineXML(_stub_xml(uuids.vm1)).create()
+        self.conn.defineXML(_stub_xml(uuids.vm2)).create()
+
+    def test_get_libvirt_proxy_classes(self):
+        proxy_classes = host.Host._get_libvirt_proxy_classes(host.libvirt)
+
+        # Assert the classes we're using currently
+        # NOTE(mdbooth): We're obviously asserting the wrong classes here
+        # because we're explicitly asserting the faked versions. This is a
+        # concession to avoid a test dependency on libvirt.
+        self.assertIn(fakelibvirt.virDomain, proxy_classes)
+        self.assertIn(fakelibvirt.virConnect, proxy_classes)
+        self.assertIn(fakelibvirt.virNodeDevice, proxy_classes)
+        self.assertIn(fakelibvirt.virSecret, proxy_classes)
+        self.assertIn(fakelibvirt.virNWFilter, proxy_classes)
+
+        # Assert that we filtered out libvirtError
+        self.assertNotIn(fakelibvirt.libvirtError, proxy_classes)
+
+    def test_tpool_get_connection(self):
+        # Test that Host.get_connection() returns a tpool.Proxy
+        self.assertIsInstance(self.conn, tpool.Proxy)
+
+    def test_tpool_instance_lookup(self):
+        # Test that domains returns by our libvirt connection are also proxied
+        dom = self.conn.lookupByUUIDString(uuids.vm1)
+        self.assertIsInstance(dom, tpool.Proxy)
+
+    def test_tpool_list_all_connections(self):
+        # Test that Host.list_all_connections() returns a list of proxied
+        # virDomain objects
+
+        domains = self.host.list_instance_domains()
+        self.assertEqual(2, len(domains))
+        for domain in domains:
+            self.assertIsInstance(domain, tpool.Proxy)
+            self.assertIn(domain.UUIDString(), (uuids.vm1, uuids.vm2))

nova/virt/libvirt/host.py

@@ -28,6 +28,7 @@ the other libvirt related classes
 """
 
 from collections import defaultdict
+import inspect
 import operator
 import os
 import socket
@@ -110,12 +111,39 @@ class Host(object):
         self._lifecycle_delay = 15
 
         self._initialized = False
+        self._libvirt_proxy_classes = self._get_libvirt_proxy_classes(libvirt)
+        self._libvirt_proxy = self._wrap_libvirt_proxy(libvirt)
 
         # AMD SEV is conditional on support in the hardware, kernel,
         # qemu, and libvirt.  This is determined on demand and
         # memoized by the supports_amd_sev property below.
         self._supports_amd_sev = None
 
+    @staticmethod
+    def _get_libvirt_proxy_classes(libvirt_module):
+        """Return a tuple for tpool.Proxy's autowrap argument containing all
+        classes defined by the libvirt module except libvirtError.
+        """
+
+        # Get a list of (name, class) tuples of libvirt classes
+        classes = inspect.getmembers(libvirt_module, inspect.isclass)
+
+        # Return a list of just the classes, filtering out libvirtError because
+        # we don't need to proxy that
+        return tuple([cls[1] for cls in classes if cls[0] != 'libvirtError'])
+
+    def _wrap_libvirt_proxy(self, obj):
+        """Return an object wrapped in a tpool.Proxy using autowrap appropriate
+        for the libvirt module.
+        """
+
+        # libvirt is not pure python, so eventlet monkey patching doesn't work
+        # on it. Consequently long-running libvirt calls will not yield to
+        # eventlet's event loop, starving all other greenthreads until
+        # completion. eventlet's tpool.Proxy handles this situation for us by
+        # executing proxied calls in a native thread.
+        return tpool.Proxy(obj, autowrap=self._libvirt_proxy_classes)
+
     def _native_thread(self):
         """Receives async events coming in from libvirtd.
 
@@ -224,8 +252,7 @@ class Host(object):
             _("Can not handle authentication request for %d credentials")
             % len(creds))
 
-    @staticmethod
-    def _connect(uri, read_only):
+    def _connect(self, uri, read_only):
         auth = [[libvirt.VIR_CRED_AUTHNAME,
                  libvirt.VIR_CRED_ECHOPROMPT,
                  libvirt.VIR_CRED_REALM,
@@ -238,12 +265,7 @@ class Host(object):
         flags = 0
         if read_only:
             flags = libvirt.VIR_CONNECT_RO
-        # tpool.proxy_call creates a native thread. Due to limitations
-        # with eventlet locking we cannot use the logging API inside
-        # the called function.
-        return tpool.proxy_call(
-            (libvirt.virDomain, libvirt.virConnect),
-            libvirt.openAuth, uri, auth, flags)
+        return self._libvirt_proxy.openAuth(uri, auth, flags)
 
     def _queue_event(self, event):
         """Puts an event on the queue for dispatch.
@@ -607,7 +629,12 @@ class Host(object):
         flags = libvirt.VIR_CONNECT_LIST_DOMAINS_ACTIVE
         if not only_running:
             flags = flags | libvirt.VIR_CONNECT_LIST_DOMAINS_INACTIVE
-        alldoms = self.get_connection().listAllDomains(flags)
+
+        # listAllDomains() returns <list of virDomain>, not <virDomain>, so
+        # tpool.Proxy's autowrap won't catch it. We need to wrap the
+        # contents of the list we return.
+        alldoms = (self._wrap_libvirt_proxy(dom)
+                   for dom in self.get_connection().listAllDomains(flags))
 
         doms = []
         for dom in alldoms: