VMware: Handle concurrent inventory folder create

Cinder volumes are organized under vCenter inventory folder
with name given by config option vmware_volume_folder. During
volume creation, this folder is created only if it doesn't
exist. Concurrent volume create operations when there is no
such folder can result in a race condition-- some of the
create operations will fail due to already existing folder.
This patch fix this by ignoring the exception and returning a
reference to the existing folder.

Change-Id: Ic9371957bb76a032985363880fbebda523574305
Closes-Bug: #1436844

diff --git a/cinder/tests/unit/test_vmware_volumeops.py b/cinder/tests/unit/test_vmware_volumeops.py
index f4b60ce610bbe1b650a35d4921844522b5f3d45d..a502de2a2114a441127f6aa658ec9719170fa523 100644 (file)
--- a/cinder/tests/unit/test_vmware_volumeops.py
+++ b/cinder/tests/unit/test_vmware_volumeops.py
@@ -403,6 +403,37 @@ class VolumeOpsTestCase(test.TestCase):
                                                         mock.sentinel.dc,
                                                         'vmFolder')
 
+    def test_create_folder_with_concurrent_create(self):
+        parent_folder = mock.sentinel.parent_folder
+        child_name = 'child_folder'
+
+        prop_val_1 = mock.Mock(ManagedObjectReference=[])
+
+        child_folder = mock.Mock(_type='Folder')
+        prop_val_2 = mock.Mock(ManagedObjectReference=[child_folder])
+
+        self.session.invoke_api.side_effect = [prop_val_1,
+                                               exceptions.DuplicateName,
+                                               prop_val_2,
+                                               child_name]
+
+        ret = self.vops.create_folder(parent_folder, child_name)
+
+        self.assertEqual(child_folder, ret)
+        expected_invoke_api = [mock.call(vim_util, 'get_object_property',
+                                         self.session.vim, parent_folder,
+                                         'childEntity'),
+                               mock.call(self.session.vim, 'CreateFolder',
+                                         parent_folder, name=child_name),
+                               mock.call(vim_util, 'get_object_property',
+                                         self.session.vim, parent_folder,
+                                         'childEntity'),
+                               mock.call(vim_util, 'get_object_property',
+                                         self.session.vim, child_folder,
+                                         'name')]
+        self.assertEqual(expected_invoke_api,
+                         self.session.invoke_api.mock_calls)
+
     def test_create_folder_with_empty_vmfolder(self):
         """Test create_folder when the datacenter vmFolder is empty"""
         child_folder = mock.sentinel.child_folder

diff --git a/cinder/volume/drivers/vmware/volumeops.py b/cinder/volume/drivers/vmware/volumeops.py
index a826133abe7b92b79fd327e63ec1c51b59267214..a99566fb7424fb3ee1ef94a172ea48e2d2bfada1 100644 (file)
--- a/cinder/volume/drivers/vmware/volumeops.py
+++ b/cinder/volume/drivers/vmware/volumeops.py
@@ -514,23 +514,7 @@ class VMwareVolumeOps(object):
                                         self._session.vim, datacenter,
                                         'vmFolder')
 
-    def create_folder(self, parent_folder, child_folder_name):
-        """Creates child folder with given name under the given parent folder.
-
-        The method first checks if a child folder already exists, if it does,
-        then it returns a moref for the folder, else it creates one and then
-        return the moref.
-
-        :param parent_folder: Reference to the folder entity
-        :param child_folder_name: Name of the child folder
-        :return: Reference to the child folder with input name if it already
-                 exists, else create one and return the reference
-        """
-        LOG.debug("Creating folder: %(child_folder_name)s under parent "
-                  "folder: %(parent_folder)s.",
-                  {'child_folder_name': child_folder_name,
-                   'parent_folder': parent_folder})
-
+    def _get_child_folder(self, parent_folder, child_folder_name):
         # Get list of child entities for the parent folder
         prop_val = self._session.invoke_api(vim_util, 'get_object_property',
                                             self._session.vim, parent_folder,
@@ -546,15 +530,40 @@ class VMwareVolumeOps(object):
                 child_entity_name = self.get_entity_name(child_entity)
                 if child_entity_name and (urllib.unquote(child_entity_name) ==
                                           child_folder_name):
-                    LOG.debug("Child folder: %s already present.",
-                              child_folder_name)
+                    LOG.debug("Child folder: %s exists.", child_folder_name)
                     return child_entity
 
-        # Need to create the child folder
-        child_folder = self._session.invoke_api(self._session.vim,
-                                                'CreateFolder', parent_folder,
-                                                name=child_folder_name)
-        LOG.debug("Created child folder: %s.", child_folder)
+    def create_folder(self, parent_folder, child_folder_name):
+        """Creates child folder with given name under the given parent folder.
+
+        The method first checks if a child folder already exists, if it does,
+        then it returns a moref for the folder, else it creates one and then
+        return the moref.
+
+        :param parent_folder: Reference to the folder entity
+        :param child_folder_name: Name of the child folder
+        :return: Reference to the child folder with input name if it already
+                 exists, else create one and return the reference
+        """
+        LOG.debug("Creating folder: %(child_folder_name)s under parent "
+                  "folder: %(parent_folder)s.",
+                  {'child_folder_name': child_folder_name,
+                   'parent_folder': parent_folder})
+
+        child_folder = self._get_child_folder(parent_folder, child_folder_name)
+        if not child_folder:
+            # Need to create the child folder.
+            try:
+                child_folder = self._session.invoke_api(self._session.vim,
+                                                        'CreateFolder',
+                                                        parent_folder,
+                                                        name=child_folder_name)
+                LOG.debug("Created child folder: %s.", child_folder)
+            except exceptions.DuplicateName:
+                # Another thread is trying to create the same folder, ignore
+                # the exception.
+                child_folder = self._get_child_folder(parent_folder,
+                                                      child_folder_name)
         return child_folder
 
     def extend_virtual_disk(self, requested_size_in_gb, name, dc_ref,