From 94df79e427752cf9b5092ddb2c4a6eca1e3252f4 Mon Sep 17 00:00:00 2001 From: Xi Yang Date: Tue, 1 Sep 2015 06:05:15 -0400 Subject: [PATCH] Retype enhancement for EMC VNX cinder driver VNX driver will trigger lun migration when retyping a thin/thick volume to a compressed volume and it is time-consuming. To enhance the performance, this patch will directly turn on the compression on a volume instead of migrating it. Change-Id: I155272d1130ac0048d226742970e021118f2f123 Closes-Bug: #1495802 --- cinder/tests/unit/test_emc_vnxdirect.py | 95 +++++++++++++++++------- cinder/volume/drivers/emc/emc_vnx_cli.py | 61 +++++++++------ 2 files changed, 108 insertions(+), 48 deletions(-) diff --git a/cinder/tests/unit/test_emc_vnxdirect.py b/cinder/tests/unit/test_emc_vnxdirect.py index e560a65e5..ed1a5f121 100644 --- a/cinder/tests/unit/test_emc_vnxdirect.py +++ b/cinder/tests/unit/test_emc_vnxdirect.py @@ -595,6 +595,11 @@ class EMCVNXCLIDriverTestData(object): return ('snap', '-modify', '-id', snap_name, '-allowReadWrite', 'yes', '-allowAutoDelete', 'yes') + def MODIFY_TIERING_CMD(self, lun_name, tiering): + cmd = ['lun', '-modify', '-name', lun_name, '-o'] + cmd.extend(self.tiering_values[tiering]) + return tuple(cmd) + provisioning_values = { 'thin': ['-type', 'Thin'], 'thick': ['-type', 'NonThin'], @@ -3045,15 +3050,10 @@ Time Remaining: 0 second(s) @mock.patch( "cinder.volume.volume_types." "get_volume_type_extra_specs", - mock.Mock(return_value={'storagetype:provisioning': 'thin'})) - def test_retype_thin_to_compressed_auto(self): - """Unit test for retype thin to compressed and auto tiering.""" - diff_data = {'encryption': {}, 'qos_specs': {}, - 'extra_specs': - {'storagetype:provsioning': ('thin', - 'compressed'), - 'storagetype:tiering': (None, 'auto')}} - + mock.Mock(side_effect=[{'provisioning:type': 'thin'}, + {'provisioning:type': 'thick'}])) + def test_retype_turn_on_compression_and_autotiering(self): + """Unit test for retype a volume to compressed and auto tiering.""" new_type_data = {'name': 'voltype0', 'qos_specs_id': None, 'deleted': False, 'extra_specs': {'storagetype:provisioning': @@ -3061,20 +3061,15 @@ Time Remaining: 0 second(s) 'storagetype:tiering': 'auto'}, 'id': 'f82f28c8-148b-416e-b1ae-32d3c02556c0'} - host_test_data = {'host': 'ubuntu-server12@pool_backend_1', + host_test_data = {'host': 'host@backendsec#unit_test_pool', 'capabilities': {'location_info': 'unit_test_pool|FNM00124500890', 'volume_backend_name': 'pool_backend_1', 'storage_protocol': 'iSCSI'}} - cmd_migrate_verify = self.testData.MIGRATION_VERIFY_CMD(1) - output_migrate_verify = (r'The specified source LUN ' - 'is not currently migrating', 23) commands = [self.testData.NDU_LIST_CMD, - self.testData.SNAP_LIST_CMD(), - cmd_migrate_verify] + self.testData.SNAP_LIST_CMD()] results = [self.testData.NDU_LIST_RESULT, - ('No snap', 1023), - output_migrate_verify] + ('No snap', 1023)] fake_cli = self.driverSetup(commands, results) self.driver.cli.enablers = ['-Compression', '-Deduplication', @@ -3082,21 +3077,69 @@ Time Remaining: 0 second(s) '-FAST'] emc_vnx_cli.CommandLineHelper.get_array_serial = mock.Mock( return_value={'array_serial': "FNM00124500890"}) - + # Retype a thin volume to a compressed volume self.driver.retype(None, self.testData.test_volume3, - new_type_data, - diff_data, - host_test_data) + new_type_data, None, host_test_data) expect_cmd = [ mock.call(*self.testData.SNAP_LIST_CMD(), poll=False), - mock.call(*self.testData.LUN_CREATION_CMD( - 'vol3-123456', 2, 'unit_test_pool', 'compressed', 'auto')), mock.call(*self.testData.ENABLE_COMPRESSION_CMD(1)), - mock.call(*self.testData.MIGRATION_CMD(), - retry_disable=True, - poll=True)] + mock.call(*self.testData.MODIFY_TIERING_CMD('vol3', 'auto')) + ] + fake_cli.assert_has_calls(expect_cmd) + + # Retype a thick volume to a compressed volume + self.driver.retype(None, self.testData.test_volume3, + new_type_data, None, host_test_data) fake_cli.assert_has_calls(expect_cmd) + @mock.patch( + "cinder.volume.drivers.emc.emc_vnx_cli.EMCVnxCliBase.get_lun_id", + mock.Mock(return_value=1)) + @mock.patch( + "cinder.volume.drivers.emc.emc_vnx_cli.CommandLineHelper." + + "get_lun_by_name", + mock.Mock(return_value={'lun_id': 1})) + @mock.patch( + "eventlet.event.Event.wait", + mock.Mock(return_value=None)) + @mock.patch( + "time.time", + mock.Mock(return_value=123456)) + @mock.patch( + "cinder.volume.volume_types." + "get_volume_type_extra_specs", + mock.Mock(return_value={'provisioning:type': 'thin'})) + def test_retype_turn_on_compression_volume_has_snap(self): + """Unit test for retype a volume which has snap to compressed.""" + new_type_data = {'name': 'voltype0', 'qos_specs_id': None, + 'deleted': False, + 'extra_specs': {'storagetype:provisioning': + 'compressed'}, + 'id': 'f82f28c8-148b-416e-b1ae-32d3c02556c0'} + + host_test_data = {'host': 'host@backendsec#unit_test_pool', + 'capabilities': + {'location_info': 'unit_test_pool|FNM00124500890', + 'volume_backend_name': 'pool_backend_1', + 'storage_protocol': 'iSCSI'}} + commands = [self.testData.NDU_LIST_CMD, + self.testData.SNAP_LIST_CMD()] + results = [self.testData.NDU_LIST_RESULT, + ('Has snap', 0)] + self.driverSetup(commands, results) + self.driver.cli.enablers = ['-Compression', + '-Deduplication', + '-ThinProvisioning', + '-FAST'] + emc_vnx_cli.CommandLineHelper.get_array_serial = mock.Mock( + return_value={'array_serial': "FNM00124500890"}) + # Retype a thin volume which has a snap to a compressed volume + retyped = self.driver.retype(None, self.testData.test_volume3, + new_type_data, None, host_test_data) + self.assertFalse(retyped, + "Retype should failed due to " + "the volume has snapshot") + @mock.patch( "cinder.volume.drivers.emc.emc_vnx_cli.EMCVnxCliBase.get_lun_id", mock.Mock(return_value=1)) diff --git a/cinder/volume/drivers/emc/emc_vnx_cli.py b/cinder/volume/drivers/emc/emc_vnx_cli.py index dbcc281bc..a875dd7e9 100644 --- a/cinder/volume/drivers/emc/emc_vnx_cli.py +++ b/cinder/volume/drivers/emc/emc_vnx_cli.py @@ -2264,17 +2264,24 @@ class EMCVnxCliBase(object): self._get_and_validate_extra_specs(new_specs)) # Check what changes are needed - migration, tiering_change = self.determine_changes_when_retype( - volume, new_type, host) - - # Reject if volume has snapshot when migration is needed - if migration and self._client.check_lun_has_snap( - self.get_lun_id(volume)): - LOG.debug('Driver is not able to do retype because the volume ' - 'has snapshot which is forbidden to migrate.') - return False + changes = self.determine_changes_when_retype(volume, new_type, host) + + if self._client.check_lun_has_snap(self.get_lun_id(volume)): + # Reject if volume has snapshot when migration is needed + if changes['migration']: + LOG.debug('Driver is not able to do retype because the volume ' + '%s has a snapshot which is forbidden to migrate.', + volume['id']) + return False + # Reject if volume has snapshot when trying to + # turn on compression + if changes['compression_on']: + LOG.debug('Driver is not able to do retype because the volume ' + '%s has a snapshot which is forbidden to turn on ' + 'compression.', volume['id']) + return False - if migration: + if changes['migration']: # Check whether the migration is valid is_valid, target_pool_name = ( self._is_valid_for_storage_assisted_migration( @@ -2294,16 +2301,21 @@ class EMCVnxCliBase(object): 'storage-assisted migration is not valid ' 'in this situation.') return False - elif tiering_change: + if changes['compression_on']: + # Turn on compression feature on the volume + self._client.enable_or_disable_compression_on_lun( + volume['name'], 'on') + if changes['tiering']: # Modify lun to change tiering policy self._client.modify_lun_tiering(volume['name'], new_tiering) - return True - else: - return True + return True def determine_changes_when_retype(self, volume, new_type, host): - migration = False - tiering_change = False + changes = { + 'migration': False, + 'tiering': False, + 'compression_on': False + } old_specs = self.get_volumetype_extraspecs(volume) old_provisioning, old_tiering, old_snapcopy = ( @@ -2316,15 +2328,20 @@ class EMCVnxCliBase(object): lun_type = self._extract_provider_location_for_lun( volume['provider_location'], 'type') - if volume['host'] != host['host'] or \ - old_provisioning != new_provisioning: - migration = True + if volume['host'] != host['host']: + changes['migration'] = True + elif old_provisioning != new_provisioning: + if (old_provisioning in ['thin', 'thick'] and + new_provisioning == 'compressed'): + changes['compression_on'] = True + else: + changes['migration'] = True if lun_type == 'smp': - migration = True + changes['migration'] = True if new_tiering != old_tiering: - tiering_change = True - return migration, tiering_change + changes['tiering'] = True + return changes def determine_all_enablers_exist(self, enablers): """Determine all wanted enablers whether exist.""" -- 2.45.2