Merge "Fix libvirt Connection.get_disks method"

openstack · May 13, 2012 · 2c7e0d1 · 2c7e0d1
2 parents 8a2d657 + a345ee5
commit 2c7e0d1
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 18 deletions.
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
@@ -693,6 +693,66 @@ def fake_lookup(id):
         devices = conn.get_all_block_devices()
         self.assertEqual(devices, ['/path/to/dev/1', '/path/to/dev/3'])
 
+    def test_get_disks(self):
+        xml = [
+            # NOTE(vish): id 0 is skipped
+            None,
+            """
+                <domain type='kvm'>
+                    <devices>
+                        <disk type='file'>
+                            <source file='filename'/>
+                            <target dev='vda' bus='virtio'/>
+                        </disk>
+                        <disk type='block'>
+                            <source dev='/path/to/dev/1'/>
+                            <target dev='vdb' bus='virtio'/>
+                        </disk>
+                    </devices>
+                </domain>
+            """,
+            """
+                <domain type='kvm'>
+                    <devices>
+                        <disk type='file'>
+                            <source file='filename'/>
+                            <target dev='vda' bus='virtio'/>
+                        </disk>
+                    </devices>
+                </domain>
+            """,
+            """
+                <domain type='kvm'>
+                    <devices>
+                        <disk type='file'>
+                            <source file='filename'/>
+                            <target dev='vda' bus='virtio'/>
+                        </disk>
+                        <disk type='block'>
+                            <source dev='/path/to/dev/3'/>
+                            <target dev='vdb' bus='virtio'/>
+                        </disk>
+                    </devices>
+                </domain>
+            """,
+        ]
+
+        def fake_lookup(id):
+            return FakeVirtDomain(xml[id])
+
+        def fake_lookup_name(name):
+            return FakeVirtDomain(xml[1])
+
+        self.mox.StubOutWithMock(connection.LibvirtConnection, '_conn')
+        connection.LibvirtConnection._conn.listDomainsID = lambda: range(4)
+        connection.LibvirtConnection._conn.lookupByID = fake_lookup
+        connection.LibvirtConnection._conn.lookupByName = fake_lookup_name
+
+        self.mox.ReplayAll()
+        conn = connection.LibvirtConnection(False)
+        devices = conn.get_disks(conn.list_instances()[0])
+        self.assertEqual(devices, ['vda', 'vdb'])
+
     @test.skip_if(missing_libvirt(), "Test requires libvirt")
     def test_snapshot_in_ami_format(self):
         self.flags(image_service='nova.image.fake.FakeImageService')

diff --git a/nova/virt/libvirt/connection.py b/nova/virt/libvirt/connection.py
@@ -1773,30 +1773,15 @@ def get_disks(self, instance_name):
         """
         domain = self._lookup_by_name(instance_name)
         xml = domain.XMLDesc(0)
-        doc = None
 
         try:
             doc = etree.fromstring(xml)
         except Exception:
             return []
 
-        disks = []
-
-        ret = doc.findall('./devices/disk')
-
-        for node in ret:
-            devdst = None
-
-            for child in node.children:
-                if child.name == 'target':
-                    devdst = child.prop('dev')
-
-            if devdst is None:
-                continue
-
-            disks.append(devdst)
-
-        return disks
+        return filter(bool,
+                      [target.get("dev") \
+                           for target in doc.findall('devices/disk/target')])
 
     def get_interfaces(self, instance_name):
         """