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 dc0f83c84a8..79eee190194 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 @@ -729,6 +729,38 @@ class PowerMaxCommonTest(test.TestCase): self.common._create_cloned_volume, volume, source_volume, extra_specs) + @mock.patch.object(common.PowerMaxCommon, + '_find_device_on_array') + def test_create_cloned_volume_not_licenced_2(self, mock_device): + volume = self.data.test_clone_volume + source_volume = self.data.test_volume + extra_specs = self.data.extra_specs + with mock.patch.object(self.rest, 'is_snapvx_licensed', + return_value=False): + self.assertRaises(exception.VolumeBackendAPIException, + self.common._create_cloned_volume, + volume, source_volume, extra_specs, + False, False) + mock_device.assert_not_called() + + @mock.patch.object(common.PowerMaxCommon, + '_find_device_on_array', + return_value=None) + @mock.patch.object(common.PowerMaxCommon, + '_clone_check') + def test_create_cloned_volume_source_not_found( + self, mock_check, mock_device): + volume = self.data.test_clone_volume + source_volume = self.data.test_volume + extra_specs = self.data.extra_specs + with mock.patch.object(self.rest, 'is_snapvx_licensed', + return_value=True): + self.assertRaises(exception.VolumeBackendAPIException, + self.common._create_cloned_volume, + volume, source_volume, extra_specs, + False, False) + mock_check.assert_not_called() + def test_parse_snap_info_found(self): ref_device_id = self.data.device_id ref_snap_name = self.data.snap_location['snap_name'] @@ -1248,6 +1280,32 @@ class PowerMaxCommonTest(test.TestCase): self.common._sync_check(array, device_id, extra_specs) mock_break.assert_not_called() + def test_do_sync_check_repeat(self): + array = self.data.array + device_id = self.data.device_id + extra_specs = self.data.extra_specs + with mock.patch.object(self.common, + '_unlink_targets_and_delete_temp_snapvx', + side_effect=Exception): + with mock.patch.object(self.common, + '_unlink_targets_and_delete_temp_snapvx', + side_effect=None): + self.common._sync_check(array, device_id, extra_specs) + + def test_do_sync_check_repeat_and_fail_again(self): + array = self.data.array + device_id = self.data.device_id + extra_specs = self.data.extra_specs + with mock.patch.object(self.common, + '_unlink_targets_and_delete_temp_snapvx', + side_effect=Exception): + with mock.patch.object(self.common, + '_unlink_targets_and_delete_temp_snapvx', + side_effect=Exception): + self.assertRaises(exception.VolumeBackendAPIException, + self.common._sync_check, array, + device_id, extra_specs) + @mock.patch.object(provision.PowerMaxProvision, 'delete_volume_snap') @mock.patch.object(provision.PowerMaxProvision, 'break_replication_relationship') diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index 574cbba6317..5db9017763a 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -30,6 +30,7 @@ from cinder import coordination from cinder import exception from cinder.i18n import _ from cinder.objects import fields +from cinder.utils import retry from cinder.volume import configuration from cinder.volume.drivers.dell_emc.powermax import masking from cinder.volume.drivers.dell_emc.powermax import metadata as volume_metadata @@ -52,6 +53,8 @@ REPLICATION_FAILOVER = fields.ReplicationStatus.FAILED_OVER FAILOVER_ERROR = fields.ReplicationStatus.FAILOVER_ERROR REPLICATION_ERROR = fields.ReplicationStatus.ERROR +retry_exc_tuple = (exception.VolumeBackendAPIException,) + powermax_opts = [ cfg.IntOpt('interval', @@ -1515,16 +1518,23 @@ class PowerMaxCommon(object): array = extra_specs[utils.ARRAY] is_clone_license = self.rest.is_snapvx_licensed(array) + if not is_clone_license: + exception_message = (_( + "SnapVx feature is not licensed on %(array)s.") + % {'array': array}) + LOG.error(exception_message) + raise exception.VolumeBackendAPIException( + message=exception_message) + if from_snapvx: source_device_id, snap_name = self._parse_snap_info( array, source_volume) else: source_device_id = self._find_device_on_array( source_volume, extra_specs) - - if not is_clone_license: + if not source_device_id: exception_message = (_( - "SnapVx feature is not licensed on %(array)s.") + "Cannot find source device on %(array)s.") % {'array': array}) LOG.error(exception_message) raise exception.VolumeBackendAPIException( @@ -2088,6 +2098,7 @@ class PowerMaxCommon(object): self._do_sync_check( array, device_id, extra_specs, tgt_only) + @retry(retry_exc_tuple, interval=2, retries=2) def _do_sync_check( self, array, device_id, extra_specs, tgt_only=False): """Check if volume is part of a SnapVx sync process. @@ -2112,31 +2123,49 @@ class PowerMaxCommon(object): snap_vx_sessions.sort( key=lambda k: k['generation'], reverse=True) for session in snap_vx_sessions: - source = session['source_vol'] - snap_name = session['snap_name'] - targets = session['target_vol_list'] - generation = session['generation'] - # Break the replication relationship - for target in targets: - LOG.debug("Unlinking source from target. Source: " - "%(volume)s, Target: %(target)s, " - "generation: %(generation)s.", - {'volume': source, 'target': target[0], - 'generation': generation}) - self.provision.break_replication_relationship( - array, target[0], source, snap_name, - extra_specs, generation) - # The snapshot name will only have 'temp' (or EMC_SMI for - # legacy volumes) if it is a temporary volume. - # Only then is it a candidate for deletion. - if 'temp' in snap_name or 'EMC_SMI' in snap_name: - LOG.debug("Deleting temporary snapshot. Source: " - "%(volume)s, snap name: %(snap_name)s, " - "generation: %(generation)s.", - {'volume': source, 'snap_name': snap_name, - 'generation': generation}) - self.provision.delete_temp_volume_snap( - array, snap_name, source, generation) + try: + self._unlink_targets_and_delete_temp_snapvx( + session, array, extra_specs) + except Exception: + exception_message = _( + "Will retry one more time.") + + LOG.warning(exception_message) + raise exception.VolumeBackendAPIException( + exception_message) + + def _unlink_targets_and_delete_temp_snapvx( + self, session, array, extra_specs): + """unlink targets and delete the temporary snapvx + + :param session: the snapvx session + :param array: the array serial number + :param extra_specs: extra specifications + """ + source = session['source_vol'] + snap_name = session['snap_name'] + targets = session['target_vol_list'] + generation = session['generation'] + for target in targets: + LOG.debug("Unlinking source from target. Source: " + "%(volume)s, Target: %(target)s, " + "generation: %(generation)s.", + {'volume': source, 'target': target[0], + 'generation': generation}) + self.provision.break_replication_relationship( + array, target[0], source, snap_name, + extra_specs, generation) + # The snapshot name will only have 'temp' (or EMC_SMI for + # legacy volumes) if it is a temporary volume. + # Only then is it a candidate for deletion. + if 'temp' in snap_name or 'EMC_SMI' in snap_name: + LOG.debug("Deleting temporary snapshot. Source: " + "%(volume)s, snap name: %(snap_name)s, " + "generation: %(generation)s.", + {'volume': source, 'snap_name': snap_name, + 'generation': generation}) + self.provision.delete_temp_volume_snap( + array, snap_name, source, generation) def _get_target_source_device( self, array, device_id, tgt_only=False):