diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 2f2f8c374460..ede0e2e5f975 100755 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -17372,9 +17372,6 @@ class LibvirtDriverTestCase(test.NoDBTestCase): fake_net = _fake_network_info(self, 1) drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - drvr.image_backend = mock.Mock() - drvr.image_backend.by_name.return_value = drvr.image_backend - drvr.image_backend.exists.return_value = False with test.nested( mock.patch('nova.compute.utils.is_volume_backed_instance', @@ -17382,12 +17379,14 @@ class LibvirtDriverTestCase(test.NoDBTestCase): mock.patch.object(os.path, 'exists'), mock.patch.object(libvirt_utils, 'get_instance_path'), mock.patch.object(utils, 'execute'), - mock.patch.object(shutil, 'rmtree'), + mock.patch.object(drvr.image_backend, 'by_name', + new_callable=mock.NonCallableMock), mock.patch.object(drvr, '_undefine_domain'), mock.patch.object(drvr, 'unplug_vifs'), mock.patch.object(drvr, 'unfilter_instance') ) as (mock_volume_backed, mock_exists, mock_get_path, - mock_exec, mock_rmtree, mock_undef, mock_unplug, mock_unfilter): + mock_exec, mock_image_by_name, mock_undef, mock_unplug, + mock_unfilter): mock_exists.return_value = True mock_get_path.return_value = '/fake/inst' @@ -17395,7 +17394,6 @@ class LibvirtDriverTestCase(test.NoDBTestCase): mock_get_path.assert_called_once_with(ins_ref) mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize', delay_on_retry=True, attempts=5) - mock_rmtree.assert_called_once_with('/fake/inst') mock_undef.assert_called_once_with(ins_ref) mock_unplug.assert_called_once_with(ins_ref, fake_net) mock_unfilter.assert_called_once_with(ins_ref, fake_net) @@ -17441,6 +17439,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase): def test_cleanup_resize_snap_backend(self): CONF.set_override('policy_dirs', [], group='oslo_policy') + self.flags(images_type='rbd', group='libvirt') ins_ref = self._create_instance({'host': CONF.host}) drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) drvr.image_backend = mock.Mock() diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 5e5f9e34060b..d74e1a01ed56 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1154,32 +1154,26 @@ class LibvirtDriver(driver.ComputeDriver): utils.execute('rm', '-rf', target, delay_on_retry=True, attempts=5) - root_disk = self.image_backend.by_name(instance, 'disk') - # TODO(nic): Set ignore_errors=False in a future release. - # It is set to True here to avoid any upgrade issues surrounding - # instances being in pending resize state when the software is updated; - # in that case there will be no snapshot to remove. Once it can be - # reasonably assumed that no such instances exist in the wild - # anymore, it should be set back to False (the default) so it will - # throw errors, like it should. - if root_disk.exists(): - root_disk.remove_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME, - ignore_errors=True) - - # NOTE(mjozefcz): - # self.image_backend.image for some backends recreates instance - # directory and image disk.info - remove it here if exists - # Do not remove inst_base for volume-backed instances since that - # could potentially remove the files on the destination host - # if using shared storage. - if (os.path.exists(inst_base) and not root_disk.exists() and - not compute_utils.is_volume_backed_instance( - context, instance)): - try: - shutil.rmtree(inst_base) - except OSError as e: - if e.errno != errno.ENOENT: - raise + # NOTE(mriedem): Some image backends will recreate the instance path + # and disk.info during init, and all we need the root disk for + # here is removing cloned snapshots which is backend-specific, so + # check that first before initializing the image backend object. If + # there is ever an image type that supports clone *and* re-creates + # the instance directory and disk.info on init, this condition will + # need to be re-visited to make sure that backend doesn't re-create + # the disk. Refer to bugs: 1666831 1728603 1769131 + if self.image_backend.backend(CONF.libvirt.images_type).SUPPORTS_CLONE: + root_disk = self.image_backend.by_name(instance, 'disk') + # TODO(nic): Set ignore_errors=False in a future release. + # It is set to True here to avoid any upgrade issues surrounding + # instances being in pending resize state when the software is + # updated; in that case there will be no snapshot to remove. + # Once it can be reasonably assumed that no such instances exist + # in the wild anymore, it should be set back to False + # (the default) so it will throw errors, like it should. + if root_disk.exists(): + root_disk.remove_snap(libvirt_utils.RESIZE_SNAPSHOT_NAME, + ignore_errors=True) if instance.host != CONF.host: self._undefine_domain(instance)