Merge "libvirt: check image type before removing snapshots in _cleanup_resize" into stable/pike16.1.3

2 files changed, 25 insertions, 32 deletions

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 2f2f8c3744..ede0e2e5f9 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 5e5f9e3406..d74e1a01ed 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)