openstack
/
deb-cinder
Code Issues Proposed changes

VMAX driver - Detaching volumes if part of two or more MVs 

When Live migration is used extensively there can be scenarios
where a regular attached volume can belong to two or more Masking
Views. Because of this, we did not remove the volume from the storage
group, which is not typical behaviour. In this fix we use a temporary
file to determine if a terminate_connection is a regular detach or a
part of the live migration process.

Change-Id: Ide38fa21d65859a5516c577a9983124d998a2e95
Closes-Bug: #1684595
(cherry picked from commit 9d2466bb29)
This commit is contained in:
Helen Walsh 2017-04-20 12:10:07 +01:00
parent 429193da22
commit 3d10a8663b
6 changed files with 194 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

118
cinder/tests/unit/volume/drivers/emc/test_emc_vmax.py
View File

@ -15,6 +15,7 @@
import os
import shutil
import sys
import tempfile
import unittest
from xml.dom import minidom
@ -3235,6 +3236,9 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
self.driver.delete_volume,
self.data.failed_delete_vol)
@mock.patch.object(
emc_vmax_utils.EMCVMAXUtils,
'insert_live_migration_record')
@mock.patch.object(
emc_vmax_common.EMCVMAXCommon,
'_is_same_host',
@ -3255,41 +3259,20 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
return_value={'volume_backend_name': 'ISCSINoFAST'})
def test_already_mapped_no_fast_success(
self, _mock_volume_type, mock_wrap_group, mock_wrap_device,
mock_is_same_host):
mock_is_same_host, mock_rec):
self.driver.common._get_correct_port_group = mock.Mock(
return_value=self.data.port_group)
self.driver.initialize_connection(self.data.test_volume,
self.data.connector)
@mock.patch.object(
emc_vmax_masking.EMCVMAXMasking,
'_check_adding_volume_to_storage_group',
return_value=None)
@mock.patch.object(
emc_vmax_utils.EMCVMAXUtils,
'find_storage_masking_group',
return_value=EMCVMAXCommonData.default_sg_instance_name)
@mock.patch.object(
emc_vmax_masking.EMCVMAXMasking,
'_wrap_get_storage_group_from_volume',
return_value=None)
@mock.patch.object(
volume_types,
'get_volume_type_extra_specs',
return_value={'volume_backend_name': 'ISCSINoFAST'})
def test_map_new_masking_view_no_fast_success(
self, _mock_volume_type, mock_wrap_group,
mock_storage_group, mock_add_volume):
self.driver.common._wrap_find_device_number = mock.Mock(
return_value=({}, False, {}))
self.driver.initialize_connection(self.data.test_volume,
self.data.connector)
@mock.patch.object(
emc_vmax_masking.EMCVMAXMasking,
'_get_port_group_from_masking_view',
return_value={'CreationClassName': 'CIM_TargetMaskingGroup',
'ElementName': 'OS-portgroup-PG'})
@mock.patch.object(
emc_vmax_utils.EMCVMAXUtils,
'insert_live_migration_record')
@mock.patch.object(
emc_vmax_common.EMCVMAXCommon,
'_get_port_group_from_source',
@ -3342,11 +3325,15 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
mock_same_host,
mock_check,
mock_sg_from_mv,
mocx_sg_from_src,
mock_pg_from_mv):
mock_pg_from_mv,
mock_file,
mock_pg_from_mv2):
self.driver.initialize_connection(self.data.test_volume,
self.data.connector)
@mock.patch.object(
emc_vmax_utils.EMCVMAXUtils,
'insert_live_migration_record')
@mock.patch.object(
emc_vmax_masking.EMCVMAXMasking,
'_get_initiator_group_from_masking_view',
@ -3369,7 +3356,7 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
return_value={'volume_backend_name': 'ISCSINoFAST'})
def test_map_existing_masking_view_no_fast_success(
self, _mock_volume_type, mock_wrap_group, mock_storage_group,
mock_initiator_group, mock_ig_from_mv):
mock_initiator_group, mock_ig_from_mv, mock_rec):
self.driver.initialize_connection(self.data.test_volume,
self.data.connector)
@ -4251,6 +4238,9 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase):
self.driver.delete_volume,
self.data.failed_delete_vol)
@mock.patch.object(
emc_vmax_utils.EMCVMAXUtils,
'insert_live_migration_record')
@mock.patch.object(
emc_vmax_common.EMCVMAXCommon,
'_is_same_host',
@ -4271,7 +4261,7 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase):
return_value={'volume_backend_name': 'ISCSIFAST'})
def test_already_mapped_fast_success(
self, _mock_volume_type, mock_wrap_group, mock_wrap_device,
mock_is_same_host):
mock_is_same_host, mock_rec):
self.driver.common._get_correct_port_group = mock.Mock(
return_value=self.data.port_group)
self.driver.initialize_connection(self.data.test_volume,
@ -4857,6 +4847,9 @@ class EMCVMAXFCDriverNoFastTestCase(test.TestCase):
self.driver.delete_volume,
self.data.failed_delete_vol)
@mock.patch.object(
emc_vmax_utils.EMCVMAXUtils,
'insert_live_migration_record')
@mock.patch.object(
emc_vmax_common.EMCVMAXCommon,
'_is_same_host',
@ -4871,7 +4864,8 @@ class EMCVMAXFCDriverNoFastTestCase(test.TestCase):
return_value={'volume_backend_name': 'FCNoFAST',
'FASTPOLICY': 'FC_GOLD1'})
def test_map_lookup_service_no_fast_success(
self, _mock_volume_type, mock_maskingview, mock_is_same_host):
self, _mock_volume_type, mock_maskingview, mock_is_same_host,
mock_rec):
self.data.test_volume['volume_name'] = "vmax-1234567"
common = self.driver.common
common.get_target_wwns_from_masking_view = mock.Mock(
@ -5458,6 +5452,9 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase):
self.driver.delete_volume,
self.data.failed_delete_vol)
@mock.patch.object(
emc_vmax_utils.EMCVMAXUtils,
'insert_live_migration_record')
@mock.patch.object(
emc_vmax_common.EMCVMAXCommon,
'_is_same_host',
@ -5472,7 +5469,7 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase):
return_value={'volume_backend_name': 'FCFAST',
'FASTPOLICY': 'FC_GOLD1'})
def test_map_fast_success(self, _mock_volume_type, mock_maskingview,
mock_is_same_host):
mock_is_same_host, mock_rec):
common = self.driver.common
common.get_target_wwns_list = mock.Mock(
return_value=EMCVMAXCommonData.target_wwns)
@ -6533,6 +6530,9 @@ class EMCV3DriverTestCase(test.TestCase):
self.data.test_ctxt, self.data.test_CG,
add_volumes, remove_volumes)
@mock.patch.object(
emc_vmax_utils.EMCVMAXUtils,
'insert_live_migration_record')
@mock.patch.object(
emc_vmax_common.EMCVMAXCommon,
'_is_same_host',
@ -6546,7 +6546,8 @@ class EMCV3DriverTestCase(test.TestCase):
'get_volume_type_extra_specs',
return_value={'volume_backend_name': 'V3_BE'})
def test_map_v3_success(
self, _mock_volume_type, mock_maskingview, mock_is_same_host):
self, _mock_volume_type, mock_maskingview, mock_is_same_host,
mock_rec):
common = self.driver.common
common.get_target_wwns_list = mock.Mock(
return_value=EMCVMAXCommonData.target_wwns)
@ -8546,6 +8547,59 @@ class EMCVMAXUtilsTest(test.TestCase):
conn, initiatorgroup)
self.assertIsNone(foundIg)
@mock.patch('builtins.open' if sys.version_info >= (3,)
else '__builtin__.open')
def test_insert_live_migration_record(self, mock_open):
volume = {'id': '12345678-87654321'}
tempdir = tempfile.mkdtemp()
emc_vmax_utils.LIVE_MIGRATION_FILE = (
tempdir + '/livemigrationarray')
lm_file_name = ("%(prefix)s-%(volid)s"
% {'prefix': emc_vmax_utils.LIVE_MIGRATION_FILE,
'volid': volume['id'][:8]})
self.driver.utils.insert_live_migration_record(volume)
mock_open.assert_called_once_with(lm_file_name, "w")
self.driver.utils.delete_live_migration_record(volume)
shutil.rmtree(tempdir)
def test_delete_live_migration_record(self):
volume = {'id': '12345678-87654321'}
tempdir = tempfile.mkdtemp()
utils.LIVE_MIGRATION_FILE = (
tempdir + '/livemigrationarray')
lm_file_name = ("%(prefix)s-%(volid)s"
% {'prefix': utils.LIVE_MIGRATION_FILE,
'volid': volume['id'][:8]})
m = mock.mock_open()
with mock.patch('{}.open'.format(__name__), m, create=True):
with open(lm_file_name, "w") as f:
f.write('live migration details')
self.driver.utils.insert_live_migration_record(volume)
self.driver.utils.delete_live_migration_record(volume)
m.assert_called_once_with(lm_file_name, "w")
shutil.rmtree(tempdir)
def test_get_live_migration_record(self):
volume = {'id': '12345678-87654321'}
tempdir = tempfile.mkdtemp()
emc_vmax_utils.LIVE_MIGRATION_FILE = (
tempdir + '/livemigrationarray')
lm_file_name = ("%(prefix)s-%(volid)s"
% {'prefix': emc_vmax_utils.LIVE_MIGRATION_FILE,
'volid': volume['id'][:8]})
self.driver.utils.insert_live_migration_record(volume)
record = self.driver.utils.get_live_migration_record(volume)
self.assertEqual(volume['id'], record[0])
os.remove(lm_file_name)
shutil.rmtree(tempdir)
def test_get_live_migration_file_name(self):
volume = {'id': '12345678-87654321'}
lm_live_migration = self.driver.utils.get_live_migration_file_name(
volume)
self.assertIn('/livemigrationarray-12345678', lm_live_migration)
self.assertIn('/tmp/', lm_live_migration)
class EMCVMAXCommonTest(test.TestCase):
def setUp(self):

28
cinder/volume/drivers/emc/emc_vmax_common.py
View File

@ -348,7 +348,12 @@ class EMCVMAXCommon(object):
vol_instance = self._find_lun(volume)
storage_system = vol_instance['SystemName']
if self._is_volume_multiple_masking_views(vol_instance):
livemigrationrecord = self.utils.get_live_migration_record(volume)
if livemigrationrecord:
self.utils.delete_live_migration_record(volume)
if livemigrationrecord and self._is_volume_multiple_masking_views(
vol_instance):
return
configservice = self.utils.find_controller_configuration_service(
@ -430,12 +435,14 @@ class EMCVMAXCommon(object):
"The device number is %(deviceNumber)s."),
{'volume': volumeName,
'deviceNumber': deviceNumber})
self.utils.insert_live_migration_record(volume)
# Special case, we still need to get the iscsi ip address.
portGroupName = (
self._get_correct_port_group(
deviceInfoDict, maskingViewDict['storageSystemName']))
else:
if isLiveMigration:
self.utils.insert_live_migration_record(volume)
maskingViewDict['storageGroupInstanceName'] = (
self._get_storage_group_from_source(sourceInfoDict))
maskingViewDict['portGroupInstanceName'] = (
@ -493,6 +500,9 @@ class EMCVMAXCommon(object):
(rollbackDict['isV3'] is not None)):
(self.masking._check_if_rollback_action_for_masking_required(
self.conn, rollbackDict))
livemigrationrecord = self.utils.get_live_migration_record(volume)
if livemigrationrecord:
self.utils.delete_live_migration_record(volume)
exception_message = (_("Error Attaching volume %(vol)s.")
% {'vol': volumeName})
raise exception.VolumeBackendAPIException(
@ -1636,14 +1646,16 @@ class EMCVMAXCommon(object):
"""
maskedvols = []
data = {}
isLiveMigration = False
source_data = {}
foundController = None
foundNumDeviceNumber = None
foundMaskingViewName = None
volumeName = volume['name']
volumeInstance = self._find_lun(volume)
storageSystemName = volumeInstance['SystemName']
isLiveMigration = False
source_data = {}
if not volumeInstance:
return data, isLiveMigration, source_data
unitnames = self.conn.ReferenceNames(
volumeInstance.path,
@ -1656,7 +1668,15 @@ class EMCVMAXCommon(object):
if index > -1:
unitinstance = self.conn.GetInstance(unitname,
LocalOnly=False)
numDeviceNumber = int(unitinstance['DeviceNumber'], 16)
if unitinstance['DeviceNumber']:
numDeviceNumber = int(unitinstance['DeviceNumber'], 16)
else:
LOG.debug(
"Device number not found for volume "
"%(volumeName)s %(volumeInstance)s.",
{'volumeName': volumeName,
'volumeInstance': volumeInstance.path})
break
foundNumDeviceNumber = numDeviceNumber
foundController = controller
controllerInstance = self.conn.GetInstance(controller,

3
cinder/volume/drivers/emc/emc_vmax_fc.py
View File

@ -72,9 +72,10 @@ class EMCVMAXFCDriver(driver.FibreChannelDriver):
- QoS support (blueprint vmax-qos)
2.4.1 - Pre-zoned port group fix (bug #456285)
2.4.2 - Rollback error on Live Migration (bug #1686174)
2.4.3 - Detaching volumes if part of two or more MVs (bug #1684595)
"""
VERSION = "2.4.2"
VERSION = "2.4.3"
# ThirdPartySystems wiki
CI_WIKI_NAME = "EMC_VMAX_CI"

3
cinder/volume/drivers/emc/emc_vmax_iscsi.py
View File

@ -78,9 +78,10 @@ class EMCVMAXISCSIDriver(driver.ISCSIDriver):
https://blueprints.launchpad.net/cinder/+spec/vmax-iscsi-multipath
2.4.1 - Pre-zoned port group fix (bug #456285)
2.4.2 - Rollback error on Live Migration (bug #1686174)
2.4.3 - Detaching volumes if part of two or more MVs (bug #1684595)
"""
VERSION = "2.4.2"
VERSION = "2.4.3"
# ThirdPartySystems wiki
CI_WIKI_NAME = "EMC_VMAX_CI"

17
cinder/volume/drivers/emc/emc_vmax_masking.py
View File

@ -2110,10 +2110,16 @@ class EMCVMAXMasking(object):
self._last_volume_delete_masking_view(
conn, controllerConfigService, mvInstanceName,
maskingViewName, extraSpecs)
self._last_volume_delete_initiator_group(
conn, controllerConfigService,
initiatorGroupInstanceName, extraSpecs, host)
initiatorGroupInstance = conn.GetInstance(initiatorGroupInstanceName)
if initiatorGroupInstance:
initiatorGroupName = initiatorGroupInstance['ElementName']
@coordination.synchronized('emc-ig-{initiatorGroupName}')
def inner_do_delete_initiator_group(initiatorGroupName):
self._last_volume_delete_initiator_group(
conn, controllerConfigService,
initiatorGroupInstanceName, extraSpecs, host)
inner_do_delete_initiator_group(initiatorGroupName)
if not isV3:
isTieringPolicySupported, tierPolicyServiceInstanceName = (
self._get_tiering_info(conn, storageSystemInstanceName,
@ -2671,8 +2677,8 @@ class EMCVMAXMasking(object):
LOG.debug("Deletion of initiator path %(hardwareIdPath)s "
"is successful.", {'hardwareIdPath': hardwareIdPath})
else:
LOG.warning(_LW("Deletion of initiator path %(hardwareIdPath)s "
"is failed."), {'hardwareIdPath': hardwareIdPath})
LOG.debug("Deletion of initiator path %(hardwareIdPath)s "
"failed.", {'hardwareIdPath': hardwareIdPath})
def _delete_initiators_from_initiator_group(self, conn,
controllerConfigService,
@ -2737,7 +2743,6 @@ class EMCVMAXMasking(object):
"OS-%(shortHostName)s-%(protocol)s-IG"
% {'shortHostName': host,
'protocol': protocol}))
if initiatorGroupName == defaultInitiatorGroupName:
maskingViewInstanceNames = (
self.get_masking_views_by_initiator_group(

70
cinder/volume/drivers/emc/emc_vmax_utils.py
View File

@ -15,11 +15,14 @@
import datetime
import hashlib
import os
import random
import re
import tempfile
from xml.dom import minidom
from oslo_log import log as logging
from oslo_serialization import jsonutils
from oslo_service import loopingcall
from oslo_utils import units
import six
@ -50,7 +53,7 @@ EMC_ROOT = 'root/emc'
CONCATENATED = 'concatenated'
CINDER_EMC_CONFIG_FILE_PREFIX = '/etc/cinder/cinder_emc_config_'
CINDER_EMC_CONFIG_FILE_POSTFIX = '.xml'
LIVE_MIGRATION_FILE = '/etc/cinder/livemigrationarray'
LIVE_MIGRATION_FILE = tempfile.gettempdir() + '/livemigrationarray'
ISCSI = 'iscsi'
FC = 'fc'
JOB_RETRIES = 60
@ -2720,3 +2723,68 @@ class EMCVMAXUtils(object):
"%(igName)s.",
{'igName': foundinitiatorGroupInstanceName})
return foundinitiatorGroupInstanceName
def insert_live_migration_record(self, volume):
"""Insert a record of live migration destination into a temporary file
:param volume: the volume dictionary
"""
lm_file_name = self.get_live_migration_file_name(volume)
live_migration_details = self.get_live_migration_record(volume)
if live_migration_details:
return
else:
live_migration_details = {volume['id']: [volume['id']]}
try:
with open(lm_file_name, "w") as f:
jsonutils.dump(live_migration_details, f)
except Exception:
exceptionMessage = (_(
"Error in processing live migration file."))
LOG.exception(exceptionMessage)
raise exception.VolumeBackendAPIException(
data=exceptionMessage)
def delete_live_migration_record(self, volume):
"""Delete record of live migration
Delete record of live migration destination from file and if
after deletion of record, delete file if empty.
:param volume: the volume dictionary
"""
lm_file_name = self.get_live_migration_file_name(volume)
live_migration_details = self.get_live_migration_record(volume)
if live_migration_details:
if volume['id'] in live_migration_details:
os.remove(lm_file_name)
def get_live_migration_record(self, volume):
"""get record of live migration destination from a temporary file
:param volume: the volume dictionary
:returns: returns a single record
"""
returned_record = None
lm_file_name = self.get_live_migration_file_name(volume)
if os.path.isfile(lm_file_name):
with open(lm_file_name, "rb") as f:
live_migration_details = jsonutils.load(f)
if volume['id'] in live_migration_details:
returned_record = live_migration_details[volume['id']]
else:
LOG.debug("%(Volume)s doesn't exist in live migration "
"record.",
{'Volume': volume['id']})
return returned_record
def get_live_migration_file_name(self, volume):
"""get name of temporary live migration file
:param volume: the volume dictionary
:returns: returns file name
"""
lm_file_name = ("%(prefix)s-%(volid)s"
% {'prefix': LIVE_MIGRATION_FILE,
'volid': volume['id'][:8]})
return lm_file_name