openstack/cinder
Code Issues Proposed changes

Merge "Dell PowerMax: Fix SnapVx unlink failure"

Browse Source
This commit is contained in:
Zuul 2024-03-24 16:41:51 +00:00 committed by Gerrit Code Review
commit 00e59c7d1d
9 changed files with 116 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_provision.py
View File

@ -171,7 +171,7 @@ class PowerMaxProvisionTest(test.TestCase):
mock_unlink.assert_called_once_with(
array, source_device_id, target_device_id,
snap_name, extra_specs, snap_id=self.data.snap_id,
list_volume_pairs=None, loop=True)
list_volume_pairs=None, loop=True, symforce=False)
@mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall',
new=test_utils.ZeroIntervalLoopingCall)
@ -184,7 +184,8 @@ class PowerMaxProvisionTest(test.TestCase):
mock_mod.assert_called_once_with(
self.data.array, self.data.device_id, self.data.device_id2,
self.data.snap_location['snap_name'], self.data.extra_specs,
snap_id=self.data.snap_id, list_volume_pairs=None, unlink=True)
snap_id=self.data.snap_id, list_volume_pairs=None, unlink=True,
symforce=False)
mock_mod.reset_mock()
self.provision._unlink_volume(
@ -194,7 +195,8 @@ class PowerMaxProvisionTest(test.TestCase):
mock_mod.assert_called_once_with(
self.data.array, self.data.device_id, self.data.device_id2,
self.data.snap_location['snap_name'], self.data.extra_specs,
snap_id=self.data.snap_id, list_volume_pairs=None, unlink=True)
snap_id=self.data.snap_id, list_volume_pairs=None, unlink=True,
symforce=False)
@mock.patch('oslo_service.loopingcall.FixedIntervalLoopingCall',
new=test_utils.ZeroIntervalLoopingCall)
@ -556,9 +558,12 @@ class PowerMaxProvisionTest(test.TestCase):
@mock.patch.object(
rest.PowerMaxRest, 'get_snap_linked_device_list',
side_effect=[[{'targetDevice': tpd.PowerMaxData.device_id2}],
[{'targetDevice': tpd.PowerMaxData.device_id2},
{'targetDevice': tpd.PowerMaxData.device_id3}]])
side_effect=[[{'targetDevice': tpd.PowerMaxData.device_id2,
'defined': False}],
[{'targetDevice': tpd.PowerMaxData.device_id2,
'defined': False},
{'targetDevice': tpd.PowerMaxData.device_id3,
'defined': False}]])
@mock.patch.object(provision.PowerMaxProvision, '_unlink_volume')
def test_delete_volume_snap_check_for_links(self, mock_unlink, mock_tgts):
self.provision.delete_volume_snap_check_for_links(
@ -568,7 +573,8 @@ class PowerMaxProvisionTest(test.TestCase):
self.data.array, "", "", self.data.test_snapshot_snap_name,
self.data.extra_specs, snap_id=self.data.snap_id,
list_volume_pairs=[
(self.data.device_id, tpd.PowerMaxData.device_id2)])
(self.data.device_id, tpd.PowerMaxData.device_id2)],
symforce=False)
mock_unlink.reset_mock()
self.provision.delete_volume_snap_check_for_links(
self.data.array, self.data.test_snapshot_snap_name,

18
cinder/volume/drivers/dell_emc/powermax/common.py
View File

@ -154,7 +154,12 @@ powermax_opts = [
cfg.IntOpt(utils.REST_API_READ_TIMEOUT,
default=30, min=1,
help='Use this value to specify read '
'timeout value (in seconds) for rest call.')]
'timeout value (in seconds) for rest call.'),
cfg.BoolOpt(utils.SNAPVX_UNLINK_SYMFORCE,
default=False,
help='Enable SnapVx unlink symforce, which forces '
'the operation to execute when normally it is rejected.'),
]
CONF.register_opts(powermax_opts, group=configuration.SHARED_CONF_GROUP)
@ -195,11 +200,8 @@ class PowerMaxCommon(object):
self.rest = rest.PowerMaxRest()
self.utils = utils.PowerMaxUtils()
self.masking = masking.PowerMaxMasking(prtcl, self.rest)
self.provision = provision.PowerMaxProvision(self.rest)
self.volume_metadata = volume_metadata.PowerMaxVolumeMetadata(
self.rest, version, LOG.isEnabledFor(logging.DEBUG))
self.migrate = migrate.PowerMaxMigrate(prtcl, self.rest)
# Configuration/Attributes
self.protocol = prtcl
@ -220,6 +222,14 @@ class PowerMaxCommon(object):
if active_backend_id == utils.PMAX_FAILOVER_START_ARRAY_PROMOTION:
self.promotion = True
# Init provision, masking and migrate instances
self.provision = provision.PowerMaxProvision(
self.rest, self.configuration)
self.masking = masking.PowerMaxMasking(
prtcl, self.rest, self.configuration)
self.migrate = migrate.PowerMaxMigrate(
prtcl, self.rest, self.configuration)
# Gather environment info
self._get_replication_info()
self._get_u4p_failover_info()

5
cinder/volume/drivers/dell_emc/powermax/masking.py
View File

@ -39,11 +39,12 @@ class PowerMaxMasking(object):
Masking code to dynamically create a masking view.
It supports VMAX 3, All Flash and PowerMax arrays.
"""
def __init__(self, prtcl, rest):
def __init__(self, prtcl, rest, configuration):
self.protocol = prtcl
self.utils = utils.PowerMaxUtils()
self.rest = rest
self.provision = provision.PowerMaxProvision(self.rest)
self.provision = provision.PowerMaxProvision(self.rest,
configuration)
def setup_masking_view(
self, serial_number, volume, masking_view_dict, extra_specs):

6
cinder/volume/drivers/dell_emc/powermax/migrate.py
View File

@ -34,11 +34,11 @@ class PowerMaxMigrate(object):
It supports VMAX 3 and VMAX All Flash and PowerMax arrays.
"""
def __init__(self, prtcl, rest):
def __init__(self, prtcl, rest, configuration):
self.rest = rest
self.utils = utils.PowerMaxUtils()
self.masking = masking.PowerMaxMasking(prtcl, self.rest)
self.provision = provision.PowerMaxProvision(self.rest)
self.masking = masking.PowerMaxMasking(prtcl, self.rest, configuration)
self.provision = provision.PowerMaxProvision(self.rest, configuration)
def do_migrate_if_candidate(
self, array, srp, device_id, volume, connector):

55
cinder/volume/drivers/dell_emc/powermax/provision.py
View File

@ -36,9 +36,11 @@ class PowerMaxProvision(object):
It supports VMAX 3, All Flash and PowerMax arrays.
"""
def __init__(self, rest):
def __init__(self, rest, configuration):
self.utils = utils.PowerMaxUtils()
self.rest = rest
self.snapvx_unlink_symforce = configuration.safe_get(
utils.SNAPVX_UNLINK_SYMFORCE) or False
def create_storage_group(
self, array, storagegroup_name, srp, slo, workload,
@ -183,6 +185,9 @@ class PowerMaxProvision(object):
{'delta': self.utils.get_time_delta(start_time,
time.time())})
def _is_symforce_enabled(self, defined):
return self.snapvx_unlink_symforce and (not defined)
def unlink_snapvx_tgt_volume(
self, array, target_device_id, source_device_id, snap_name,
extra_specs, snap_id, loop=True):
@ -197,20 +202,33 @@ class PowerMaxProvision(object):
:param loop: if looping call is required for handling retries
"""
@coordination.synchronized("emc-snapvx-{src_device_id}")
def do_unlink_volume(src_device_id):
def do_unlink_volume(src_device_id, symforce=False):
LOG.debug("Break snap vx link relationship between: %(src)s "
"and: %(tgt)s.",
{'src': src_device_id, 'tgt': target_device_id})
self._unlink_volume(array, src_device_id, target_device_id,
snap_name, extra_specs, snap_id=snap_id,
list_volume_pairs=None, loop=loop)
list_volume_pairs=None, loop=loop,
symforce=symforce)
do_unlink_volume(source_device_id)
# Get the link defined status
defined = True
linked_list = self.rest.get_snap_linked_device_list(
array, source_device_id, snap_name, snap_id)
for link in linked_list:
if target_device_id == link['targetDevice']:
if not link['defined']:
defined = False
break
LOG.debug("The link defined status: %s", defined)
do_unlink_volume(
source_device_id, symforce=self._is_symforce_enabled(defined))
def _unlink_volume(
self, array, source_device_id, target_device_id, snap_name,
extra_specs, snap_id=None, list_volume_pairs=None, loop=True):
extra_specs, snap_id=None, list_volume_pairs=None, loop=True,
symforce=False):
"""Unlink a target volume from its source volume.
:param array: the array serial number
@ -221,6 +239,7 @@ class PowerMaxProvision(object):
:param snap_id: the unique snap id of the SnapVX
:param list_volume_pairs: list of volume pairs, optional
:param loop: if looping call is required for handling retries
:param symforce: if symforce is enabled
:returns: return code
"""
def _unlink_vol():
@ -234,7 +253,8 @@ class PowerMaxProvision(object):
if not kwargs['modify_vol_success']:
self.rest.modify_volume_snap(
array, source_device_id, target_device_id, snap_name,
extra_specs, snap_id=snap_id, unlink=True,
extra_specs, snap_id=snap_id,
unlink=True, symforce=symforce,
list_volume_pairs=list_volume_pairs)
kwargs['modify_vol_success'] = True
except exception.VolumeBackendAPIException:
@ -250,7 +270,8 @@ class PowerMaxProvision(object):
if not loop:
self.rest.modify_volume_snap(
array, source_device_id, target_device_id, snap_name,
extra_specs, snap_id=snap_id, unlink=True,
extra_specs, snap_id=snap_id,
unlink=True, symforce=symforce,
list_volume_pairs=list_volume_pairs)
else:
kwargs = {'retries': 0,
@ -387,6 +408,7 @@ class PowerMaxProvision(object):
:param snap_id: the unique snap id of the SnapVX
"""
list_device_pairs = []
list_device_pairs_defined = True
if not isinstance(source_devices, list):
source_devices = [source_devices]
for source_device in source_devices:
@ -398,6 +420,8 @@ class PowerMaxProvision(object):
if len(linked_list) == 1:
target_device = linked_list[0]['targetDevice']
list_device_pairs.append((source_device, target_device))
if not linked_list[0]['defined']:
list_device_pairs_defined = False
else:
for link in linked_list:
# If a single source volume has multiple targets,
@ -405,11 +429,13 @@ class PowerMaxProvision(object):
target_device = link['targetDevice']
self._unlink_volume(
array, source_device, target_device, snap_name,
extra_specs, snap_id=snap_id)
extra_specs, snap_id=snap_id,
symforce=self._is_symforce_enabled(link['defined']))
if list_device_pairs:
self._unlink_volume(
array, "", "", snap_name, extra_specs, snap_id=snap_id,
list_volume_pairs=list_device_pairs)
list_volume_pairs=list_device_pairs,
symforce=self._is_symforce_enabled(list_device_pairs_defined))
if source_devices:
self.delete_volume_snap(
array, snap_name, source_devices, snap_id, restored=False)
@ -703,6 +729,14 @@ class PowerMaxProvision(object):
self.rest.modify_volume_snap(
array, None, None, snap_name, extra_specs, snap_id=snap_id,
link=True, list_volume_pairs=list_volume_pairs)
# Get the link defined status
defined = True
for src, dst in list_volume_pairs:
linked_list = self.rest.get_snap_linked_device_list(
array, src, snap_name, snap_id)
if not linked_list[0]['defined']:
defined = False
break
# Unlink the snapshot
LOG.debug("Unlinking Snap Vx snapshot: source group: %(srcGroup)s "
"targetGroup: %(tgtGroup)s.",
@ -710,7 +744,8 @@ class PowerMaxProvision(object):
'tgtGroup': target_group_name})
self._unlink_volume(
array, None, None, snap_name, extra_specs, snap_id=snap_id,
list_volume_pairs=list_volume_pairs)
list_volume_pairs=list_volume_pairs,
symforce=self._is_symforce_enabled(defined))
# Delete the snapshot if necessary
if delete_snapshot:
LOG.debug("Deleting Snap Vx snapshot: source group: %(srcGroup)s "

4
cinder/volume/drivers/dell_emc/powermax/rest.py
View File

@ -2197,7 +2197,7 @@ class PowerMaxRest(object):
def modify_volume_snap(self, array, source_id, target_id, snap_name,
extra_specs, snap_id=None, link=False, unlink=False,
rename=False, new_snap_name=None, restore=False,
list_volume_pairs=None, copy=False):
list_volume_pairs=None, copy=False, symforce=False):
"""Modify a snapvx snapshot
:param array: the array serial number
@ -2248,7 +2248,7 @@ class PowerMaxRest(object):
"copy": copy, "action": action,
"star": 'false', "force": force,
"exact": 'false', "remote": 'false',
"symforce": 'false'}
"symforce": str(symforce).lower()}
elif action == "Rename":
operation = 'Rename snapVx snapshot'
payload = {"deviceNameListSource": [{"name": source_id}],

1
cinder/volume/drivers/dell_emc/powermax/utils.py
View File

@ -180,6 +180,7 @@ LOAD_LOOKBACK = 'load_look_back'
LOAD_LOOKBACK_RT = 'load_look_back_real_time'
PORT_GROUP_LOAD_METRIC = 'port_group_load_metric'
PORT_LOAD_METRIC = 'port_load_metric'
SNAPVX_UNLINK_SYMFORCE = 'snapvx_unlink_symforce'
# One minute in milliseconds
ONE_MINUTE = 60000

3
doc/source/configuration/block-storage/drivers/dell-emc-powermax-driver.rst
View File

@ -3243,6 +3243,9 @@ options are detailed in the table below.
| | | | | load calculation, minimum of 1 |
| | | | | maximum of 60 (24 hours). |
+-----------------------------+----------------+-----------------+----------------------------------------+
| ``snapvx_unlink_symforce`` | ``True/False`` | ``False`` | | Enable/disable symforce |
| | | | | for SnapVx unlink. |
+-----------------------------+----------------+-----------------+----------------------------------------+
| ``port_group_load_metric`` | See below | ``PercentBusy`` | | Metric used for port group load |
| | | | | calculation. |
+-----------------------------+----------------+-----------------+----------------------------------------+

32
releasenotes/notes/bug-2045230-dell-powermax-fix-snapvx-unlink-e27d67d6b217d706.yaml Normal file
View File

@ -0,0 +1,32 @@
---
fixes:
- |
Dell PowerMax Driver `Bug #2045230
<https://bugs.launchpad.net/cinder/+bug/2045230>`_: Fixed
the issue that Dell PowerMax SnapVx link fails as the linked device is not
yet fully defined.
Previously, the below operations could fail if the linked device was not
yet fully defined at the time of the call.
Now, when ``snapvx_unlink_symforce`` is enabled, those operations are not
interrupted by not fully defined devices.
By default, ``snapvx_unlink_symforce`` is ``False``. Use extreme caution
with this option. If used when a link is copy in progress or when a restore
is restore in progress, this will cause an incomplete copy and data on the
copy target would not be usable.
Impacted operations:
* Clone a volume
* Create a volume from a snapshot
* Create volume snapshots
* Delete volume snapshots
* Revert volume to snapshot
* Create generic volume group from source
* Unmanage volumes
upgrade:
- |
Dell PowerMax Driver: introduced a new configuration option,
``snapvx_unlink_symforce``, to address Bug #2045230. See the Bug Fixes
section for details.