Merge "PowerMax Driver - Fix non-temporary snapshot delete" into stable/ussuri

This commit is contained in:
Zuul 2020-11-14 20:22:36 +00:00 committed by Gerrit Code Review
commit bd4f882fd1
5 changed files with 74 additions and 41 deletions

View File

@ -3122,19 +3122,6 @@ class PowerMaxCommonTest(test.TestCase):
port = self.common._get_unisphere_port()
self.assertEqual(ref_port, port)
@mock.patch.object(common.PowerMaxCommon,
'_do_sync_check')
def test_sync_check_no_source_device_on_array(self, mock_check):
with mock.patch.object(self.rest, 'get_volume',
side_effect=exception.VolumeBackendAPIException(
"404 00123 does not exist")):
array = self.data.array
device_id = self.data.device_id
extra_specs = self.data.extra_specs
self.common._sync_check(array, device_id, extra_specs,
source_device_id='00123')
mock_check.assert_not_called()
def test_sync_check(self):
array = self.data.array
device_id = self.data.device_id
@ -3143,13 +3130,13 @@ class PowerMaxCommonTest(test.TestCase):
with mock.patch.object(self.common, '_do_sync_check') as mck_sync:
self.common._sync_check(array, device_id, extra_specs, False,
self.data.device_id2)
mck_sync.assert_called_with(array, self.data.device_id2,
mck_sync.assert_called_with(array, self.data.device_id,
extra_specs, False)
mck_sync.reset_mock()
with mock.patch.object(self.common, '_get_target_source_device',
return_value=self.data.device_id3):
self.common._sync_check(array, device_id, extra_specs, True)
mck_sync.assert_called_with(array, self.data.device_id3,
mck_sync.assert_called_with(array, self.data.device_id,
extra_specs, True)
mck_sync.reset_mock()
self.common._sync_check(array, device_id, extra_specs)
@ -3172,30 +3159,64 @@ class PowerMaxCommonTest(test.TestCase):
@mock.patch.object(provision.PowerMaxProvision, 'delete_temp_volume_snap')
@mock.patch.object(provision.PowerMaxProvision, 'unlink_snapvx_tgt_volume')
def test_unlink_targets_and_delete_temp_snapvx(self, mck_break, mck_del):
def test_unlink_targets_and_delete_temp_snapvx_legacy(
self, mck_break, mck_del):
array = self.data.array
extra_specs = self.data.extra_specs
session = self.data.snap_tgt_session_cm_enabled
session = deepcopy(self.data.snap_tgt_session_cm_enabled)
legacy_snap_name = 'EMC_SMI'
session['snap_name'] = legacy_snap_name
source = session['source_vol_id']
generation = session['generation']
target = session['target_vol_id']
self.common._unlink_targets_and_delete_temp_snapvx(
session, array, extra_specs)
mck_break.assert_called_with(array, target, source, legacy_snap_name,
extra_specs, generation, True)
mck_del.assert_called_once_with(
array, legacy_snap_name, source, generation)
@mock.patch.object(provision.PowerMaxProvision, 'delete_temp_volume_snap')
@mock.patch.object(provision.PowerMaxProvision, 'unlink_snapvx_tgt_volume')
def test_unlink_targets_and_delete_temp_snapvx_is_temp(
self, mck_break, mck_del):
array = self.data.array
extra_specs = self.data.extra_specs
session = deepcopy(self.data.snap_tgt_session_cm_enabled)
snap_name = session['snap_name']
source = session['source_vol_id']
generation = session['generation']
target = session['target_vol_id']
self.common._unlink_targets_and_delete_temp_snapvx(
session, array, extra_specs)
mck_break.assert_called_with(array, target, source, snap_name,
extra_specs, generation, True)
mck_del.assert_called_once_with(array, snap_name, source, generation)
mck_break.reset_mock()
mck_del.reset_mock()
def test_unlink_targets_and_delete_temp_snapvx_no_snap_name_found(self):
array = self.data.array
extra_specs = self.data.extra_specs
session = dict()
self.assertRaises(exception.VolumeBackendAPIException,
self.common._unlink_targets_and_delete_temp_snapvx,
session, array, extra_specs)
session['copy_mode'] = False
session['expired'] = True
@mock.patch.object(provision.PowerMaxProvision, 'delete_temp_volume_snap')
@mock.patch.object(provision.PowerMaxProvision, 'unlink_snapvx_tgt_volume')
def test_unlink_targets_and_delete_temp_snapvx_not_legacy_not_temp(
self, mck_break, mck_del):
array = self.data.array
extra_specs = self.data.extra_specs
session = deepcopy(self.data.snap_tgt_session_cm_enabled)
session['snap_name'] = 'not_leg_or_tmp'
snap_name = session['snap_name']
source = session['source_vol_id']
generation = session['generation']
target = session['target_vol_id']
self.common._unlink_targets_and_delete_temp_snapvx(
session, array, extra_specs)
mck_break.assert_called_with(array, target, source, snap_name,
extra_specs, generation, False)
extra_specs, generation, True)
mck_del.assert_not_called()
@mock.patch.object(rest.PowerMaxRest, 'find_snap_vx_sessions',

View File

@ -2793,16 +2793,10 @@ class PowerMaxCommon(object):
if source_device_id:
@coordination.synchronized("emc-source-{src_device_id}")
def do_unlink_and_delete_snap(src_device_id):
# Check if source device exists on the array
try:
self.rest.get_volume(array, src_device_id)
except exception.VolumeBackendAPIException:
LOG.debug("Device %(device_id)s not found on array, no "
"sync check required.",
{'device_id': src_device_id})
return
LOG.debug("Locking on source device %(device_id)s.",
{'device_id': src_device_id})
self._do_sync_check(
array, src_device_id, extra_specs, tgt_only)
array, device_id, extra_specs, tgt_only)
do_unlink_and_delete_snap(source_device_id)
else:
@ -2843,10 +2837,16 @@ class PowerMaxCommon(object):
:param array: the array serial number
:param extra_specs: extra specifications
"""
snap_name = session['snap_name']
source = session['source_vol_id']
snap_name = session.get('snap_name')
if not snap_name:
msg = _("Snap_name not present in volume's snapvx session. "
"Unable to perform unlink and cleanup.")
LOG.error(msg)
raise exception.VolumeBackendAPIException(msg)
source = session.get('source_vol_id')
generation = session['generation']
expired = session['expired']
is_legacy = 'EMC_SMI' in snap_name
is_temp = utils.CLONE_SNAPSHOT_NAME in snap_name
target, cm_enabled = None, False
if session.get('target_vol_id'):
@ -2865,10 +2865,13 @@ class PowerMaxCommon(object):
# Candidates for deletion:
# 1. If legacy snapshot with 'EMC_SMI' in snapshot name
# 2. If snapVX snapshot with copy mode enabled
# 3. If snapVX snapshot with copy mode disabled and not expired
if ('EMC_SMI' in snap_name or cm_enabled or (
not cm_enabled and not expired)):
# 2. If snapVX snapshot is temporary
# If a target is present in the snap session it will be unlinked by
# the provision.unlink_snapvx_tgt_volume call above. This accounts
# for both CM enabled and disabled, as such by this point there will
# either be no target or it would have been unlinked, therefore
# delete if it is legacy or temp.
if is_legacy or is_temp:
LOG.debug(
"Deleting temporary snapshot. Source: %(vol)s, snap name: "
"%(name)s, generation: %(gen)s.", {

View File

@ -132,9 +132,10 @@ class PowerMaxFCDriver(san.SanDriver, driver.FibreChannelDriver):
4.2.8 - Force flag to add to rep enable storage groups (#1892057)
4.2.9 - Failover group volume update (#1888831)
4.2.10 - Replica RDFG suspend fix (#1892718)
4.2.11 - Fix non-temporary snapshot delete (#1887962)
"""
VERSION = "4.2.10"
VERSION = "4.2.11"
# ThirdPartySystems wiki
CI_WIKI_NAME = "EMC_VMAX_CI"

View File

@ -137,9 +137,10 @@ class PowerMaxISCSIDriver(san.SanISCSIDriver):
4.2.8 - Force flag to add to rep enable storage groups (#1892057)
4.2.9 - Failover group volume update (#1888831)
4.2.10 - Replica RDFG suspend fix (#1892718)
4.2.11 - Fix non-temporary snapshot delete (#1887962)
"""
VERSION = "4.2.10"
VERSION = "4.2.11"
# ThirdPartySystems wiki
CI_WIKI_NAME = "EMC_VMAX_CI"

View File

@ -0,0 +1,7 @@
---
fixes:
- |
`Bug #1887962 <https://bugs.launchpad.net/cinder/+bug/1887962>`_:
PowerMax driver fix to rectify incorrectly deleted non-temporary
snapshots when calling do_sync_check used in multiple operations due
to missing check for temporary snapshot name.