From 92be3ae71ad2162091f92524a8c3034b85d222ad Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Mon, 27 Jul 2015 13:27:46 +0530 Subject: [PATCH] VMware: Set virtual disk UUID to volume ID The symlinks in /dev/disk/by-id/ (in a Linux guest OS running in VMware ESX based Nova instance) use UUID of virtual disk as the SCSI device ID when disk.EnableUUID is set to True in the virtual machine configuration file. This patch sets the UUID of virtual disk corresponding to a Cinder volume (at the time of volume creation) to volume ID so that end-users (of Nova instance) can identify the device corresponding to an attached volume in the guest using /dev/disk/by-id symlink. For example, given a Cinder volume with ID = xyz, user can identify the device corresponding to the volume in instance's guest using the symlink /dev/disk/by-id/scsi-xyz provided disk.EnableUUID is set to True in the instance's virtual machine configuration file (in ESX). Note: If the UUID of the virtual disk corresponding to a volume is unset at the time of volume creation, a random UUID will be used. Closes-Bug: #1475738 Change-Id: I8f9d210083010b25833a7b108ae135417c72fd09 --- cinder/tests/unit/test_vmware_vmdk.py | 72 ++++++++++++++++++---- cinder/tests/unit/test_vmware_volumeops.py | 29 +++++++++ cinder/volume/drivers/vmware/vmdk.py | 48 +++++++++------ cinder/volume/drivers/vmware/volumeops.py | 28 +++++++++ 4 files changed, 147 insertions(+), 30 deletions(-) diff --git a/cinder/tests/unit/test_vmware_vmdk.py b/cinder/tests/unit/test_vmware_vmdk.py index e05f44f58..81f329241 100644 --- a/cinder/tests/unit/test_vmware_vmdk.py +++ b/cinder/tests/unit/test_vmware_vmdk.py @@ -753,6 +753,9 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): create_disk_from_sparse_image.return_value = path create_disk_from_preallocated_image.return_value = path + clone = mock.sentinel.clone + vops.clone_backing.return_value = clone + volume_size = 2 vops.get_disk_size.return_value = volume_size * units.Gi @@ -780,12 +783,15 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): volume['name'], backing, None, volumeops.FULL_CLONE_TYPE, summary.datastore, disk_type, mock.sentinel.host, mock.sentinel.rp) vops.delete_backing.assert_called_once_with(backing) + vops.update_backing_disk_uuid.assert_called_once_with(clone, + volume['id']) self.assertFalse(extend_disk.called) vops.get_disk_size.return_value = 1 * units.Gi create_backing.reset_mock() vops.attach_disk_to_backing.reset_mock() vops.delete_backing.reset_mock() + vops.update_backing_disk_uuid.reset_mock() image_meta['properties']['vmware_disktype'] = 'preallocated' self._driver.copy_image_to_volume( @@ -800,6 +806,8 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): vops.attach_disk_to_backing.assert_called_once_with( backing, image_size_in_bytes / units.Ki, disk_type, adapter_type, path.get_descriptor_ds_file_path()) + vops.update_backing_disk_uuid.assert_called_once_with(backing, + volume['id']) extend_disk.assert_called_once_with(volume['name'], volume['size']) extend_disk.reset_mock() @@ -1089,6 +1097,9 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): volumeops.get_disk_size.return_value = size + backing = mock.sentinel.backing + download_image.return_value = backing + self._driver.copy_image_to_volume(fake_context, fake_volume, image_service, fake_image_id) @@ -1114,6 +1125,8 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): vm_folder=fake_folder, vm_import_spec=vm_import_spec, image_size=size) + volumeops.update_backing_disk_uuid.assert_called_once_with( + backing, fake_volume['id']) _extend_virtual_disk.assert_called_once_with(fake_volume['name'], fake_volume_size) @@ -1269,6 +1282,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): # Test with in-use volume. vol = {'size': 1, 'status': 'retyping', 'name': 'vol-1', + 'id': 'd11a82de-ddaa-448d-b50a-a255a7e61a1e', 'volume_type_id': 'def'} vol['volume_attachment'] = [mock.sentinel.volume_attachment] self.assertFalse(self._driver.retype(context, vol, new_type, diff, @@ -1401,6 +1415,7 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): vops.clone_backing.assert_called_once_with( vol['name'], backing, None, volumeops.FULL_CLONE_TYPE, datastore, vmdk.THIN_VMDK_TYPE, host, rp) + vops.update_backing_disk_uuid.assert_called_once_with(clone, vol['id']) delete_temp_backing.assert_called_once_with(backing) vops.change_backing_profile.assert_called_once_with(clone, profile_id) @@ -1413,12 +1428,14 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): vops.clone_backing.side_effect = exceptions.VimException('error') + vops.update_backing_disk_uuid.reset_mock() vops.rename_backing.reset_mock() vops.change_backing_profile.reset_mock() self.assertRaises( exceptions.VimException, self._driver.retype, context, vol, new_type, diff, host) + self.assertFalse(vops.update_backing_disk_uuid.called) exp_rename_calls = [mock.call(backing, uuid), mock.call(backing, vol['name'])] self.assertEqual(exp_rename_calls, vops.rename_backing.call_args_list) @@ -1618,8 +1635,12 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): disk_type = vmdk.THIN_VMDK_TYPE get_disk_type.return_value = disk_type + dest = mock.sentinel.dest + vops.clone_backing.return_value = dest + context = mock.sentinel.context - volume = {'name': 'vol-1', 'id': 1, 'size': 1} + volume = {'name': 'vol-1', + 'id': 'bd45dfe5-d411-435d-85ac-2605fe7d5d8f', 'size': 1} backing = None tmp_file_path = mock.sentinel.tmp_file_path backup_size = units.Gi @@ -1631,19 +1652,19 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): vops.clone_backing.assert_called_once_with( volume['name'], src, None, volumeops.FULL_CLONE_TYPE, summary.datastore, disk_type, mock.sentinel.host, mock.sentinel.rp) + vops.update_backing_disk_uuid.assert_called_once_with(dest, + volume['id']) delete_temp_backing.assert_called_once_with(src) create_backing.reset_mock() vops.clone_backing.reset_mock() + vops.update_backing_disk_uuid.reset_mock() delete_temp_backing.reset_mock() dest_uuid = "de4b0708-f947-4abe-98f8-75e52ce03b7b" tmp_uuid = "82c2a4f0-9064-4d95-bd88-6567a36018fa" generate_uuid.side_effect = [src_uuid, dest_uuid, tmp_uuid] - dest = mock.sentinel.dest - vops.clone_backing.return_value = dest - backing = mock.sentinel.backing self._driver._restore_backing( context, volume, backing, tmp_file_path, backup_size) @@ -1653,6 +1674,8 @@ class VMwareEsxVmdkDriverTestCase(test.TestCase): vops.clone_backing.assert_called_once_with( dest_uuid, src, None, volumeops.FULL_CLONE_TYPE, summary.datastore, disk_type, mock.sentinel.host, mock.sentinel.rp) + vops.update_backing_disk_uuid.assert_called_once_with(dest, + volume['id']) exp_rename_calls = [mock.call(backing, tmp_uuid), mock.call(dest, volume['name'])] self.assertEqual(exp_rename_calls, vops.rename_backing.call_args_list) @@ -2058,6 +2081,9 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): @mock.patch.object(VMDK_DRIVER, 'volumeops') def test_clone_backing_linked(self, volume_ops, _extend_vmdk_virtual_disk): """Test _clone_backing with clone type - linked.""" + clone = mock.sentinel.clone + volume_ops.clone_backing.return_value = clone + fake_size = 3 fake_volume = {'volume_type_id': None, 'name': 'fake_name', 'id': '51e47214-8e3c-475d-b44b-aea6cd3eef53', @@ -2070,6 +2096,7 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): self._driver._clone_backing(fake_volume, fake_backing, fake_snapshot, volumeops.LINKED_CLONE_TYPE, fake_snapshot['volume_size']) + extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: fake_volume['id']} volume_ops.clone_backing.assert_called_with(fake_volume['name'], fake_backing, @@ -2079,6 +2106,9 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): host=None, resource_pool=None, extra_config=extra_config) + volume_ops.update_backing_disk_uuid.assert_called_once_with( + clone, fake_volume['id']) + # 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'], @@ -2101,24 +2131,29 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): _extend_vmdk_virtual_disk): """Test _clone_backing with clone type - full.""" fake_host = mock.sentinel.host - fake_backing = mock.sentinel.backing fake_folder = mock.sentinel.folder fake_datastore = mock.sentinel.datastore fake_resource_pool = mock.sentinel.resourcePool fake_summary = mock.Mock(spec=object) fake_summary.datastore = fake_datastore fake_size = 3 + _select_ds_for_volume.return_value = (fake_host, + fake_resource_pool, + fake_folder, fake_summary) + + clone = mock.sentinel.clone + volume_ops.clone_backing.return_value = clone + + fake_backing = mock.sentinel.backing fake_volume = {'volume_type_id': None, 'name': 'fake_name', 'id': '51e47214-8e3c-475d-b44b-aea6cd3eef53', 'size': fake_size} fake_snapshot = {'volume_name': 'volume_name', 'name': 'snapshot_name', 'volume_size': 2} - _select_ds_for_volume.return_value = (fake_host, - fake_resource_pool, - fake_folder, fake_summary) self._driver._clone_backing(fake_volume, fake_backing, fake_snapshot, volumeops.FULL_CLONE_TYPE, fake_snapshot['volume_size']) + _select_ds_for_volume.assert_called_with(fake_volume) extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: fake_volume['id']} volume_ops.clone_backing.assert_called_with(fake_volume['name'], @@ -2130,6 +2165,9 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): resource_pool= fake_resource_pool, extra_config=extra_config) + volume_ops.update_backing_disk_uuid.assert_called_once_with( + clone, fake_volume['id']) + # 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'], @@ -2563,12 +2601,15 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): summary = mock.sentinel.summary select_ds_for_volume.return_value = (host, resource_pool, folder, summary) + backing = mock.sentinel.backing + vops.create_backing_disk_less.return_value = backing volume = {'name': 'vol-1', 'volume_type_id': None, 'size': 1, 'id': 'd11a82de-ddaa-448d-b50a-a255a7e61a1e'} create_params = {vmdk.CREATE_PARAM_DISK_LESS: True} - self._driver._create_backing(volume, host, create_params) + ret = self._driver._create_backing(volume, host, create_params) + self.assertEqual(backing, ret) extra_config = {vmdk.EXTRA_CONFIG_VOLUME_ID_KEY: volume['id']} vops.create_backing_disk_less.assert_called_once_with( 'vol-1', @@ -2578,10 +2619,13 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): summary.name, profileId=None, extra_config=extra_config) + self.assertFalse(vops.update_backing_disk_uuid.called) + vops.create_backing.return_value = backing create_params = {vmdk.CREATE_PARAM_ADAPTER_TYPE: 'ide'} - self._driver._create_backing(volume, host, create_params) + ret = self._driver._create_backing(volume, host, create_params) + self.assertEqual(backing, ret) vops.create_backing.assert_called_once_with('vol-1', units.Mi, vmdk.THIN_VMDK_TYPE, @@ -2592,12 +2636,16 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): profileId=None, adapter_type='ide', extra_config=extra_config) + vops.update_backing_disk_uuid.assert_called_once_with(backing, + volume['id']) vops.create_backing.reset_mock() + vops.update_backing_disk_uuid.reset_mock() backing_name = "temp-vol" create_params = {vmdk.CREATE_PARAM_BACKING_NAME: backing_name} - self._driver._create_backing(volume, host, create_params) + ret = self._driver._create_backing(volume, host, create_params) + self.assertEqual(backing, ret) vops.create_backing.assert_called_once_with(backing_name, units.Mi, vmdk.THIN_VMDK_TYPE, @@ -2608,6 +2656,8 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): profileId=None, adapter_type='lsiLogic', extra_config=extra_config) + vops.update_backing_disk_uuid.assert_called_once_with(backing, + volume['id']) @mock.patch('cinder.openstack.common.fileutils.ensure_tree') @mock.patch('cinder.openstack.common.fileutils.delete_if_exists') diff --git a/cinder/tests/unit/test_vmware_volumeops.py b/cinder/tests/unit/test_vmware_volumeops.py index d2510ad1c..fea5a00c3 100644 --- a/cinder/tests/unit/test_vmware_volumeops.py +++ b/cinder/tests/unit/test_vmware_volumeops.py @@ -1199,6 +1199,35 @@ class VolumeOpsTestCase(test.TestCase): newName=new_name) self.session.wait_for_task.assert_called_once_with(task) + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + '_get_disk_device') + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + '_reconfigure_backing') + def test_update_backing_disk_uuid(self, reconfigure_backing, + get_disk_device): + disk_spec = mock.Mock() + reconfig_spec = mock.Mock() + self.session.vim.client.factory.create.side_effect = [disk_spec, + reconfig_spec] + + disk_device = mock.Mock() + get_disk_device.return_value = disk_device + + self.vops.update_backing_disk_uuid(mock.sentinel.backing, + mock.sentinel.disk_uuid) + + get_disk_device.assert_called_once_with(mock.sentinel.backing) + self.assertEqual(mock.sentinel.disk_uuid, disk_device.backing.uuid) + self.assertEqual('edit', disk_spec.operation) + self.assertEqual(disk_device, disk_spec.device) + self.assertEqual([disk_spec], reconfig_spec.deviceChange) + reconfigure_backing.assert_called_once_with(mock.sentinel.backing, + reconfig_spec) + exp_factory_create_calls = [mock.call('ns0:VirtualDeviceConfigSpec'), + mock.call('ns0:VirtualMachineConfigSpec')] + self.assertEqual(exp_factory_create_calls, + self.session.vim.client.factory.create.call_args_list) + 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 d29a056d8..48628f328 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -462,16 +462,19 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): size_kb = volume['size'] * units.Mi adapter_type = create_params.get(CREATE_PARAM_ADAPTER_TYPE, 'lsiLogic') - return self.volumeops.create_backing(backing_name, - size_kb, - disk_type, - folder, - resource_pool, - host_ref, - summary.name, - profileId=profile_id, - adapter_type=adapter_type, - extra_config=extra_config) + backing = self.volumeops.create_backing(backing_name, + size_kb, + disk_type, + folder, + resource_pool, + host_ref, + summary.name, + profileId=profile_id, + adapter_type=adapter_type, + extra_config=extra_config) + + self.volumeops.update_backing_disk_uuid(backing, volume['id']) + return backing def _relocate_backing(self, volume, backing, host): pass @@ -1072,15 +1075,17 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): datastore = summary.datastore LOG.debug("Cloning temporary backing: %s for disk type " "conversion.", backing) - self.volumeops.clone_backing(volume['name'], - backing, - None, - volumeops.FULL_CLONE_TYPE, - datastore, - disk_type, - host, - rp) + clone = self.volumeops.clone_backing(volume['name'], + backing, + None, + volumeops.FULL_CLONE_TYPE, + datastore, + disk_type, + host, + rp) 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. with excutils.save_and_reraise_exception(): @@ -1143,7 +1148,7 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): host_ip = self.configuration.vmware_host_ip LOG.debug("Fetching glance image: %(id)s to server: %(host)s.", {'id': image_id, 'host': host_ip}) - image_transfer.download_stream_optimized_image( + backing = image_transfer.download_stream_optimized_image( context, timeout, image_service, @@ -1155,6 +1160,7 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): vm_folder=folder, vm_import_spec=vm_import_spec, image_size=image_size) + self.volumeops.update_backing_disk_uuid(backing, volume['id']) except (exceptions.VimException, exceptions.VMwareDriverException): with excutils.save_and_reraise_exception(): @@ -1463,6 +1469,8 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): volume['name'], backing, None, volumeops.FULL_CLONE_TYPE, datastore, new_disk_type, host, rp) + self.volumeops.update_backing_disk_uuid(new_backing, + volume['id']) self._delete_temp_backing(backing) backing = new_backing except exceptions.VimException: @@ -1699,6 +1707,7 @@ class VMwareEsxVmdkDriver(driver.VolumeDriver): dest = self.volumeops.clone_backing(dest_name, src, None, volumeops.FULL_CLONE_TYPE, datastore, disk_type, host, rp) + self.volumeops.update_backing_disk_uuid(dest, volume['id']) if new_backing: LOG.debug("Created new backing: %s for restoring backup.", dest_name) @@ -1974,6 +1983,7 @@ class VMwareVcVmdkDriver(VMwareEsxVmdkDriver): snapshot, clone_type, datastore, host=host, resource_pool=rp, extra_config=extra_config) + self.volumeops.update_backing_disk_uuid(clone, volume['id']) # 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 0f0b30bd8..f063638f6 100644 --- a/cinder/volume/drivers/vmware/volumeops.py +++ b/cinder/volume/drivers/vmware/volumeops.py @@ -1225,6 +1225,34 @@ class VMwareVolumeOps(object): {'backing': backing, 'profile': profile_id}) + def update_backing_disk_uuid(self, backing, disk_uuid): + """Update backing VM's disk UUID. + + :param backing: Reference to backing VM + :param disk_uuid: New disk UUID + """ + LOG.debug("Reconfiguring backing VM: %(backing)s to change disk UUID " + "to: %(disk_uuid)s.", + {'backing': backing, + 'disk_uuid': disk_uuid}) + + disk_device = self._get_disk_device(backing) + disk_device.backing.uuid = disk_uuid + + cf = self._session.vim.client.factory + disk_spec = cf.create('ns0:VirtualDeviceConfigSpec') + disk_spec.device = disk_device + disk_spec.operation = 'edit' + + reconfig_spec = cf.create('ns0:VirtualMachineConfigSpec') + reconfig_spec.deviceChange = [disk_spec] + self._reconfigure_backing(backing, reconfig_spec) + + LOG.debug("Backing VM: %(backing)s reconfigured with new disk UUID: " + "%(disk_uuid)s.", + {'backing': backing, + 'disk_uuid': disk_uuid}) + def delete_file(self, file_path, datacenter=None): """Delete file or folder on the datastore. -- 2.45.2