From 15a93312641bb6121d20c7749481104a1ab34bd0 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Sun, 29 Jun 2014 19:00:04 +0530 Subject: [PATCH] VMware:Support for attaching disk to backing This change adds support for attaching a virtual disk to an existing backing VM. Currently volume creation from preallocated/sparse image doesn't honour the disk type in the volume extra spec and adapter type in the image meta-data. The workflow to address these issues requires the above mentioned method. This change also sets the disk size of backing to at least 1MB which is the minimum required by VIM APIs. Closes-Bug: #1340315 Partial-Bug: #1284284 Partial-Bug: #1287185 Partial-Bug: #1287176 Change-Id: Icdb1f26d5c8eaa42408a14c57c7394d96dda078f --- cinder/tests/test_vmware_volumeops.py | 130 +++++++++++++++++++--- cinder/volume/drivers/vmware/volumeops.py | 123 ++++++++++++++++---- 2 files changed, 216 insertions(+), 37 deletions(-) diff --git a/cinder/tests/test_vmware_volumeops.py b/cinder/tests/test_vmware_volumeops.py index 6b080a16d..11745ef39 100644 --- a/cinder/tests/test_vmware_volumeops.py +++ b/cinder/tests/test_vmware_volumeops.py @@ -418,35 +418,108 @@ class VolumeOpsTestCase(test.TestCase): self.assertEqual(expected_invoke_api, self.session.invoke_api.mock_calls) + def test_create_disk_backing_thin(self): + backing = mock.Mock() + del backing.eagerlyScrub + cf = self.session.vim.client.factory + cf.create.return_value = backing + + disk_type = 'thin' + ret = self.vops._create_disk_backing(disk_type, None) + + self.assertEqual(backing, ret) + self.assertIsInstance(ret.thinProvisioned, bool) + self.assertTrue(ret.thinProvisioned) + self.assertEqual('', ret.fileName) + self.assertEqual('persistent', ret.diskMode) + + def test_create_disk_backing_thick(self): + backing = mock.Mock() + del backing.eagerlyScrub + del backing.thinProvisioned + cf = self.session.vim.client.factory + cf.create.return_value = backing + + disk_type = 'thick' + ret = self.vops._create_disk_backing(disk_type, None) + + self.assertEqual(backing, ret) + self.assertEqual('', ret.fileName) + self.assertEqual('persistent', ret.diskMode) + + def test_create_disk_backing_eager_zeroed_thick(self): + backing = mock.Mock() + del backing.thinProvisioned + cf = self.session.vim.client.factory + cf.create.return_value = backing + + disk_type = 'eagerZeroedThick' + ret = self.vops._create_disk_backing(disk_type, None) + + self.assertEqual(backing, ret) + self.assertIsInstance(ret.eagerlyScrub, bool) + self.assertTrue(ret.eagerlyScrub) + self.assertEqual('', ret.fileName) + self.assertEqual('persistent', ret.diskMode) + + def test_create_virtual_disk_config_spec(self): + + cf = self.session.vim.client.factory + cf.create.side_effect = lambda *args: mock.Mock() + + size_kb = units.Ki + controller_key = 200 + disk_type = 'thick' + spec = self.vops._create_virtual_disk_config_spec(size_kb, + disk_type, + controller_key, + None) + + cf.create.side_effect = None + self.assertEqual('add', spec.operation) + self.assertEqual('create', spec.fileOperation) + device = spec.device + self.assertEqual(size_kb, device.capacityInKB) + self.assertEqual(-101, device.key) + self.assertEqual(0, device.unitNumber) + self.assertEqual(controller_key, device.controllerKey) + backing = device.backing + self.assertEqual('', backing.fileName) + self.assertEqual('persistent', backing.diskMode) + def test_create_specs_for_ide_disk_add(self): factory = self.session.vim.client.factory - factory.create.return_value = mock.Mock(spec=object) + factory.create.side_effect = lambda *args: mock.Mock() - size_kb = 0.5 + size_kb = 1 disk_type = 'thin' adapter_type = 'ide' ret = self.vops._create_specs_for_disk_add(size_kb, disk_type, adapter_type) - self.assertFalse(hasattr(ret[0].device, 'sharedBus')) - self.assertEqual(1, ret[1].device.capacityInKB) - expected = [mock.call.create('ns0:VirtualIDEController'), - mock.call.create('ns0:VirtualDeviceConfigSpec'), + + factory.create.side_effect = None + self.assertEqual(1, len(ret)) + self.assertEqual(units.Ki, ret[0].device.capacityInKB) + self.assertEqual(200, ret[0].device.controllerKey) + expected = [mock.call.create('ns0:VirtualDeviceConfigSpec'), mock.call.create('ns0:VirtualDisk'), - mock.call.create('ns0:VirtualDiskFlatVer2BackingInfo'), - mock.call.create('ns0:VirtualDeviceConfigSpec')] + mock.call.create('ns0:VirtualDiskFlatVer2BackingInfo')] factory.create.assert_has_calls(expected, any_order=True) def test_create_specs_for_scsi_disk_add(self): factory = self.session.vim.client.factory - factory.create.return_value = mock.Mock(spec=object) + factory.create.side_effect = lambda *args: mock.Mock() - size_kb = 2 + size_kb = 2 * units.Ki disk_type = 'thin' adapter_type = 'lsiLogicsas' ret = self.vops._create_specs_for_disk_add(size_kb, disk_type, adapter_type) - self.assertEqual('noSharing', ret[0].device.sharedBus) - self.assertEqual(size_kb, ret[1].device.capacityInKB) + + factory.create.side_effect = None + self.assertEqual(2, len(ret)) + self.assertEqual('noSharing', ret[1].device.sharedBus) + self.assertEqual(size_kb, ret[0].device.capacityInKB) expected = [mock.call.create('ns0:VirtualLsiLogicSASController'), mock.call.create('ns0:VirtualDeviceConfigSpec'), mock.call.create('ns0:VirtualDisk'), @@ -456,11 +529,14 @@ class VolumeOpsTestCase(test.TestCase): def test_get_create_spec_disk_less(self): factory = self.session.vim.client.factory - factory.create.return_value = mock.Mock(spec=object) + factory.create.side_effect = lambda *args: mock.Mock() + name = mock.sentinel.name ds_name = mock.sentinel.ds_name profile_id = mock.sentinel.profile_id ret = self.vops._get_create_spec_disk_less(name, ds_name, profile_id) + + factory.create.side_effect = None self.assertEqual(name, ret.name) self.assertEqual('[%s]' % ds_name, ret.files.vmPathName) self.assertEqual("vmx-08", ret.version) @@ -782,6 +858,34 @@ class VolumeOpsTestCase(test.TestCase): folder=folder, name=name, spec=clone_spec)] self.assertEqual(expected, self.session.invoke_api.mock_calls) + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + '_create_specs_for_disk_add') + def test_attach_disk_to_backing(self, create_spec): + reconfig_spec = mock.Mock() + self.session.vim.client.factory.create.return_value = reconfig_spec + disk_add_config_specs = mock.Mock() + create_spec.return_value = disk_add_config_specs + task = mock.Mock() + self.session.invoke_api.return_value = task + + backing = mock.Mock() + size_in_kb = units.Ki + disk_type = "thin" + adapter_type = "ide" + vmdk_ds_file_path = mock.Mock() + self.vops.attach_disk_to_backing(backing, size_in_kb, disk_type, + adapter_type, vmdk_ds_file_path) + + self.assertEqual(disk_add_config_specs, reconfig_spec.deviceChange) + create_spec.assert_called_once_with(size_in_kb, disk_type, + adapter_type, + vmdk_ds_file_path) + self.session.invoke_api.assert_called_once_with(self.session.vim, + "ReconfigVM_Task", + backing, + spec=reconfig_spec) + self.session.wait_for_task.assert_called_once_with(task) + def test_delete_file(self): file_mgr = mock.sentinel.file_manager self.session.vim.service_content.fileManager = file_mgr diff --git a/cinder/volume/drivers/vmware/volumeops.py b/cinder/volume/drivers/vmware/volumeops.py index 33649b501..b5ac3c5e1 100644 --- a/cinder/volume/drivers/vmware/volumeops.py +++ b/cinder/volume/drivers/vmware/volumeops.py @@ -568,45 +568,90 @@ class VMwareVolumeOps(object): "%(size)s GB."), {'name': name, 'size': requested_size_in_gb}) - def _create_specs_for_disk_add(self, size_kb, disk_type, adapter_type): - """Create controller and disk specs for adding a new disk. - - :param size_kb: disk size in KB - :param disk_type: disk provisioning type - :param adapter_type: disk adapter type - :return: list containing controller and disk specs - """ + def _create_controller_config_spec(self, adapter_type): + """Returns config spec for adding a disk controller.""" cf = self._session.vim.client.factory + controller_type = ControllerType.get_controller_type(adapter_type) controller_device = cf.create('ns0:%s' % controller_type) controller_device.key = -100 controller_device.busNumber = 0 if ControllerType.is_scsi_controller(controller_type): controller_device.sharedBus = 'noSharing' + controller_spec = cf.create('ns0:VirtualDeviceConfigSpec') controller_spec.operation = 'add' controller_spec.device = controller_device + return controller_spec - disk_device = cf.create('ns0:VirtualDisk') - # for very small disks allocate at least 1KB - disk_device.capacityInKB = max(1, int(size_kb)) - disk_device.key = -101 - disk_device.unitNumber = 0 - disk_device.controllerKey = -100 + def _create_disk_backing(self, disk_type, vmdk_ds_file_path): + """Creates file backing for virtual disk.""" + cf = self._session.vim.client.factory disk_device_bkng = cf.create('ns0:VirtualDiskFlatVer2BackingInfo') - if disk_type == 'eagerZeroedThick': + + if disk_type == VirtualDiskType.EAGER_ZEROED_THICK: disk_device_bkng.eagerlyScrub = True - elif disk_type == 'thin': + elif disk_type == VirtualDiskType.THIN: disk_device_bkng.thinProvisioned = True - disk_device_bkng.fileName = '' + + disk_device_bkng.fileName = vmdk_ds_file_path or '' disk_device_bkng.diskMode = 'persistent' - disk_device.backing = disk_device_bkng + + return disk_device_bkng + + def _create_virtual_disk_config_spec(self, size_kb, disk_type, + controller_key, vmdk_ds_file_path): + """Returns config spec for adding a virtual disk.""" + cf = self._session.vim.client.factory + + disk_device = cf.create('ns0:VirtualDisk') + # disk size should be at least 1024KB + disk_device.capacityInKB = max(units.Ki, int(size_kb)) + if controller_key < 0: + disk_device.key = controller_key - 1 + else: + disk_device.key = -101 + disk_device.unitNumber = 0 + disk_device.controllerKey = controller_key + disk_device.backing = self._create_disk_backing(disk_type, + vmdk_ds_file_path) + disk_spec = cf.create('ns0:VirtualDeviceConfigSpec') disk_spec.operation = 'add' - disk_spec.fileOperation = 'create' + if vmdk_ds_file_path is None: + disk_spec.fileOperation = 'create' disk_spec.device = disk_device + return disk_spec - return [controller_spec, disk_spec] + def _create_specs_for_disk_add(self, size_kb, disk_type, adapter_type, + vmdk_ds_file_path=None): + """Create controller and disk config specs for adding a new disk. + + :param size_kb: disk size in KB + :param disk_type: disk provisioning type + :param adapter_type: disk adapter type + :param vmdk_ds_file_path: Optional datastore file path of an existing + virtual disk. If specified, file backing is + not created for the virtual disk. + :return: list containing controller and disk config specs + """ + controller_spec = None + if adapter_type == 'ide': + # For IDE disks, use one of the default IDE controllers (with keys + # 200 and 201) created as part of backing VM creation. + controller_key = 200 + else: + controller_spec = self._create_controller_config_spec(adapter_type) + controller_key = controller_spec.device.key + + disk_spec = self._create_virtual_disk_config_spec(size_kb, + disk_type, + controller_key, + vmdk_ds_file_path) + specs = [disk_spec] + if controller_spec is not None: + specs.append(controller_spec) + return specs def _get_create_spec_disk_less(self, name, ds_name, profileId=None): """Return spec for creating disk-less backing. @@ -626,10 +671,10 @@ class VMwareVolumeOps(object): create_spec.numCPUs = 1 create_spec.memoryMB = 128 create_spec.files = vm_file_info - # set the Hardware version to the lowest version supported by ESXi5.0 - # and compatible with vCenter Server 5.0 - # This ensures migration of volume created on a later ESX server - # works on any ESX server 5.0 and above. + # Set the hardware version to a compatible version supported by + # vSphere 5.0. This will ensure that the backing VM can be migrated + # without any incompatibility issues in a mixed cluster of ESX hosts + # with versions 5.0 or above. create_spec.version = "vmx-08" if profileId: @@ -959,6 +1004,36 @@ class VMwareVolumeOps(object): LOG.info(_("Successfully created clone: %s.") % new_backing) return new_backing + def attach_disk_to_backing(self, backing, size_in_kb, disk_type, + adapter_type, vmdk_ds_file_path): + """Attach an existing virtual disk to the backing VM. + + :param backing: reference to the backing VM + :param size_in_kb: disk size in KB + :param disk_type: virtual disk type + :param adapter_type: disk adapter type + :param vmdk_ds_file_path: datastore file path of the virtual disk to + be attached + """ + cf = self._session.vim.client.factory + reconfig_spec = cf.create('ns0:VirtualMachineConfigSpec') + specs = self._create_specs_for_disk_add(size_in_kb, + disk_type, + adapter_type, + vmdk_ds_file_path) + reconfig_spec.deviceChange = specs + LOG.debug("Reconfiguring backing VM: %(backing)s with spec: %(spec)s.", + {'backing': backing, + 'spec': reconfig_spec}) + reconfig_task = self._session.invoke_api(self._session.vim, + "ReconfigVM_Task", + backing, + spec=reconfig_spec) + LOG.debug("Task: %s created for reconfiguring backing VM.", + reconfig_task) + self._session.wait_for_task(reconfig_task) + LOG.debug("Backing VM: %s reconfigured with new disk.", backing) + def delete_file(self, file_path, datacenter=None): """Delete file or folder on the datastore. -- 2.45.2