VMware: fix snapshot failure when host in maintenance mode

The root cause is due to a bug in the VC's handling of the
VirtualDiskManager.DeleteVirtualDisk_Task API, which allows the picking
of any host in a datacenter with access to the datastore participating
in the disk deletion picked be to perform the operation, even when the
host is in maintenance mode and hence will always reject the call when
sent.

The fix uses an alternative API (FileManager.DeleteDatastoreFile_Task)
to delete the vmdk and -flat vmdk files separately. This API does not
suffer from the above-mentioned failure mode.

Closes-Bug: #1229994

(cherry picked from commit 7910385)

Conflicts:
	nova/virt/vmwareapi/fake.py
	nova/virt/vmwareapi/vmops.py

Change-Id: I786365847673e5192a21b654cba951b2e7a6f291

nova/tests/virt/vmwareapi/test_vmwareapi.py

@@ -405,7 +405,6 @@ def _test_snapshot(self):
                  {'task_state': task_states.IMAGE_UPLOADING,
                   'expected_state': task_states.IMAGE_PENDING_UPLOAD}}]
         func_call_matcher = matchers.FunctionCallMatcher(expected_calls)
-        self._create_vm()
         info = self.conn.get_info({'uuid': self.uuid,
                                    'node': self.instance_node})
         self._check_vm_info(info, power_state.RUNNING)
@@ -417,6 +416,7 @@ def _test_snapshot(self):
         self.assertIsNone(func_call_matcher.match())
 
     def test_snapshot(self):
+        self._create_vm()
         self._test_snapshot()
 
     def test_snapshot_non_existent(self):
@@ -1138,6 +1138,29 @@ def test_snapshot(self):
 
         self.mox.ReplayAll()
 
+        self._create_vm()
+        self._test_snapshot()
+
+    def test_snapshot_using_file_manager(self):
+        self._create_vm()
+        uuid_str = uuidutils.generate_uuid()
+        self.mox.StubOutWithMock(uuidutils,
+                                 'generate_uuid')
+        uuidutils.generate_uuid().AndReturn(uuid_str)
+
+        self.mox.StubOutWithMock(vmops.VMwareVMOps,
+                                 '_delete_datastore_file')
+        # Check calls for delete vmdk and -flat.vmdk pair
+        self.conn._vmops._delete_datastore_file(
+                mox.IgnoreArg(),
+                "[fake-ds] vmware-tmp/%s-flat.vmdk" % uuid_str,
+                mox.IgnoreArg()).AndReturn(None)
+        self.conn._vmops._delete_datastore_file(
+                mox.IgnoreArg(),
+                "[fake-ds] vmware-tmp/%s.vmdk" % uuid_str,
+                mox.IgnoreArg()).AndReturn(None)
+
+        self.mox.ReplayAll()
         self._test_snapshot()
 
     def test_spawn_invalid_node(self):

nova/virt/vmwareapi/fake.py

@@ -23,12 +23,12 @@
 
 import collections
 import pprint
-import uuid
 
 from nova import exception
 from nova.openstack.common.gettextutils import _
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
+from nova.openstack.common import uuidutils
 from nova.virt.vmwareapi import error_util
 
 _CLASSES = ['Datacenter', 'Datastore', 'ResourcePool', 'VirtualMachine',
@@ -863,7 +863,7 @@ def __str__(self):
 
     def _login(self):
         """Logs in and sets the session object in the db."""
-        self._session = str(uuid.uuid4())
+        self._session = uuidutils.generate_uuid()
         session = DataObject()
         session.key = self._session
         _db_content['session'][self._session] = session
@@ -1096,9 +1096,6 @@ def __getattr__(self, attr_name):
         elif attr_name == "ExtendVirtualDisk_Task":
             return lambda *args, **kwargs: self._extend_disk(attr_name,
                                                 kwargs.get("size"))
-        elif attr_name == "DeleteVirtualDisk_Task":
-            return lambda *args, **kwargs: self._delete_disk(attr_name,
-                                                *args, **kwargs)
         elif attr_name == "Destroy_Task":
             return lambda *args, **kwargs: self._unregister_vm(attr_name,
                                                                *args, **kwargs)

nova/virt/vmwareapi/vmops.py

@@ -27,7 +27,6 @@
 import time
 import urllib
 import urllib2
-import uuid
 
 from oslo.config import cfg
 
@@ -40,6 +39,7 @@
 from nova.openstack.common import excutils
 from nova.openstack.common.gettextutils import _
 from nova.openstack.common import log as logging
+from nova.openstack.common import uuidutils
 from nova import utils
 from nova.virt import configdrive
 from nova.virt import driver
@@ -126,22 +126,35 @@ def list_instances(self):
         LOG.debug(_("Got total of %s instances") % str(len(lst_vm_names)))
         return lst_vm_names
 
-    def _extend_virtual_disk(self, instance, requested_size, name,
-                             datacenter):
+    def _extend_virtual_disk(self, instance, requested_size, name, dc_ref):
         service_content = self._session._get_vim().get_service_content()
         LOG.debug(_("Extending root virtual disk to %s"), requested_size)
         vmdk_extend_task = self._session._call_method(
                 self._session._get_vim(),
                 "ExtendVirtualDisk_Task",
                 service_content.virtualDiskManager,
                 name=name,
-                datacenter=datacenter,
+                datacenter=dc_ref,
                 newCapacityKb=requested_size,
                 eagerZero=False)
         self._session._wait_for_task(instance['uuid'],
                                      vmdk_extend_task)
         LOG.debug(_("Extended root virtual disk"))
 
+    def _delete_datastore_file(self, instance, datastore_path, dc_ref):
+        LOG.debug(_("Deleting the datastore file %s") % datastore_path,
+                  instance=instance)
+        vim = self._session._get_vim()
+        file_delete_task = self._session._call_method(
+                self._session._get_vim(),
+                "DeleteDatastoreFile_Task",
+                vim.get_service_content().fileManager,
+                name=datastore_path,
+                datacenter=dc_ref)
+        self._session._wait_for_task(instance['uuid'],
+                                     file_delete_task)
+        LOG.debug(_("Deleted the datastore file"), instance=instance)
+
     def spawn(self, context, instance, image_meta, injected_files,
               admin_password, network_info, block_device_info=None):
         """
@@ -320,27 +333,6 @@ def _create_virtual_disk():
                          "data_store_name": data_store_name},
                       instance=instance)
 
-        def _delete_disk_file(vmdk_path):
-            LOG.debug(_("Deleting the file %(vmdk_path)s "
-                        "on the ESX host local"
-                        "store %(data_store_name)s") %
-                        {"vmdk_path": vmdk_path,
-                         "data_store_name": data_store_name},
-                      instance=instance)
-            # Delete the vmdk file.
-            vmdk_delete_task = self._session._call_method(
-                        self._session._get_vim(),
-                        "DeleteDatastoreFile_Task",
-                        service_content.fileManager,
-                        name=vmdk_path,
-                        datacenter=dc_ref)
-            self._session._wait_for_task(instance['uuid'], vmdk_delete_task)
-            LOG.debug(_("Deleted the file %(vmdk_path)s on the "
-                        "ESX host local store %(data_store_name)s") %
-                        {"vmdk_path": vmdk_path,
-                         "data_store_name": data_store_name},
-                      instance=instance)
-
         def _fetch_image_on_esx_datastore():
             """Fetch image from Glance to ESX datastore."""
             LOG.debug(_("Downloading image file data %(image_ref)s to the ESX "
@@ -448,7 +440,9 @@ def _copy_virtual_disk(source, dest):
                 if disk_type != "sparse":
                    # Create a flat virtual disk and retain the metadata file.
                     _create_virtual_disk()
-                    _delete_disk_file(flat_uploaded_vmdk_path)
+                    self._delete_datastore_file(instance,
+                                                flat_uploaded_vmdk_path,
+                                                dc_ref)
 
                 _fetch_image_on_esx_datastore()
 
@@ -457,7 +451,9 @@ def _copy_virtual_disk(source, dest):
                     disk_type = "thin"
                     _copy_virtual_disk(sparse_uploaded_vmdk_path,
                                        uploaded_vmdk_path)
-                    _delete_disk_file(sparse_uploaded_vmdk_path)
+                    self._delete_datastore_file(instance,
+                                                sparse_uploaded_vmdk_path,
+                                                dc_ref)
             else:
                 # linked clone base disk exists
                 if disk_type == "sparse":
@@ -744,9 +740,11 @@ def _check_if_tmp_folder_exists():
         # Generate a random vmdk file name to which the coalesced vmdk content
         # will be copied to. A random name is chosen so that we don't have
         # name clashes.
-        random_name = str(uuid.uuid4())
-        dest_vmdk_file_location = vm_util.build_datastore_path(datastore_name,
+        random_name = uuidutils.generate_uuid()
+        dest_vmdk_file_path = vm_util.build_datastore_path(datastore_name,
                    "vmware-tmp/%s.vmdk" % random_name)
+        dest_vmdk_data_file_path = vm_util.build_datastore_path(datastore_name,
+                   "vmware-tmp/%s-flat.vmdk" % random_name)
         dc_ref = self._get_datacenter_ref_and_name()[0]
 
         def _copy_vmdk_content():
@@ -763,7 +761,7 @@ def _copy_vmdk_content():
                 service_content.virtualDiskManager,
                 sourceName=vmdk_file_path_before_snapshot,
                 sourceDatacenter=dc_ref,
-                destName=dest_vmdk_file_location,
+                destName=dest_vmdk_file_path,
                 destDatacenter=dc_ref,
                 destSpec=copy_spec,
                 force=False)
@@ -803,18 +801,13 @@ def _clean_temp_data():
             Delete temporary vmdk files generated in image handling
             operations.
             """
-            # Delete the temporary vmdk created above.
-            LOG.debug(_("Deleting temporary vmdk file %s")
-                        % dest_vmdk_file_location, instance=instance)
-            remove_disk_task = self._session._call_method(
-                self._session._get_vim(),
-                "DeleteVirtualDisk_Task",
-                service_content.virtualDiskManager,
-                name=dest_vmdk_file_location,
-                datacenter=dc_ref)
-            self._session._wait_for_task(instance['uuid'], remove_disk_task)
-            LOG.debug(_("Deleted temporary vmdk file %s")
-                        % dest_vmdk_file_location, instance=instance)
+            # The data file is the one occupying space, and likelier to see
+            # deletion problems, so prioritize its deletion first. In the
+            # unlikely event that its deletion fails, the small descriptor file
+            # is retained too by design since it makes little sense to remove
+            # it when the data disk it refers to still lingers.
+            for f in dest_vmdk_data_file_path, dest_vmdk_file_path:
+                self._delete_datastore_file(instance, f, dc_ref)
 
         _clean_temp_data()