From c56bbb74f196b43baabcd949109643b4bff3b65e Mon Sep 17 00:00:00 2001 From: Johnson koil raj Date: Sat, 31 Jan 2015 15:24:57 +0530 Subject: [PATCH] VMware: Fix missing target resource pool The volume is cloned while creating it from another volume, snapshot or image. Currently, the target resource pool is unset while calling vCenter CloneVM_Task API. So when a clone happens across clusters in a vCenter clone fails. This patch fixes the problem by setting the target resource pool (returned by datastore selection logic) while invoking clone. Closes-Bug: #1416456 Change-Id: Ib1023efe6fd19d604af89f7e4b4f67640d778d39 (cherry picked from commit 0df929e60c9053240b792dea7ed7e2c10de69fc1) --- cinder/tests/test_vmware_vmdk.py | 19 +++++++++++-------- cinder/tests/test_vmware_volumeops.py | 9 +++++---- cinder/volume/drivers/vmware/vmdk.py | 16 +++++++++------- cinder/volume/drivers/vmware/volumeops.py | 17 ++++++++++------- 4 files changed, 35 insertions(+), 26 deletions(-) diff --git a/cinder/tests/test_vmware_vmdk.py b/cinder/tests/test_vmware_vmdk.py index 9da3376d5..cf76beb2a 100644 --- a/cinder/tests/test_vmware_vmdk.py +++ b/cinder/tests/test_vmware_vmdk.py @@ -864,7 +864,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): select_ds_for_volume.assert_called_once_with(volume) vops.clone_backing.assert_called_once_with( volume['name'], backing, None, volumeops.FULL_CLONE_TYPE, - summary.datastore, disk_type, mock.sentinel.host) + summary.datastore, disk_type, mock.sentinel.host, mock.sentinel.rp) vops.delete_backing.assert_called_once_with(backing) self.assertFalse(extend_disk.called) @@ -1484,7 +1484,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): vops.rename_backing.assert_called_once_with(backing, uuid) vops.clone_backing.assert_called_once_with( vol['name'], backing, None, volumeops.FULL_CLONE_TYPE, - datastore, vmdk.THIN_VMDK_TYPE, host) + datastore, vmdk.THIN_VMDK_TYPE, host, rp) delete_temp_backing.assert_called_once_with(backing) vops.change_backing_profile.assert_called_once_with(clone, profile_id) @@ -1696,8 +1696,8 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): summary = mock.Mock() summary.datastore = mock.sentinel.datastore - select_ds.return_value = (mock.sentinel.host, mock.ANY, mock.ANY, - summary) + select_ds.return_value = (mock.sentinel.host, mock.sentinel.rp, + mock.ANY, summary) disk_type = vmdk.THIN_VMDK_TYPE get_disk_type.return_value = disk_type @@ -1714,7 +1714,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): context, src_uuid, volume, tmp_file_path, backup_size) vops.clone_backing.assert_called_once_with( volume['name'], src, None, volumeops.FULL_CLONE_TYPE, - summary.datastore, disk_type, mock.sentinel.host) + summary.datastore, disk_type, mock.sentinel.host, mock.sentinel.rp) delete_temp_backing.assert_called_once_with(src) create_backing.reset_mock() @@ -1736,7 +1736,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): context, src_uuid, volume, tmp_file_path, backup_size) vops.clone_backing.assert_called_once_with( dest_uuid, src, None, volumeops.FULL_CLONE_TYPE, - summary.datastore, disk_type, mock.sentinel.host) + summary.datastore, disk_type, mock.sentinel.host, mock.sentinel.rp) exp_rename_calls = [mock.call(backing, tmp_uuid), mock.call(dest, volume['name'])] self.assertEqual(exp_rename_calls, vops.rename_backing.call_args_list) @@ -2122,7 +2122,8 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): fake_snapshot, fake_type, None, - host=None) + host=None, + resource_pool=None) # If the volume size is greater than the original snapshot size, # _extend_vmdk_virtual_disk will be called. _extend_vmdk_virtual_disk.assert_called_with(fake_volume['name'], @@ -2168,7 +2169,9 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): fake_snapshot, volumeops.FULL_CLONE_TYPE, fake_datastore, - host=fake_host) + host=fake_host, + resource_pool= + fake_resource_pool) # If the volume size is greater than the original snapshot size, # _extend_vmdk_virtual_disk will be called. _extend_vmdk_virtual_disk.assert_called_with(fake_volume['name'], diff --git a/cinder/tests/test_vmware_volumeops.py b/cinder/tests/test_vmware_volumeops.py index 50a1f0cbd..f4b60ce61 100644 --- a/cinder/tests/test_vmware_volumeops.py +++ b/cinder/tests/test_vmware_volumeops.py @@ -1026,7 +1026,7 @@ class VolumeOpsTestCase(test.TestCase): self.assertEqual(mock.sentinel.new_backing, ret) disk_move_type = 'moveAllDiskBackingsAndDisallowSharing' get_clone_spec.assert_called_with(datastore, disk_move_type, snapshot, - backing, None, None) + backing, None, None, None) expected = [mock.call(vim_util, 'get_object_property', self.session.vim, backing, 'parent'), mock.call(self.session.vim, 'CloneVM_Task', backing, @@ -1042,7 +1042,7 @@ class VolumeOpsTestCase(test.TestCase): self.assertEqual(mock.sentinel.new_backing, ret) disk_move_type = 'createNewChildDiskBacking' get_clone_spec.assert_called_with(datastore, disk_move_type, snapshot, - backing, None, None) + backing, None, None, None) expected = [mock.call(vim_util, 'get_object_property', self.session.vim, backing, 'parent'), mock.call(self.session.vim, 'CloneVM_Task', backing, @@ -1053,14 +1053,15 @@ class VolumeOpsTestCase(test.TestCase): clone_type = None disk_type = 'thin' host = mock.sentinel.host + rp = mock.sentinel.rp self.session.invoke_api.reset_mock() ret = self.vops.clone_backing(name, backing, snapshot, clone_type, - datastore, disk_type, host) + datastore, disk_type, host, rp) self.assertEqual(mock.sentinel.new_backing, ret) disk_move_type = 'moveAllDiskBackingsAndDisallowSharing' get_clone_spec.assert_called_with(datastore, disk_move_type, snapshot, - backing, disk_type, host) + backing, disk_type, host, rp) expected = [mock.call(vim_util, 'get_object_property', self.session.vim, backing, 'parent'), mock.call(self.session.vim, 'CloneVM_Task', backing, diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py index 338765f84..c8c2eab3f 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -1144,7 +1144,7 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): if disk_conversion: # Clone the temporary backing for disk type conversion. - (host, _rp, _folder, summary) = self._select_ds_for_volume( + (host, rp, _folder, summary) = self._select_ds_for_volume( volume) datastore = summary.datastore LOG.debug("Cloning temporary backing: %s for disk type " @@ -1155,7 +1155,8 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): volumeops.FULL_CLONE_TYPE, datastore, disk_type, - host) + host, + rp) self._delete_temp_backing(backing) except Exception: # Delete backing and virtual disk created from image. @@ -1535,7 +1536,7 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): new_backing = self.volumeops.clone_backing( volume['name'], backing, None, volumeops.FULL_CLONE_TYPE, datastore, new_disk_type, - host) + host, rp) self._delete_temp_backing(backing) backing = new_backing except exceptions.VimException: @@ -1762,13 +1763,13 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): renamed = False try: # Find datastore for clone. - (host, _rp, _folder, summary) = self._select_ds_for_volume(volume) + (host, rp, _folder, summary) = self._select_ds_for_volume(volume) datastore = summary.datastore disk_type = VMwareEsxVmdkDriver._get_disk_type(volume) dest = self.volumeops.clone_backing(dest_name, src, None, volumeops.FULL_CLONE_TYPE, - datastore, disk_type, host) + datastore, disk_type, host, rp) if new_backing: LOG.debug("Created new backing: %s for restoring backup.", dest_name) @@ -2031,13 +2032,14 @@ class VMwareVcVmdkDriver(VMwareEsxVmdkDriver): """ datastore = None host = None + rp = None if not clone_type == volumeops.LINKED_CLONE_TYPE: # Pick a datastore where to create the full clone under any host - (host, _rp, _folder, summary) = self._select_ds_for_volume(volume) + (host, rp, _folder, summary) = self._select_ds_for_volume(volume) datastore = summary.datastore clone = self.volumeops.clone_backing(volume['name'], backing, snapshot, clone_type, datastore, - host=host) + host=host, resource_pool=rp) # If the volume size specified by the user is greater than # the size of the source volume, the newly created volume will # allocate the capacity to the size of the source volume in the backend diff --git a/cinder/volume/drivers/vmware/volumeops.py b/cinder/volume/drivers/vmware/volumeops.py index e401f7126..a826133ab 100644 --- a/cinder/volume/drivers/vmware/volumeops.py +++ b/cinder/volume/drivers/vmware/volumeops.py @@ -1012,7 +1012,7 @@ class VMwareVolumeOps(object): return self._get_parent(backing, 'Folder') def _get_clone_spec(self, datastore, disk_move_type, snapshot, backing, - disk_type, host=None): + disk_type, host=None, resource_pool=None): """Get the clone spec. :param datastore: Reference to datastore @@ -1021,6 +1021,7 @@ class VMwareVolumeOps(object): :param backing: Source backing VM :param disk_type: Disk type of clone :param host: Target host + :param resource_pool: Target resource pool :return: Clone spec """ if disk_type is not None: @@ -1028,7 +1029,7 @@ class VMwareVolumeOps(object): else: disk_device = None - relocate_spec = self._get_relocate_spec(datastore, None, host, + relocate_spec = self._get_relocate_spec(datastore, resource_pool, host, disk_move_type, disk_type, disk_device) cf = self._session.vim.client.factory @@ -1042,7 +1043,7 @@ class VMwareVolumeOps(object): return clone_spec def clone_backing(self, name, backing, snapshot, clone_type, datastore, - disk_type=None, host=None): + disk_type=None, host=None, resource_pool=None): """Clone backing. If the clone_type is 'full', then a full clone of the source volume @@ -1056,21 +1057,23 @@ class VMwareVolumeOps(object): :param datastore: Reference to the datastore entity :param disk_type: Disk type of the clone :param host: Target host + :param resource_pool: Target resource pool """ LOG.debug("Creating a clone of backing: %(back)s, named: %(name)s, " "clone type: %(type)s from snapshot: %(snap)s on " - "host: %(host)s, datastore: %(ds)s with disk type: " - "%(disk_type)s.", + "resource pool: %(resource_pool)s, host: %(host)s, " + "datastore: %(ds)s with disk type: %(disk_type)s.", {'back': backing, 'name': name, 'type': clone_type, 'snap': snapshot, 'ds': datastore, 'disk_type': disk_type, - 'host': host}) + 'host': host, 'resource_pool': resource_pool}) folder = self._get_folder(backing) if clone_type == LINKED_CLONE_TYPE: disk_move_type = 'createNewChildDiskBacking' else: disk_move_type = 'moveAllDiskBackingsAndDisallowSharing' clone_spec = self._get_clone_spec(datastore, disk_move_type, snapshot, - backing, disk_type, host) + backing, disk_type, host, + resource_pool) task = self._session.invoke_api(self._session.vim, 'CloneVM_Task', backing, folder=folder, name=name, spec=clone_spec) -- 2.45.2