diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 0d169e7a4cc4..d5434a15cd0b 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -9205,7 +9205,9 @@ class ComputeManager(manager.Manager):
# vpmem must be cleaned
do_cleanup = (not migrate_data.is_shared_instance_path or
has_vpmem or has_mdevs or power_management_possible)
- destroy_disks = not migrate_data.is_shared_block_storage
+ destroy_disks = not (
+ migrate_data.is_shared_block_storage or
+ migrate_data.is_shared_instance_path)
elif isinstance(migrate_data, migrate_data_obj.HyperVLiveMigrateData):
# NOTE(claudiub): We need to cleanup any zombie Planned VM.
do_cleanup = True
diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index 0b16fb821880..4b1b22f78009 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -11380,7 +11380,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
do_cleanup, destroy_disks = self.compute._live_migration_cleanup_flags(
migrate_data, migr_ctxt)
self.assertTrue(do_cleanup)
- self.assertTrue(destroy_disks)
+ self.assertFalse(destroy_disks)
def test_live_migration_cleanup_flags_block_migrate_libvirt(self):
migrate_data = objects.LibvirtLiveMigrateData(
@@ -11407,7 +11407,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
do_cleanup, destroy_disks = self.compute._live_migration_cleanup_flags(
migrate_data)
self.assertFalse(do_cleanup)
- self.assertTrue(destroy_disks)
+ self.assertFalse(destroy_disks)
def test_live_migration_cleanup_flags_shared_libvirt(self):
migrate_data = objects.LibvirtLiveMigrateData(
@@ -11418,6 +11418,16 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
self.assertFalse(do_cleanup)
self.assertFalse(destroy_disks)
+ def test_live_migration_cleanup_flags_shared_path_libvirt_mdev(self):
+ migrate_data = objects.LibvirtLiveMigrateData(
+ is_shared_block_storage=False,
+ is_shared_instance_path=True,
+ target_mdevs={})
+ do_cleanup, destroy_disks = self.compute._live_migration_cleanup_flags(
+ migrate_data)
+ self.assertTrue(do_cleanup)
+ self.assertFalse(destroy_disks)
+
def test_live_migration_cleanup_flags_live_migrate(self):
do_cleanup, destroy_disks = self.compute._live_migration_cleanup_flags(
{})
diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index edea0177ac17..170d8bfc9a61 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -21002,7 +21002,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
# is_shared_block_storage=True and destroy_disks=False.
instance = objects.Instance(self.context, **self.test_instance)
migrate_data = objects.LibvirtLiveMigrateData(
- is_shared_block_storage=True)
+ is_shared_block_storage=True,
+ is_shared_instance_path=False)
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
drvr.cleanup(
self.context, instance, network_info={}, destroy_disks=False,
@@ -21012,6 +21013,25 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertTrue(instance.cleaned)
save.assert_called_once_with()
+ @mock.patch.object(libvirt_driver.LibvirtDriver, 'delete_instance_files',
+ return_value=True)
+ @mock.patch.object(objects.Instance, 'save')
+ @mock.patch.object(libvirt_driver.LibvirtDriver, '_undefine_domain')
+ def test_cleanup_migrate_data_block_storage_and_share_instance_dir(
+ self, _undefine_domain, save, delete_instance_files
+ ):
+ # Test the case when the instance directory is on shared storage
+ # (e.g. NFS) and the instance is booted form volume.
+ instance = objects.Instance(self.context, **self.test_instance)
+ migrate_data = objects.LibvirtLiveMigrateData(
+ is_shared_block_storage=True,
+ is_shared_instance_path=True)
+ drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
+ drvr.cleanup(
+ self.context, instance, network_info={}, destroy_disks=False,
+ migrate_data=migrate_data, destroy_vifs=False)
+ delete_instance_files.assert_not_called()
+
@mock.patch.object(libvirt_driver.LibvirtDriver, 'delete_instance_files',
return_value=True)
@mock.patch.object(objects.Instance, 'save')
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 16834846e2d4..4e1fe43b236a 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1705,12 +1705,12 @@ class LibvirtDriver(driver.ComputeDriver):
cleanup_instance_dir = True
cleanup_instance_disks = True
else:
- # NOTE(mdbooth): I think the theory here was that if this is a
- # migration with shared block storage then we need to delete the
- # instance directory because that's not shared. I'm pretty sure
- # this is wrong.
+ # NOTE(mheler): For shared block storage we only need to clean up
+ # the instance directory when it's not on a shared path.
if migrate_data and 'is_shared_block_storage' in migrate_data:
- cleanup_instance_dir = migrate_data.is_shared_block_storage
+ cleanup_instance_dir = (
+ migrate_data.is_shared_block_storage and
+ not migrate_data.is_shared_instance_path)
# NOTE(lyarwood): The following workaround allows operators to
# ensure that non-shared instance directories are removed after an
diff --git a/releasenotes/notes/bug-2080436-568b03b5b5ba5760.yaml b/releasenotes/notes/bug-2080436-568b03b5b5ba5760.yaml
new file mode 100644
index 000000000000..fad6e75b0024
--- /dev/null
+++ b/releasenotes/notes/bug-2080436-568b03b5b5ba5760.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+ - |
+ Fixes a regression for live migration on shared storage that
+ was removing the backing disk and instance folder during the
+ cleanup of a virtual machine post live migration.
+ `bug 2080436
+ <https://bugs.launchpad.net/nova/+bug/2080436>`__ for details.