From: Vipin Balachandran Date: Thu, 26 Mar 2015 13:10:27 +0000 (-0700) Subject: VMware: Handle concurrent inventory folder create X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=2a20781029f9f978269bba508a6d6873917839d8;p=openstack-build%2Fcinder-build.git 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 f4b60ce61..a502de2a2 100644 --- 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 a826133ab..a99566fb7 100644 --- 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,