[SVF] Fix extend issue for mirroring volumes

[Spectrum Virtualize Family] Fixed volume extend issue for volumes
with replication enabled by avoiding both delete RC relationship
and create RC relationship of that volume.

Closes-Bug: #1920870

Change-Id: Id8dfa3d756142a17a50edf969d2ec84cfe524f4f

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': '<is> True',
+                'replication_type': '<in> 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',

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()

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 <https://bugs.launchpad.net/cinder/+bug/1920870>`_:
+    Fixed extend issues for volumes with replication enabled by
+    avoiding volume remote-copy relationship deletion and creation.