Fix resizes with attached file-based volumes

Bug 1190364

get_instance_disk_info() now takes an additional, optional argument
of block_device_info. When passed in, we use it to filter out
volumes, such that they are not returned. Previously, any devices
classified as "file" as parsed from the instance XML were considered
local disks. This assumption is incorrect, since some volumes are
simply files, rather than block devices.

Added a new test, based almost entirely off the existing test case
for this function, which represents an XML file with two fake
file-based (NFS-like) volumes and associated block_device_info to
claim those volumes.

Change-Id: I4809a8fd68eb0f709347aa7cad0fa85389ade41c

3 files changed, 103 insertions, 6 deletions

diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py
index 2a6884483..dc31109cb 100644
--- a/nova/tests/virt/libvirt/test_libvirt.py
+++ b/nova/tests/virt/libvirt/test_libvirt.py
@@ -2691,6 +2691,80 @@ class LibvirtConnTestCase(test.TestCase):
 
         db.instance_destroy(self.context, instance_ref['uuid'])
 
+    def test_get_instance_disk_info_excludes_volumes(self):
+        # Test data
+        instance_ref = db.instance_create(self.context, self.test_instance)
+        dummyxml = ("<domain type='kvm'><name>instance-0000000a</name>"
+                    "<devices>"
+                    "<disk type='file'><driver name='qemu' type='raw'/>"
+                    "<source file='/test/disk'/>"
+                    "<target dev='vda' bus='virtio'/></disk>"
+                    "<disk type='file'><driver name='qemu' type='qcow2'/>"
+                    "<source file='/test/disk.local'/>"
+                    "<target dev='vdb' bus='virtio'/></disk>"
+                    "<disk type='file'><driver name='qemu' type='qcow2'/>"
+                    "<source file='/fake/path/to/volume1'/>"
+                    "<target dev='vdc' bus='virtio'/></disk>"
+                    "<disk type='file'><driver name='qemu' type='qcow2'/>"
+                    "<source file='/fake/path/to/volume2'/>"
+                    "<target dev='vdd' bus='virtio'/></disk>"
+                    "</devices></domain>")
+
+        # Preparing mocks
+        vdmock = self.mox.CreateMock(libvirt.virDomain)
+        self.mox.StubOutWithMock(vdmock, "XMLDesc")
+        vdmock.XMLDesc(0).AndReturn(dummyxml)
+
+        def fake_lookup(instance_name):
+            if instance_name == instance_ref['name']:
+                return vdmock
+        self.create_fake_libvirt_mock(lookupByName=fake_lookup)
+
+        GB = 1024 * 1024 * 1024
+        fake_libvirt_utils.disk_sizes['/test/disk'] = 10 * GB
+        fake_libvirt_utils.disk_sizes['/test/disk.local'] = 20 * GB
+        fake_libvirt_utils.disk_backing_files['/test/disk.local'] = 'file'
+
+        self.mox.StubOutWithMock(os.path, "getsize")
+        os.path.getsize('/test/disk').AndReturn((10737418240))
+        os.path.getsize('/test/disk.local').AndReturn((3328599655))
+
+        ret = ("image: /test/disk\n"
+               "file format: raw\n"
+               "virtual size: 20G (21474836480 bytes)\n"
+               "disk size: 3.1G\n"
+               "cluster_size: 2097152\n"
+               "backing file: /test/dummy (actual path: /backing/file)\n")
+
+        self.mox.StubOutWithMock(os.path, "exists")
+        os.path.exists('/test/disk.local').AndReturn(True)
+
+        self.mox.StubOutWithMock(utils, "execute")
+        utils.execute('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info',
+                      '/test/disk.local').AndReturn((ret, ''))
+
+        self.mox.ReplayAll()
+        conn_info = {'driver_volume_type': 'fake'}
+        info = {'block_device_mapping': [
+                  {'connection_info': conn_info, 'mount_device': '/dev/vdc'},
+                  {'connection_info': conn_info, 'mount_device': '/dev/vdd'}]}
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        info = conn.get_instance_disk_info(instance_ref['name'],
+                                           block_device_info=info)
+        info = jsonutils.loads(info)
+        self.assertEquals(info[0]['type'], 'raw')
+        self.assertEquals(info[0]['path'], '/test/disk')
+        self.assertEquals(info[0]['disk_size'], 10737418240)
+        self.assertEquals(info[0]['backing_file'], "")
+        self.assertEquals(info[0]['over_committed_disk_size'], 0)
+        self.assertEquals(info[1]['type'], 'qcow2')
+        self.assertEquals(info[1]['path'], '/test/disk.local')
+        self.assertEquals(info[1]['virt_disk_size'], 21474836480)
+        self.assertEquals(info[1]['backing_file'], "file")
+        self.assertEquals(info[1]['over_committed_disk_size'], 18146236825)
+
+        db.instance_destroy(self.context, instance_ref['uuid'])
+
     def test_spawn_with_network_info(self):
         # Preparing mocks
         def fake_none(*args, **kwargs):
@@ -5137,7 +5211,8 @@ class LibvirtDriverTestCase(test.TestCase):
         self.counter = 0
         self.checked_shared_storage = False
 
-        def fake_get_instance_disk_info(instance, xml=None):
+        def fake_get_instance_disk_info(instance, xml=None,
+                                        block_device_info=None):
             return '[]'
 
         def fake_destroy(instance):
@@ -5189,7 +5264,8 @@ class LibvirtDriverTestCase(test.TestCase):
                       'disk_size': '83886080'}]
         disk_info_text = jsonutils.dumps(disk_info)
 
-        def fake_get_instance_disk_info(instance, xml=None):
+        def fake_get_instance_disk_info(instance, xml=None,
+                                        block_device_info=None):
             return disk_info_text
 
         def fake_destroy(instance):
diff --git a/nova/tests/virt/test_virt_drivers.py b/nova/tests/virt/test_virt_drivers.py
index a21993597..6cc9bef43 100644
--- a/nova/tests/virt/test_virt_drivers.py
+++ b/nova/tests/virt/test_virt_drivers.py
@@ -106,7 +106,8 @@ class _FakeDriverBackendTestCase(object):
         def fake_make_drive(_self, _path):
             pass
 
-        def fake_get_instance_disk_info(_self, instance, xml=None):
+        def fake_get_instance_disk_info(_self, instance, xml=None,
+                                        block_device_info=None):
             return '[]'
 
         self.stubs.Set(nova.virt.libvirt.driver.LibvirtDriver,
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 4544d35fb..4168e38c6 100755
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1434,7 +1434,8 @@ class LibvirtDriver(driver.ComputeDriver):
                           write_to_disk=True)
 
         # NOTE (rmk): Re-populate any missing backing files.
-        disk_info_json = self.get_instance_disk_info(instance['name'], xml)
+        disk_info_json = self.get_instance_disk_info(instance['name'], xml,
+                                                     block_device_info)
         self._create_images_and_backing(context, instance, disk_info_json)
 
         # Initialize all the necessary networking, block devices and
@@ -3540,7 +3541,8 @@ class LibvirtDriver(driver.ComputeDriver):
             dom = self._lookup_by_name(instance["name"])
             self._conn.defineXML(dom.XMLDesc(0))
 
-    def get_instance_disk_info(self, instance_name, xml=None):
+    def get_instance_disk_info(self, instance_name, xml=None,
+                               block_device_info=None):
         """Preparation block migration.
 
         :params instance:
@@ -3573,15 +3575,27 @@ class LibvirtDriver(driver.ComputeDriver):
                 LOG.warn(msg)
                 raise exception.InstanceNotFound(instance_id=instance_name)
 
+        # NOTE (rmk): When block_device_info is provided, we will use it to
+        #             filter out devices which are actually volumes.
+        block_device_mapping = driver.block_device_info_get_mapping(
+            block_device_info)
+
+        volume_devices = set()
+        for vol in block_device_mapping:
+            disk_dev = vol['mount_device'].rpartition("/")[2]
+            volume_devices.add(disk_dev)
+
         disk_info = []
         doc = etree.fromstring(xml)
         disk_nodes = doc.findall('.//devices/disk')
         path_nodes = doc.findall('.//devices/disk/source')
         driver_nodes = doc.findall('.//devices/disk/driver')
+        target_nodes = doc.findall('.//devices/disk/target')
 
         for cnt, path_node in enumerate(path_nodes):
             disk_type = disk_nodes[cnt].get('type')
             path = path_node.get('file')
+            target = target_nodes[cnt].attrib['dev']
 
             if disk_type != 'file':
                 LOG.debug(_('skipping %s since it looks like volume'), path)
@@ -3592,6 +3606,11 @@ class LibvirtDriver(driver.ComputeDriver):
                           instance_name)
                 continue
 
+            if target in volume_devices:
+                LOG.debug(_('skipping disk %(path)s (%(target)s) as it is a '
+                            'volume'), {'path': path, 'target': target})
+                continue
+
             # get the real disk size or
             # raise a localized error if image is unavailable
             dk_size = int(os.path.getsize(path))
@@ -3700,7 +3719,8 @@ class LibvirtDriver(driver.ComputeDriver):
                                    block_device_info=None):
         LOG.debug(_("Starting migrate_disk_and_power_off"),
                    instance=instance)
-        disk_info_text = self.get_instance_disk_info(instance['name'])
+        disk_info_text = self.get_instance_disk_info(instance['name'],
+                block_device_info=block_device_info)
         disk_info = jsonutils.loads(disk_info_text)
 
         # copy disks to destination