From d2f91755ab74372a966b0ebae710beccabef0821 Mon Sep 17 00:00:00 2001 From: rsritesh Date: Wed, 19 Apr 2017 12:02:30 +0530 Subject: [PATCH] libvirt: flatten rbd images when unshelving an instance Previously attempts to remove the shelved snapshot of an unshelved instance when using the rbd backends for both Nova and Glance would fail. This was due to the instance disk being cloned from and still referencing the shelved snapshot image in Glance, blocking any attempt to remove this image later in the unshelve process. After much debate this change attempts to fix this issue by flattening the instance disk while the instance is being spawned as part of an unshelve. For the rbd imagebackend this removes any reference to the shelved snapshot in Glance allowing this image to be removed. For all other imagebackends the call to flatten the image is currently a no-op. Co-Authored-By: Lee Yarwood Co-Authored-By: Vladyslav Drok NOTE(lyarwood): Test conflicts due to Ie3130e104d7ca80289f1bd9f0fee9a7a198c263c and I407034374fe17c4795762aa32575ba72d3a46fe8 not being present in stable/queens. Note that the latter was backported but then reverted via Ibf2b5eeafd962e93ae4ab6290015d58c33024132 resulting in this conflict. Conflicts: nova/tests/unit/virt/libvirt/test_driver.py Closes-Bug: #1653953 Change-Id: If3c9d1de3ce0fe394405bd1e1f0fa08ce2baeda8 (cherry picked from commit d89e7d7857e0ab56c3b088338272c24d0618c07f) (cherry picked from commit e802ede4b30b21c7590620abc142300a57bcf349) (cherry picked from commit e93bc57a73d8642012f759a4ffbe5289112ba490) --- nova/tests/unit/virt/libvirt/test_driver.py | 54 ++++++++++++++++++- .../unit/virt/libvirt/test_imagebackend.py | 6 +++ nova/virt/libvirt/driver.py | 20 +++++++ nova/virt/libvirt/imagebackend.py | 11 ++++ 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 9abf8ac8b7eb..036a518e34b4 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -764,6 +764,7 @@ def _create_test_instance(): 'vcpu_model': None, 'host': 'fake-host', 'task_state': None, + 'vm_state': None, } @@ -16092,8 +16093,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, prepare.side_effect = fake_prepare drvr = libvirt_driver.LibvirtDriver(virtapi, False) - instance = objects.Instance(vm_state=vm_states.BUILDING, - **self.test_instance) + instance = objects.Instance(**self.test_instance) + instance.vm_state = vm_states.BUILDING vifs = [{'id': 'vif1', 'active': False}, {'id': 'vif2', 'active': False}] @@ -17410,6 +17411,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase): inst['system_metadata'] = {} inst['metadata'] = {} inst['task_state'] = None + inst['vm_state'] = None inst.update(params) @@ -18460,6 +18462,54 @@ class LibvirtDriverTestCase(test.NoDBTestCase): bdm.append({'boot_index': 0}) self.assertTrue(func(bdi)) + def test_unshelve_noop_flatten_fetch_image_cache(self): + instance = self._create_instance( + params={'vm_state': vm_states.SHELVED_OFFLOADED}) + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + mock_imagebackend = mock.Mock(spec=imagebackend.Lvm) + mock_imagebackend.flatten.side_effect = NotImplementedError() + + # Assert that this doesn't raise NotImplementedError + drvr._try_fetch_image_cache(mock_imagebackend, mock.sentinel.fetch, + self.context, mock.sentinel.filename, uuids.image_id, + instance, mock.sentinel.size) + + # Assert that we cache and then flatten the image when an instance is + # still SHELVED_OFFLOADED during _try_fetch_image_cache. + mock_imagebackend.cache.assert_called_once_with( + fetch_func=mock.sentinel.fetch, context=self.context, + filename=mock.sentinel.filename, image_id=uuids.image_id, + size=mock.sentinel.size) + mock_imagebackend.flatten.assert_called_once() + + def test_unshelve_rbd_image_flatten_during_fetch_image_cache(self): + instance = self._create_instance( + params={'vm_state': vm_states.SHELVED_OFFLOADED}) + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + mock_rbd_driver = mock.Mock(spec=rbd_utils.RBDDriver) + mock_rbd_imagebackend = mock.Mock(spec=imagebackend.Rbd) + mock_rbd_imagebackend.rbd_name = mock.sentinel.rbd_name + mock_rbd_imagebackend.pool = mock.sentinel.rbd_pool + # This is logged so we can't use a sentinel + mock_rbd_imagebackend.path = 'rbd:pool/vol_disk' + mock_rbd_imagebackend.driver = mock_rbd_driver + mock_rbd_imagebackend.flatten.side_effect = \ + imagebackend.Rbd.flatten(mock_rbd_imagebackend) + + drvr._try_fetch_image_cache(mock_rbd_imagebackend, mock.sentinel.fetch, + self.context, mock.sentinel.filename, uuids.image_id, + instance, mock.sentinel.size) + + # Assert that we cache and then flatten the image when an instance is + # still SHELVED_OFFLOADED during _try_fetch_image_cache. + mock_rbd_imagebackend.cache.assert_called_once_with( + fetch_func=mock.sentinel.fetch, context=self.context, + filename=mock.sentinel.filename, image_id=uuids.image_id, + size=mock.sentinel.size) + mock_rbd_imagebackend.flatten.assert_called_once() + mock_rbd_driver.flatten.assert_called_once_with( + mock.sentinel.rbd_name, pool=mock.sentinel.rbd_pool) + @mock.patch('nova.virt.libvirt.driver.imagebackend') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._inject_data') @mock.patch('nova.virt.libvirt.driver.imagecache') diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index 4971758617e2..a76c4f60bd3c 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py @@ -1558,6 +1558,12 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): ["server1:1899", "server2:1920"]), model) + @mock.patch.object(rbd_utils.RBDDriver, 'flatten') + def test_flatten(self, mock_flatten): + image = self.image_class(self.INSTANCE, self.NAME) + image.flatten() + mock_flatten.assert_called_once_with(image.rbd_name, pool=self.POOL) + def test_import_file(self): image = self.image_class(self.INSTANCE, self.NAME) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 321af13dd988..d52d48221994 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -7680,6 +7680,26 @@ class LibvirtDriver(driver.ComputeDriver): image.cache(fetch_func=copy_from_host, size=size, filename=filename) + # NOTE(lyarwood): If the instance vm_state is shelved offloaded then we + # must be unshelving for _try_fetch_image_cache to be called. + if instance.vm_state == vm_states.SHELVED_OFFLOADED: + # NOTE(lyarwood): When using the rbd imagebackend the call to cache + # above will attempt to clone from the shelved snapshot in Glance + # if available from this compute. We then need to flatten the + # resulting image to avoid it still referencing and ultimately + # blocking the removal of the shelved snapshot at the end of the + # unshelve. This is a no-op for all but the rbd imagebackend. + try: + image.flatten() + LOG.debug('Image %s flattened successfully while unshelving ' + 'instance.', image.path, instance=instance) + except NotImplementedError: + # NOTE(lyarwood): There's an argument to be made for logging + # our inability to call flatten here, however given this isn't + # implemented for most of the backends it may do more harm than + # good, concerning operators etc so for now just pass. + pass + def _create_images_and_backing(self, context, instance, instance_dir, disk_info, fallback_from_host=None): """:param context: security context diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index f260296d2e81..d185b7e1eb2a 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -430,6 +430,14 @@ class Image(object): raise exception.ImageUnacceptable(image_id=image_id_or_uri, reason=reason) + def flatten(self): + """Flatten an image. + + The implementation of this method is optional and therefore is + not an abstractmethod. + """ + raise NotImplementedError('flatten() is not implemented') + def direct_snapshot(self, context, snapshot_name, image_format, image_id, base_image_id): """Prepare a snapshot for direct reference from glance. @@ -960,6 +968,9 @@ class Rbd(Image): raise exception.ImageUnacceptable(image_id=image_id_or_uri, reason=reason) + def flatten(self): + self.driver.flatten(self.rbd_name, pool=self.pool) + def get_model(self, connection): secret = None if CONF.libvirt.rbd_secret_uuid: