From 257c1c16fa0711130772bb36a5d04e9563c583b1 Mon Sep 17 00:00:00 2001 From: Xuchu Jiang Date: Sat, 15 Feb 2020 00:34:06 +0800 Subject: [PATCH] Fix revert snapshot issue While there is a related fcmap stays in 100% copying clone state, and this volume act as target volume in fcmap relationship, revert snapshot will fail. This fix will stop 100% copying clone fcmap, then let revert snapshot go. Closes-bug: #1863070 Change-Id: I6b5f6396ad5524f4086ce5192e32fd5e7fee58a2 (cherry picked from commit 9b4321f8462b32206c8d684442606b57a6af20da) --- .../volume/drivers/ibm/test_storwize_svc.py | 63 +++++++++++++++++++ .../ibm/storwize_svc/storwize_svc_common.py | 39 +++++++++++- 2 files changed, 100 insertions(+), 2 deletions(-) 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 0bd96145d22..e37be31d519 100644 --- a/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py +++ b/cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py @@ -8053,6 +8053,29 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase): self.driver.retype, ctxt, volume, new_type, diff, host) + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + '_get_flashcopy_mapping_attributes') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + '_get_vdisk_fc_mappings') + def test_revert_to_snapshot_with_uncompleted_clone( + self, + _get_vdisk_fc_mappings, + _get_flashcopy_mapping_attributes): + vol1 = self._generate_vol_info() + snap1 = self._generate_snap_info(vol1.id) + + self.driver._helpers._get_vdisk_fc_mappings.return_value = ['4'] + self.driver._helpers._get_flashcopy_mapping_attributes.return_value = { + 'copy_rate': '50', + 'progress': '3', + 'status': 'copying', + 'target_vdisk_name': 'testvol'} + + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.revert_to_snapshot, + self.ctxt, + vol1, snap1) + class CLIResponseTestCase(test.TestCase): def test_empty(self): @@ -8247,6 +8270,46 @@ class StorwizeHelpersTestCase(test.TestCase): self.assertTrue(iog in state['available_iogrps']) self.assertEqual(1, iog) + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + '_get_flashcopy_mapping_attributes') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + '_get_vdisk_fc_mappings') + def test_pretreatment_before_revert_uncompleted_clone( + self, + _get_vdisk_fc_mappings, + _get_flashcopy_mapping_attributes): + vol = 'testvol' + _get_vdisk_fc_mappings.return_value = ['4'] + _get_flashcopy_mapping_attributes.return_value = { + 'copy_rate': '50', + 'progress': '3', + 'status': 'copying', + 'target_vdisk_name': 'testvol'} + + self.assertRaises(exception.VolumeDriverException, + self.storwize_svc_common.pretreatment_before_revert, + vol) + + @mock.patch.object(storwize_svc_common.StorwizeSSH, 'stopfcmap') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + '_get_flashcopy_mapping_attributes') + @mock.patch.object(storwize_svc_common.StorwizeHelpers, + '_get_vdisk_fc_mappings') + def test_pretreatment_before_revert_completed_clone( + self, + _get_vdisk_fc_mappings, + _get_flashcopy_mapping_attributes, + stopfcmap): + vol = 'testvol' + _get_vdisk_fc_mappings.return_value = ['4'] + _get_flashcopy_mapping_attributes.return_value = { + 'copy_rate': '50', + 'progress': '100', + 'status': 'copying', + 'target_vdisk_name': 'testvol'} + self.storwize_svc_common.pretreatment_before_revert(vol) + stopfcmap.assert_called_once_with('4', split=True) + @ddt.ddt class StorwizeSSHTestCase(test.TestCase): 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 87f91b97a3d..255731d8e7b 100644 --- a/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py +++ b/cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py @@ -612,8 +612,13 @@ class StorwizeSSH(object): '-autodelete', autodel, fc_map_id] self.run_ssh_assert_no_output(ssh_cmd) - def stopfcmap(self, fc_map_id): - ssh_cmd = ['svctask', 'stopfcmap', fc_map_id] + def stopfcmap(self, fc_map_id, force=False, split=False): + ssh_cmd = ['svctask', 'stopfcmap'] + if force: + ssh_cmd += ['-force'] + if split: + ssh_cmd += ['-split'] + ssh_cmd += [fc_map_id] self.run_ssh_assert_no_output(ssh_cmd) def rmfcmap(self, fc_map_id): @@ -2510,6 +2515,28 @@ class StorwizeHelpers(object): 'same site as peer_pool %(peer_pool)s. ') % {'pool': pool, 'peer_pool': peer_pool}) + def pretreatment_before_revert(self, name): + mapping_ids = self._get_vdisk_fc_mappings(name) + for map_id in mapping_ids: + attrs = self._get_flashcopy_mapping_attributes(map_id) + if not attrs: + continue + target = attrs['target_vdisk_name'] + copy_rate = attrs['copy_rate'] + progress = attrs['progress'] + status = attrs['status'] + if status in ['copying', 'prepared'] and target == name: + if copy_rate != '0' and progress != '100': + msg = (_('Cannot start revert since fcmap %(map_id)s ' + 'in progress, current progress is %(progress)s') + % {'map_id': map_id, 'progress': progress}) + LOG.error(msg) + raise exception.VolumeDriverException(message=msg) + elif copy_rate != '0' and progress == '100': + LOG.debug('Split completed clone map_id=%(map_id)s fcmap', + {'map_id': map_id}) + self.ssh.stopfcmap(map_id, split=True) + class CLIResponse(object): """Parse SVC CLI output and generate iterable.""" @@ -5475,6 +5502,14 @@ class StorwizeSVCCommonDriver(san.SanDriver, if rep_type: raise exception.InvalidInput( reason=_('Reverting replication volume is not supported.')) + try: + self._helpers.pretreatment_before_revert(volume.name) + except Exception as err: + msg = (_("Pretreatment before revert volume %(vol)s to snapshot " + "%(snap)s failed due to: %(err)s.") + % {"vol": volume.name, "snap": snapshot.name, "err": err}) + LOG.error(msg) + raise exception.VolumeBackendAPIException(data=msg) opts = self._get_vdisk_params(volume.volume_type_id) try: self._helpers.run_flashcopy(