From cfe138bf81cfaed7c7aa60a53e52069e65492fa0 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Tue, 2 May 2017 16:58:56 -0700 Subject: [PATCH] VMware: Apply policy at vmdk level during retype Currently we do not update the vmdk storage policy when the volume's storage policy is changed due to retype. This patch fixes it. Change-Id: I647afbcbfc005e419c9740f0e3a6e1d0e5ee3a63 Closes-bug: #1687821 --- .../drivers/vmware/test_vmware_volumeops.py | 66 +++++++++++-------- cinder/volume/drivers/vmware/volumeops.py | 10 ++- 2 files changed, 48 insertions(+), 28 deletions(-) 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 d06e5921629..0e3cb51e8c4 100644 --- a/cinder/tests/unit/volume/drivers/vmware/test_vmware_volumeops.py +++ b/cinder/tests/unit/volume/drivers/vmware/test_vmware_volumeops.py @@ -1201,49 +1201,61 @@ class VolumeOpsTestCase(test.TestCase): 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. + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + '_get_disk_device') + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + '_reconfigure_backing') + def test_change_backing_profile_to_empty_profile( + self, reconfigure_backing, get_disk_device): reconfig_spec = mock.Mock() empty_profile_spec = mock.sentinel.empty_profile_spec + disk_spec = mock.Mock() self.session.vim.client.factory.create.side_effect = [ - reconfig_spec, empty_profile_spec] + empty_profile_spec, reconfig_spec, disk_spec] - task = mock.sentinel.task - self.session.invoke_api.return_value = task + disk_device = mock.sentinel.disk_device + get_disk_device.return_value = disk_device backing = mock.sentinel.backing - unique_profile_id = mock.sentinel.unique_profile_id - profile_id = mock.Mock(uniqueId=unique_profile_id) - self.vops.change_backing_profile(backing, profile_id) + self.vops.change_backing_profile(backing, None) + self.assertEqual('profile', empty_profile_spec.dynamicType) self.assertEqual([empty_profile_spec], reconfig_spec.vmProfile) - 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) + get_disk_device.assert_called_once_with(backing) + self.assertEqual(disk_device, disk_spec.device) + self.assertEqual('edit', disk_spec.operation) + self.assertEqual([empty_profile_spec], disk_spec.profile) + self.assertEqual([disk_spec], reconfig_spec.deviceChange) + reconfigure_backing.assert_called_once_with(backing, reconfig_spec) - # Test change to non-empty profile. + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + '_get_disk_device') + @mock.patch('cinder.volume.drivers.vmware.volumeops.VMwareVolumeOps.' + '_reconfigure_backing') + def test_change_backing_profile( + self, reconfigure_backing, get_disk_device): + reconfig_spec = mock.Mock() profile_spec = mock.Mock() + disk_spec = mock.Mock() self.session.vim.client.factory.create.side_effect = [ - reconfig_spec, profile_spec] + profile_spec, reconfig_spec, disk_spec] - self.session.invoke_api.reset_mock() - self.session.wait_for_task.reset_mock() + disk_device = mock.sentinel.disk_device + get_disk_device.return_value = disk_device + backing = mock.sentinel.backing + unique_id = mock.sentinel.unique_id + profile_id = mock.Mock(uniqueId=unique_id) self.vops.change_backing_profile(backing, profile_id) + self.assertEqual(unique_id, profile_spec.profileId) self.assertEqual([profile_spec], reconfig_spec.vmProfile) - self.assertEqual(unique_profile_id, - reconfig_spec.vmProfile[0].profileId) - 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) - - # Clear side effects. - self.session.vim.client.factory.create.side_effect = None + get_disk_device.assert_called_once_with(backing) + self.assertEqual(disk_device, disk_spec.device) + self.assertEqual('edit', disk_spec.operation) + self.assertEqual([profile_spec], disk_spec.profile) + self.assertEqual([disk_spec], reconfig_spec.deviceChange) + reconfigure_backing.assert_called_once_with(backing, reconfig_spec) def test_delete_file(self): file_mgr = mock.sentinel.file_manager diff --git a/cinder/volume/drivers/vmware/volumeops.py b/cinder/volume/drivers/vmware/volumeops.py index f31e742c285..27e7ffebfa4 100644 --- a/cinder/volume/drivers/vmware/volumeops.py +++ b/cinder/volume/drivers/vmware/volumeops.py @@ -1217,7 +1217,6 @@ class VMwareVolumeOps(object): {'backing': backing, 'profile': profile_id}) cf = self._session.vim.client.factory - reconfig_spec = cf.create('ns0:VirtualMachineConfigSpec') if profile_id is None: vm_profile = cf.create('ns0:VirtualMachineEmptyProfileSpec') @@ -1226,7 +1225,16 @@ class VMwareVolumeOps(object): vm_profile = cf.create('ns0:VirtualMachineDefinedProfileSpec') vm_profile.profileId = profile_id.uniqueId + reconfig_spec = cf.create('ns0:VirtualMachineConfigSpec') reconfig_spec.vmProfile = [vm_profile] + + disk_device = self._get_disk_device(backing) + disk_spec = cf.create('ns0:VirtualDeviceConfigSpec') + disk_spec.device = disk_device + disk_spec.operation = 'edit' + disk_spec.profile = [vm_profile] + reconfig_spec.deviceChange = [disk_spec] + self._reconfigure_backing(backing, reconfig_spec) LOG.debug("Backing VM: %(backing)s reconfigured with new profile: " "%(profile)s.",