From 2bad1fb2cf73921259d967897f050b4ac258f71b Mon Sep 17 00:00:00 2001 From: Xiangfei Zhu Date: Thu, 30 Jul 2015 18:03:46 +0800 Subject: [PATCH] VMware: Fix re-attach volume error for VC 5.1 vCenter server version 5.1 does not support Storage Policy Based Management. This patch adds a check before retrieving storage profiles which depends on SPBM. Storage profiles are used to check datastore compliance. Change-Id: Iba465becdc3e7bac2f47c3984ea42cc6537911ae Closes-Bug: #1479402 --- cinder/tests/unit/test_vmware_vmdk.py | 36 +++++++++++++++++++++++++++ cinder/volume/drivers/vmware/vmdk.py | 15 +++++++---- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/cinder/tests/unit/test_vmware_vmdk.py b/cinder/tests/unit/test_vmware_vmdk.py index 663e0d144a8..ce69bc053d4 100644 --- a/cinder/tests/unit/test_vmware_vmdk.py +++ b/cinder/tests/unit/test_vmware_vmdk.py @@ -2759,6 +2759,7 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): @mock.patch.object(VMDK_DRIVER, 'volumeops') @mock.patch.object(VMDK_DRIVER, 'ds_sel') def test_relocate_backing_nop(self, ds_sel, vops): + self._driver._storage_policy_enabled = True volume = {'name': 'vol-1', 'size': 1} datastore = mock.sentinel.datastore @@ -2783,6 +2784,7 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): @mock.patch.object(VMDK_DRIVER, 'ds_sel') def test_relocate_backing_with_no_datastore( self, ds_sel, vops): + self._driver._storage_policy_enabled = True volume = {'name': 'vol-1', 'size': 1} profile = mock.sentinel.profile @@ -2837,6 +2839,40 @@ class VMwareVcVmdkDriverTestCase(VMwareEsxVmdkDriverTestCase): vops.move_backing_to_folder.assert_called_once_with(backing, folder) + @mock.patch.object(VMDK_DRIVER, 'volumeops') + @mock.patch.object(VMDK_DRIVER, '_get_volume_group_folder') + @mock.patch.object(VMDK_DRIVER, 'ds_sel') + def test_relocate_backing_with_pbm_disabled( + self, ds_sel, get_volume_group_folder, vops): + self._driver._storage_policy_enabled = False + volume = {'name': 'vol-1', 'size': 1, 'project_id': 'abc'} + + vops.is_datastore_accessible.return_value = False + + backing = mock.sentinel.backing + host = mock.sentinel.host + + rp = mock.sentinel.rp + datastore = mock.sentinel.datastore + summary = mock.Mock(datastore=datastore) + ds_sel.select_datastore.return_value = (host, rp, summary) + + folder = mock.sentinel.folder + get_volume_group_folder.return_value = folder + + self._driver._relocate_backing(volume, backing, host) + + self.assertFalse(vops.get_profile.called) + vops.relocate_backing.assert_called_once_with(backing, + datastore, + rp, + host) + vops.move_backing_to_folder.assert_called_once_with(backing, + folder) + ds_sel.select_datastore.assert_called_once_with( + {hub.DatastoreSelector.SIZE_BYTES: volume['size'] * units.Gi, + hub.DatastoreSelector.PROFILE_NAME: None}, hosts=[host]) + @mock.patch('oslo_vmware.api.VMwareAPISession') def test_session(self, apiSession): self._session = None diff --git a/cinder/volume/drivers/vmware/vmdk.py b/cinder/volume/drivers/vmware/vmdk.py index 0f925ee5ba4..9ed2a11ba3c 100644 --- a/cinder/volume/drivers/vmware/vmdk.py +++ b/cinder/volume/drivers/vmware/vmdk.py @@ -1945,17 +1945,22 @@ class VMwareVcVmdkDriver(VMwareEsxVmdkDriver): # Check if the current datastore is visible to the host managing # the instance and compliant with the storage profile. datastore = self.volumeops.get_datastore(backing) - backing_profile = self.volumeops.get_profile(backing) + backing_profile = None + if self._storage_policy_enabled: + backing_profile = self.volumeops.get_profile(backing) if (self.volumeops.is_datastore_accessible(datastore, host) and self.ds_sel.is_datastore_compliant(datastore, backing_profile)): LOG.debug("Datastore: %(datastore)s of backing: %(backing)s is " - "already accessible to instance's host: %(host)s and " - "compliant with storage profile: %(profile)s.", + "already accessible to instance's host: %(host)s.", {'backing': backing, 'datastore': datastore, - 'host': host, - 'profile': backing_profile}) + 'host': host}) + if backing_profile: + LOG.debug("Backing: %(backing)s is compliant with " + "storage profile: %(profile)s.", + {'backing': backing, + 'profile': backing_profile}) return # We need to relocate the backing to an accessible and profile