From d7b831e38cbc029b7975d1059ed212762a5f6a5e Mon Sep 17 00:00:00 2001 From: Chris Friesen Date: Fri, 22 Jul 2016 16:33:43 -0600 Subject: [PATCH] Fix cold migration with qcow2 ephemeral disks If we have qcow2 ephemeral disks we need to ensure that the backing file gets created on a cold migration. This requires passing in the block_device_info when calling _create_image() so that we can loop over the ephemeral disks. Closes-Bug: #1605720 Co-Authored-By: Feodor Tersin Change-Id: Ie278bb10e1675ba1d903aaa3c0249be0d1cf147b --- .../unit/virt/libvirt/fake_imagebackend.py | 2 +- nova/tests/unit/virt/libvirt/test_driver.py | 88 ++++++++++++++++++- nova/virt/libvirt/driver.py | 31 +++++-- 3 files changed, 109 insertions(+), 12 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/fake_imagebackend.py b/nova/tests/unit/virt/libvirt/fake_imagebackend.py index ba3b2a49b109..d92e5a370b5f 100644 --- a/nova/tests/unit/virt/libvirt/fake_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/fake_imagebackend.py @@ -138,7 +138,7 @@ class ImageBackendFixture(fixtures.Fixture): # Used directly by callers. These would have been set if called # the real constructor. setattr(disk, 'path', path) - setattr(disk, 'is_block_dev', False) + setattr(disk, 'is_block_dev', mock.sentinel.is_block_dev) # Used by tests. Note that image_init is a closure over image_type. setattr(disk, 'image_type', image_type) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 653ee7fbe756..6290f4ae129e 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -19,6 +19,7 @@ import contextlib import copy import datetime import errno +import functools import glob import os import random @@ -10523,7 +10524,7 @@ class LibvirtConnTestCase(test.NoDBTestCase): return gotFiles, imported_files - def test_create_image_with_swap(self): + def test_create_image_with_flavor_swap(self): def enable_swap(instance_ref): # Turn on some swap to exercise that codepath in _create_image instance_ref['system_metadata']['instance_type_swap'] = 500 @@ -10554,6 +10555,52 @@ class LibvirtConnTestCase(test.NoDBTestCase): self.assertTrue(imported_files[0][0].endswith('/disk.config')) self.assertEqual('disk.config', imported_files[0][1]) + def test_create_image_with_swap(self): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + instance_ref = self.test_instance + instance_ref['image_ref'] = '' + instance = objects.Instance(**instance_ref) + # Also check that bdm specified swap takes precedence over flavor + # specified swap + instance.flavor.swap = 200 + image_meta = objects.ImageMeta.from_dict(self.test_image_meta) + bdi = {'swap': {'swap_size': 100, + 'device_name': '/dev/vdb'}, + 'block_device_mapping': [{'boot_index': 0}]} + disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type, + instance, image_meta, + block_device_info=bdi) + backend = self.useFixture(fake_imagebackend.ImageBackendFixture()) + + drvr._create_image(self.context, instance, disk_info['mapping'], + block_device_info=bdi) + + backend.disks['disk.swap'].cache.assert_called_once_with( + fetch_func=drvr._create_swap, context=self.context, + filename='swap_100', size=100 * units.Mi, swap_mb=100) + + def test_create_image_with_legacy_swap_resizing(self): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + instance_ref = self.test_instance + instance_ref['image_ref'] = '' + instance = objects.Instance(**instance_ref) + instance.flavor.swap = 200 + image_meta = objects.ImageMeta.from_dict(self.test_image_meta) + bdi = {'swap': {'swap_size': 100, + 'device_name': '/dev/vdb'}, + 'block_device_mapping': [{'boot_index': 0}]} + disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type, + instance, image_meta, + block_device_info=bdi) + backend = self.useFixture(fake_imagebackend.ImageBackendFixture()) + + drvr._create_image(self.context, instance, disk_info['mapping'], + block_device_info=bdi, ignore_bdi_for_swap=True) + + backend.disks['disk.swap'].cache.assert_called_once_with( + fetch_func=drvr._create_swap, context=self.context, + filename='swap_200', size=200 * units.Mi, swap_mb=200) + @mock.patch.object(nova.virt.libvirt.imagebackend.Image, 'cache', side_effect=exception.ImageNotFound(image_id='fake-id')) def test_create_image_not_exist_no_fallback(self, mock_cache): @@ -10597,6 +10644,37 @@ class LibvirtConnTestCase(test.NoDBTestCase): host='fake-source-host', receive=True) + @mock.patch('nova.virt.disk.api.get_file_extension_for_os_type') + def test_create_image_with_ephemerals(self, mock_get_ext): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + instance_ref = self.test_instance + instance_ref['image_ref'] = '' + instance = objects.Instance(**instance_ref) + image_meta = objects.ImageMeta.from_dict(self.test_image_meta) + bdi = {'ephemerals': [{'size': 100}], + 'block_device_mapping': [{'boot_index': 0}]} + disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type, + instance, image_meta, + block_device_info=bdi) + mock_get_ext.return_value = mock.sentinel.file_ext + backend = self.useFixture(fake_imagebackend.ImageBackendFixture()) + + drvr._create_image(self.context, instance, disk_info['mapping'], + block_device_info=bdi) + + backend.disks['disk.eph0'].cache.assert_called_once_with( + fetch_func=mock.ANY, context=self.context, + filename=('ephemeral_100_%s' % mock.sentinel.file_ext), + size=100 * units.Gi, ephemeral_size=100, specified_fs=None) + fetch_func = (backend.disks['disk.eph0'].cache. + mock_calls[0][2]['fetch_func']) + self.assertIsInstance(fetch_func, functools.partial) + self.assertEqual(drvr._create_ephemeral, fetch_func.func) + self.assertEqual( + dict(fs_label='ephemeral0', os_type=instance.os_type, + is_block_dev=mock.sentinel.is_block_dev), + fetch_func.keywords) + @mock.patch.object(nova.virt.libvirt.imagebackend.Image, 'cache') def test_create_image_resize_snap_backend(self, mock_cache): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -15579,6 +15657,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase): self.fake_create_domain_called = False self.fake_disk_resize_called = False create_image_called = [False] + bdi = {'block_device_mapping': []} def fake_to_xml(context, instance, network_info, disk_info, image_meta=None, rescue=None, @@ -15592,8 +15671,11 @@ class LibvirtDriverTestCase(test.NoDBTestCase): disk_mapping, suffix='', disk_images=None, network_info=None, block_device_info=None, inject_files=True, - fallback_from_host=None): + fallback_from_host=None, + ignore_bdi_for_swap=False): + self.assertTrue(ignore_bdi_for_swap) self.assertFalse(inject_files) + self.assertEqual(bdi, block_device_info) create_image_called[0] = True def fake_create_domain_and_network( @@ -15660,7 +15742,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase): self.drvr.finish_migration( context.get_admin_context(), migration, instance, disk_info, [], image_meta, - resize_instance, None, power_on) + resize_instance, bdi, power_on) mock_ensure_console_log.assert_called_once_with(instance) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 10bc929a52d6..5aebb9906efa 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -2960,7 +2960,8 @@ class LibvirtDriver(driver.ComputeDriver): disk_images=None, network_info=None, block_device_info=None, files=None, admin_pass=None, inject_files=True, - fallback_from_host=None): + fallback_from_host=None, + ignore_bdi_for_swap=False): booted_from_volume = self._is_booted_from_volume( instance, disk_mapping) @@ -3052,13 +3053,26 @@ class LibvirtDriver(driver.ComputeDriver): mapping = disk_mapping['disk.swap'] swap_mb = 0 - swap = driver.block_device_info_get_swap(block_device_info) - if driver.swap_is_usable(swap): - swap_mb = swap['swap_size'] - elif (inst_type['swap'] > 0 and - not block_device.volume_in_mapping( - mapping['dev'], block_device_info)): + if ignore_bdi_for_swap: + # This is a workaround to support legacy swap resizing, + # which does not touch swap size specified in bdm, + # but works with flavor specified size only. + # In this case we follow the legacy logic and ignore block + # device info completely. + # NOTE(ft): This workaround must be removed when a correct + # implementation of resize operation changing sizes in bdms is + # developed. Also at that stage we probably may get rid of + # the direct usage of flavor swap size here, + # leaving the work with bdm only. swap_mb = inst_type['swap'] + else: + swap = driver.block_device_info_get_swap(block_device_info) + if driver.swap_is_usable(swap): + swap_mb = swap['swap_size'] + elif (inst_type['swap'] > 0 and + not block_device.volume_in_mapping( + mapping['dev'], block_device_info)): + swap_mb = inst_type['swap'] if swap_mb > 0: size = swap_mb * units.Mi @@ -7192,7 +7206,8 @@ class LibvirtDriver(driver.ComputeDriver): # backing file. self._create_image(context, instance, block_disk_info['mapping'], network_info=network_info, - block_device_info=None, inject_files=False, + block_device_info=block_device_info, + inject_files=False, ignore_bdi_for_swap=True, fallback_from_host=migration.source_compute) # Required by Quobyte CI