From e2f52da0d37f77e7a7d619ad1b6c6e098971dac1 Mon Sep 17 00:00:00 2001 From: michael-mcaleer Date: Tue, 21 May 2019 16:09:51 +0100 Subject: [PATCH] PowerMax Driver - PowerMax Formatted Vols Fix PowerMax OS (uCode 5978) arrays support adding volumes to a Metro RDF group without the need for formatting the volume in advance. This bug fix addresses a bug in the code where PowerMax volumes are formatted regardless of support. Change-Id: Ib840d885c8e3d4d13c9d4bc3c88ef8e5d8477788 Closes-Bug: #1829876 --- .../dell_emc/powermax/test_powermax_rest.py | 84 ++++++++++++++++--- cinder/volume/drivers/dell_emc/powermax/fc.py | 1 + .../volume/drivers/dell_emc/powermax/iscsi.py | 1 + .../volume/drivers/dell_emc/powermax/rest.py | 24 +++++- .../volume/drivers/dell_emc/powermax/utils.py | 1 + 5 files changed, 95 insertions(+), 16 deletions(-) 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 6a7401c2bc5..0da67b28cf1 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 @@ -1373,23 +1373,81 @@ class PowerMaxRestTest(test.TestCase): self.data.device_id2, self.data.remote_array, extra_specs) self.assertEqual(ref_dict, rdf_dict) + @mock.patch.object(rest.PowerMaxRest, 'wait_for_job') + @mock.patch.object(rest.PowerMaxRest, 'create_resource', + return_value=(200, 'job')) + @mock.patch.object(rest.PowerMaxRest, 'is_next_gen_array', + side_effect=[True, True, False, False]) + def test_test_create_rdf_device_pair_metro_cons_exempt( + self, mck_nxt_gen, mck_create, mck_wait): + extra_specs = deepcopy(self.data.extra_specs) + extra_specs[utils.REP_MODE] = utils.REP_METRO + extra_specs[utils.METROBIAS] = True + + ref_payload = ({ + "deviceNameListSource": [{"name": self.data.device_id}], + "deviceNameListTarget": [{"name": self.data.device_id2}], + "replicationMode": 'Active', + "establish": 'true', + "rdfType": 'RDF1'}) + + get_payload_true = {'rdfType': 'RDF1', 'consExempt': 'true'} + get_payload_false = {'rdfType': 'RDF1', 'consExempt': 'false'} + + with mock.patch.object( + self.rest, 'get_metro_payload_info', + side_effect=[get_payload_true, + get_payload_false]) as mock_payload: + ref_extra_specs = deepcopy(extra_specs) + + ref_extra_specs[utils.RDF_CONS_EXEMPT] = True + self.rest.create_rdf_device_pair( + self.data.array, self.data.device_id, self.data.rdf_group_no, + self.data.device_id2, self.data.remote_array, extra_specs) + mock_payload.assert_called_once_with( + self.data.array, ref_payload, self.data.rdf_group_no, + ref_extra_specs) + + mock_payload.reset_mock() + + ref_extra_specs[utils.RDF_CONS_EXEMPT] = False + self.rest.create_rdf_device_pair( + self.data.array, self.data.device_id, self.data.rdf_group_no, + self.data.device_id2, self.data.remote_array, extra_specs) + mock_payload.assert_called_once_with( + self.data.array, ref_payload, self.data.rdf_group_no, + ref_extra_specs) + @mock.patch.object(rest.PowerMaxRest, 'get_rdf_group', side_effect=[{'numDevices': 0}, {'numDevices': 0}, - {'numDevices': 1}]) + {'numDevices': 1}, {'numDevices': 1}]) def test_get_metro_payload_info(self, mock_rdfg): - ref_payload = {'establish': 'true', 'rdfType': 'RDF1'} - payload1 = self.rest.get_metro_payload_info( - self.data.array, ref_payload, self.data.rdf_group_no, {}) - self.assertEqual(ref_payload, payload1) - payload2 = self.rest.get_metro_payload_info( - self.data.array, ref_payload, self.data.rdf_group_no, + payload_in = {'establish': 'true', 'rdfType': 'RDF1'} + + # First volume out, Metro use bias not set + act_payload_1 = self.rest.get_metro_payload_info( + self.data.array, payload_in.copy(), self.data.rdf_group_no, {}) + self.assertEqual(payload_in, act_payload_1) + + # First volume out, Metro use bias set + act_payload_2 = self.rest.get_metro_payload_info( + self.data.array, payload_in.copy(), self.data.rdf_group_no, {'metro_bias': True}) - self.assertEqual('true', payload2['metroBias']) - ref_payload2 = {'establish': 'true', 'rdfType': 'RDF1'} - payload3 = self.rest.get_metro_payload_info( - self.data.array, ref_payload2, self.data.rdf_group_no, {}) - ref_payload3 = {'rdfType': 'RDF1', 'format': 'true'} - self.assertEqual(ref_payload3, payload3) + self.assertEqual('true', act_payload_2['metroBias']) + + # Not first vol in RDFG, consistency exempt not set + act_payload_3 = self.rest.get_metro_payload_info( + self.data.array, payload_in.copy(), self.data.rdf_group_no, + {'consExempt': False}) + ref_payload_3 = {'rdfType': 'NA', 'format': 'true'} + self.assertEqual(ref_payload_3, act_payload_3) + + # Not first vol in RDFG, consistency exempt set + act_payload_4 = self.rest.get_metro_payload_info( + self.data.array, payload_in.copy(), self.data.rdf_group_no, + {'consExempt': True}) + ref_payload_4 = {'rdfType': 'RDF1', 'consExempt': 'true'} + self.assertEqual(ref_payload_4, act_payload_4) def test_modify_rdf_device_pair(self): resource_name = '70/volume/00001' diff --git a/cinder/volume/drivers/dell_emc/powermax/fc.py b/cinder/volume/drivers/dell_emc/powermax/fc.py index 4783b441ad2..8b1a83ba559 100644 --- a/cinder/volume/drivers/dell_emc/powermax/fc.py +++ b/cinder/volume/drivers/dell_emc/powermax/fc.py @@ -110,6 +110,7 @@ class PowerMaxFCDriver(san.SanDriver, driver.FibreChannelDriver): (bp/powermax-storage-assisted-inuse-retype) 4.1.0 - Changing from 90 to 91 rest endpoints - Support for Rapid TDEV Delete (bp powermax-tdev-deallocation) + - PowerMax OS Metro formatted volumes fix (bug #1829876) """ VERSION = "4.1.0" diff --git a/cinder/volume/drivers/dell_emc/powermax/iscsi.py b/cinder/volume/drivers/dell_emc/powermax/iscsi.py index 434de4e3aef..17d41cdb0ea 100644 --- a/cinder/volume/drivers/dell_emc/powermax/iscsi.py +++ b/cinder/volume/drivers/dell_emc/powermax/iscsi.py @@ -115,6 +115,7 @@ class PowerMaxISCSIDriver(san.SanISCSIDriver): (bp/powermax-storage-assisted-inuse-retype) 4.1.0 - Changing from 90 to 91 rest endpoints - Support for Rapid TDEV Delete (bp powermax-tdev-deallocation) + - PowerMax OS Metro formatted volumes fix (bug #1829876) """ VERSION = "4.1.0" diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index ad106486c2d..501241eb8e2 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -2310,8 +2310,17 @@ class PowerMaxRest(object): payload_update = self._get_async_payload_info(array, rdf_group_no) payload.update(payload_update) elif rep_mode == 'Active': + # Check if arrays are next gen to support add data vol to existing + # metro enabled rdfg, else format drive before adding + r1_nxt_gen = self.is_next_gen_array(array) + r2_nxt_gen = self.is_next_gen_array(remote_array) + if r1_nxt_gen and r2_nxt_gen: + extra_specs[utils.RDF_CONS_EXEMPT] = True + else: + extra_specs[utils.RDF_CONS_EXEMPT] = False payload = self.get_metro_payload_info( array, payload, rdf_group_no, extra_specs) + resource_type = ("rdf_group/%(rdf_num)s/volume" % {'rdf_num': rdf_group_no}) status_code, job = self.create_resource(array, REPLICATION, @@ -2357,10 +2366,19 @@ class PowerMaxRest(object): and extra_specs[utils.METROBIAS] is True): payload.update({'metroBias': 'true'}) else: - # Need to format subsequent volumes - payload['format'] = 'true' + if (extra_specs.get(utils.RDF_CONS_EXEMPT) + and extra_specs[utils.RDF_CONS_EXEMPT] is True): + payload['consExempt'] = 'true' + payload['rdfType'] = 'RDF1' + else: + LOG.warning("Adding HyperMax OS volumes to an existing RDFG " + "requires the volumes to be formatted in advance," + "please upgrade to PowerMax OS to bypass this " + "restriction.") + payload['format'] = 'true' + payload['rdfType'] = 'NA' + payload.pop('establish') - payload['rdfType'] = 'RDF1' return payload def modify_rdf_device_pair( diff --git a/cinder/volume/drivers/dell_emc/powermax/utils.py b/cinder/volume/drivers/dell_emc/powermax/utils.py index 0ad7f56d7b0..fa59921070f 100644 --- a/cinder/volume/drivers/dell_emc/powermax/utils.py +++ b/cinder/volume/drivers/dell_emc/powermax/utils.py @@ -73,6 +73,7 @@ RDF_FAILEDOVER_STATE = 'failed over' RDF_ACTIVE = 'active' RDF_ACTIVEACTIVE = 'activeactive' RDF_ACTIVEBIAS = 'activebias' +RDF_CONS_EXEMPT = 'consExempt' METROBIAS = 'metro_bias' DEFAULT_PORT = 8443 CLONE_SNAPSHOT_NAME = "snapshot_for_clone"