From 6b9464e91840f6e006f537dcd3c0f656ae90a179 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Fri, 2 Dec 2016 19:44:32 +0530 Subject: [PATCH] VMware: Set backend UUID to volume UUID Currently we do not set the UUID of volume backing in vCenter to volume UUID. We identify the volume in the backend by its name. Querying the volume by name in vCenter is slow for large deployments. This patch sets the UUID of the volume backing in vCenter to Cinder volume UUID so that we can use vCenter SearchIndex API to query the volume by UUID which is much faster than query by name. Partial-bug: #1600754 Change-Id: Ie14ae1c3d49f7fc9d3ee9dc8173b305dff57a105 --- .../volume/drivers/vmware/test_vmware_vmdk.py | 27 ++++++++++++----- .../drivers/vmware/test_vmware_volumeops.py | 30 ++++++++++++++++--- cinder/volume/drivers/vmware/vmdk.py | 18 ++++++++--- cinder/volume/drivers/vmware/volumeops.py | 17 +++++++++++ 4 files changed, 76 insertions(+), 16 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py b/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py index d743f7eb6..42b9d2d84 100644 --- a/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py +++ b/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py @@ -496,7 +496,8 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): create_backing.assert_called_once_with( volume, create_params={vmdk.CREATE_PARAM_DISK_LESS: True, - vmdk.CREATE_PARAM_BACKING_NAME: disk_name}) + vmdk.CREATE_PARAM_BACKING_NAME: disk_name, + vmdk.CREATE_PARAM_TEMP_BACKING: True}) else: create_backing.assert_called_once_with( volume, create_params={vmdk.CREATE_PARAM_DISK_LESS: True}) @@ -517,10 +518,12 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): if disk_conversion: select_ds_for_volume.assert_called_once_with(volume) + extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: volume['id'], + volumeops.BACKING_UUID_KEY: volume['id']} vops.clone_backing.assert_called_once_with( volume['name'], backing, None, volumeops.FULL_CLONE_TYPE, datastore, disk_type=disk_type, host=host, resource_pool=rp, - folder=folder) + extra_config=extra_config, folder=folder) delete_tmp_backing.assert_called_once_with(backing) vops.update_backing_disk_uuid(clone, volume['id']) else: @@ -1211,6 +1214,8 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): vops.rename_backing.call_args_list) else: vops.rename_backing.assert_called_once_with(backing, uuid) + vops.update_backing_uuid.assert_called_once_with( + new_backing, volume['id']) vops.update_backing_disk_uuid.assert_called_once_with( new_backing, volume['id']) delete_temp_backing.assert_called_once_with(backing) @@ -1520,7 +1525,8 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): context, name, volume, tmp_file_path, file_size_bytes) self.assertEqual(vm_ref, ret) - extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: volume['id']} + extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: volume['id'], + volumeops.BACKING_UUID_KEY: volume['id']} vops.get_create_spec.assert_called_once_with( name, 0, disk_type, summary.name, profileId=profile_id, extra_config=extra_config) @@ -1895,7 +1901,8 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): volumeops.LINKED_CLONE_TYPE, fake_snapshot['volume_size']) - extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: fake_volume['id']} + extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: fake_volume['id'], + volumeops.BACKING_UUID_KEY: fake_volume['id']} volume_ops.clone_backing.assert_called_with(fake_volume['name'], fake_backing, fake_snapshot, @@ -1933,7 +1940,8 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): volume, backing, snapshot_ref, volumeops.LINKED_CLONE_TYPE, volume['size']) - extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: volume['id']} + extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: volume['id'], + volumeops.BACKING_UUID_KEY: volume['id']} vops.clone_backing.assert_called_once_with( volume['name'], backing, snapshot_ref, volumeops.LINKED_CLONE_TYPE, None, host=None, resource_pool=None, extra_config=extra_config, @@ -1971,7 +1979,8 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): fake_snapshot['volume_size']) _select_ds_for_volume.assert_called_with(fake_volume) - extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: fake_volume['id']} + extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: fake_volume['id'], + volumeops.BACKING_UUID_KEY: fake_volume['id']} volume_ops.clone_backing.assert_called_with( fake_volume['name'], fake_backing, @@ -2315,7 +2324,8 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): ret = self._driver._create_backing(volume, host, create_params) self.assertEqual(backing, ret) - extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: volume['id']} + extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: volume['id'], + volumeops.BACKING_UUID_KEY: volume['id']} vops.create_backing_disk_less.assert_called_once_with( 'vol-1', folder, @@ -2736,7 +2746,8 @@ class VMwareVcVmdkDriverTestCase(test.TestCase): vops.get_backing.assert_called_once_with(volume['name']) vops.update_backing_extra_config.assert_called_once_with( - backing, {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: ''}) + backing, {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: '', + volumeops.BACKING_UUID_KEY: ''}) @mock.patch('oslo_vmware.api.VMwareAPISession') def test_session(self, apiSession): diff --git a/cinder/tests/unit/volume/drivers/vmware/test_vmware_volumeops.py b/cinder/tests/unit/volume/drivers/vmware/test_vmware_volumeops.py index b8f94d819..f5dedfcdb 100644 --- a/cinder/tests/unit/volume/drivers/vmware/test_vmware_volumeops.py +++ b/cinder/tests/unit/volume/drivers/vmware/test_vmware_volumeops.py @@ -576,12 +576,14 @@ class VolumeOpsTestCase(test.TestCase): profile_id = mock.sentinel.profile_id option_key = mock.sentinel.key option_value = mock.sentinel.value - extra_config = {option_key: option_value} + extra_config = {option_key: option_value, + volumeops.BACKING_UUID_KEY: mock.sentinel.uuid} ret = self.vops._get_create_spec_disk_less(name, ds_name, profile_id, extra_config) factory.create.side_effect = None self.assertEqual(name, ret.name) + self.assertEqual(mock.sentinel.uuid, ret.instanceUuid) self.assertEqual('[%s]' % ds_name, ret.files.vmPathName) self.assertEqual("vmx-08", ret.version) self.assertEqual(profile_id, ret.vmProfile[0].profileId) @@ -959,7 +961,8 @@ class VolumeOpsTestCase(test.TestCase): rp = mock.sentinel.rp key = mock.sentinel.key value = mock.sentinel.value - extra_config = {key: value} + extra_config = {key: value, + volumeops.BACKING_UUID_KEY: mock.sentinel.uuid} ret = self.vops._get_clone_spec(datastore, disk_move_type, snapshot, backing, disk_type, host, rp, extra_config) @@ -968,6 +971,7 @@ class VolumeOpsTestCase(test.TestCase): self.assertFalse(ret.powerOn) self.assertFalse(ret.template) self.assertEqual(snapshot, ret.snapshot) + self.assertEqual(mock.sentinel.uuid, ret.config.instanceUuid) get_relocate_spec.assert_called_once_with(datastore, rp, host, disk_move_type, disk_type, None) @@ -1172,13 +1176,31 @@ class VolumeOpsTestCase(test.TestCase): get_extra_config_option_values.return_value = option_values backing = mock.sentinel.backing - extra_config = mock.sentinel.extra_config + option_key = mock.sentinel.key + option_value = mock.sentinel.value + extra_config = {option_key: option_value, + volumeops.BACKING_UUID_KEY: mock.sentinel.uuid} self.vops.update_backing_extra_config(backing, extra_config) - get_extra_config_option_values.assert_called_once_with(extra_config) + get_extra_config_option_values.assert_called_once_with( + {option_key: option_value}) + self.assertEqual(mock.sentinel.uuid, reconfig_spec.instanceUuid) self.assertEqual(option_values, reconfig_spec.extraConfig) reconfigure_backing.assert_called_once_with(backing, reconfig_spec) + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + '_reconfigure_backing') + def test_update_backing_uuid(self, reconfigure_backing): + reconfig_spec = mock.Mock() + self.session.vim.client.factory.create.return_value = reconfig_spec + + backing = mock.sentinel.backing + uuid = mock.sentinel.uuid + self.vops.update_backing_uuid(backing, uuid) + + self.assertEqual(mock.sentinel.uuid, reconfig_spec.instanceUuid) + reconfigure_backing.assert_called_once_with(backing, reconfig_spec) + def test_change_backing_profile(self): # Test change to empty profile. reconfig_spec = mock.Mock() diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py index b7ba0157e..7804aec9e 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -60,6 +60,7 @@ CREATE_PARAM_ADAPTER_TYPE = 'adapter_type' CREATE_PARAM_DISK_LESS = 'disk_less' CREATE_PARAM_BACKING_NAME = 'name' CREATE_PARAM_DISK_SIZE = 'disk_size' +CREATE_PARAM_TEMP_BACKING = 'temp_backing' TMP_IMAGES_DATASTORE_FOLDER_PATH = "cinder_temp/" @@ -393,7 +394,8 @@ class VMwareVcVmdkDriver(driver.VolumeDriver): return profile_id def _get_extra_config(self, volume): - return {EXTRA_CONFIG_VOLUME_ID_KEY: volume['id']} + return {EXTRA_CONFIG_VOLUME_ID_KEY: volume['id'], + volumeops.BACKING_UUID_KEY: volume['id']} def _create_backing(self, volume, host=None, create_params=None): """Create volume backing under the given host. @@ -418,6 +420,9 @@ class VMwareVcVmdkDriver(driver.VolumeDriver): volume['name']) extra_config = self._get_extra_config(volume) + # We shoudln't set backing UUID to volume UUID for temporary backing. + if create_params.get(CREATE_PARAM_TEMP_BACKING): + del extra_config[volumeops.BACKING_UUID_KEY] # default is a backing with single disk disk_less = create_params.get(CREATE_PARAM_DISK_LESS, False) @@ -954,6 +959,7 @@ class VMwareVcVmdkDriver(driver.VolumeDriver): # for clone operation. disk_name = uuidutils.generate_uuid() create_params[CREATE_PARAM_BACKING_NAME] = disk_name + create_params[CREATE_PARAM_TEMP_BACKING] = True else: disk_name = volume['name'] @@ -1006,6 +1012,7 @@ class VMwareVcVmdkDriver(driver.VolumeDriver): datastore = summary.datastore LOG.debug("Cloning temporary backing: %s for disk type " "conversion.", backing) + extra_config = self._get_extra_config(volume) clone = self.volumeops.clone_backing(volume['name'], backing, None, @@ -1014,9 +1021,11 @@ class VMwareVcVmdkDriver(driver.VolumeDriver): disk_type=disk_type, host=host, resource_pool=rp, + extra_config=extra_config, folder=folder) self._delete_temp_backing(backing) backing = clone + self.volumeops.update_backing_disk_uuid(backing, volume['id']) except Exception: # Delete backing and virtual disk created from image. @@ -1379,17 +1388,18 @@ class VMwareVcVmdkDriver(driver.VolumeDriver): volumeops.FULL_CLONE_TYPE, datastore, disk_type=new_disk_type, host=host, resource_pool=rp, folder=folder) - self.volumeops.update_backing_disk_uuid(new_backing, - volume['id']) self._delete_temp_backing(backing) backing = new_backing + self.volumeops.update_backing_uuid(backing, volume['id']) + self.volumeops.update_backing_disk_uuid(backing, + volume['id']) except exceptions.VimException: with excutils.save_and_reraise_exception(): LOG.exception(_LE("Error occurred while cloning " "backing:" " %s during retype."), backing) - if renamed: + if renamed and not new_backing: LOG.debug("Undo rename of backing: %(backing)s; " "changing name from %(new_name)s to " "%(old_name)s.", diff --git a/cinder/volume/drivers/vmware/volumeops.py b/cinder/volume/drivers/vmware/volumeops.py index e0fc2ed21..39a845f57 100644 --- a/cinder/volume/drivers/vmware/volumeops.py +++ b/cinder/volume/drivers/vmware/volumeops.py @@ -33,6 +33,8 @@ LOG = logging.getLogger(__name__) LINKED_CLONE_TYPE = 'linked' FULL_CLONE_TYPE = 'full' +BACKING_UUID_KEY = 'instanceUuid' + def split_datastore_path(datastore_path): """Split the datastore path to components. @@ -704,6 +706,8 @@ class VMwareVolumeOps(object): create_spec.vmProfile = [vmProfile] if extra_config: + if BACKING_UUID_KEY in extra_config: + create_spec.instanceUuid = extra_config.pop(BACKING_UUID_KEY) create_spec.extraConfig = self._get_extra_config_option_values( extra_config) @@ -1063,6 +1067,8 @@ class VMwareVolumeOps(object): if extra_config: config_spec = cf.create('ns0:VirtualMachineConfigSpec') + if BACKING_UUID_KEY in extra_config: + config_spec.instanceUuid = extra_config.pop(BACKING_UUID_KEY) config_spec.extraConfig = self._get_extra_config_option_values( extra_config) clone_spec.config = config_spec @@ -1259,6 +1265,8 @@ class VMwareVolumeOps(object): def update_backing_extra_config(self, backing, extra_config): cf = self._session.vim.client.factory reconfig_spec = cf.create('ns0:VirtualMachineConfigSpec') + if BACKING_UUID_KEY in extra_config: + reconfig_spec.instanceUuid = extra_config.pop(BACKING_UUID_KEY) reconfig_spec.extraConfig = self._get_extra_config_option_values( extra_config) self._reconfigure_backing(backing, reconfig_spec) @@ -1267,6 +1275,15 @@ class VMwareVolumeOps(object): {'backing': backing, 'extra_config': extra_config}) + def update_backing_uuid(self, backing, uuid): + cf = self._session.vim.client.factory + reconfig_spec = cf.create('ns0:VirtualMachineConfigSpec') + reconfig_spec.instanceUuid = uuid + self._reconfigure_backing(backing, reconfig_spec) + LOG.debug("Backing: %(backing)s reconfigured with uuid: %(uuid)s.", + {'backing': backing, + 'uuid': uuid}) + def delete_file(self, file_path, datacenter=None): """Delete file or folder on the datastore.