From e71081b856b06513cc1e56a9adb2a619967f0eae Mon Sep 17 00:00:00 2001 From: Rafi Khardalian Date: Thu, 13 Jun 2013 05:09:45 +0000 Subject: 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 --- nova/tests/virt/libvirt/test_libvirt.py | 80 ++++++++++++++++++++++++++++++++- nova/tests/virt/test_virt_drivers.py | 3 +- nova/virt/libvirt/driver.py | 26 +++++++++-- 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 = ("instance-0000000a" + "" + "" + "" + "" + "" + "" + "" + "" + "" + "" + "" + "" + "" + "") + + # 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 -- cgit