From 8d972bec8ab0ea9b1b038e71e3d39d0e1eceaf02 Mon Sep 17 00:00:00 2001 From: Helen Walsh Date: Tue, 12 Feb 2019 17:12:33 +0000 Subject: [PATCH] PowerMax driver - performance improvements 1. Changing a payload parameter to in modifying storage group so that a new volume does not have to be created on every operation. This is especially relevent as when we delete a volume we do not wait for the tracks to deallocate. This volume (once deallocation is eventually complete) can be reused. 2. Optimization on driver initialization to call method to determine whether array is a powermax only once. Change-Id: Iea8f445052be4882fb6450d939774eeb7124384e --- .../dell_emc/powermax/powermax_data.py | 4 +- .../dell_emc/powermax/test_powermax_common.py | 22 +++--- .../powermax/test_powermax_metadata.py | 2 +- .../powermax/test_powermax_replication.py | 71 ++++++++++++++----- .../dell_emc/powermax/test_powermax_rest.py | 36 ++++++---- .../drivers/dell_emc/powermax/common.py | 39 +++++----- .../drivers/dell_emc/powermax/metadata.py | 7 +- .../drivers/dell_emc/powermax/provision.py | 14 ++-- .../volume/drivers/dell_emc/powermax/rest.py | 41 ++++++++--- 9 files changed, 158 insertions(+), 78 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py index 11ca7dfa47d..e1c0ef5c4f7 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py @@ -291,6 +291,8 @@ class PowerMaxData(object): rep_extra_specs3['workload'] = workload rep_extra_specs4 = deepcopy(rep_extra_specs3) rep_extra_specs4['rdf_group_label'] = rdf_group_name + rep_extra_specs5 = deepcopy(rep_extra_specs2) + rep_extra_specs5['target_array_model'] = 'VMAX250F' test_volume_type_1 = volume_type.VolumeType( id='2b06255d-f5f0-4520-a953-b029196add6a', name='abc', @@ -744,7 +746,7 @@ class PowerMaxData(object): 'model': 'VMAX250F', 'ucode': '5977.1091.1092'}, {'symmetrixId': array_herc, - 'model': 'VMAXHERC', + 'model': 'PowerMax 2000', 'ucode': '5978.1091.1092'}] version_details = {'version': 'V9.0.0.1'} diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py index 1fd500feaa0..b7e1cc6875b 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py @@ -69,8 +69,8 @@ class PowerMaxCommonTest(test.TestCase): None, 'config_group', None, None) fc.PowerMaxFCDriver(configuration=configuration) - @mock.patch.object(rest.PowerMaxRest, 'is_next_gen_array', - return_value=True) + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('PowerMax 2000', True)) @mock.patch.object(rest.PowerMaxRest, 'set_rest_credentials') @mock.patch.object(common.PowerMaxCommon, '_get_slo_workload_combinations', return_value=[]) @@ -80,7 +80,7 @@ class PowerMaxCommonTest(test.TestCase): def test_gather_info_next_gen(self, mock_parse, mock_combo, mock_rest, mock_nextgen): self.common._gather_info() - self.assertTrue(self.common.nextGen) + self.assertTrue(self.common.next_gen) def test_get_slo_workload_combinations_powermax(self): array_info = self.common.get_attributes_from_cinder_config() @@ -110,7 +110,8 @@ class PowerMaxCommonTest(test.TestCase): return_value=tpd.PowerMaxData.powermax_slo_details['sloId']) def test_get_slo_workload_combinations_next_gen(self, mck_slo, mck_wl, mck_model): - self.common.nextGen = True + self.common.next_gen = True + self.common.array_model = 'PowerMax 2000' finalarrayinfolist = self.common._get_slo_workload_combinations( self.data.array_info_no_wl) self.assertTrue(len(finalarrayinfolist) == 14) @@ -125,7 +126,7 @@ class PowerMaxCommonTest(test.TestCase): return_value=tpd.PowerMaxData.powermax_slo_details['sloId']) def test_get_slo_workload_combinations_next_gen_vmax( self, mck_slo, mck_wl, mck_model): - self.common.nextGen = True + self.common.next_gen = True finalarrayinfolist = self.common._get_slo_workload_combinations( self.data.array_info_no_wl) self.assertTrue(len(finalarrayinfolist) == 18) @@ -641,7 +642,7 @@ class PowerMaxCommonTest(test.TestCase): extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f extra_specs[utils.WORKLOAD] = self.data.workload ref_mv_dict = self.data.masking_view_dict - self.common.nextGen = False + self.common.next_gen = False masking_view_dict = self.common._populate_masking_dict( volume, connector, extra_specs) self.assertEqual(ref_mv_dict, masking_view_dict) @@ -686,7 +687,7 @@ class PowerMaxCommonTest(test.TestCase): connector = self.data.connector extra_specs = deepcopy(self.data.extra_specs) extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f - self.common.nextGen = True + self.common.next_gen = True masking_view_dict = self.common._populate_masking_dict( volume, connector, extra_specs) self.assertEqual('NONE', masking_view_dict[utils.WORKLOAD]) @@ -822,11 +823,12 @@ class PowerMaxCommonTest(test.TestCase): volume_name = '1' volume_size = self.data.test_volume.size extra_specs = self.data.extra_specs - self.common.nextGen = True + self.common.next_gen = True with mock.patch.object( self.utils, 'is_compression_disabled', return_value=True): with mock.patch.object( - self.rest, 'is_next_gen_array', return_value=True): + self.rest, 'get_array_model_info', + return_value=('PowerMax 2000', True)): with mock.patch.object( self.masking, 'get_or_create_default_storage_group') as mock_get: @@ -947,7 +949,7 @@ class PowerMaxCommonTest(test.TestCase): def test_set_vmax_extra_specs_next_gen(self): srp_record = self.common.get_attributes_from_cinder_config() - self.common.nextGen = True + self.common.next_gen = True extra_specs = self.common._set_vmax_extra_specs( self.data.vol_type_extra_specs, srp_record) ref_extra_specs = deepcopy(self.data.extra_specs_intervals_set) diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_metadata.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_metadata.py index e23955c005a..7b18c24a371 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_metadata.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_metadata.py @@ -247,7 +247,7 @@ class PowerMaxVolumeMetadataDebugTest(test.TestCase): rest.PowerMaxRest, 'get_unisphere_version', return_value={'version': tpd.PowerMaxData.unisphere_version}) @mock.patch.object( - rest.PowerMaxRest, 'get_array_serial', + rest.PowerMaxRest, 'get_array_detail', return_value={'ucode': tpd.PowerMaxData.vmax_firmware_version, 'model': tpd.PowerMaxData.vmax_model}) def test_gather_version_info( diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py index 4aed5d2be5f..e45f827e4f4 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py @@ -220,7 +220,7 @@ class PowerMaxReplicationTest(test.TestCase): @mock.patch.object(common.PowerMaxCommon, '_remove_members') @mock.patch.object( common.PowerMaxCommon, '_get_replication_extra_specs', - return_value=tpd.PowerMaxData.rep_extra_specs) + return_value=tpd.PowerMaxData.rep_extra_specs2) @mock.patch.object( utils.PowerMaxUtils, 'is_volume_failed_over', return_value=True) def test_unmap_lun_volume_failed_over(self, mock_fo, mock_es, mock_rm): @@ -260,7 +260,9 @@ class PowerMaxReplicationTest(test.TestCase): @mock.patch.object(utils.PowerMaxUtils, 'is_metro_device', return_value=True) - def test_initialize_connection_vol_metro(self, mock_md): + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) + def test_initialize_connection_vol_metro(self, mock_model, mock_md): metro_connector = deepcopy(self.data.connector) metro_connector['multipath'] = True info_dict = self.common.initialize_connection( @@ -324,8 +326,10 @@ class PowerMaxReplicationTest(test.TestCase): return_value=(False, False, None)) @mock.patch.object(common.PowerMaxCommon, 'extend_volume_is_replicated') @mock.patch.object(common.PowerMaxCommon, '_sync_check') - def test_extend_volume_rep_enabled(self, mock_sync, mock_ex_re, - mock_is_re): + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) + def test_extend_volume_rep_enabled(self, mock_model, mock_sync, + mock_ex_re, mock_is_re): extra_specs = deepcopy(self.extra_specs) extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f volume_name = self.data.test_volume.name @@ -351,7 +355,9 @@ class PowerMaxReplicationTest(test.TestCase): @mock.patch.object(common.PowerMaxCommon, '_replicate_volume', return_value=({}, {})) - def test_manage_existing_is_replicated(self, mock_rep): + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) + def test_manage_existing_is_replicated(self, mock_model, mock_rep): extra_specs = deepcopy(self.extra_specs) extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f external_ref = {u'source-name': u'00002'} @@ -368,7 +374,9 @@ class PowerMaxReplicationTest(test.TestCase): extra_specs, delete_src=False) @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') - def test_setup_volume_replication(self, mock_rm): + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) + def test_setup_volume_replication(self, mock_model, mock_rm): rep_status, rep_data, __ = self.common.setup_volume_replication( self.data.array, self.data.test_volume, self.data.device_id, self.extra_specs) @@ -378,7 +386,10 @@ class PowerMaxReplicationTest(test.TestCase): @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') @mock.patch.object(common.PowerMaxCommon, '_create_volume') - def test_setup_volume_replication_target(self, mock_create, mock_rm): + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) + def test_setup_volume_replication_target( + self, mock_model, mock_create, mock_rm): rep_status, rep_data, __ = self.common.setup_volume_replication( self.data.array, self.data.test_volume, self.data.device_id, self.extra_specs, self.data.device_id2) @@ -393,7 +404,7 @@ class PowerMaxReplicationTest(test.TestCase): @mock.patch.object(rest.PowerMaxRest, 'get_size_of_device_on_array', return_value=2) @mock.patch.object(common.PowerMaxCommon, '_get_replication_extra_specs', - return_value=tpd.PowerMaxData.rep_extra_specs) + return_value=tpd.PowerMaxData.rep_extra_specs5) @mock.patch.object(common.PowerMaxCommon, '_create_volume', return_value=tpd.PowerMaxData.provider_location) @mock.patch.object(common.PowerMaxCommon, '_sync_check') @@ -414,10 +425,13 @@ class PowerMaxReplicationTest(test.TestCase): self.assertEqual('enabled', rep_status) self.assertEqual(self.data.rdf_group_details, rep_data) + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) @mock.patch.object(common.PowerMaxCommon, '_cleanup_remote_target') - def test_cleanup_lun_replication_success(self, mock_clean): + def test_cleanup_lun_replication_success(self, mock_clean, mock_model): rep_extra_specs = deepcopy(self.data.rep_extra_specs) rep_extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f + rep_extra_specs['target_array_model'] = 'VMAX250F' self.common.cleanup_lun_replication( self.data.test_volume, '1', self.data.device_id, self.extra_specs) @@ -436,8 +450,10 @@ class PowerMaxReplicationTest(test.TestCase): self.data.device_id2, self.data.rdf_group_no, '1', rep_extra_specs) + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) @mock.patch.object(common.PowerMaxCommon, '_cleanup_remote_target') - def test_cleanup_lun_replication_no_target(self, mock_clean): + def test_cleanup_lun_replication_no_target(self, mock_clean, mock_model): with mock.patch.object(self.common, 'get_remote_target_device', return_value=(None, '', '', '', '')): self.common.cleanup_lun_replication( @@ -591,12 +607,14 @@ class PowerMaxReplicationTest(test.TestCase): self.data.device_id)) self.assertIsNone(target_device4) + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('PowerMax 2000', True)) @mock.patch.object(common.PowerMaxCommon, 'setup_volume_replication') @mock.patch.object(provision.PowerMaxProvision, 'extend_volume') @mock.patch.object(provision.PowerMaxProvision, 'break_rdf_relationship') @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') - def test_extend_volume_is_replicated(self, mock_remove, - mock_break, mock_extend, mock_setup): + def test_extend_volume_is_replicated(self, mock_remove, mock_break, + mock_extend, mock_setup, mock_model): self.common.extend_volume_is_replicated( self.data.array, self.data.test_volume, self.data.device_id, 'vol1', '5', self.data.extra_specs_rep_enabled) @@ -626,10 +644,12 @@ class PowerMaxReplicationTest(test.TestCase): self.data.device_id, 'vol1', '1', self.data.extra_specs_rep_enabled) + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) @mock.patch.object(common.PowerMaxCommon, 'add_volume_to_replication_group') @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') - def test_enable_rdf(self, mock_remove, mock_add): + def test_enable_rdf(self, mock_remove, mock_add, mock_model): rep_config = self.utils.get_replication_config( [self.replication_device]) self.common.enable_rdf( @@ -639,10 +659,12 @@ class PowerMaxReplicationTest(test.TestCase): self.assertEqual(2, mock_remove.call_count) self.assertEqual(2, mock_add.call_count) + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) @mock.patch.object(masking.PowerMaxMasking, 'remove_vol_from_storage_group') @mock.patch.object(common.PowerMaxCommon, '_cleanup_remote_target') - def test_enable_rdf_exception(self, mock_cleanup, mock_rm): + def test_enable_rdf_exception(self, mock_cleanup, mock_rm, mock_model): rep_config = self.utils.get_replication_config( [self.replication_device]) self.assertRaises( @@ -668,25 +690,31 @@ class PowerMaxReplicationTest(test.TestCase): self.data.array, self.data.device_id, 'vol1', self.extra_specs) - def test_get_replication_extra_specs(self): + @mock.patch.object(rest.PowerMaxRest, + 'get_array_model_info', + return_value=('VMAX250F', False)) + def test_get_replication_extra_specs(self, mock_model): rep_config = self.utils.get_replication_config( [self.replication_device]) # Path one - disable compression extra_specs1 = deepcopy(self.extra_specs) extra_specs1[utils.DISABLECOMPRESSION] = 'true' - ref_specs1 = deepcopy(self.data.rep_extra_specs2) + ref_specs1 = deepcopy(self.data.rep_extra_specs5) rep_extra_specs1 = self.common._get_replication_extra_specs( extra_specs1, rep_config) self.assertEqual(ref_specs1, rep_extra_specs1) # Path two - disable compression, not all flash - ref_specs2 = deepcopy(self.data.rep_extra_specs2) + ref_specs2 = deepcopy(self.data.rep_extra_specs5) with mock.patch.object(self.rest, 'is_compression_capable', return_value=False): rep_extra_specs2 = self.common._get_replication_extra_specs( extra_specs1, rep_config) self.assertEqual(ref_specs2, rep_extra_specs2) - def test_get_replication_extra_specs_powermax(self): + @mock.patch.object(rest.PowerMaxRest, + 'get_array_model_info', + return_value=('PowerMax 2000', True)) + def test_get_replication_extra_specs_powermax(self, mock_model): rep_config = self.utils.get_replication_config( [self.replication_device]) rep_specs = deepcopy(self.data.rep_extra_specs2) @@ -695,6 +723,7 @@ class PowerMaxReplicationTest(test.TestCase): # SLO not valid, both SLO and Workload set to NONE rep_specs['slo'] = None rep_specs['workload'] = None + rep_specs['target_array_model'] = 'PowerMax 2000' with mock.patch.object(self.provision, 'verify_slo_workload', return_value=(False, False)): rep_extra_specs = self.common._get_replication_extra_specs( @@ -703,6 +732,7 @@ class PowerMaxReplicationTest(test.TestCase): # SL valid, workload invalid, only workload set to NONE rep_specs['slo'] = 'Diamond' rep_specs['workload'] = None + rep_specs['target_array_model'] = 'PowerMax 2000' with mock.patch.object(self.provision, 'verify_slo_workload', return_value=(True, False)): rep_extra_specs = self.common._get_replication_extra_specs( @@ -905,10 +935,13 @@ class PowerMaxReplicationTest(test.TestCase): self.data.failed_resource, self.data.device_id, 'name', self.data.remote_array, self.data.device_id2, extra_specs) + @mock.patch.object(rest.PowerMaxRest, 'get_array_model_info', + return_value=('VMAX250F', False)) @mock.patch.object(common.PowerMaxCommon, '_add_volume_to_async_rdf_managed_grp') @mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members') - def test_setup_volume_replication_async(self, mock_rm, mock_add): + def test_setup_volume_replication_async( + self, mock_rm, mock_add, mock_model): extra_specs = deepcopy(self.extra_specs) extra_specs['rep_mode'] = utils.REP_ASYNC rep_status, rep_data, __ = ( diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py index 45a5450a244..c1c6708de0c 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py @@ -217,13 +217,13 @@ class PowerMaxRestTest(test.TestCase): self.rest.modify_resource, array, category, resource_type, resource_name) - def test_get_array_serial(self): + def test_get_array_detail(self): ref_details = self.data.symmetrix[0] - array_details = self.rest.get_array_serial(self.data.array) + array_details = self.rest.get_array_detail(self.data.array) self.assertEqual(ref_details, array_details) - def test_get_array_serial_failed(self): - array_details = self.rest.get_array_serial(self.data.failed_resource) + def test_get_array_detail_failed(self): + array_details = self.rest.get_array_detail(self.data.failed_resource) self.assertIsNone(array_details) def test_get_uni_version(self): @@ -241,32 +241,32 @@ class PowerMaxRestTest(test.TestCase): def test_get_slo_list_powermax(self): ref_settings = self.data.powermax_slo_details['sloId'] - slo_settings = self.rest.get_slo_list(self.data.array) + slo_settings = self.rest.get_slo_list( + self.data.array, True, 'PowerMax 2000') self.assertEqual(ref_settings, slo_settings) def test_get_slo_list_vmax(self): ref_settings = ['Diamond'] with mock.patch.object(self.rest, 'get_resource', return_value=self.data.vmax_slo_details): - slo_settings = self.rest.get_slo_list(self.data.array) + slo_settings = self.rest.get_slo_list( + self.data.array, False, 'VMAX250F') self.assertEqual(ref_settings, slo_settings) def test_get_workload_settings(self): ref_settings = self.data.workloadtype['workloadId'] wl_settings = self.rest.get_workload_settings( - self.data.array) + self.data.array, False) self.assertEqual(ref_settings, wl_settings) def test_get_workload_settings_next_gen(self): - with mock.patch.object(self.rest, 'is_next_gen_array', - return_value=True): - wl_settings = self.rest.get_workload_settings( - self.data.array_herc) - self.assertEqual(['None'], wl_settings) + wl_settings = self.rest.get_workload_settings( + self.data.array_herc, True) + self.assertEqual(['None'], wl_settings) def test_get_workload_settings_failed(self): wl_settings = self.rest.get_workload_settings( - self.data.failed_resource) + self.data.failed_resource, False) self.assertEqual([], wl_settings) def test_is_compression_capable_true(self): @@ -1479,6 +1479,16 @@ class PowerMaxRestTest(test.TestCase): is_next_gen2 = self.rest.is_next_gen_array(self.data.array_herc) self.assertTrue(is_next_gen2) + def test_get_array_model_info(self): + array_model_vmax, is_next_gen = self.rest.get_array_model_info( + self.data.array) + self.assertEqual('VMAX250F', array_model_vmax) + self.assertFalse(is_next_gen) + array_model_powermax, is_next_gen2 = self.rest.get_array_model_info( + self.data.array_herc) + self.assertTrue(is_next_gen2) + self.assertEqual('PowerMax 2000', array_model_powermax) + @mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall', new=test_utils.ZeroIntervalLoopingCall) @mock.patch.object(rest.PowerMaxRest, 'are_vols_rdf_paired', diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index 5378742bba9..38d6a8110ed 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -184,7 +184,7 @@ class PowerMaxCommon(object): self.failover = False self._get_replication_info() self._get_u4p_failover_info() - self.nextGen = False + self.next_gen = False self._gather_info() self.version_dict = {} @@ -199,7 +199,7 @@ class PowerMaxCommon(object): "longer supported.") self.rest.set_rest_credentials(array_info) if array_info: - self.nextGen = self.rest.is_next_gen_array( + self.array_model, self.next_gen = self.rest.get_array_model_info( array_info['SerialNumber']) finalarrayinfolist = self._get_slo_workload_combinations( array_info) @@ -351,10 +351,12 @@ class PowerMaxCommon(object): if self.failover: array = self.active_backend_id - slo_settings = self.rest.get_slo_list(array) + slo_settings = self.rest.get_slo_list( + array, self.next_gen, self.array_model) slo_list = [x for x in slo_settings if x.lower() not in ['none', 'optimized']] - workload_settings = self.rest.get_workload_settings(array) + workload_settings = self.rest.get_workload_settings( + array, self.next_gen) workload_settings.append('None') slo_workload_set = set( ['%(slo)s:%(workload)s' % {'slo': slo, @@ -362,7 +364,7 @@ class PowerMaxCommon(object): for slo in slo_list for workload in workload_settings]) slo_workload_set.add('None:None') - if self.nextGen: + if self.next_gen: LOG.warning("Workloads have been deprecated for arrays " "running PowerMax OS uCode level 5978 or higher. " "Any supplied workloads will be treated as None " @@ -375,7 +377,7 @@ class PowerMaxCommon(object): slo_workload_set.add('Optimized:None') # If array is 5978 or greater and a VMAX AFA add legacy SL/WL # combinations - if any(self.rest.get_vmax_model(array) in x for x in + if any(self.array_model in x for x in utils.VMAX_AFA_MODELS): slo_workload_set.add('Diamond:OLTP') slo_workload_set.add('Diamond:OLTP_REP') @@ -383,7 +385,7 @@ class PowerMaxCommon(object): slo_workload_set.add('Diamond:DSS_REP') slo_workload_set.add('Diamond:None') - if not any(self.rest.get_vmax_model(array) in x for x in + if not any(self.array_model in x for x in utils.VMAX_AFA_MODELS): slo_workload_set.add('Optimized:None') @@ -1450,7 +1452,7 @@ class PowerMaxCommon(object): protocol = self.utils.get_short_protocol_type(self.protocol) short_host_name = self.utils.get_host_short_name(connector['host']) masking_view_dict[utils.SLO] = extra_specs[utils.SLO] - masking_view_dict[utils.WORKLOAD] = 'NONE' if self.nextGen else ( + masking_view_dict[utils.WORKLOAD] = 'NONE' if self.next_gen else ( extra_specs[utils.WORKLOAD]) masking_view_dict[utils.ARRAY] = extra_specs[utils.ARRAY] masking_view_dict[utils.SRP] = extra_specs[utils.SRP] @@ -1666,12 +1668,12 @@ class PowerMaxCommon(object): :raises: VolumeBackendAPIException: """ array = extra_specs[utils.ARRAY] - next_gen = self.rest.is_next_gen_array(array) + array_model, next_gen = self.rest.get_array_model_info(array) if next_gen: extra_specs[utils.WORKLOAD] = 'NONE' is_valid_slo, is_valid_workload = self.provision.verify_slo_workload( array, extra_specs[utils.SLO], - extra_specs[utils.WORKLOAD], extra_specs[utils.SRP]) + extra_specs[utils.WORKLOAD], next_gen, array_model) if not is_valid_slo or not is_valid_workload: exception_message = (_( "Either SLO: %(slo)s or workload %(workload)s is invalid. " @@ -1769,14 +1771,14 @@ class PowerMaxCommon(object): workload_from_extra_spec = pool_details[1] # Check if legacy pool chosen if (workload_from_extra_spec == pool_record['srpName'] or - self.nextGen): + self.next_gen): workload_from_extra_spec = 'NONE' elif pool_record.get('ServiceLevel'): slo_from_extra_spec = pool_record['ServiceLevel'] workload_from_extra_spec = pool_record.get('Workload', 'None') # If workload is None in cinder.conf, convert to string - if not workload_from_extra_spec or self.nextGen: + if not workload_from_extra_spec or self.next_gen: workload_from_extra_spec = 'NONE' LOG.info("Pool_name is not present in the extra_specs " "- using slo/ workload from cinder.conf: %(slo)s/%(wl)s.", @@ -1784,7 +1786,8 @@ class PowerMaxCommon(object): 'wl': workload_from_extra_spec}) else: - slo_list = self.rest.get_slo_list(pool_record['SerialNumber']) + slo_list = self.rest.get_slo_list( + pool_record['SerialNumber'], self.next_gen, self.array_model) if 'Optimized' in slo_list: slo_from_extra_spec = 'Optimized' elif 'Diamond' in slo_list: @@ -3428,7 +3431,8 @@ class PowerMaxCommon(object): target_device_id=target_device_id, replication_status=replication_status, rep_mode=rep_extra_specs['rep_mode'], - rdf_group_label=self.rep_config['rdf_group_label']) + rdf_group_label=self.rep_config['rdf_group_label'], + target_array_model=rep_extra_specs['target_array_model']) return replication_status, replication_driver_data, rep_info_dict @@ -4084,12 +4088,14 @@ class PowerMaxCommon(object): # Check to see if SLO and Workload are configured on the target array. if extra_specs[utils.SLO]: + rep_extra_specs['target_array_model'], next_gen = ( + self.rest.get_array_model_info(rep_config['array'])) is_valid_slo, is_valid_workload = ( self.provision.verify_slo_workload( rep_extra_specs[utils.ARRAY], extra_specs[utils.SLO], - rep_extra_specs[utils.WORKLOAD], - rep_extra_specs[utils.SRP])) + rep_extra_specs[utils.WORKLOAD], next_gen, + rep_extra_specs['target_array_model'])) if not is_valid_slo: LOG.warning("The target array does not support the " "storage pool setting for SLO %(slo)s, " @@ -4102,7 +4108,6 @@ class PowerMaxCommon(object): "%(workload)s, setting to NONE.", {'workload': extra_specs[utils.WORKLOAD]}) rep_extra_specs[utils.WORKLOAD] = None - return rep_extra_specs @staticmethod diff --git a/cinder/volume/drivers/dell_emc/powermax/metadata.py b/cinder/volume/drivers/dell_emc/powermax/metadata.py index e2e90881211..9ec3c85d23a 100644 --- a/cinder/volume/drivers/dell_emc/powermax/metadata.py +++ b/cinder/volume/drivers/dell_emc/powermax/metadata.py @@ -131,7 +131,7 @@ class PowerMaxVolumeMetadata(object): self.version_dict['unisphere_for_powermax_version'] = ( u4p_version_dict['version']) self.version_dict['serial_number'] = serial_number - array_info_dict = self.rest.get_array_serial(serial_number) + array_info_dict = self.rest.get_array_detail(serial_number) self.version_dict['storage_firmware_version'] = ( array_info_dict['ucode']) self.version_dict['storage_model'] = array_info_dict['model'] @@ -465,6 +465,7 @@ class PowerMaxVolumeMetadata(object): None, None, None, None) rep_mode, replication_status, rdf_group_label, use_bias = ( None, None, None, None) + target_array_model = None if rep_info_dict: rdf_group_no = rep_info_dict['rdf_group_no'] target_name = rep_info_dict['target_name'] @@ -475,6 +476,7 @@ class PowerMaxVolumeMetadata(object): rdf_group_label = rep_info_dict['rdf_group_label'] if utils.METROBIAS in extra_specs: use_bias = extra_specs[utils.METROBIAS] + target_array_model = rep_info_dict['target_array_model'] default_sg = self.utils.derive_default_sg_from_extra_specs( extra_specs, rep_mode) @@ -500,7 +502,8 @@ class PowerMaxVolumeMetadata(object): 'yes' if self.utils.is_compression_disabled( extra_specs) else 'no'), source_device_id=source_device_id, - temporary_snapvx=temporary_snapvx + temporary_snapvx=temporary_snapvx, + target_array_model=target_array_model ) volume_metadata = self.update_volume_info_metadata( diff --git a/cinder/volume/drivers/dell_emc/powermax/provision.py b/cinder/volume/drivers/dell_emc/powermax/provision.py index 5f94c1c36a8..2c934537dbc 100644 --- a/cinder/volume/drivers/dell_emc/powermax/provision.py +++ b/cinder/volume/drivers/dell_emc/powermax/provision.py @@ -452,13 +452,15 @@ class PowerMaxProvision(object): return (total_capacity_gb, remaining_capacity_gb, subscribed_capacity_gb, array_reserve_percent) - def verify_slo_workload(self, array, slo, workload, srp): + def verify_slo_workload( + self, array, slo, workload, is_next_gen=None, array_model=None): """Check if SLO and workload values are valid. :param array: the array serial number :param slo: Service Level Object e.g bronze :param workload: workload e.g DSS - :param srp: the storage resource pool name + :param is_next_gen: can be None + :returns: boolean """ is_valid_slo, is_valid_workload = False, False @@ -472,8 +474,12 @@ class PowerMaxProvision(object): if slo and slo.lower() == 'none': slo = None - valid_slos = self.rest.get_slo_list(array) - valid_workloads = self.rest.get_workload_settings(array) + if is_next_gen or is_next_gen is None: + array_model, is_next_gen = self.rest.get_array_model_info( + array) + valid_slos = self.rest.get_slo_list(array, is_next_gen, array_model) + + valid_workloads = self.rest.get_workload_settings(array, is_next_gen) for valid_slo in valid_slos: if slo == valid_slo: is_valid_slo = True diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index d5a53c631d4..7161181c490 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -539,7 +539,7 @@ class PowerMaxRest(object): operation = 'delete %(res)s resource' % {'res': resource_type} self.check_status_code_success(operation, status_code, message) - def get_array_serial(self, array): + def get_array_detail(self, array): """Get an array from its serial number. :param array: the array serial number @@ -559,7 +559,7 @@ class PowerMaxRest(object): :returns: bool """ is_next_gen = False - array_details = self.get_array_serial(array) + array_details = self.get_array_detail(array) if array_details: ucode_version = array_details['ucode'].split('.')[0] if ucode_version >= UCODE_5978: @@ -603,17 +603,19 @@ class PowerMaxRest(object): resource_name=srp, params=None) return srp_details - def get_slo_list(self, array): + def get_slo_list(self, array, is_next_gen, array_model): """Retrieve the list of slo's from the array :param array: the array serial number + :param is_next_gen: next generation flag + :param array_model :returns: slo_list -- list of service level names """ slo_list = [] slo_dict = self.get_resource(array, SLOPROVISIONING, 'slo') if slo_dict and slo_dict.get('sloId'): - if not self.is_next_gen_array(array) and ( - any(self.get_vmax_model(array) in x for x in + if not is_next_gen and ( + any(array_model in x for x in utils.VMAX_AFA_MODELS)): if 'Optimized' in slo_dict.get('sloId'): slo_dict['sloId'].remove('Optimized') @@ -622,15 +624,16 @@ class PowerMaxRest(object): slo_list.append(slo) return slo_list - def get_workload_settings(self, array): + def get_workload_settings(self, array, is_next_gen): """Get valid workload options from array. Workloads are no longer supported from HyperMaxOS 5978 onwards. :param array: the array serial number + :param is_next_gen: is next generation flag :returns: workload_setting -- list of workload names """ workload_setting = [] - if self.is_next_gen_array(array): + if is_next_gen: workload_setting.append('None') else: wl_details = self.get_resource( @@ -645,14 +648,29 @@ class PowerMaxRest(object): :param array: the array serial number :return: the PowerMax/VMAX model """ - vmax_version = '' - system_uri = ("/%(version)s/system/symmetrix/%(array)s" % { - 'version': U4V_VERSION, 'array': array}) - system_info = self._get_request(system_uri, SYSTEM) + vmax_version = None + system_info = self.get_array_detail(array) if system_info and system_info.get('model'): vmax_version = system_info.get('model') return vmax_version + def get_array_model_info(self, array): + """Get the PowerMax/VMAX model. + + :param array: the array serial number + :return: the PowerMax/VMAX model + """ + array_model = None + is_next_gen = False + system_info = self.get_array_detail(array) + if system_info and system_info.get('model'): + array_model = system_info.get('model') + if system_info: + ucode_version = system_info['ucode'].split('.')[0] + if ucode_version >= UCODE_5978: + is_next_gen = True + return array_model, is_next_gen + def is_compression_capable(self, array): """Check if array is compression capable. @@ -839,6 +857,7 @@ class PowerMaxRest(object): "addVolumeParam": { "num_of_vols": 1, "emulation": "FBA", + "create_new_volumes": "False", "volumeIdentifier": { "identifier_name": volume_name, "volumeIdentifierChoice": "identifier_name"},