From c5f694e008feb2b1299273d12df1d395a747985c Mon Sep 17 00:00:00 2001 From: odonos12 Date: Mon, 17 Aug 2020 16:06:39 +0100 Subject: [PATCH] PowerMax Driver - Prevent unmanage with snapvx Current unmanage operations attempt to clean up any temporary snapshot links that exist on the volume. However these links are not cleaned unless they have reached their expiration timeout. Adding additional check to prevent unmanage from completing unless all temporary links have been cleared. Change-Id: I3409308eb0b37352b069ec250db6adb9e082a27d --- .../dell_emc/powermax/test_powermax_common.py | 12 +++++++++++- .../volume/drivers/dell_emc/powermax/common.py | 16 +++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) 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 569d3b5154c..d14ddac3d46 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 @@ -1903,10 +1903,12 @@ class PowerMaxCommonTest(test.TestCase): self.common.manage_existing_get_size, self.data.test_volume, external_ref) + @mock.patch.object(rest.PowerMaxRest, 'is_vol_in_rep_session', + return_value=(False, False, False)) @mock.patch.object(common.PowerMaxCommon, '_remove_vol_and_cleanup_replication') @mock.patch.object(common.PowerMaxCommon, '_clone_check') - def test_unmanage_success(self, mck_clone, mock_rm): + def test_unmanage_success(self, mck_clone, mock_rm, mck_sess): volume = self.data.test_volume with mock.patch.object(self.rest, 'rename_volume') as mock_rename: self.common.unmanage(volume) @@ -1931,6 +1933,14 @@ class PowerMaxCommonTest(test.TestCase): self.common.unmanage(volume) mock_rename.assert_not_called() + @mock.patch.object(rest.PowerMaxRest, 'is_vol_in_rep_session', + return_value=(True, True, False)) + @mock.patch.object(common.PowerMaxCommon, '_clone_check') + def test_unmanage_temp_snapshot_links(self, mck_clone, mck_sess): + volume = self.data.test_volume + self.assertRaises(exception.VolumeIsBusy, self.common.unmanage, + volume) + @mock.patch.object(common.PowerMaxCommon, '_slo_workload_migration') def test_retype(self, mock_migrate): device_id = self.data.device_id diff --git a/cinder/volume/drivers/dell_emc/powermax/common.py b/cinder/volume/drivers/dell_emc/powermax/common.py index ccf0f9393e3..d6872af019e 100644 --- a/cinder/volume/drivers/dell_emc/powermax/common.py +++ b/cinder/volume/drivers/dell_emc/powermax/common.py @@ -3330,13 +3330,27 @@ class PowerMaxCommon(object): {'name': volume_name, 'id': volume_id}) extra_specs = self._initial_setup(volume) device_id = self._find_device_on_array(volume, extra_specs) + array = extra_specs['array'] if device_id is None: LOG.error("Cannot find Volume: %(id)s for " "unmanage operation. Exiting...", {'id': volume_id}) else: # Check if volume is snap source - self._clone_check(extra_specs['array'], device_id, extra_specs) + self._clone_check(array, device_id, extra_specs) + snapvx_tgt, snapvx_src, __ = self.rest.is_vol_in_rep_session( + array, device_id) + if snapvx_src or snapvx_tgt: + msg = _( + 'Cannot unmanage volume %s with device id %s as it is ' + 'busy. Please either wait until all temporary snapshot ' + 'have expired or manually unlink and terminate any ' + 'remaining temporary sessions when they have been ' + 'fully copied to their targets. Volume is a snapvx ' + 'source: %s. Volume is a snapvx target: %s' % + (volume_id, device_id, snapvx_src, snapvx_tgt)) + LOG.error(msg) + raise exception.VolumeIsBusy(volume.id) # Remove volume from any openstack storage groups # and remove any replication self._remove_vol_and_cleanup_replication(