openstack/cinder
Code Issues Proposed changes

PowerMax Driver - Improve error handling around deletes

Browse Source 
1. Bring storage group check within delete volume so that
it can be used in every place it is called and not just the
delete method.
2. Remove the reset identifier_name name as this makes the
device available for use straight away even if is not deleted.
3. Allow for None value in unlink_snapshot.
4. Add extra debugging information for device id.
5. Retry on getting connection info for volume.
6. Validate a masking view on an attach operation.
7. Reset identifier name on a rollback.
8. Retry a cleanup if snap id not available at the time.
9. Pylint fixes.
10. Retry on a failed delete with a cleanup snapvx in exception.

Change-Id: I0c0f3d2246b840326579748027b9cd15bf174ff8
This commit is contained in:
Helen Walsh 2021-06-14 15:32:18 +01:00
parent 5917307a10
commit e1f6de6f83
10 changed files with 496 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_data.py
View File

@ -665,6 +665,13 @@ class PowerMaxData(object):
{'host_lun_address': '0003'}]},
{}]
maskingview_no_lun = {
'maskingViewId': masking_view_name_f,
'portGroupId': port_group_name_f,
'storageGroupId': storagegroup_name_f,
'hostId': initiatorgroup_name_f,
'maskingViewConnection': []}
portgroup = [{'portGroupId': port_group_name_f,
'symmetrixPortKey': [
{'directorId': 'FA-1D',

2
cinder/tests/unit/volume/drivers/dell_emc/powermax/powermax_fake_objects.py
View File

@ -146,7 +146,7 @@ class FakeRequestsSession(object):
if '/private' in url:
return_object = self.data.private_vol_details
elif params:
if '1' in params.values():
if '1' in params.values() or 'volume_identifier' in params:
return_object = self.data.volume_list[0]
elif '2' in params.values():
return_object = self.data.volume_list[1]

99
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py
View File

@ -399,7 +399,7 @@ class PowerMaxCommonTest(test.TestCase):
extra_specs = self.common._initial_setup(test_volume)
self.assertRaises(exception.VolumeBackendAPIException,
self.common._delete_volume, test_volume)
mck_cleanup.assert_called_once_with(array, device_id, extra_specs)
mck_cleanup.assert_called_with(array, device_id, extra_specs)
mck_delete.assert_not_called()
@mock.patch.object(common.PowerMaxCommon, '_delete_from_srp')
@ -417,7 +417,7 @@ class PowerMaxCommonTest(test.TestCase):
extra_specs = self.common._initial_setup(test_volume)
self.assertRaises(exception.VolumeBackendAPIException,
self.common._delete_volume, test_volume)
mck_cleanup.assert_called_once_with(array, device_id, extra_specs)
mck_cleanup.assert_called_with(array, device_id, extra_specs)
mck_delete.assert_not_called()
@mock.patch.object(common.PowerMaxCommon, '_cleanup_device_snapvx')
@ -1941,12 +1941,15 @@ class PowerMaxCommonTest(test.TestCase):
clone_name, snap_name, self.data.extra_specs,
target_volume=clone_volume)
@mock.patch.object(rest.PowerMaxRest, 'get_storage_groups_from_volume',
return_value=[])
@mock.patch.object(rest.PowerMaxRest, 'get_snap_id',
return_value=tpd.PowerMaxData.snap_id)
@mock.patch.object(
masking.PowerMaxMasking,
'remove_and_reset_members')
def test_cleanup_target_sync_present(self, mock_remove, mock_snaps):
def test_cleanup_target_sync_present(
self, mock_remove, mock_snaps, mock_sgs):
array = self.data.array
clone_volume = self.data.test_clone_volume
source_device_id = self.data.device_id
@ -2974,6 +2977,7 @@ class PowerMaxCommonTest(test.TestCase):
model_update, __ = self.common._delete_group(group, volumes)
self.assertEqual(ref_model_update, model_update)
@mock.patch.object(common.PowerMaxCommon, '_delete_from_srp')
@mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members')
@mock.patch.object(common.PowerMaxCommon, '_cleanup_device_snapvx')
@mock.patch.object(common.PowerMaxCommon, '_get_members_of_volume_group',
@ -2983,7 +2987,7 @@ class PowerMaxCommonTest(test.TestCase):
@mock.patch.object(volume_utils, 'is_group_a_type', return_value=False)
def test_delete_group_snapshot_and_volume_cleanup(
self, mock_check, mck_get_snaps, mock_members, mock_cleanup,
mock_remove):
mock_remove, mock_del):
group = self.data.test_group_1
volumes = [fake_volume.fake_volume_obj(
context='cxt', provider_location=None)]
@ -3837,6 +3841,13 @@ class PowerMaxCommonTest(test.TestCase):
act_metadata = self.common.get_volume_metadata(array, device_id)
self.assertEqual(ref_metadata, act_metadata)
def test_get_volume_metadata_device_none(self):
ref_metadata = {}
array = self.data.array
device_id = None
act_metadata = self.common.get_volume_metadata(array, device_id)
self.assertEqual(ref_metadata, act_metadata)
@mock.patch.object(rest.PowerMaxRest, 'get_volume_snap_info',
return_value=tpd.PowerMaxData.priv_snap_response)
def test_get_snapshot_metadata(self, mck_snap):
@ -3925,6 +3936,23 @@ class PowerMaxCommonTest(test.TestCase):
model_update, existing_metadata, object_metadata)
self.assertEqual(ref_model_update, model_update)
def test_update_metadata_no_object_metadata(self):
model_update = {'provider_location': six.text_type(
self.data.provider_location)}
ref_model_update = (
{'provider_location': six.text_type(self.data.provider_location),
'metadata': {'user-meta-key-1': 'user-meta-value-1',
'user-meta-key-2': 'user-meta-value-2'}})
existing_metadata = {'user-meta-key-1': 'user-meta-value-1',
'user-meta-key-2': 'user-meta-value-2'}
object_metadata = {}
model_update = self.common.update_metadata(
model_update, existing_metadata, object_metadata)
self.assertEqual(ref_model_update, model_update)
def test_update_metadata_model_list_exception(self):
model_update = [{'provider_location': six.text_type(
self.data.provider_location)}]
@ -4562,3 +4590,66 @@ class PowerMaxCommonTest(test.TestCase):
self.assertTrue(rnr.rdf_pair_broken)
self.assertTrue(rnr.resume_original_sg)
self.assertFalse(rnr.is_partitioned)
@mock.patch.object(common.PowerMaxCommon, '_cleanup_device_snapvx')
@mock.patch.object(rest.PowerMaxRest, 'get_volume_snapshot_list',
return_value=[])
@mock.patch.object(rest.PowerMaxRest, 'find_snap_vx_sessions',
side_effect=[(None, tpd.PowerMaxData.snap_tgt_session),
(None, None)])
def test_cleanup_device_retry(self, mock_snapvx, mock_ss_list, mock_clean):
self.common._cleanup_device_retry(
self.data.array, self.data.device_id, self.data.extra_specs)
self.assertEqual(2, mock_clean.call_count)
@mock.patch.object(rest.PowerMaxRest, 'find_volume_device_id',
return_value=[tpd.PowerMaxData.device_id,
tpd.PowerMaxData.device_id2])
def test_get_device_id_from_identifier_list(self, mock_dev_id):
ret_dev = self.common._get_device_id_from_identifier(
self.data.array, 'vol', self.data.device_id)
self.assertEqual(self.data.device_id, ret_dev)
@mock.patch.object(rest.PowerMaxRest, 'find_volume_device_id',
return_value=tpd.PowerMaxData.device_id2)
def test_get_device_id_from_identifier_wrong(self, mock_dev_id):
ret_dev = self.common._get_device_id_from_identifier(
self.data.array, 'vol', self.data.device_id)
self.assertEqual(self.data.device_id2, ret_dev)
@mock.patch.object(rest.PowerMaxRest, 'find_volume_device_id',
return_value=tpd.PowerMaxData.device_id)
def test_get_device_id_from_identifier_same(self, mock_dev_id):
ret_dev = self.common._get_device_id_from_identifier(
self.data.array, 'vol', self.data.device_id)
self.assertIsNone(ret_dev)
@mock.patch.object(rest.PowerMaxRest, 'rename_volume')
@mock.patch.object(rest.PowerMaxRest, 'find_volume_identifier',
return_value='vol')
@mock.patch.object(rest.PowerMaxRest, 'find_volume_device_id',
return_value=tpd.PowerMaxData.device_id)
def test_reset_identifier_on_rollback_rename(
self, mock_dev, mock_ident, mock_rename):
self.common._reset_identifier_on_rollback(self.data.array, 'vol')
mock_rename.assert_called_once()
@mock.patch.object(rest.PowerMaxRest, 'rename_volume')
@mock.patch.object(rest.PowerMaxRest, 'find_volume_identifier',
return_value='diff_vol_name')
@mock.patch.object(rest.PowerMaxRest, 'find_volume_device_id',
return_value=tpd.PowerMaxData.device_id)
def test_reset_identifier_on_rollback_no_rename(
self, mock_dev, mock_ident, mock_rename):
self.common._reset_identifier_on_rollback(self.data.array, 'vol')
mock_rename.assert_not_called()
@mock.patch.object(common.PowerMaxCommon, '_cleanup_device_snapvx')
@mock.patch.object(
provision.PowerMaxProvision, 'delete_volume_from_srp',
side_effect=[exception.VolumeBackendAPIException, None])
def test_test_delete_from_srp(self, mock_del, mock_clean):
self.common._delete_from_srp(
self.data.array, 'vol_name', self.data.device_id,
self.data.extra_specs)
mock_clean.assert_called_once()

42
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_masking.py
View File

@ -96,6 +96,8 @@ class PowerMaxMaskingTest(test.TestCase):
self.maskingviewdict, self.extra_specs)
mock_get_or_create_mv.assert_called_once()
@mock.patch.object(masking.PowerMaxMasking, '_validate_attach',
return_value=True)
@mock.patch.object(masking.PowerMaxMasking,
'_check_adding_volume_to_storage_group')
@mock.patch.object(masking.PowerMaxMasking, '_move_vol_from_default_sg',
@ -108,7 +110,7 @@ class PowerMaxMaskingTest(test.TestCase):
Exception('Exception')])
def test_get_or_create_masking_view_and_map_lun(
self, mock_masking_view_element, mock_masking, mock_move,
mock_add_volume):
mock_add_volume, mock_validate):
rollback_dict = (
self.driver.masking.get_or_create_masking_view_and_map_lun(
self.data.array, self.data.test_volume,
@ -864,6 +866,9 @@ class PowerMaxMaskingTest(test.TestCase):
self.data.array, self.data.masking_view_name_i)
mock_delete_mv.assert_called_once()
@mock.patch.object(
rest.PowerMaxRest, 'get_storage_groups_from_volume',
return_value=list())
@mock.patch.object(masking.PowerMaxMasking,
'return_volume_to_volume_group')
@mock.patch.object(rest.PowerMaxRest, 'move_volume_between_storage_groups')
@ -871,7 +876,7 @@ class PowerMaxMaskingTest(test.TestCase):
'get_or_create_default_storage_group')
@mock.patch.object(masking.PowerMaxMasking, 'add_volume_to_storage_group')
def test_add_volume_to_default_storage_group(
self, mock_add_sg, mock_get_sg, mock_move, mock_return):
self, mock_add_sg, mock_get_sg, mock_move, mock_return, mock_sgs):
self.mask.add_volume_to_default_storage_group(
self.data.array, self.device_id, self.volume_name,
self.extra_specs)
@ -1372,3 +1377,36 @@ class PowerMaxMaskingTest(test.TestCase):
exception_message):
self.mask._check_director_and_port_status(
self.data.array, self.data.port_group_name_f)
@mock.patch.object(
rest.PowerMaxRest, 'get_storage_groups_from_volume',
return_value=([tpd.PowerMaxData.storagegroup_name_f]))
@mock.patch.object(
rest.PowerMaxRest, 'get_masking_views_from_storage_group',
return_value=[tpd.PowerMaxData.masking_view_name_f])
def test_validate_attach(self, mock_sgs, mock_mvs):
self.assertTrue(self.mask._validate_attach(
self.data.array, self.data.device_id,
self.data.storagegroup_name_f, self.data.masking_view_name_f))
@mock.patch.object(
rest.PowerMaxRest, 'get_storage_groups_from_volume',
return_value=([tpd.PowerMaxData.storagegroup_name_f]))
@mock.patch.object(
rest.PowerMaxRest, 'get_masking_views_from_storage_group',
return_value=[])
def test_validate_attach_no_mvs(self, mock_sgs, mock_mvs):
self.assertFalse(self.mask._validate_attach(
self.data.array, self.data.device_id,
self.data.storagegroup_name_f, self.data.masking_view_name_f))
@mock.patch.object(
rest.PowerMaxRest, 'get_storage_groups_from_volume',
return_value=([tpd.PowerMaxData.defaultstoragegroup_name]))
@mock.patch.object(
rest.PowerMaxRest, 'get_masking_views_from_storage_group',
return_value=[])
def test_validate_attach_incorrect_sg(self, mock_sgs, mock_mvs):
self.assertFalse(self.mask._validate_attach(
self.data.array, self.data.device_id,
self.data.storagegroup_name_f, self.data.masking_view_name_f))

15
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_replication.py
View File

@ -216,12 +216,21 @@ class PowerMaxReplicationTest(test.TestCase):
self.iscsi_common.initialize_connection,
self.data.test_volume, self.data.connector)
@mock.patch.object(masking.PowerMaxMasking, '_validate_attach',
return_value=True)
@mock.patch.object(
rest.PowerMaxRest, 'get_storage_groups_from_volume',
side_effect=[
[], [], [], [], [tpd.PowerMaxData.storagegroup_name_f], [],
[tpd.PowerMaxData.storagegroup_name_f],
[tpd.PowerMaxData.storagegroup_name_f]], )
@mock.patch.object(
masking.PowerMaxMasking, '_check_director_and_port_status')
@mock.patch.object(
masking.PowerMaxMasking, 'pre_multiattach',
return_value=tpd.PowerMaxData.masking_view_dict_multiattach)
def test_attach_metro_volume(self, mock_pre, mock_check):
def test_attach_metro_volume(
self, mock_pre, mock_check, mock_sgs, mock_validate):
rep_extra_specs = deepcopy(tpd.PowerMaxData.rep_extra_specs)
rep_extra_specs[utils.PORTGROUPNAME] = self.data.port_group_name_f
hostlunid, remote_port_group = self.common._attach_metro_volume(
@ -802,10 +811,12 @@ class PowerMaxReplicationTest(test.TestCase):
self.data.test_rep_group, self.data.rep_extra_specs)
mock_rm.assert_called_once()
@mock.patch.object(rest.PowerMaxRest, 'get_storage_groups_from_volume',
return_value=[])
@mock.patch.object(masking.PowerMaxMasking, 'remove_and_reset_members')
@mock.patch.object(masking.PowerMaxMasking,
'remove_volumes_from_storage_group')
def test_cleanup_group_replication(self, mock_rm, mock_rm_reset):
def test_cleanup_group_replication(self, mock_rm, mock_rm_reset, mock_sgs):
self.common._cleanup_group_replication(
self.data.array, self.data.test_vol_grp_name,
[self.data.device_id], self.extra_specs, self.data.rep_config_sync)

33
cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py
View File

@ -741,7 +741,9 @@ class PowerMaxRestTest(test.TestCase):
mck_modify.assert_called_once_with(array, device_id,
extend_vol_payload)
def test_legacy_delete_volume(self):
@mock.patch.object(rest.PowerMaxRest, 'get_storage_groups_from_volume',
return_value=[])
def test_legacy_delete_volume(self, mock_sgs):
device_id = self.data.device_id
vb_except = exception.VolumeBackendAPIException
with mock.patch.object(self.rest, 'delete_resource') as mock_delete, (
@ -755,21 +757,28 @@ class PowerMaxRestTest(test.TestCase):
mock_delete.assert_called_once_with(
self.data.array, 'sloprovisioning', 'volume', device_id)
def test_delete_volume(self):
@mock.patch.object(rest.PowerMaxRest, 'get_storage_groups_from_volume',
return_value=[])
def test_delete_volume(self, mock_sgs):
device_id = self.data.device_id
self.rest.ucode_major_level = utils.UCODE_5978
self.rest.ucode_minor_level = utils.UCODE_5978_HICKORY
with mock.patch.object(
self.rest, 'delete_resource') as mock_delete, (
mock.patch.object(
self.rest, '_modify_volume')) as mock_modify:
self.rest, 'delete_resource') as mock_delete:
self.rest.delete_volume(self.data.array, device_id)
mod_call_count = mock_modify.call_count
self.assertEqual(1, mod_call_count)
mock_delete.assert_called_once_with(
self.data.array, 'sloprovisioning', 'volume', device_id)
@mock.patch.object(rest.PowerMaxRest, 'get_storage_groups_from_volume',
return_value=['OS-SG'])
def test_delete_volume_in_sg(self, mock_sgs):
device_id = self.data.device_id
self.rest.ucode_major_level = utils.UCODE_5978
self.rest.ucode_minor_level = utils.UCODE_5978_HICKORY
self.assertRaises(
exception.VolumeBackendAPIException,
self.rest.delete_volume, self.data.array, device_id)
def test_rename_volume(self):
device_id = self.data.device_id
payload = {'editVolumeActionParam': {
@ -847,6 +856,14 @@ class PowerMaxRestTest(test.TestCase):
self.data.array, self.data.masking_view_name_f, device_id)
self.assertEqual(ref_lun_id, host_lun_id)
def test_find_mv_connections_for_vol_missing_host_lun_address(self):
with mock.patch.object(self.rest, 'get_resource',
return_value=self.data.maskingview_no_lun):
host_lun_id = self.rest.find_mv_connections_for_vol(
self.data.array, self.data.masking_view_name_f,
self.data.device_id)
self.assertIsNone(host_lun_id)
def test_find_mv_connections_for_vol_failed(self):
# no masking view info retrieved
device_id = self.data.volume_details[0]['volumeId']

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

@ -2057,15 +2057,32 @@ class PowerMaxCommon(object):
device_id = self._find_device_on_array(volume, extra_specs)
if device_id is None:
LOG.error("Volume %(name)s not found on the array. "
"No volume to delete.",
{'name': volume_name})
LOG.warning("Volume %(name)s not found on the array. "
"No volume to delete.",
{'name': volume_name})
return volume_name
array = extra_specs[utils.ARRAY]
if self.utils.is_replication_enabled(extra_specs):
self._validate_rdfg_status(array, extra_specs)
self._cleanup_device_retry(array, device_id, extra_specs)
# Remove from any storage groups and cleanup replication
self._remove_vol_and_cleanup_replication(
array, device_id, volume_name, extra_specs, volume)
self._delete_from_srp(
array, device_id, volume_name, extra_specs)
return volume_name
@retry(retry_exc_tuple, interval=1, retries=3)
def _cleanup_device_retry(self, array, device_id, extra_specs):
"""Cleanup snapvx on the device
:param array: the serial number of the array -- str
:param device_id: the device id -- str
:param extra_specs: extra specs -- dict
"""
# Check if the volume being deleted is a
# source or target for copy session
self._cleanup_device_snapvx(array, device_id, extra_specs)
@ -2083,24 +2100,12 @@ class PowerMaxCommon(object):
if snapvx_target_details:
source_device = snapvx_target_details.get('source_vol_id')
snapshot_name = snapvx_target_details.get('snap_name')
raise exception.VolumeBackendAPIException(_(
'Cannot delete device %s as it is currently a linked target '
'of snapshot %s. The source device of this link is %s. '
'Please try again once this snapshots is no longer '
'active.') % (device_id, snapshot_name, source_device))
# Remove from any storage groups and cleanup replication
self._remove_vol_and_cleanup_replication(
array, device_id, volume_name, extra_specs, volume)
# Check if volume is in any storage group
sg_list = self.rest.get_storage_groups_from_volume(array, device_id)
if sg_list:
LOG.error("Device %(device_id)s is in storage group(s) "
"%(sg_list)s prior to delete. Delete will fail.",
{'device_id': device_id, 'sg_list': sg_list})
self._delete_from_srp(
array, device_id, volume_name, extra_specs)
return volume_name
if snapshot_name:
raise exception.VolumeBackendAPIException(_(
'Cannot delete device %s as it is currently a linked '
'target of snapshot %s. The source device of this link '
'is %s. Please try again once this snapshot is no longer '
'active.') % (device_id, snapshot_name, source_device))
def _create_volume(self, volume, volume_name, volume_size, extra_specs):
"""Create a volume.
@ -2162,15 +2167,40 @@ class PowerMaxCommon(object):
array, volume, volume_name, volume_size, extra_specs,
storagegroup_name, rep_mode))
# Compare volume ID against identifier on array. Update if needed.
# This can occur in cases where multiple edits are occurring at once.
found_device_id = self.rest.find_volume_device_id(array, volume_name)
returning_device_id = volume_dict['device_id']
if found_device_id != returning_device_id:
volume_dict['device_id'] = found_device_id
device_id = self._get_device_id_from_identifier(
array, volume_name, volume_dict['device_id'])
if device_id:
volume_dict['device_id'] = device_id
return volume_dict, rep_update, rep_info_dict
def _get_device_id_from_identifier(
self, array, volume_name, orig_device_id):
"""Get the device(s) using the identifier name
:param array: the serial number of the array -- str
:param volume_name: the user supplied volume name -- str
:param orig_device_id: the original device id -- str
:returns: device id -- str
"""
# Compare volume ID against identifier on array. Update if needed.
# This can occur in cases where multiple edits are occurring at once.
dev_id_from_identifier = self.rest.find_volume_device_id(
array, volume_name)
if isinstance(dev_id_from_identifier, list):
if orig_device_id in dev_id_from_identifier:
return orig_device_id
else:
if dev_id_from_identifier != orig_device_id:
LOG.warning(
"The device id %(dev_ident)s associated with %(vol_name)s "
"is not the same as device %(dev_orig)s.",
{'dev_ident': dev_id_from_identifier,
'vol_name': volume_name,
'dev_orig': orig_device_id})
return dev_id_from_identifier
return None
@coordination.synchronized("emc-nonrdf-vol-{storagegroup_name}-{array}")
def _create_non_replicated_volume(
self, array, volume, volume_name, storagegroup_name, volume_size,
@ -2195,6 +2225,7 @@ class PowerMaxCommon(object):
return volume_dict
except Exception as e:
try:
self._reset_identifier_on_rollback(array, volume_name)
# Attempt cleanup of storage group post exception.
updated_devices = set(self.rest.get_volumes_in_storage_group(
array, storagegroup_name))
@ -2518,6 +2549,7 @@ class PowerMaxCommon(object):
LOG.info("The tag list %(tag_list)s has been verified.",
{'tag_list': array_tag_list})
@retry(retry_exc_tuple, interval=3, retries=3)
def _delete_from_srp(self, array, device_id, volume_name,
extra_specs):
"""Delete from srp.
@ -2534,11 +2566,17 @@ class PowerMaxCommon(object):
self.provision.delete_volume_from_srp(
array, device_id, volume_name)
except Exception as e:
error_message = (_("Failed to delete volume %(volume_name)s. "
"Exception received: %(e)s") %
{'volume_name': volume_name,
'e': six.text_type(e)})
error_message = (_(
"Failed to delete volume %(volume_name)s with device id "
"%(dev)s. Exception received: %(e)s.") %
{'volume_name': volume_name,
'dev': device_id,
'e': six.text_type(e)})
LOG.error(error_message)
LOG.warning("Attempting device cleanup after a failed delete of: "
"%(name)s. device_id: %(device_id)s.",
{'name': volume_name, 'device_id': device_id})
self._cleanup_device_snapvx(array, device_id, extra_specs)
raise exception.VolumeBackendAPIException(message=error_message)
def _remove_vol_and_cleanup_replication(
@ -2953,7 +2991,7 @@ class PowerMaxCommon(object):
return source_device_id
@retry(retry_exc_tuple, interval=1, retries=3)
@retry(retry_exc_tuple, interval=10, retries=3)
def _cleanup_device_snapvx(
self, array, device_id, extra_specs):
"""Perform any snapvx cleanup before creating clones or snapshots
@ -2990,17 +3028,15 @@ class PowerMaxCommon(object):
:param array: the array serial number
:param extra_specs: extra specifications
"""
try:
session_unlinked = self._unlink_snapshot(
session, array, extra_specs)
if session_unlinked:
self._delete_temp_snapshot(session, array)
except exception.VolumeBackendAPIException as e:
# Ignore and continue as snapshot has been unlinked
# successfully with incorrect status code returned
if ('404' and session['snap_name'] and
'does not exist' in six.text_type(e)):
pass
session_unlinked = self._unlink_snapshot(
session, array, extra_specs)
if session_unlinked:
self._delete_temp_snapshot(session, array)
else:
LOG.warning(
"Snap name %(snap)s is still linked. The delete of "
"the temporary snapshot has not occurred.",
{'snap': session.get('snap_name')})
def _unlink_snapshot(self, session, array, extra_specs):
"""Helper for unlinking temporary snapshot during cleanup.
@ -3013,10 +3049,19 @@ class PowerMaxCommon(object):
snap_name = session.get('snap_name')
source = session.get('source_vol_id')
snap_id = session.get('snapid')
if snap_id is None:
exception_message = (
_("Unable to get snapid from session %(session)s for source "
"device %(dev)s. Retrying...")
% {'dev': source, 'session': session})
# Warning only as there will be a retry
LOG.warning(exception_message)
raise exception.VolumeBackendAPIException(
message=exception_message)
snap_info = self.rest.get_volume_snap(
array, source, snap_name, snap_id)
is_linked = snap_info.get('linkedDevices')
is_linked = snap_info.get('linkedDevices') if snap_info else None
target, cm_enabled = None, False
if session.get('target_vol_id'):
@ -3252,7 +3297,7 @@ class PowerMaxCommon(object):
volume_identifier = None
# Check if volume is already cinder managed
if volume_details.get('volume_identifier'):
volume_identifier = volume_details['volume_identifier']
volume_identifier = volume_details.get('volume_identifier')
if volume_identifier.startswith(utils.VOLUME_ELEMENT_NAME_PREFIX):
raise exception.ManageExistingAlreadyManaged(
volume_ref=volume_id)
@ -3569,6 +3614,7 @@ class PowerMaxCommon(object):
"they can be managed into Cinder.")
return manageable_vols
volumes = volumes or list()
for device in volumes:
# Determine if volume is valid for management
if self.utils.is_volume_manageable(device):
@ -3677,6 +3723,7 @@ class PowerMaxCommon(object):
"Cinder.")
return manageable_snaps
volumes = volumes or list()
for device in volumes:
# Determine if volume is valid for management
manageable_snaps = self._is_snapshot_valid_for_management(
@ -4760,7 +4807,7 @@ class PowerMaxCommon(object):
if self.rest.is_next_gen_array(target_array_serial):
target_workload = 'NONE'
except IndexError:
LOG.error("Error parsing array, pool, SLO and workload.")
LOG.debug("Error parsing array, pool, SLO and workload.")
return false_ret
if self.promotion:
@ -6092,7 +6139,7 @@ class PowerMaxCommon(object):
# Get the volume group dict for getting the group name
volume_group = (self._find_volume_group(array, source_group))
if volume_group and volume_group.get('name'):
vol_grp_name = volume_group['name']
vol_grp_name = volume_group.get('name')
if vol_grp_name is None:
LOG.warning("Cannot find generic volume group %(grp_ss_id)s. "
"on array %(array)s",
@ -7107,6 +7154,8 @@ class PowerMaxCommon(object):
:param device_id: the device ID
:returns: dict -- volume metadata
"""
if device_id is None:
return dict()
vol_info = self.rest._get_private_volume(array, device_id)
vol_header = vol_info['volumeHeader']
array_model, __ = self.rest.get_array_model_info(array)
@ -7576,3 +7625,26 @@ class PowerMaxCommon(object):
management_sg_name, rdf_group_number, missing_volumes_str)
is_valid = False
return is_valid
def _reset_identifier_on_rollback(self, array, volume_name):
"""Reset the user supplied name on a rollback
:param array: the serial number -- str
:param volume_name: the volume name assigned -- str
"""
# Find volume based on identifier name
dev_id_from_identifier = self.rest.find_volume_device_id(
array, volume_name)
if dev_id_from_identifier and isinstance(
dev_id_from_identifier, str):
vol_identifier_name = self.rest.find_volume_identifier(
array, dev_id_from_identifier)
if vol_identifier_name and (
vol_identifier_name == volume_name):
LOG.warning(
"Attempting to reset name of %(vol_name)s on device "
"%(dev_ident)s on a create volume rollback operation.",
{'vol_name': volume_name,
'dev_ident': dev_id_from_identifier})
self.rest.rename_volume(
array, dev_id_from_identifier, None)

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

@ -90,12 +90,14 @@ class PowerMaxMasking(object):
error_message = self._get_or_create_masking_view(
serial_number, masking_view_dict,
default_sg_name, extra_specs)
LOG.debug(
"The masking view in the attach operation is "
"%(masking_name)s. The storage group "
"in the masking view is %(storage_name)s.",
{'masking_name': maskingview_name,
'storage_name': storagegroup_name})
# Check that the device is in the correct storage group
if not self._validate_attach(
serial_number, device_id, storagegroup_name,
maskingview_name):
error_message = ("The attach validation for device "
"%(dev)s was unsuccessful.")
raise exception.VolumeBackendAPIException(
message=error_message)
rollback_dict['portgroup_name'] = (
self.rest.get_element_from_masking_view(
serial_number, maskingview_name, portgroup=True))
@ -131,6 +133,71 @@ class PowerMaxMasking(object):
return rollback_dict
def _validate_attach(
self, serial_number, device_id, dest_sg_name, dest_mv_name):
"""Validate that the attach was successful.
:param serial_number: the array serial number
:param device_id: the device id
:param dest_sg_name: the correct storage group
:param dest_mv_name: the correct masking view
:returns: bool
"""
sg_list = self.rest.get_storage_groups_from_volume(
serial_number, device_id)
def _validate_masking_view():
mv_list = self.rest.get_masking_views_from_storage_group(
serial_number, dest_sg_name)
if not mv_list:
LOG.error(
"The masking view list of %(sg_name)s where device "
"%(dev)s exists is empty.",
{'sg_name': dest_sg_name,
'dev': device_id})
return False
if dest_mv_name.lower() in (
mv_name.lower() for mv_name in mv_list):
LOG.debug(
"The masking view in the attach operation is "
"%(masking_name)s. The storage group "
"in the masking view is %(storage_name)s. "
"The device id is %(dev)s.",
{'masking_name': dest_mv_name,
'storage_name': dest_sg_name,
'dev': device_id})
return True
else:
LOG.error(
"The masking view %(masking_name)s is not in "
"the masking view list %(mv_list)s. "
"%(sg_name)s is the storage group where device "
"%(dev)s exists.",
{'masking_name': dest_mv_name,
'mv_list': mv_list,
'sg_name': dest_sg_name,
'dev': device_id})
return False
if not sg_list:
LOG.error(
"Device %(dev)s does not exist in any storage group.",
{'dev': device_id})
return False
if dest_sg_name.lower() in (
sg_name.lower() for sg_name in sg_list):
return _validate_masking_view()
else:
LOG.error(
"The storage group %(sg_name)s is not in "
"the storage group list %(sg_list)s "
"where device %(dev)s exists.",
{'sg_name': dest_sg_name,
'sg_list': sg_list,
'dev': device_id})
return False
def _move_vol_from_default_sg(
self, serial_number, device_id, volume_name,
default_sg_name, dest_storagegroup, extra_specs,
@ -725,6 +792,13 @@ class PowerMaxMasking(object):
{'volume_name': volume_name,
'sg_name': storagegroup_name})
else:
storage_group_list = self.rest.get_storage_groups_from_volume(
serial_number, device_id)
if storage_group_list:
msg = self._check_add_volume_suitability(
serial_number, device_id, volume_name, storagegroup_name)
if msg:
return msg
try:
force = True if extra_specs.get(utils.IS_RE) else False
self.add_volume_to_storage_group(
@ -738,6 +812,42 @@ class PowerMaxMasking(object):
LOG.error(msg)
return msg
def _check_add_volume_suitability(
self, serial_number, device_id, volume_name, add_sg_name):
"""Check if possible to add a volume to a storage group
If a volume already belongs to a storage group that is associated
with FAST it is not possible to add that same volume to another
storage group which is also associated with FAST
:param serial_number: the array serial number
:param device_id: the device id
:param volume_name: the client supplied volume name
:param add_sg_name: storage group that the volume is to be added to
:returns: msg -- str or None
"""
storage_group = self.rest.get_storage_group(
serial_number, add_sg_name)
if storage_group and storage_group.get('slo') and (
storage_group.get('slo').lower() == 'none'):
return None
storage_group_list = self.rest.get_storage_groups_from_volume(
serial_number, device_id)
if storage_group_list:
msg = ("Volume %(vol)s with device id %(dev)s belongs "
"to storage group(s) %(sgs)s. Cannot add volume "
"to another storage group associated with FAST."
% {'vol': volume_name, 'dev': device_id,
'sgs': storage_group_list})
for storage_group_name in storage_group_list:
storage_group = self.rest.get_storage_group(
serial_number, storage_group_name)
if storage_group and storage_group.get('slo') and (
storage_group.get('slo').lower() != 'none'):
LOG.error(msg)
return msg
return None
def add_volume_to_storage_group(
self, serial_number, device_id, storagegroup_name,
volume_name, extra_specs, force=False):

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

@ -119,8 +119,9 @@ class PowerMaxProvision(object):
:param volume_name: the volume name
"""
start_time = time.time()
LOG.debug("Delete volume %(volume_name)s from srp.",
{'volume_name': volume_name})
LOG.debug("Delete volume %(volume_name)s with device id %(dev)s "
"from srp.", {'volume_name': volume_name,
'dev': device_id})
self.rest.delete_volume(array, device_id)
LOG.debug("Delete volume took: %(delta)s H:MM:SS.",
{'delta': self.utils.get_time_delta(

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

@ -1224,6 +1224,11 @@ class PowerMaxRest(object):
element_name = self.utils.get_volume_element_name(name_id)
if vol_identifier == element_name:
found_device_id = device_id
else:
LOG.error("We cannot verify that device %(dev)s was "
"created/managed by openstack by its "
"identifier name.", {'dev': device_id})
return found_device_id
def add_vol_to_sg(self, array, storagegroup_name, device_id, extra_specs,
@ -1412,6 +1417,9 @@ class PowerMaxRest(object):
:returns: volume dict
:raises: VolumeBackendAPIException
"""
if not device_id:
LOG.warning('No device id supplied to get_volume.')
return dict()
version = self.get_uni_version()[1]
volume_dict = self.get_resource(
array, SLOPROVISIONING, 'volume', resource_name=device_id,
@ -1530,6 +1538,9 @@ class PowerMaxRest(object):
:param device_id: the volume device id
:param new_name: the new name for the volume, can be None
"""
if not device_id:
LOG.warning('No device id supplied to rename operation.')
return
if new_name is not None:
vol_identifier_dict = {
"identifier_name": new_name,
@ -1547,17 +1558,26 @@ class PowerMaxRest(object):
:param array: the array serial number
:param device_id: volume device id
"""
# Check if volume is in any storage group
sg_list = self.get_storage_groups_from_volume(
array, device_id)
if sg_list:
exception_message = (_(
"Device %(device_id)s is in storage group(s) "
"%(sg_list)s prior to delete.")
% {'device_id': device_id, 'sg_list': sg_list})
LOG.error(exception_message)
raise exception.VolumeBackendAPIException(exception_message)
if ((self.ucode_major_level >= utils.UCODE_5978)
and (self.ucode_minor_level > utils.UCODE_5978_ELMSR)):
# Use Rapid TDEV Deallocation to delete after ELMSR
try:
# Rename volume, removing the OS-<cinderUUID>
self.rename_volume(array, device_id, None)
self.delete_resource(array, SLOPROVISIONING,
"volume", device_id)
except Exception as e:
LOG.warning('Delete volume failed with %(e)s.', {'e': e})
LOG.warning('Delete volume %(dev)s failed with %(e)s.',
{'dev': device_id, 'e': e})
raise
else:
# Pre-Foxtail, deallocation and delete are separate calls
@ -1569,12 +1589,14 @@ class PowerMaxRest(object):
self._modify_volume(array, device_id, payload)
pass
except Exception as e:
LOG.warning('Deallocate volume failed with %(e)s.'
'Attempting delete.', {'e': e})
LOG.warning('Deallocate volume %(dev)s failed with %(e)s.'
'Attempting delete.',
{'dev': device_id, 'e': e})
# Try to delete the volume if deallocate failed.
self.delete_resource(array, SLOPROVISIONING,
"volume", device_id)
@retry(retry_exc_tuple, interval=2, retries=3)
def find_mv_connections_for_vol(self, array, maskingview, device_id):
"""Find the host_lun_id for a volume in a masking view.
@ -1595,16 +1617,35 @@ class PowerMaxRest(object):
'for %(mv)s.', {'mv': maskingview})
else:
try:
host_lun_id = (
connection_info[
'maskingViewConnection'][0]['host_lun_address'])
host_lun_id = int(host_lun_id, 16)
masking_view_conn = connection_info.get(
'maskingViewConnection')
if masking_view_conn and isinstance(
masking_view_conn, list):
host_lun_id = masking_view_conn[0].get(
'host_lun_address')
if host_lun_id:
host_lun_id = int(host_lun_id, 16)
else:
exception_message = (
_('Unable to get host_lun_address for '
'device %(dev)s on masking view %(mv)s. '
'Retrying...')
% {'dev': device_id, 'mv': maskingview})
LOG.warning(exception_message)
raise exception.VolumeBackendAPIException(
message=exception_message)
except Exception as e:
LOG.error("Unable to retrieve connection information "
"for volume %(vol)s in masking view %(mv)s. "
"Exception received: %(e)s.",
{'vol': device_id, 'mv': maskingview,
'e': e})
exception_message = (
_("Unable to retrieve connection information "
"for volume %(vol)s in masking view %(mv)s. "
"Exception received: %(e)s. Retrying...")
% {'vol': device_id, 'mv': maskingview,
'e': e})
LOG.warning(exception_message)
raise exception.VolumeBackendAPIException(
message=exception_message)
LOG.debug("The hostlunid is %(hli)s for %(dev)s.",
{'hli': host_lun_id, 'dev': device_id})
return host_lun_id
def get_storage_groups_from_volume(self, array, device_id):
@ -1615,7 +1656,16 @@ class PowerMaxRest(object):
:returns: storagegroup_list
"""
sg_list = []
if not device_id:
return sg_list
vol = self.get_volume(array, device_id)
if vol and isinstance(vol, list):
LOG.warning(
"Device id %(dev_id)s has brought back "
"multiple volume objects.",
{'vol_name': device_id})
return sg_list
if vol and vol.get('storageGroupId'):
sg_list = vol['storageGroupId']
num_storage_groups = len(sg_list)
@ -1647,13 +1697,19 @@ class PowerMaxRest(object):
"""
device_id = None
params = {"volume_identifier": volume_name}
volume_list = self.get_volume_list(array, params)
if not volume_list:
LOG.debug("Cannot find record for volume %(volumeId)s.",
{'volumeId': volume_name})
device_list = self.get_volume_list(array, params)
if not device_list:
LOG.debug("Cannot find record for volume %(vol_name)s.",
{'vol_name': volume_name})
else:
device_id = volume_list[0]
LOG.debug("The device id list is %(dev_list)s for %(vol_name)s.",
{'dev_list': device_list,
'vol_name': volume_name})
device_id = device_list[0] if len(device_list) == 1 else (
device_list)
if isinstance(device_id, list):
LOG.warning("More than one devices returned for %(vol_name)s.",
{'vol_name': volume_name})
return device_id
def find_volume_identifier(self, array, device_id):
@ -1664,7 +1720,7 @@ class PowerMaxRest(object):
:returns: the volume identifier -- string
"""
vol = self.get_volume(array, device_id)
return vol['volume_identifier']
return vol.get('volume_identifier') if vol else None
def get_size_of_device_on_array(self, array, device_id):
"""Get the size of the volume from the array.