Some libvirt driver lookups lacks proper exception handling

Add error handling to list_instance_uuids() and get_memory_mb_used().
While fixing list_instance_uuids, this change also removes an
unnecessary extra lookup for every running instance.

Improve test coverage for list_instances() and list_instance_uuids().

Change-Id: I0cabe5f38d19143a3eb7b2ebce1ea6ea63f36929

3 files changed, 101 insertions, 4 deletions

diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py
index 6b70fe1c6..317a62d45 100644
--- a/nova/tests/virt/libvirt/test_libvirt.py
+++ b/nova/tests/virt/libvirt/test_libvirt.py
@@ -1046,7 +1046,20 @@ class LibvirtConnTestCase(test.TestCase):
         self.mox.ReplayAll()
         conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
         instances = conn.list_instances()
-        # Only one should be listed, since domain with ID 0 must be skiped
+        # Only one should be listed, since domain with ID 0 must be skipped
+        self.assertEquals(len(instances), 1)
+
+    def test_list_instance_uuids(self):
+        self.mox.StubOutWithMock(libvirt_driver.LibvirtDriver, '_conn')
+        libvirt_driver.LibvirtDriver._conn.lookupByID = self.fake_lookup
+        libvirt_driver.LibvirtDriver._conn.numOfDomains = lambda: 2
+        libvirt_driver.LibvirtDriver._conn.listDomainsID = lambda: [0, 1]
+        libvirt_driver.LibvirtDriver._conn.listDefinedDomains = lambda: []
+
+        self.mox.ReplayAll()
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        instances = conn.list_instance_uuids()
+        # Only one should be listed, since domain with ID 0 must be skipped
         self.assertEquals(len(instances), 1)
 
     def test_list_defined_instances(self):
@@ -1083,6 +1096,63 @@ class LibvirtConnTestCase(test.TestCase):
         # None should be listed, since we fake deleted the last one
         self.assertEquals(len(instances), 0)
 
+    def test_list_instance_uuids_when_instance_deleted(self):
+
+        def fake_lookup(instance_name):
+            raise libvirt.libvirtError("we deleted an instance!")
+
+        self.mox.StubOutWithMock(libvirt_driver.LibvirtDriver, '_conn')
+        libvirt_driver.LibvirtDriver._conn.lookupByID = fake_lookup
+        libvirt_driver.LibvirtDriver._conn.numOfDomains = lambda: 1
+        libvirt_driver.LibvirtDriver._conn.listDomainsID = lambda: [0, 1]
+        libvirt_driver.LibvirtDriver._conn.listDefinedDomains = lambda: []
+
+        self.mox.StubOutWithMock(libvirt.libvirtError, "get_error_code")
+        libvirt.libvirtError.get_error_code().AndReturn(
+            libvirt.VIR_ERR_NO_DOMAIN)
+
+        self.mox.ReplayAll()
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        instances = conn.list_instance_uuids()
+        # None should be listed, since we fake deleted the last one
+        self.assertEquals(len(instances), 0)
+
+    def test_list_instances_throws_nova_exception(self):
+        def fake_lookup(instance_name):
+            raise libvirt.libvirtError("we deleted an instance!")
+
+        self.mox.StubOutWithMock(libvirt_driver.LibvirtDriver, '_conn')
+        libvirt_driver.LibvirtDriver._conn.lookupByID = fake_lookup
+        libvirt_driver.LibvirtDriver._conn.numOfDomains = lambda: 1
+        libvirt_driver.LibvirtDriver._conn.listDomainsID = lambda: [0, 1]
+        libvirt_driver.LibvirtDriver._conn.listDefinedDomains = lambda: []
+
+        self.mox.StubOutWithMock(libvirt.libvirtError, "get_error_code")
+        libvirt.libvirtError.get_error_code().AndReturn(
+            libvirt.VIR_ERR_INTERNAL_ERROR)
+
+        self.mox.ReplayAll()
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        self.assertRaises(exception.NovaException, conn.list_instances)
+
+    def test_list_instance_uuids_throws_nova_exception(self):
+        def fake_lookup(instance_name):
+            raise libvirt.libvirtError("we deleted an instance!")
+
+        self.mox.StubOutWithMock(libvirt_driver.LibvirtDriver, '_conn')
+        libvirt_driver.LibvirtDriver._conn.lookupByID = fake_lookup
+        libvirt_driver.LibvirtDriver._conn.numOfDomains = lambda: 1
+        libvirt_driver.LibvirtDriver._conn.listDomainsID = lambda: [0, 1]
+        libvirt_driver.LibvirtDriver._conn.listDefinedDomains = lambda: []
+
+        self.mox.StubOutWithMock(libvirt.libvirtError, "get_error_code")
+        libvirt.libvirtError.get_error_code().AndReturn(
+            libvirt.VIR_ERR_INTERNAL_ERROR)
+
+        self.mox.ReplayAll()
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        self.assertRaises(exception.NovaException, conn.list_instance_uuids)
+
     def test_get_all_block_devices(self):
         xml = [
             # NOTE(vish): id 0 is skipped
diff --git a/nova/tests/virt/test_virt_drivers.py b/nova/tests/virt/test_virt_drivers.py
index c054b9624..6dc1d83d6 100644
--- a/nova/tests/virt/test_virt_drivers.py
+++ b/nova/tests/virt/test_virt_drivers.py
@@ -209,6 +209,10 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase):
         self.connection.list_instances()
 
     @catch_notimplementederror
+    def test_list_instance_uuids(self):
+        self.connection.list_instance_uuids()
+
+    @catch_notimplementederror
     def test_spawn(self):
         instance_ref, network_info = self._get_running_instance()
         domains = self.connection.list_instances()
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index a2d2d4a6f..de80efb02 100755
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -684,8 +684,26 @@ class LibvirtDriver(driver.ComputeDriver):
         return names
 
     def list_instance_uuids(self):
-        return [self._conn.lookupByName(name).UUIDString()
-                for name in self.list_instances()]
+        uuids = set()
+        for domain_id in self.list_instance_ids():
+            try:
+                # We skip domains with ID 0 (hypervisors).
+                if domain_id != 0:
+                    domain = self._lookup_by_id(domain_id)
+                    uuids.add(domain.UUIDString())
+            except exception.InstanceNotFound:
+                # Ignore deleted instance while listing
+                continue
+
+        # extend instance list to contain also defined domains
+        for domain_name in self._conn.listDefinedDomains():
+            try:
+                uuids.add(self._lookup_by_name(domain_name).UUIDString())
+            except exception.InstanceNotFound:
+                # Ignore deleted instance while listing
+                continue
+
+        return list(uuids)
 
     def plug_vifs(self, instance, network_info):
         """Plug VIFs into networks."""
@@ -2698,8 +2716,13 @@ class LibvirtDriver(driver.ComputeDriver):
         if CONF.libvirt_type == 'xen':
             used = 0
             for domain_id in self.list_instance_ids():
+                try:
+                    dom_mem = int(self._lookup_by_id(domain_id).info()[2])
+                except exception.InstanceNotFound:
+                    LOG.info(_("libvirt can't find a domain with id: %s")
+                             % domain_id)
+                    continue
                 # skip dom0
-                dom_mem = int(self._conn.lookupByID(domain_id).info()[2])
                 if domain_id != 0:
                     used += dom_mem
                 else: