openstack/cinder
Code Issues Proposed changes

Storwize: self assign the SCSI lun id for volume attaching

Browse Source 
Currently, the SCSI lun id is assigned by storage. But Storwize may
assigned the same SCSI lun id for different volumes on different i/o
groups. The wrong volume may be discovered with the same wwpn and same
lun id.

This patch reverts the patch https://review.openstack.org/#/c/360394/,
Storwize volume driver calculates an unused SCSI lun id and sends the
SCSI lun id to storage.
To avoid the problem of patch 360394,  this patch retries 3 times to
map volume to host if there is CMMVC5879E error.

Change-Id: I3d3c61f913ff7ec8d35c8105cecc1a51accd21e7
Closes-Bug: 1757922
This commit is contained in:
yixuanzhang
2018-03-23 13:31:05 +08:00
committed by yixuan zhang
parent fb6f96e6c3
commit c42aba603b
2 changed files with 86 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
cinder/tests/unit/volume/drivers/ibm/test_storwize_svc.py
View File

@@ -4000,8 +4000,34 @@ class StorwizeSVCFcDriverTestCase(test.TestCase):
'wwpns': ['ff00000000000000', 'ff00000000000001'],
'initiator': 'iqn.1993-08.org.debian:01:eac5ccc1aaa'}
self.fc_driver.initialize_connection(volume_fc, connector)
self.fc_driver.initialize_connection(volume_fc, connector)
self.fc_driver.terminate_connection(volume_fc, connector)
with mock.patch.object(
storwize_svc_common.StorwizeSSH,
'mkvdiskhostmap') as mkvdiskhostmap:
ex = exception.VolumeBackendAPIException(data='CMMVC5879E')
mkvdiskhostmap.side_effect = [ex, ex, mock.MagicMock()]
self.fc_driver.initialize_connection(volume_fc, connector)
self.fc_driver.terminate_connection(volume_fc, connector)
mkvdiskhostmap.side_effect = ex
self.assertRaises(exception.VolumeBackendAPIException,
self.fc_driver.initialize_connection,
volume_fc,
connector)
ex1 = exception.VolumeBackendAPIException(data='CMMVC6071E')
mkvdiskhostmap.side_effect = ex1
self._set_flag('storwize_svc_multihostmap_enabled', False)
self.assertRaises(exception.VolumeDriverException,
self.fc_driver.initialize_connection,
volume_fc,
connector)
ex2 = exception.VolumeBackendAPIException(data='CMMVC5707E')
mkvdiskhostmap.side_effect = ex2
self.assertRaises(exception.VolumeBackendAPIException,
self.fc_driver.initialize_connection,
volume_fc,
connector)
def test_storwize_initialize_fc_connection_with_host_site(self):
connector = {'host': 'storwize-svc-host',
@@ -7893,33 +7919,19 @@ class StorwizeSSHTestCase(test.TestCase):
def test_mkvdiskhostmap(self):
# mkvdiskhostmap should not be returning anything
self.fake_driver.fake_storage._volumes_list['9999'] = {
'name': ' 9999', 'id': '0', 'uid': '0',
'IO_group_id': '0', 'IO_group_name': 'fakepool'}
self.fake_driver.fake_storage._hosts_list['HOST1'] = {
'name': 'HOST1', 'id': '0', 'host_name': 'HOST1'}
self.fake_driver.fake_storage._hosts_list['HOST2'] = {
'name': 'HOST2', 'id': '1', 'host_name': 'HOST2'}
self.fake_driver.fake_storage._hosts_list['HOST3'] = {
'name': 'HOST3', 'id': '2', 'host_name': 'HOST3'}
ret = self.storwize_ssh.mkvdiskhostmap('HOST1', '9999', '511', False)
self.assertEqual('511', ret)
ret = self.storwize_ssh.mkvdiskhostmap('HOST2', '9999', '512', True)
self.assertEqual('512', ret)
ret = self.storwize_ssh.mkvdiskhostmap('HOST3', '9999', None, True)
self.assertIsNotNone(ret)
with mock.patch.object(
storwize_svc_common.StorwizeSSH,
'run_ssh_check_created') as run_ssh_check_created:
run_ssh_check_created.return_value = None
ret = self.storwize_ssh.mkvdiskhostmap('HOST1', 9999, 511, False)
self.assertIsNone(ret)
ret = self.storwize_ssh.mkvdiskhostmap('HOST2', 9999, 511, True)
self.assertIsNone(ret)
ex = exception.VolumeBackendAPIException(data='CMMVC6071E')
run_ssh_check_created.side_effect = ex
self.assertRaises(exception.VolumeBackendAPIException,
self.storwize_ssh.mkvdiskhostmap,
'HOST3', '9999', 511, True)
'HOST3', 9999, 511, True)
@ddt.data((exception.VolumeBackendAPIException(data='CMMVC6372W'), None),
(exception.VolumeBackendAPIException(data='CMMVC6372W'),

95
cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py
View File

@@ -31,6 +31,7 @@ from oslo_utils import encodeutils
from oslo_utils import excutils
from oslo_utils import strutils
from oslo_utils import units
from retrying import retry
import six
from cinder import context
@@ -301,37 +302,12 @@ class StorwizeSSH(object):
If vdisk already mapped and multihostmap is True, use the force flag.
"""
ssh_cmd = ['svctask', 'mkvdiskhostmap', '-host', '"%s"' % host, vdisk]
if lun:
ssh_cmd.insert(ssh_cmd.index(vdisk), '-scsi')
ssh_cmd.insert(ssh_cmd.index(vdisk), lun)
ssh_cmd = ['svctask', 'mkvdiskhostmap', '-host', '"%s"' % host,
'-scsi', lun, '"%s"' % vdisk]
if multihostmap:
ssh_cmd.insert(ssh_cmd.index('mkvdiskhostmap') + 1, '-force')
try:
self.run_ssh_check_created(ssh_cmd)
result_lun = self.get_vdiskhostmapid(vdisk, host)
if result_lun is None or (lun and lun != result_lun):
msg = (_('mkvdiskhostmap error:\n command: %(cmd)s\n '
'lun: %(lun)s\n result_lun: %(result_lun)s') %
{'cmd': ssh_cmd,
'lun': lun,
'result_lun': result_lun})
LOG.error(msg)
raise exception.VolumeDriverException(message=msg)
return result_lun
except Exception as ex:
if (not multihostmap and hasattr(ex, 'message') and
'CMMVC6071E' in ex.message):
LOG.error('storwize_svc_multihostmap_enabled is set '
'to False, not allowing multi host mapping.')
raise exception.VolumeDriverException(
message=_('CMMVC6071E The VDisk-to-host mapping was not '
'created because the VDisk is already mapped '
'to a host.\n"'))
with excutils.save_and_reraise_exception():
LOG.error('Error mapping VDisk-to-host')
self.run_ssh_check_created(ssh_cmd)
def mkrcrelationship(self, master, aux, system, asyncmirror,
cyclingmode=False):
@@ -1177,25 +1153,62 @@ class StorwizeHelpers(object):
def delete_host(self, host_name):
self.ssh.rmhost(host_name)
def _get_unused_lun_id(self, host_name):
luns_used = []
result_lun = '-1'
resp = self.ssh.lshostvdiskmap(host_name)
for mapping_info in resp:
luns_used.append(int(mapping_info['SCSI_id']))
luns_used.sort()
result_lun = str(len(luns_used))
for index, n in enumerate(luns_used):
if n > index:
result_lun = str(index)
break
return result_lun
@cinder_utils.trace
def map_vol_to_host(self, volume_name, host_name, multihostmap):
"""Create a mapping between a volume to a host."""
LOG.debug('Enter: map_vol_to_host: volume %(volume_name)s to '
'host %(host_name)s.',
{'volume_name': volume_name, 'host_name': host_name})
# Check if this volume is already mapped to this host
result_lun = self.ssh.get_vdiskhostmapid(volume_name, host_name)
if result_lun is None:
result_lun = self.ssh.mkvdiskhostmap(host_name, volume_name, None,
multihostmap)
if result_lun:
LOG.debug('volume %(volume_name)s is already mapped to the host '
'%(host_name)s.',
{'volume_name': volume_name, 'host_name': host_name})
return int(result_lun)
LOG.debug('Leave: map_vol_to_host: LUN %(result_lun)s, volume '
'%(volume_name)s, host %(host_name)s.',
{'result_lun': result_lun,
'volume_name': volume_name,
'host_name': host_name})
return int(result_lun)
def _retry_on_exception(e):
if hasattr(e, 'msg') and 'CMMVC5879E' in e.msg:
return True
return False
@retry(retry_on_exception=_retry_on_exception,
stop_max_attempt_number=3,
wait_random_min=1,
wait_random_max=10)
def make_vdisk_host_map():
try:
result_lun = self._get_unused_lun_id(host_name)
self.ssh.mkvdiskhostmap(host_name, volume_name, result_lun,
multihostmap)
return int(result_lun)
except Exception as ex:
if (not multihostmap and hasattr(ex, 'msg') and
'CMMVC6071E' in ex.msg):
LOG.warning('storwize_svc_multihostmap_enabled is set '
'to False, not allowing multi host mapping.')
raise exception.VolumeDriverException(
message=_('CMMVC6071E The VDisk-to-host mapping was '
'not created because the VDisk is already '
'mapped to a host.'))
with excutils.save_and_reraise_exception():
LOG.error('Error mapping VDisk-to-host.')
return make_vdisk_host_map()
def unmap_vol_from_host(self, volume_name, host_name):
"""Unmap the volume and delete the host if it has no more mappings."""