diff --git a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py index 7859729e526..9bd22b84453 100644 --- a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py +++ b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py @@ -170,6 +170,9 @@ class StorwizeSVCManagementSimulator(object): 'is not in a group.'), 'CMMVC9012E': ('', 'CMMVC9012E The copy type differs from other ' 'copies already in the consistency group.'), + 'CMMVC5951E': ('', 'CMMVC5951E The operation cannot be performed ' + 'because the relationship is not a stand-alone ' + 'relationship.'), 'CMMVC9201E': ('', 'CMMVC9201E Task failed because volume has a ' 'copy that is fully allocated and is part of a ' 'Metro Mirror or Global Mirror relationship.'), @@ -2234,6 +2237,9 @@ port_speed!N/A except KeyError: return self._errors['CMMVC5753E'] + if rcrel.get('consistency_group_name', None): + return self._errors['CMMVC5951E'] + function = 'stop_access' if force_access else 'stop' self._rc_state_transition(function, rcrel) if force_access: @@ -2360,6 +2366,11 @@ port_speed!N/A cycleperiodseconds = kwargs['cycleperiodseconds'].strip('\'\"') nonull_num += 1 + cyclingmode = None + if 'cyclingmode' in kwargs: + cyclingmode = kwargs['cyclingmode'].strip('\'\"') + nonull_num += 1 + if nonull_num > 1: return self._errors['CMMVC5713E'] elif masterchange: @@ -2370,6 +2381,8 @@ port_speed!N/A return ('', '') elif cycleperiodseconds: rcrel['cycle_period_seconds'] = cycleperiodseconds + elif cyclingmode: + rcrel['cycling_mode'] = cyclingmode return ('', '') def _rc_state_transition(self, function, rcrel): @@ -9833,6 +9846,32 @@ class StorwizeHelpersTestCase(test.TestCase): access=access) startrcrelationship.assert_called_once_with(opts['RC_name'], None) + @ddt.data(({'RC_name': None, + 'name': 'volume-12d-7'}, 'multi'), + ({'RC_name': 'fake_rcrel', + 'name': 'rep_volume-12d-8'}, 'multi'), + ({'RC_name': 'fake_rcrel-2', + 'name': 'rep_volume-12d-9'}, 'none'), + ({'RC_name': 'fake_rcrel-3', + 'name': 'rep_volume-12d-10'}, None)) + @mock.patch.object(storwize_svc_common.StorwizeSSH, + 'ch_rcrelationship_cyclingmode') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'get_vdisk_attributes') + @ddt.unpack + def test_change_relationship_cyclingmode(self, opts, cycling_mode, + get_vdisk_attributes, + ch_rcrelationship_cyclingmode): + get_vdisk_attributes.side_effect = [{'RC_name': opts['RC_name']}] + self.storwize_svc_common.change_relationship_cyclingmode( + opts['name'], cycling_mode) + get_vdisk_attributes.assert_called_with(opts['name']) + if not opts['RC_name'] or not cycling_mode: + ch_rcrelationship_cyclingmode.assert_not_called() + else: + ch_rcrelationship_cyclingmode.assert_called_once_with( + opts['RC_name'], cycling_mode) + @mock.patch.object(storwize_svc_common.StorwizeHelpers, 'get_host_from_host_info') @mock.patch.object(storwize_svc_common.StorwizeSSH, 'lshost') @@ -9898,6 +9937,18 @@ class StorwizeSSHTestCase(test.TestCase): self.storwize_ssh = storwize_svc_common.StorwizeSSH( self.fake_driver._run_ssh) + def test_ch_rcrelationship_cyclingmode(self): + with mock.patch.object( + storwize_svc_common.StorwizeSSH, + 'run_ssh_assert_no_output') as run_ssh_assert_no_output: + run_ssh_assert_no_output.return_value = None + ret = self.storwize_ssh.ch_rcrelationship_cyclingmode('rcrel0', + 'multi') + self.assertIsNone(ret) + ret = self.storwize_ssh.ch_rcrelationship_cyclingmode('rcrel1', + 'none') + self.assertIsNone(ret) + def test_mkvdiskhostmap(self): # mkvdiskhostmap should not be returning anything with mock.patch.object( @@ -11167,6 +11218,127 @@ class StorwizeSVCReplicationTestCase(test.TestCase): self.driver.delete_volume(volume) self._validate_replic_vol_deletion(volume) + def test_storwize_extend_gmcv_volume_invalid(self): + # Set replication target. + self.driver.configuration.set_override('replication_device', + [self.rep_target]) + self.driver.do_setup(self.ctxt) + + # Extend gmcv volume with thick_provisioning_support. + spec = {'replication_enabled': ' True', + 'replication_type': ' gmcv', + 'drivers:rsize': -1} + + gmcv_thick_type = self._create_replica_volume_type( + False, opts=spec, vol_type_name='test_gmcv_thik_type') + gmcv_volume, model_update = self._create_test_volume(gmcv_thick_type) + self.assertEqual(fields.ReplicationStatus.ENABLED, + model_update['replication_status']) + + if self.USESIM: + # tell expandvdisksize to fail while called extend_volume + # because volume is fast formatting + self.sim.error_injection('expandvdisksize', 'fast_formatting') + self.assertRaises(exception.VolumeDriverException, + self.driver.extend_volume, gmcv_volume, 15) + attrs = ( + self.driver._helpers.get_vdisk_attributes(gmcv_volume['name'])) + vol_size = int(attrs['capacity']) / units.Gi + self.assertAlmostEqual(vol_size, 1) + + self.driver.delete_volume(gmcv_volume) + self._validate_replic_vol_deletion(gmcv_volume) + + # Extend gmcv volume that added to group with replication. + with mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'extend_vdisk') as extend_vdisk: + # Create group with replication. + group = self._create_test_rccg(self.rccg_type, + [self.gmcv_default_type.id]) + rccg_name = self.driver._get_rccg_name(group) + # Create gloabl mirror replication with change volumes. + volume, model_update = self._create_test_volume( + self.gmcv_default_type) + self._validate_replic_vol_creation(volume, True) + rcrel = self.driver._helpers.get_relationship_info(volume.name) + self.sim._rc_state_transition('wait', rcrel) + # Add gmcv volume to group. + add_vols = [volume] + (model_update, add_volumes_update, + remove_volumes_update) = self.driver.update_group( + self.ctxt, group, add_vols, []) + self.assertEqual( + rccg_name, + self.driver._helpers.get_rccg_info(volume.name)['name']) + self.assertEqual(fields.GroupStatus.AVAILABLE, + model_update['status']) + self.assertEqual([{'id': volume.id, 'group_id': group.id}], + add_volumes_update) + self.assertEqual([], remove_volumes_update) + + self.assertRaises(exception.VolumeDriverException, + self.driver.extend_volume, volume, 15) + + self.assertFalse(extend_vdisk.called) + attrs = self.driver._helpers.get_vdisk_attributes(volume['name']) + vol_size = int(attrs['capacity']) / units.Gi + self.assertAlmostEqual(vol_size, 1) + + self.driver.delete_volume(volume) + self._validate_replic_vol_deletion(volume) + + def test_convert_global_mirror_volume_to_gmcv(self): + """Test volume conversion from global to gmcv.""" + self.driver.configuration.set_override('replication_device', + [self.rep_target]) + self.driver.do_setup(self.ctxt) + # Create global mirror replication. + gm_vol, model_update = self._create_test_volume(self.gm_type) + self.assertEqual(fields.ReplicationStatus.ENABLED, + model_update['replication_status']) + self._validate_replic_vol_creation(gm_vol) + rcrel = self.driver._helpers.get_relationship_info(gm_vol.name) + self.assertEqual(rcrel['cycling_mode'], '') + self.assertEqual(rcrel['master_change_vdisk_name'], '') + self.assertEqual(rcrel['aux_change_vdisk_name'], '') + + # Validating volume conversion from global to gmcv by checking a few + # property values of RC relationship + target_vol = storwize_const.REPLICA_AUX_VOL_PREFIX + gm_vol.name + master_change_vol_name = ( + storwize_const.REPLICA_CHG_VOL_PREFIX + gm_vol.name) + aux_change_vol_name = ( + storwize_const.REPLICA_CHG_VOL_PREFIX + target_vol) + size = 1 + self.driver._convert_global_mirror_volume_to_gmcv(gm_vol, target_vol, + size) + rcrel = self.driver._helpers.get_relationship_info(gm_vol.name) + self.assertEqual(rcrel['cycling_mode'], 'multi') + self.assertEqual(rcrel['master_change_vdisk_name'], + master_change_vol_name) + self.assertEqual(rcrel['aux_change_vdisk_name'], + aux_change_vol_name) + self.driver.delete_volume(gm_vol) + self._validate_replic_vol_deletion(gm_vol) + + gm_vol, model_update = self._create_test_volume(self.gm_type) + self._validate_replic_vol_creation(gm_vol) + with (mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'create_vdisk')) as create_vdisk: + with (mock.patch.object(storwize_svc_common.StorwizeHelpers, + 'change_relationship_cyclingmode') + ) as ch_relationship_cyclingmode: + target_vol = ( + storwize_const.REPLICA_AUX_VOL_PREFIX + gm_vol.name) + size = 1 + self.driver._convert_global_mirror_volume_to_gmcv( + gm_vol, target_vol, size) + create_vdisk.assert_called() + self.assertEqual(2, create_vdisk.call_count) + ch_relationship_cyclingmode.assert_called() + self.driver.delete_volume(gm_vol) + self._validate_replic_vol_deletion(gm_vol) + def test_storwize_manage_existing_mismatch_with_volume_replication(self): # Set replication target. self.driver.configuration.set_override('replication_device', diff --git a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py index 5a07d382cb8..5d5c0f4291f 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py @@ -366,10 +366,21 @@ class StorwizeSSH(object): ssh_cmd.append(rc_rel) self.run_ssh_assert_no_output(ssh_cmd) + def ch_rcrelationship_cyclingmode(self, relationship, + cyclingmode): + # Note: Can only change one attribute at a time, + # so define three ch_rcrelationship_xxx here + if cyclingmode: + ssh_cmd = ['svctask', 'chrcrelationship'] + ssh_cmd.extend(['-cyclingmode', + str(cyclingmode)]) + ssh_cmd.append(relationship) + self.run_ssh_assert_no_output(ssh_cmd) + def ch_rcrelationship_cycleperiod(self, relationship, cycle_period_seconds): # Note: Can only change one attribute at a time, - # so define two ch_rcrelationship_xxx here + # so define three ch_rcrelationship_xxx here if cycle_period_seconds: ssh_cmd = ['svctask', 'chrcrelationship'] ssh_cmd.extend(['-cycleperiodseconds', @@ -380,7 +391,7 @@ class StorwizeSSH(object): def ch_rcrelationship_changevolume(self, relationship, changevolume, master): # Note: Can only change one attribute at a time, - # so define two ch_rcrelationship_xxx here + # so define three ch_rcrelationship_xxx here if changevolume: ssh_cmd = ['svctask', 'chrcrelationship'] if master: @@ -2530,6 +2541,12 @@ class StorwizeHelpers(object): self.ssh.ch_rcrelationship_cycleperiod(vol_attrs['RC_name'], cycle_period_seconds) + def change_relationship_cyclingmode(self, volume_name, cyclingmode): + vol_attrs = self.get_vdisk_attributes(volume_name) + if vol_attrs['RC_name'] and cyclingmode: + self.ssh.ch_rcrelationship_cyclingmode(vol_attrs['RC_name'], + cyclingmode) + def delete_relationship(self, volume_name): vol_attrs = self.get_vdisk_attributes(volume_name) if vol_attrs['RC_name']: @@ -3843,6 +3860,10 @@ class StorwizeSVCCommonDriver(san.SanDriver, def _extend_volume_op(self, volume, new_size, old_size=None): LOG.debug('enter: _extend_volume_op: volume %s', volume['id']) + if self._state['code_level'] < (7, 7, 0, 0): + force_unmap = False + else: + force_unmap = True volume_name = self._get_target_vol(volume) rel_info = self._helpers.get_relationship_info(volume_name) @@ -3903,41 +3924,55 @@ class StorwizeSVCCommonDriver(san.SanDriver, raise exception.VolumeDriverException(message=msg) else: try: - master_helper.delete_relationship(volume.name) tgt_vol = (storwize_const.REPLICA_AUX_VOL_PREFIX + volume.name) - master_helper.extend_vdisk(volume.name, extend_amt) - target_helper.extend_vdisk(tgt_vol, extend_amt) - tgt_sys = target_helper.get_system_info() - if storwize_const.GMCV_MULTI == cyclingmode: + if storwize_const.GMCV_MULTI != cyclingmode: + target_helper.extend_vdisk(tgt_vol, extend_amt) + master_helper.extend_vdisk(volume.name, extend_amt) + else: + # Update gmcv volume cyclingmode to 'none' + master_helper.stop_relationship(volume.name) + master_helper.change_relationship_cyclingmode( + volume.name, 'none') + master_helper.start_relationship(volume.name) + tgt_change_vol = ( - storwize_const.REPLICA_CHG_VOL_PREFIX + - tgt_vol) + storwize_const.REPLICA_CHG_VOL_PREFIX + tgt_vol) source_change_vol = ( storwize_const.REPLICA_CHG_VOL_PREFIX + volume.name) - master_helper.extend_vdisk(source_change_vol, - extend_amt) - target_helper.extend_vdisk(tgt_change_vol, extend_amt) - src_change_opts = self._get_vdisk_params( - volume.volume_type_id) - cycle_period_seconds = src_change_opts.get( - 'cycle_period_seconds') - master_helper.create_relationship( - volume.name, tgt_vol, tgt_sys.get('system_name'), - True, True, source_change_vol, - cycle_period_seconds) - target_helper.change_relationship_changevolume( - tgt_vol, tgt_change_vol, False) - master_helper.start_relationship(volume.name) - else: - master_helper.create_relationship( - volume.name, tgt_vol, tgt_sys.get('system_name'), - True if cyclingmode == 'none' else False) + + # Delete source_change_volume and target_change_volume + master_helper.delete_vdisk(source_change_vol, + force_unmap=force_unmap, + force_delete=True) + target_helper.delete_vdisk(tgt_change_vol, + force_unmap=force_unmap, + force_delete=True) + + # Extend primary volume and auxiliary volume + target_helper.extend_vdisk(tgt_vol, extend_amt) + master_helper.extend_vdisk(volume.name, extend_amt) + + # Convert global mirror volume to GMCV volume with + # the new volume-size + self._convert_global_mirror_volume_to_gmcv( + volume, tgt_vol, new_size) except Exception as e: msg = (_('Failed to extend a volume with remote copy ' '%(volume)s. Exception: ' - '%(err)s.') % {'volume': volume.id, 'err': e}) + '%(err)s.') % {'volume': volume.id, + 'err': e}) + rel_info = self._helpers.get_relationship_info(volume_name) + new_cyclingmode = ( + rel_info['cycling_mode'] if rel_info else 'multi') + if (storwize_const.GMCV_MULTI == cyclingmode and + cyclingmode != new_cyclingmode): + # Convert global mirror volume to GMCV volume with + # the current volume-size + self._convert_global_mirror_volume_to_gmcv( + volume, tgt_vol, volume['size']) + LOG.error(msg) raise exception.VolumeDriverException(message=msg) else: @@ -3961,6 +3996,48 @@ class StorwizeSVCCommonDriver(san.SanDriver, context.get_admin_context(), volume['id'], model_update['metadata'], False) + def _convert_global_mirror_volume_to_gmcv(self, volume, target_vol, size): + master_helper = self._master_backend_helpers + target_helper = self._aux_backend_helpers + tgt_change_vol = (storwize_const.REPLICA_CHG_VOL_PREFIX + target_vol) + src_change_vol = (storwize_const.REPLICA_CHG_VOL_PREFIX + volume.name) + + # Create source change volume if it doesn't exist + src_attr = master_helper.get_vdisk_attributes(volume.name) + src_change_attr = master_helper.get_vdisk_attributes(src_change_vol) + if not src_change_attr: + src_change_opts = self._get_vdisk_params(volume.volume_type_id) + src_change_opts['iogrp'] = src_attr['IO_group_id'] + # Change volumes would usually be thin-provisioned + src_change_opts['autoexpand'] = True + master_helper.create_vdisk(src_change_vol, str(int(size)), 'gb', + src_attr['mdisk_grp_name'], + src_change_opts) + + # Create target change volume if it doesn't exist + target_change_attr = ( + target_helper.get_vdisk_attributes(tgt_change_vol)) + if not target_change_attr: + target_change_opts = self._get_vdisk_params( + volume.volume_type_id) + target_change_pool = self._replica_target.get('pool_name') + target_change_opts['iogrp'] = src_attr['IO_group_id'] + # Change Volumes would usually be thin-provisioned + target_change_opts['autoexpand'] = True + target_helper.create_vdisk(tgt_change_vol, str(int(size)), 'gb', + target_change_pool, target_change_opts) + + # Update volume cyclingmode to 'multi' + master_helper.stop_relationship(volume.name) + master_helper.change_relationship_cyclingmode(volume.name, 'multi') + # Set source_change_volume and target_change_volume + master_helper.change_relationship_changevolume(volume.name, + src_change_vol, True) + target_helper.change_relationship_changevolume(target_vol, + tgt_change_vol, False) + # Start gmcv volume relationship + master_helper.start_relationship(volume.name) + def _qos_model_update(self, model_update, volume): """add volume wwn and IOThrottle_rate to the metadata of the volume""" model_update = model_update or dict() diff --git a/releasenotes/notes/bug-1920870-ibm-svf-fix-extend-issue-for-mirroring-volumes-31b1a9119c49e112.yaml b/releasenotes/notes/bug-1920870-ibm-svf-fix-extend-issue-for-mirroring-volumes-31b1a9119c49e112.yaml new file mode 100644 index 00000000000..45dc71bc4ad --- /dev/null +++ b/releasenotes/notes/bug-1920870-ibm-svf-fix-extend-issue-for-mirroring-volumes-31b1a9119c49e112.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + IBM Spectrum Virtualize Family driver + `Bug #1920870 `_: + Fixed extend issues for volumes with replication enabled by + avoiding volume remote-copy relationship deletion and creation.