Merge "libvirt: do not remove inst_base when volume-backed during resize" into stable/ocata

3 files changed, 65 insertions, 13 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index aeb095ec7b..60271eb63a 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -3522,6 +3522,7 @@ class ComputeManager(manager.Manager):
 
             network_info = self.network_api.get_instance_nw_info(context,
                                                                  instance)
+            # TODO(mriedem): Get BDMs here and pass them to the driver.
             self.driver.confirm_migration(context, migration, instance,
                                           network_info)
 
diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 0fe2cc8aa8..0efab9ec50 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -16524,8 +16524,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
         ins_ref = self._create_instance()
 
         self.mox.StubOutWithMock(self.drvr, "_cleanup_resize")
-        self.drvr._cleanup_resize(ins_ref,
-                             _fake_network_info(self, 1))
+        self.drvr._cleanup_resize(self.context, ins_ref,
+                                  _fake_network_info(self, 1))
 
         self.mox.ReplayAll()
         self.drvr.confirm_migration(self.context, "migration_ref", ins_ref,
@@ -16548,7 +16548,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
             mock_exists.return_value = True
             mock_get_path.return_value = '/fake/inst'
 
-            drvr._cleanup_resize(ins_ref, _fake_network_info(self, 1))
+            drvr._cleanup_resize(
+                self.context, ins_ref, _fake_network_info(self, 1))
             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)
@@ -16566,6 +16567,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
         drvr.image_backend.exists.return_value = False
 
         with test.nested(
+                mock.patch('nova.compute.utils.is_volume_backed_instance',
+                           return_value=False),
                 mock.patch.object(os.path, 'exists'),
                 mock.patch.object(libvirt_utils, 'get_instance_path'),
                 mock.patch.object(utils, 'execute'),
@@ -16573,12 +16576,12 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
                 mock.patch.object(drvr, '_undefine_domain'),
                 mock.patch.object(drvr, 'unplug_vifs'),
                 mock.patch.object(drvr, 'unfilter_instance')
-        ) as (mock_exists, mock_get_path, mock_exec, mock_rmtree,
-              mock_undef, mock_unplug, mock_unfilter):
+        ) as (mock_volume_backed, mock_exists, mock_get_path,
+              mock_exec, mock_rmtree, mock_undef, mock_unplug, mock_unfilter):
             mock_exists.return_value = True
             mock_get_path.return_value = '/fake/inst'
 
-            drvr._cleanup_resize(ins_ref, fake_net)
+            drvr._cleanup_resize(self.context, ins_ref, fake_net)
             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)
@@ -16587,6 +16590,45 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
             mock_unplug.assert_called_once_with(ins_ref, fake_net)
             mock_unfilter.assert_called_once_with(ins_ref, fake_net)
 
+    def test_cleanup_resize_not_same_host_volume_backed(self):
+        """Tests cleaning up after a resize is confirmed with a volume-backed
+        instance. The key point is that the instance base directory should not
+        be removed for volume-backed instances.
+        """
+        CONF.set_override('policy_dirs', [], group='oslo_policy')
+        host = 'not' + CONF.host
+        ins_ref = self._create_instance({'host': host})
+        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',
+                           return_value=True),
+                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, '_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_exists.return_value = True
+            mock_get_path.return_value = '/fake/inst'
+
+            drvr._cleanup_resize(self.context, ins_ref, fake_net)
+            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_not_called()
+            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)
+
     def test_cleanup_resize_snap_backend(self):
         CONF.set_override('policy_dirs', [], group='oslo_policy')
         ins_ref = self._create_instance({'host': CONF.host})
@@ -16605,7 +16647,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
             mock_exists.return_value = True
             mock_get_path.return_value = '/fake/inst'
 
-            drvr._cleanup_resize(ins_ref, _fake_network_info(self, 1))
+            drvr._cleanup_resize(
+                self.context, ins_ref, _fake_network_info(self, 1))
             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)
@@ -16622,17 +16665,20 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
         drvr.image_backend.exists.return_value = False
 
         with test.nested(
+                mock.patch('nova.compute.utils.is_volume_backed_instance',
+                           return_value=False),
                 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, 'remove_snap')) as (
-                mock_exists, mock_get_path, mock_exec, mock_rmtree,
-                mock_remove):
+                mock_volume_backed, mock_exists, mock_get_path,
+                mock_exec, mock_rmtree, mock_remove):
             mock_exists.return_value = True
             mock_get_path.return_value = '/fake/inst'
 
-            drvr._cleanup_resize(ins_ref, _fake_network_info(self, 1))
+            drvr._cleanup_resize(
+                self.context, ins_ref, _fake_network_info(self, 1))
             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)
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 5ebaedc256..d47971125f 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1120,7 +1120,7 @@ class LibvirtDriver(driver.ComputeDriver):
             enforce_multipath=True,
             host=CONF.host)
 
-    def _cleanup_resize(self, instance, network_info):
+    def _cleanup_resize(self, context, instance, network_info):
         inst_base = libvirt_utils.get_instance_path(instance)
         target = inst_base + '_resize'
 
@@ -1146,7 +1146,12 @@ class LibvirtDriver(driver.ComputeDriver):
         # NOTE(mjozefcz):
         # self.image_backend.image for some backends recreates instance
         # directory and image disk.info - remove it here if exists
-        if os.path.exists(inst_base) and not root_disk.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:
@@ -7584,7 +7589,7 @@ class LibvirtDriver(driver.ComputeDriver):
 
     def confirm_migration(self, context, migration, instance, network_info):
         """Confirms a resize, destroying the source VM."""
-        self._cleanup_resize(instance, network_info)
+        self._cleanup_resize(context, instance, network_info)
 
     @staticmethod
     def _get_io_devices(xml_doc):