From d71445c7d2d2921d10a08f82330f0ab8ef4f7df2 Mon Sep 17 00:00:00 2001 From: ZHU ZHU Date: Wed, 3 Sep 2014 03:59:13 -0500 Subject: [PATCH] VMWare: Fix VM leak when deletion of VM during resizing During the VM resizing, before VM arrive RESIZED state, driver migrate_disk_and_power_off will initially rename orginal vm 'uuid' to be 'uuid-orig' and clone a new vm with 'uuid' name. When deletion VM is triggered at this time window, it wouldn't be able to delete the VM uuid-orig in VCenter and so cause VM leak. As VM task state will be set to 'deleting' and it can not be used to determine the resize migrating/migrated state, this fix will attempt to delete orig VM within destroy phase. Conflicts: nova/tests/virt/vmwareapi/test_driver_api.py nova/virt/vmwareapi/vmops.py Closes-Bug: #1359138 NOTE: the aformentioned patch broke Minesweeper. The fix was also cherry picked from commit e464bc518e8590d59c2741948466777982ca3319. This was to do two things: 1. Solve the actual bug 2. Ensure that the unit tests and Minesweeper passed Change-Id: I7598afbf0dc3c527471af34224003d28e64daaff (cherry-picked from e1f8664c9fa83f77f5bb763ffcc3157905ed954c) --- nova/tests/virt/vmwareapi/test_driver_api.py | 40 ++++++++++++++++++++ nova/virt/vmwareapi/vmops.py | 15 ++++++++ 2 files changed, 55 insertions(+) diff --git a/nova/tests/virt/vmwareapi/test_driver_api.py b/nova/tests/virt/vmwareapi/test_driver_api.py index 08adaadb52f8..310457cd2730 100644 --- a/nova/tests/virt/vmwareapi/test_driver_api.py +++ b/nova/tests/virt/vmwareapi/test_driver_api.py @@ -1441,6 +1441,46 @@ class VMwareAPIVMTestCase(test.NoDBTestCase): None, self.destroy_disks) self.assertFalse(mock_destroy.called) + def _destroy_instance_without_vm_ref(self, resize_exists=False, + task_state=None): + + def fake_vm_ref_from_name(session, vm_name): + if resize_exists: + return 'fake-ref' + + self._create_instance() + with contextlib.nested( + mock.patch.object(vm_util, 'get_vm_ref_from_name', + fake_vm_ref_from_name), + mock.patch.object(self.conn._session, + '_call_method'), + mock.patch.object(self.conn._vmops, + '_destroy_instance') + ) as (mock_get, mock_call, mock_destroy): + self.instance.task_state = task_state + self.conn.destroy(self.context, self.instance, + self.network_info, + None, True) + if resize_exists: + if task_state == task_states.RESIZE_REVERTING: + expected = 1 + else: + expected = 2 + else: + expected = 1 + self.assertEqual(expected, mock_destroy.call_count) + self.assertFalse(mock_call.called) + + def test_destroy_instance_without_vm_ref(self): + self._destroy_instance_without_vm_ref() + + def test_destroy_instance_without_vm_ref_with_resize(self): + self._destroy_instance_without_vm_ref(resize_exists=True) + + def test_destroy_instance_without_vm_ref_with_resize_revert(self): + self._destroy_instance_without_vm_ref(resize_exists=True, + task_state=task_states.RESIZE_REVERTING) + def _rescue(self, config_drive=False): def fake_attach_disk_to_vm(vm_ref, instance, adapter_type, disk_type, vmdk_path=None, diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py index c437360ac7ac..d93785459a33 100644 --- a/nova/virt/vmwareapi/vmops.py +++ b/nova/virt/vmwareapi/vmops.py @@ -1129,6 +1129,21 @@ class VMwareVMOps(object): self._destroy_instance(instance, network_info, destroy_disks=destroy_disks, instance_name=rescue_name) + # NOTE(arnaud): Destroy uuid-orig and uuid VMs iff it is not + # triggered by the revert resize api call. This prevents + # the uuid-orig VM to be deleted to be able to associate it later. + if instance['task_state'] != task_states.RESIZE_REVERTING: + # When VM deletion is triggered in middle of VM resize before VM + # arrive RESIZED state, uuid-orig VM need to deleted to avoid + # VM leak. Within method _destroy_instance it will check vmref + # exist or not before attempt deletion. + resize_orig_vmname = instance['uuid'] + self._migrate_suffix + vm_orig_ref = vm_util.get_vm_ref_from_name(self._session, + resize_orig_vmname) + if vm_orig_ref: + self._destroy_instance(instance, network_info, + destroy_disks=destroy_disks, + instance_name=resize_orig_vmname) self._destroy_instance(instance, network_info, destroy_disks=destroy_disks) LOG.debug(_("Instance destroyed"), instance=instance)