From 28e7a46afea688bd9639fbfc0892f296e3bf1252 Mon Sep 17 00:00:00 2001 From: odonos12 Date: Mon, 23 Nov 2020 11:51:22 +0000 Subject: [PATCH] PowerMax Driver - Update unsupported retype combinations 1. Allowing for metro to metro as the volume is simply being moved within the masking view. 2. Updating the message, as target metro is not possible when source is not metro and host assisted migration does not support replicated volumes. Change-Id: I12d05d2ec670e8db037060b29e59eeb7cdd30ebc --- .../dell_emc/powermax/test_powermax_common.py | 58 ++++++++++++++++++- .../drivers/dell_emc/powermax/common.py | 55 +++++++++++++----- 2 files changed, 97 insertions(+), 16 deletions(-) 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 bd1dcd056e4..d60ce2198eb 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 @@ -4141,6 +4141,61 @@ class PowerMaxCommonTest(test.TestCase): self.assertEqual(2, mck_get_sg.call_count) self.assertTrue(success) + @mock.patch.object( + utils.PowerMaxUtils, 'get_port_name_label', + return_value='my_pg') + @mock.patch.object( + utils.PowerMaxUtils, 'get_volume_attached_hostname', + return_value='HostX') + @mock.patch.object( + rest.PowerMaxRest, 'is_volume_in_storagegroup', return_value=True) + @mock.patch.object( + masking.PowerMaxMasking, 'return_volume_to_volume_group') + @mock.patch.object( + masking.PowerMaxMasking, 'move_volume_between_storage_groups') + @mock.patch.object( + masking.PowerMaxMasking, 'add_child_sg_to_parent_sg') + @mock.patch.object( + provision.PowerMaxProvision, 'create_storage_group') + @mock.patch.object( + rest.PowerMaxRest, 'get_storage_group', + side_effect=[None, tpd.PowerMaxData.volume_info_dict]) + @mock.patch.object( + rest.PowerMaxRest, 'get_volume', + return_value=tpd.PowerMaxData.volume_details[0]) + @mock.patch.object( + utils.PowerMaxUtils, 'get_rdf_management_group_name', + return_value=tpd.PowerMaxData.rdf_managed_async_grp) + def test_retype_volume_attached_metro( + self, mck_get_rdf, mck_get_vol, mck_get_sg, mck_create, mck_add, + mck_move_vol, mck_return_vol, mck_is_vol, mck_host, mck_pg): + + array = self.data.array + srp = self.data.srp + device_id = self.data.device_id + volume = self.data.test_attached_volume + volume_name = self.data.volume_id + extra_specs = self.data.rep_extra_specs_rep_config_metro + target_slo = self.data.slo_silver + target_workload = self.data.workload + target_extra_specs = deepcopy(self.data.rep_extra_specs) + target_extra_specs[utils.DISABLECOMPRESSION] = False + target_extra_specs[utils.REP_CONFIG] = self.data.rep_config_sync + + success, target_sg_name = self.common._retype_volume( + array, srp, device_id, volume, volume_name, extra_specs, + target_slo, target_workload, target_extra_specs, remote=True, + metro_attach=True) + mck_get_rdf.assert_called_once() + mck_get_vol.assert_called_once() + mck_create.assert_called_once() + mck_add.assert_called_once() + mck_move_vol.assert_called_once() + mck_return_vol.assert_called_once() + mck_is_vol.assert_called_once() + self.assertEqual(2, mck_get_sg.call_count) + self.assertTrue(success) + @mock.patch.object( utils.PowerMaxUtils, 'get_volume_attached_hostname', return_value=None) @mock.patch.object( @@ -4157,7 +4212,8 @@ class PowerMaxCommonTest(test.TestCase): device_id = self.data.device_id volume = self.data.test_attached_volume volume_name = self.data.volume_id - extra_specs = self.data.rep_extra_specs_rep_config + extra_specs = deepcopy(self.data.rep_extra_specs_rep_config) + extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f target_slo = self.data.slo_silver target_workload = self.data.workload target_extra_specs = deepcopy(self.data.rep_extra_specs) diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index e4b99a5f28f..1935ef35b83 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -3797,7 +3797,7 @@ class PowerMaxCommon(object): new_type['extra_specs'], self.rep_configs): src_mode = extra_specs.get('rep_mode', 'non-replicated') - LOG.error("It is not possible to perform host-assisted retype " + LOG.error("It is not possible to perform storage-assisted retype " "from %(src_mode)s to Metro replication type whilst the " "volume is attached to a host. To perform this " "operation please first detach the volume.", @@ -4293,7 +4293,8 @@ class PowerMaxCommon(object): def _retype_volume( self, array, srp, device_id, volume, volume_name, extra_specs, - target_slo, target_workload, target_extra_specs, remote=False): + target_slo, target_workload, target_extra_specs, remote=False, + metro_attach=False): """Retype a volume from one volume type to another. The target storage group ID is returned so the next phase in the @@ -4310,6 +4311,7 @@ class PowerMaxCommon(object): :param target_extra_specs: target extra specs :param remote: if the volume being retyped is on a remote replication target + :param metro_attach: is it metro attached -- bool :returns: retype success, target storage group -- bool, str """ is_re, rep_mode, mgmt_sg_name = False, None, None @@ -4342,7 +4344,27 @@ class PowerMaxCommon(object): try: # If volume is attached set up the parent/child SGs if not already # present on array - if volume.attach_status == 'attached' and not remote: + is_attached_vol = False + if volume.attach_status == 'attached' and remote and metro_attach: + is_attached_vol = True + rep_config = target_extra_specs.get('rep_config') + if rep_config: + port_group_name = rep_config.get('portgroup') + if port_group_name: + port_group_label = self.utils.get_port_name_label( + port_group_name, + self.powermax_port_group_name_template) + else: + LOG.error("Unable to get the port group name from " + "replication configuration.") + return False, None + + elif volume.attach_status == 'attached' and not remote: + is_attached_vol = True + port_group_label = self.utils.get_port_name_label( + target_extra_specs[utils.PORTGROUPNAME], + self.powermax_port_group_name_template) + if is_attached_vol: attached_host = self.utils.get_volume_attached_hostname( volume) if not attached_host: @@ -4352,10 +4374,6 @@ class PowerMaxCommon(object): "migration.", {'volume_name': device_id}) return False, None - port_group_label = self.utils.get_port_name_label( - target_extra_specs[utils.PORTGROUPNAME], - self.powermax_port_group_name_template) - target_sg_name, __, __ = self.utils.get_child_sg_name( attached_host, target_extra_specs, port_group_label) target_sg = self.rest.get_storage_group(array, target_sg_name) @@ -4377,7 +4395,8 @@ class PowerMaxCommon(object): target_extra_specs) add_sg_to_parent = True - # Else volume is not attached or is remote volume, use default SGs + # Else volume is not attached or is remote (not attached) volume, + # use default SGs else: target_sg_name = ( self.masking.get_or_create_default_storage_group( @@ -4585,12 +4604,18 @@ class PowerMaxCommon(object): remote_array = rep_extra_specs['array'] rep_compr_disabled = self.utils.is_compression_disabled( rep_extra_specs) - - remote_sg_name = self.masking.get_or_create_default_storage_group( - remote_array, rep_extra_specs[utils.SRP], - rep_extra_specs[utils.SLO], rep_extra_specs[utils.WORKLOAD], - rep_extra_specs, rep_compr_disabled, - is_re=is_re, rep_mode=rep_mode) + remote_sg_name = None + metro_attach = False + if volume.attach_status == 'detached' or ( + rep_mode in [utils.REP_SYNC, utils.REP_ASYNC]): + remote_sg_name = self.masking.get_or_create_default_storage_group( + remote_array, rep_extra_specs[utils.SRP], + rep_extra_specs[utils.SLO], rep_extra_specs[utils.WORKLOAD], + rep_extra_specs, rep_compr_disabled, + is_re=is_re, rep_mode=rep_mode) + elif volume.attach_status == 'attached' and ( + rep_mode in [utils.REP_METRO]): + metro_attach = True found_storage_group_list = self.rest.get_storage_groups_from_volume( remote_array, target_device_id) @@ -4606,7 +4631,7 @@ class PowerMaxCommon(object): remote_array, rep_extra_specs[utils.SRP], target_device_id, volume, volume_name, rep_extra_specs, extra_specs[utils.SLO], extra_specs[utils.WORKLOAD], - extra_specs, remote=True) + extra_specs, remote=True, metro_attach=metro_attach) except Exception as e: try: volumes = self.rest.get_volumes_in_storage_group(