From 6c6ffc0476b56fe0bf822ddcf3c9a2d543912f38 Mon Sep 17 00:00:00 2001 From: "zhu.boxiang" Date: Fri, 1 Mar 2019 16:16:31 +0800 Subject: [PATCH] Fix failure to boot instances with qcow2 format images Ceph doesn't support QCOW2 for hosting a virtual machine disk: http://docs.ceph.com/docs/master/rbd/rbd-openstack/ When we set image_type as rbd and force_raw_images as False and we don't launch an instance with boot-from-volume, the instance is spawned using qcow2 as root disk but fails to boot because data is accessed as raw. To fix this, we raise an error and refuse to start nova-compute service when force_raw_images and image_type are incompatible. When we import image into rbd, check the format of cache images. If the format is not raw, remove it first and fetch it again. It will be raw format now. Change-Id: I1aa471e8df69fbb6f5d9aeb35651bd32c7123d78 Closes-Bug: 1816686 --- nova/conf/compute.py | 3 ++ nova/conf/libvirt.py | 1 + nova/exception.py | 4 ++ .../unit/virt/libvirt/test_imagebackend.py | 46 +++++++++++++++++-- nova/tests/unit/virt/test_virt_drivers.py | 15 ++++++ nova/virt/libvirt/driver.py | 12 +++++ nova/virt/libvirt/imagebackend.py | 19 ++++++++ .../notes/bug-1816686-77060eb8f8bd4092.yaml | 13 ++++++ 8 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/bug-1816686-77060eb8f8bd4092.yaml diff --git a/nova/conf/compute.py b/nova/conf/compute.py index cf83090fdd16..f5c5a7f8c024 100644 --- a/nova/conf/compute.py +++ b/nova/conf/compute.py @@ -232,6 +232,9 @@ Possible values: Related options: * ``compute_driver``: Only the libvirt driver uses this option. +* ``[libvirt]/images_type``: If images_type is rbd, setting this option + to False is not allowed. See the bug + https://bugs.launchpad.net/nova/+bug/1816686 for more details. """), # NOTE(yamahata): ListOpt won't work because the command may include a comma. # For example: diff --git a/nova/conf/libvirt.py b/nova/conf/libvirt.py index 2046c350b738..5f8dd49e11a4 100644 --- a/nova/conf/libvirt.py +++ b/nova/conf/libvirt.py @@ -856,6 +856,7 @@ Related options: * compute.use_cow_images * images_volume_group * [workarounds]/ensure_libvirt_rbd_instance_dir_cleanup +* compute.force_raw_images """), cfg.StrOpt('images_volume_group', help=""" diff --git a/nova/exception.py b/nova/exception.py index 61839fd1e18a..eebc01127f82 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -187,6 +187,10 @@ class Invalid(NovaException): code = 400 +class InvalidConfiguration(Invalid): + msg_fmt = _("Configuration is Invalid.") + + class InvalidBDM(Invalid): msg_fmt = _("Block Device Mapping is Invalid.") diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index c9c1dc2fc563..a9d1f19f9744 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py @@ -1411,12 +1411,44 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): mock_exists.assert_has_calls([mock.call(), mock.call()]) fn.assert_called_once_with(target=self.TEMPLATE_PATH) + @mock.patch.object(images, 'qemu_img_info') + @mock.patch.object(os.path, 'exists', return_value=False) + def test__remove_non_raw_cache_image_not_exists( + self, mock_exists, mock_qemu): + image = self.image_class(self.INSTANCE, self.NAME) + image._remove_non_raw_cache_image(self.TEMPLATE_PATH) + mock_qemu.assert_not_called() + + @mock.patch.object(os, 'remove') + @mock.patch.object(images, 'qemu_img_info', + return_value=imageutils.QemuImgInfo()) + @mock.patch.object(os.path, 'exists', return_value=True) + def test__remove_non_raw_cache_image_with_raw_cache( + self, mock_exists, mock_qemu, mock_remove): + mock_qemu.return_value.file_format = 'raw' + image = self.image_class(self.INSTANCE, self.NAME) + image._remove_non_raw_cache_image(self.TEMPLATE_PATH) + mock_remove.assert_not_called() + + @mock.patch.object(os, 'remove') + @mock.patch.object(images, 'qemu_img_info', + return_value=imageutils.QemuImgInfo()) + @mock.patch.object(os.path, 'exists', return_value=True) + def test__remove_non_raw_cache_image_with_qcow2_cache( + self, mock_exists, mock_qemu, mock_remove): + mock_qemu.return_value.file_format = 'qcow2' + image = self.image_class(self.INSTANCE, self.NAME) + image._remove_non_raw_cache_image(self.TEMPLATE_PATH) + mock_remove.assert_called_once_with(self.TEMPLATE_PATH) + + @mock.patch.object(images, 'qemu_img_info', + return_value=imageutils.QemuImgInfo()) @mock.patch.object(rbd_utils.RBDDriver, 'resize') @mock.patch.object(imagebackend.Rbd, 'verify_base_size') @mock.patch.object(imagebackend.Rbd, 'get_disk_size') @mock.patch.object(imagebackend.Rbd, 'exists') def test_create_image_resize(self, mock_exists, mock_get, - mock_verify, mock_resize): + mock_verify, mock_resize, mock_qemu): fn = mock.MagicMock() full_size = self.SIZE * 2 @@ -1427,6 +1459,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): image = self.image_class(self.INSTANCE, self.NAME) mock_exists.return_value = False + mock_qemu.return_value.file_format = 'raw' mock_get.return_value = self.SIZE rbd_name = "%s_%s" % (self.INSTANCE['uuid'], self.NAME) cmd = ('rbd', 'import', '--pool', self.POOL, self.TEMPLATE_PATH, @@ -1443,13 +1476,17 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): mock_verify.assert_called_once_with(self.TEMPLATE_PATH, full_size) fn.assert_called_once_with(target=self.TEMPLATE_PATH) + @mock.patch.object(images, 'qemu_img_info', + return_value=imageutils.QemuImgInfo()) @mock.patch.object(imagebackend.Rbd, 'get_disk_size') @mock.patch.object(imagebackend.Rbd, 'exists') - def test_create_image_already_exists(self, mock_exists, mock_get): + def test_create_image_already_exists(self, mock_exists, mock_get, + mock_qemu): rbd_utils.rbd.RBD_FEATURE_LAYERING = 1 image = self.image_class(self.INSTANCE, self.NAME) mock_exists.return_value = True + mock_qemu.return_value.file_format = 'raw' mock_get.return_value = self.SIZE rbd_name = "%s_%s" % (self.INSTANCE['uuid'], self.NAME) fn = mock.MagicMock() @@ -1507,7 +1544,10 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): self.assertEqual(2361393152, image.get_disk_size(image.path)) size_mock.assert_called_once_with(image.rbd_name) - def test_create_image_too_small(self): + @mock.patch.object(images, 'qemu_img_info', + return_value=imageutils.QemuImgInfo()) + def test_create_image_too_small(self, mock_qemu): + mock_qemu.return_value.file_format = 'raw' image = self.image_class(self.INSTANCE, self.NAME) with mock.patch.object(image, 'driver') as driver_mock: driver_mock.exists.return_value = True diff --git a/nova/tests/unit/virt/test_virt_drivers.py b/nova/tests/unit/virt/test_virt_drivers.py index 7b40bea1ff68..adb6e06174a2 100644 --- a/nova/tests/unit/virt/test_virt_drivers.py +++ b/nova/tests/unit/virt/test_virt_drivers.py @@ -901,6 +901,21 @@ class LibvirtConnTestCase(_VirtDriverTestCase, test.TestCase): # stub out the unplug call to os-vif since we don't care about it. self.stub_out('os_vif.unplug', lambda a, kw: None) + def test_init_host_image_type_rbd_force_raw_images_true(self): + CONF.set_override('images_type', 'rbd', group='libvirt') + CONF.set_override('force_raw_images', True) + self.connection.init_host('myhostname') + + def test_init_host_image_type_non_rbd(self): + CONF.set_override('images_type', 'default', group='libvirt') + self.connection.init_host('myhostname') + + def test_init_host_raise_invalid_configuration(self): + CONF.set_override('images_type', 'rbd', group='libvirt') + CONF.set_override('force_raw_images', False) + self.assertRaises(exception.InvalidConfiguration, + self.connection.init_host, 'myhostname') + def test_force_hard_reboot(self): self.flags(wait_soft_reboot_seconds=0, group='libvirt') self.test_reboot() diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index d1c08ff3383d..6a384c80fabe 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -612,6 +612,18 @@ class LibvirtDriver(driver.ComputeDriver): "'live_migration_with_native_tls'.") raise exception.Invalid(msg) + # Some imagebackends are only able to import raw disk images, + # and will fail if given any other format. See the bug + # https://bugs.launchpad.net/nova/+bug/1816686 for more details. + if CONF.libvirt.images_type in ('rbd',): + if not CONF.force_raw_images: + msg = _("'[DEFAULT]/force_raw_images = False' is not " + "allowed with '[libvirt]/images_type = rbd'. " + "Please check the two configs and if you really " + "do want to use rbd as images_type, set " + "force_raw_images to True.") + raise exception.InvalidConfiguration(msg) + # TODO(sbauza): Remove this code once mediated devices are persisted # across reboots. if self._host.has_min_version(MIN_LIBVIRT_MDEV_SUPPORT): diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index 56fb7e76d1d9..b2347b373ca8 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -914,9 +914,28 @@ class Rbd(Image): """ return self.driver.size(self.rbd_name) + @staticmethod + def _remove_non_raw_cache_image(base): + # NOTE(boxiang): If the cache image file exists, we will check + # the format of it. Only raw format image is compatible for + # RBD image backend. If format is not raw, we will remove it + # at first. We limit force_raw_images to True this time. So + # the format of new cache image must be raw. + # We can remove this in 'U' version later. + if not os.path.exists(base): + return True + image_format = images.qemu_img_info(base) + if image_format.file_format != 'raw': + try: + os.remove(base) + except OSError as e: + LOG.warning("Ignoring failure to remove %(path)s: " + "%(error)s", {'path': base, 'error': e}) + def create_image(self, prepare_template, base, size, *args, **kwargs): if not self.exists(): + self._remove_non_raw_cache_image(base) prepare_template(target=base, *args, **kwargs) # prepare_template() may have cloned the image into a new rbd diff --git a/releasenotes/notes/bug-1816686-77060eb8f8bd4092.yaml b/releasenotes/notes/bug-1816686-77060eb8f8bd4092.yaml new file mode 100644 index 000000000000..268d65b1ea7a --- /dev/null +++ b/releasenotes/notes/bug-1816686-77060eb8f8bd4092.yaml @@ -0,0 +1,13 @@ +--- +upgrade: + - | + The libvirt driver's RBD imagebackend no longer supports setting + force_raw_images to False. Setting force_raw_images = False and + images_type = rbd in nova.conf will cause the nova compute service + to refuse to start. To fix this, set force_raw_images = True. This + change was required to fix the `bug 1816686`_. + + Note that non-raw cache image files will be removed if you set + force_raw_images = True and images_type = rbd now. + + .. _bug 1816686: https://bugs.launchpad.net/nova/+bug/1816686