From 52d2ef021fab8513c68bbf40a9e3990c09920f33 Mon Sep 17 00:00:00 2001
From: Vipin Balachandran <vbala@vmware.com>
Date: Fri, 15 Dec 2017 18:09:41 -0800
Subject: [PATCH] VMware: Support volume adapter type change
Adding support for volume adapter type change using volume
retype.
Change-Id: I7731eaf9561965d4e0f87c0597bfdb784ef6ca7a
---
.../volume/drivers/vmware/test_vmware_vmdk.py | 44 ++++++++++++-
.../drivers/vmware/test_vmware_volumeops.py | 63 ++++++++++++++++++-
cinder/volume/drivers/vmware/vmdk.py | 17 ++++-
cinder/volume/drivers/vmware/volumeops.py | 35 +++++++++--
..._retype_adapter_type-dbd8935b8d3bcb1b.yaml | 6 ++
5 files changed, 154 insertions(+), 11 deletions(-)
create mode 100644 releasenotes/notes/vmware_retype_adapter_type-dbd8935b8d3bcb1b.yaml
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 ee4dda53b7e..d17a95068e3 100644
--- a/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py
+++ b/cinder/tests/unit/volume/drivers/vmware/test_vmware_vmdk.py
@@ -1238,10 +1238,22 @@ class VMwareVcVmdkDriverTestCase(test.TestCase):
@mock.patch.object(VMDK_DRIVER, '_get_extra_spec_storage_profile')
@mock.patch.object(VMDK_DRIVER, 'ds_sel')
@mock.patch.object(VMDK_DRIVER, '_select_datastore')
+ @mock.patch.object(
+ VMDK_DRIVER, '_get_adapter_type', return_value='lsiLogic')
+ @mock.patch.object(
+ VMDK_DRIVER, '_get_extra_spec_adapter_type', return_value='lsiLogic')
def test_retype_with_diff_profile_and_ds_compliance(
- self, select_datastore, ds_sel, get_extra_spec_storage_profile,
- get_storage_profile, get_extra_spec_disk_type, get_disk_type,
- vops, in_use):
+ self,
+ _get_extra_spec_adapter_type,
+ _get_adapter_type,
+ select_datastore,
+ ds_sel,
+ get_extra_spec_storage_profile,
+ get_storage_profile,
+ get_extra_spec_disk_type,
+ get_disk_type,
+ vops,
+ in_use):
backing = mock.sentinel.backing
vops.get_backing.return_value = backing
@@ -1337,8 +1349,14 @@ class VMwareVcVmdkDriverTestCase(test.TestCase):
@mock.patch.object(VMDK_DRIVER, '_select_datastore')
@mock.patch.object(VMDK_DRIVER, '_get_dc')
@mock.patch.object(VMDK_DRIVER, '_get_volume_group_folder')
+ @mock.patch.object(
+ VMDK_DRIVER, '_get_adapter_type', return_value='lsiLogic')
+ @mock.patch.object(
+ VMDK_DRIVER, '_get_extra_spec_adapter_type', return_value='lsiLogic')
def test_retype_with_diff_extra_spec_and_vol_snapshot(
self,
+ get_extra_spec_adapter_type,
+ get_adapter_type,
get_volume_group_folder,
get_dc,
select_datastore,
@@ -1418,8 +1436,12 @@ class VMwareVcVmdkDriverTestCase(test.TestCase):
@mock.patch.object(VMDK_DRIVER, '_get_volume_group_folder')
@mock.patch('oslo_utils.uuidutils.generate_uuid')
@mock.patch.object(VMDK_DRIVER, '_delete_temp_backing')
+ @mock.patch.object(VMDK_DRIVER, '_get_adapter_type')
+ @mock.patch.object(VMDK_DRIVER, '_get_extra_spec_adapter_type')
def _test_retype_with_diff_extra_spec_and_ds_compliance(
self,
+ get_extra_spec_adapter_type,
+ get_adapter_type,
delete_temp_backing,
generate_uuid,
get_volume_group_folder,
@@ -1475,6 +1497,17 @@ class VMwareVcVmdkDriverTestCase(test.TestCase):
new_backing = mock.sentinel.new_backing
vops.clone_backing.return_value = new_backing
+ adapter_type = 'lsiLogic'
+ get_adapter_type.return_value = adapter_type
+ new_adapter_type = 'paraVirtual'
+ get_extra_spec_adapter_type.return_value = new_adapter_type
+
+ capacity = self.VOL_SIZE * units.Mi
+ filename = mock.sentinel.filename
+ disk_backing = mock.Mock(filename=filename)
+ disk_device = mock.Mock(capacityInKB=capacity, backing=disk_backing)
+ vops._get_disk_device.return_value = disk_device
+
context = mock.sentinel.context
volume = self._create_volume_dict(status='retyping')
new_type = {'id': 'f04a65e0-d10c-4db7-b4a5-f933d57aa2b5'}
@@ -1510,6 +1543,11 @@ class VMwareVcVmdkDriverTestCase(test.TestCase):
vops.update_backing_disk_uuid.assert_called_once_with(
new_backing, volume['id'])
delete_temp_backing.assert_called_once_with(backing)
+ vops.detach_disk_from_backing.assert_called_once_with(
+ new_backing, disk_device)
+ vops.attach_disk_to_backing.assert_called_once_with(
+ new_backing, disk_device.capacityInKB, new_disk_type,
+ new_adapter_type, None, disk_device.backing.fileName)
vops.change_backing_profile.assert_called_once_with(new_backing,
new_profile_id)
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 de467f22564..d61b0116e11 100644
--- a/cinder/tests/unit/volume/drivers/vmware/test_vmware_volumeops.py
+++ b/cinder/tests/unit/volume/drivers/vmware/test_vmware_volumeops.py
@@ -1184,9 +1184,32 @@ class VolumeOpsTestCase(test.TestCase):
def test_clone_backing_with_empty_folder(self):
self._test_clone_backing('linked', None)
+ def _create_controller_device(self, controller_type):
+ dev = mock.Mock()
+ dev.__class__.__name__ = controller_type
+ return dev
+
+ def test_get_controller(self):
+ disk = self._create_disk_device('foo.vmdk')
+ controller1 = self._create_controller_device(
+ volumeops.ControllerType.LSI_LOGIC)
+ controller2 = self._create_controller_device(
+ volumeops.ControllerType.PARA_VIRTUAL)
+ self.session.invoke_api.return_value = [disk, controller1, controller2]
+
+ backing = mock.sentinel.backing
+ ret = self.vops._get_controller(
+ backing, volumeops.VirtualDiskAdapterType.PARA_VIRTUAL)
+ self.assertEqual(controller2, ret)
+ self.session.invoke_api.assert_called_once_with(
+ vim_util, 'get_object_property', self.session.vim, backing,
+ 'config.hardware.device')
+
+ @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.'
+ '_get_controller', return_value=None)
@mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.'
'_create_specs_for_disk_add')
- def test_attach_disk_to_backing(self, create_spec):
+ def test_attach_disk_to_backing(self, create_spec, get_controller):
reconfig_spec = mock.Mock()
self.session.vim.client.factory.create.return_value = reconfig_spec
disk_add_config_specs = mock.Mock()
@@ -1204,6 +1227,7 @@ class VolumeOpsTestCase(test.TestCase):
adapter_type, profile_id,
vmdk_ds_file_path)
+ get_controller.assert_called_once_with(backing, adapter_type)
self.assertEqual(disk_add_config_specs, reconfig_spec.deviceChange)
create_spec.assert_called_once_with(
size_in_kb, disk_type, adapter_type, profile_id,
@@ -1214,6 +1238,43 @@ class VolumeOpsTestCase(test.TestCase):
spec=reconfig_spec)
self.session.wait_for_task.assert_called_once_with(task)
+ @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.'
+ '_get_controller')
+ @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.'
+ '_create_virtual_disk_config_spec')
+ def test_attach_disk_to_backing_existing_controller(
+ self, create_disk_spec, get_controller):
+ key = mock.sentinel.key
+ controller = mock.Mock(key=key)
+ get_controller.return_value = controller
+
+ reconfig_spec = mock.Mock()
+ self.session.vim.client.factory.create.return_value = reconfig_spec
+ disk_spec = mock.Mock()
+ create_disk_spec.return_value = disk_spec
+ task = mock.Mock()
+ self.session.invoke_api.return_value = task
+
+ backing = mock.Mock()
+ size_in_kb = units.Ki
+ disk_type = "thin"
+ adapter_type = "ide"
+ profile_id = mock.sentinel.profile_id
+ vmdk_ds_file_path = mock.sentinel.vmdk_ds_file_path
+ self.vops.attach_disk_to_backing(backing, size_in_kb, disk_type,
+ adapter_type, profile_id,
+ vmdk_ds_file_path)
+
+ get_controller.assert_called_once_with(backing, adapter_type)
+ self.assertEqual([disk_spec], reconfig_spec.deviceChange)
+ create_disk_spec.assert_called_once_with(
+ size_in_kb, disk_type, key, profile_id, 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_create_spec_for_disk_remove(self):
disk_spec = mock.Mock()
self.session.vim.client.factory.create.return_value = disk_spec
diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py
index de0cb9d1753..72ee32fc5bb 100644
--- a/cinder/volume/drivers/vmware/vmdk.py
+++ b/cinder/volume/drivers/vmware/vmdk.py
@@ -250,7 +250,8 @@ class VMwareVcVmdkDriver(driver.VolumeDriver):
# add storage profile ID to connection info
# support for revert-to-snapshot
# improve scalability of querying volumes in backend (bug 1600754)
- VERSION = '3.0.0'
+ # 3.1.0 - support adapter type change using retype
+ VERSION = '3.1.0'
# ThirdPartySystems wiki page
CI_WIKI_NAME = "VMware_CI"
@@ -1555,6 +1556,20 @@ class VMwareVcVmdkDriver(driver.VolumeDriver):
'new_name': tmp_name,
'old_name': volume['name']})
+ adapter_type = self._get_adapter_type(volume)
+ new_adapter_type = self._get_extra_spec_adapter_type(new_type['id'])
+ if new_adapter_type != adapter_type:
+ LOG.debug("Changing volume: %(name)s adapter type from "
+ "%(adapter_type)s to %(new_adapter_type)s.",
+ {'name': volume['name'],
+ 'adapter_type': adapter_type,
+ 'new_adapter_type': new_adapter_type})
+ disk_device = self.volumeops._get_disk_device(backing)
+ self.volumeops.detach_disk_from_backing(backing, disk_device)
+ self.volumeops.attach_disk_to_backing(
+ backing, disk_device.capacityInKB, new_disk_type,
+ new_adapter_type, None, disk_device.backing.fileName)
+
# Update the backing's storage profile if needed.
if need_profile_change:
LOG.debug("Backing: %(backing)s needs a profile change to:"
diff --git a/cinder/volume/drivers/vmware/volumeops.py b/cinder/volume/drivers/vmware/volumeops.py
index c059708b771..110c96116f5 100644
--- a/cinder/volume/drivers/vmware/volumeops.py
+++ b/cinder/volume/drivers/vmware/volumeops.py
@@ -1235,6 +1235,18 @@ class VMwareVolumeOps(object):
reconfig_task)
self._session.wait_for_task(reconfig_task)
+ def _get_controller(self, backing, adapter_type):
+ devices = self._session.invoke_api(vim_util,
+ 'get_object_property',
+ self._session.vim,
+ backing,
+ 'config.hardware.device')
+
+ controller_type = ControllerType.get_controller_type(adapter_type)
+ for device in devices:
+ if device.__class__.__name__ == controller_type:
+ return device
+
def attach_disk_to_backing(self, backing, size_in_kb, disk_type,
adapter_type, profile_id, vmdk_ds_file_path):
"""Attach an existing virtual disk to the backing VM.
@@ -1256,12 +1268,23 @@ class VMwareVolumeOps(object):
'adapter_type': adapter_type})
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,
- profile_id,
- vmdk_ds_file_path=vmdk_ds_file_path)
+
+ controller = self._get_controller(backing, adapter_type)
+ if controller:
+ disk_spec = self._create_virtual_disk_config_spec(
+ size_in_kb,
+ disk_type,
+ controller.key,
+ profile_id,
+ vmdk_ds_file_path)
+ specs = [disk_spec]
+ else:
+ specs = self._create_specs_for_disk_add(
+ size_in_kb,
+ disk_type,
+ adapter_type,
+ profile_id,
+ vmdk_ds_file_path=vmdk_ds_file_path)
reconfig_spec.deviceChange = specs
self._reconfigure_backing(backing, reconfig_spec)
LOG.debug("Backing VM: %s reconfigured with new disk.", backing)
diff --git a/releasenotes/notes/vmware_retype_adapter_type-dbd8935b8d3bcb1b.yaml b/releasenotes/notes/vmware_retype_adapter_type-dbd8935b8d3bcb1b.yaml
new file mode 100644
index 00000000000..21c40c8510e
--- /dev/null
+++ b/releasenotes/notes/vmware_retype_adapter_type-dbd8935b8d3bcb1b.yaml
@@ -0,0 +1,6 @@
+---
+features:
+ - |
+ VMware VMDK driver now supports changing adpater type using retype.
+ To change the adapter type, set ``vmware:adapter_type`` in the
+ new volume type.