From 6dd75f4554ee53649a3be309ecfe27a6a8752b6d Mon Sep 17 00:00:00 2001 From: Sean McGinnis Date: Mon, 27 Jul 2015 09:18:44 -0500 Subject: [PATCH] Dell SC: Add check of current value on retype The recently merged retype code for the Dell Storage Center driver does not check if the new Storage Profile value is different than the currently set value. While this is mostly harmless since there isn't a lot of overhead to setting the Storage Profile to its existing value, it is still extra overhead and API calls that need to be performed to complete the operation. This adds a simple check to compare the current value against the requested value to decide whether or not to perform the retype operation. We only support retyping of the Storage Profile, so if they are the same, just return true to prevent a migration from happening. Change-Id: I2d16b4129170cc8ecedfd3674b609badb27b9095 --- cinder/tests/unit/test_dellsc.py | 10 +++++++ .../drivers/dell/dell_storagecenter_common.py | 29 ++++++++++++------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/cinder/tests/unit/test_dellsc.py b/cinder/tests/unit/test_dellsc.py index 2d4c55244ba..e23ae71dfb0 100644 --- a/cinder/tests/unit/test_dellsc.py +++ b/cinder/tests/unit/test_dellsc.py @@ -1652,6 +1652,16 @@ class DellSCSanISCSIDriverTestCase(test.TestCase): None, None, None, {'extra_specs': {'something': 'else'}}, None) self.assertFalse(res) + def test_retype_same(self, + mock_close_connection, + mock_open_connection, + mock_init): + res = self.driver.retype( + None, None, None, + {'extra_specs': {'storagetype:storageprofile': ['A', 'A']}}, + None) + self.assertTrue(res) + def test_retype_malformed(self, mock_close_connection, mock_open_connection, diff --git a/cinder/volume/drivers/dell/dell_storagecenter_common.py b/cinder/volume/drivers/dell/dell_storagecenter_common.py index 5627dd57aa6..7044fe27622 100644 --- a/cinder/volume/drivers/dell/dell_storagecenter_common.py +++ b/cinder/volume/drivers/dell/dell_storagecenter_common.py @@ -663,16 +663,25 @@ class DellCommonDriver(driver.ConsistencyGroupVD, driver.ManageableVD, storage_profiles) return False + current = storage_profiles[0] requested = storage_profiles[1] - volume_name = volume.get('id') - LOG.debug('Retyping volume %(vol)s to use storage ' - 'profile %(profile)s', - {'vol': volume_name, - 'profile': requested}) - with self._client.open_connection() as api: - if api.find_sc(): - scvolume = api.find_volume(volume_name) - return api.update_storage_profile( - scvolume, requested) + + if current != requested: + volume_name = volume.get('id') + LOG.debug('Retyping volume %(vol)s to use storage ' + 'profile %(profile)s.', + {'vol': volume_name, + 'profile': requested}) + with self._client.open_connection() as api: + if api.find_sc(): + scvolume = api.find_volume(volume_name) + return api.update_storage_profile( + scvolume, requested) + else: + # We only support retype of Storage Profile and they are + # the same, so just return True to avoid unnecessary data + # migration. + LOG.info(_LI('Retype was to same Storage Profile.')) + return True return False