openstack/cinder
Code Issues Proposed changes

Do not conjecture volume-id from iscsi_name

Browse Source 
Now iscsi target driver assumes that volume name always start with
"volume-". In fact the name can be configured[1]. This change gets
the volume id from volume object directly.

Closes-Bug: #1597317

[1]: 9da9ebb345/cinder/objects/volume.py (L142)

Change-Id: Iaa366fbc4ddc0265255e5a4d2bb9d166a665856c
This commit is contained in:
Cao Shufeng 2016-07-04 06:41:10 -04:00
parent 9da9ebb345
commit f9d52850e7
3 changed files with 26 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
cinder/tests/unit/targets/test_base_iscsi_driver.py
View File

@ -34,7 +34,8 @@ class TestBaseISCSITargetDriver(tf.TargetDriverFixture):
self.target = fake.FakeTarget(root_helper=utils.get_root_helper(), self.target = fake.FakeTarget(root_helper=utils.get_root_helper(),
configuration=self.configuration) configuration=self.configuration)
self.target.db = mock.MagicMock( self.target.db = mock.MagicMock(
volume_get=lambda x, y: {'provider_auth': 'CHAP otzL 234Z'}) volume_get=mock.MagicMock(return_value={'provider_auth':
'CHAP otzL 234Z'}))
def test_abc_methods_not_present_fails(self): def test_abc_methods_not_present_fails(self):
configuration = conf.Configuration(cfg.StrOpt('iscsi_target_prefix', configuration = conf.Configuration(cfg.StrOpt('iscsi_target_prefix',
@ -161,4 +162,6 @@ class TestBaseISCSITargetDriver(tf.TargetDriverFixture):
ctxt = context.get_admin_context() ctxt = context.get_admin_context()
self.assertEqual(('otzL', '234Z'), self.assertEqual(('otzL', '234Z'),
self.target._get_target_chap_auth(ctxt, self.target._get_target_chap_auth(ctxt,
self.test_vol)) self.testvol))
self.target.db.volume_get.assert_called_once_with(
ctxt, self.testvol['id'])

15
cinder/volume/targets/iscsi.py
View File

@ -192,7 +192,7 @@ class ISCSITarget(driver.Target):
# Verify we haven't setup a CHAP creds file already # Verify we haven't setup a CHAP creds file already
# if DNE no big deal, we'll just create it # if DNE no big deal, we'll just create it
chap_auth = self._get_target_chap_auth(context, iscsi_name) chap_auth = self._get_target_chap_auth(context, volume)
if not chap_auth: if not chap_auth:
chap_auth = (vutils.generate_username(), chap_auth = (vutils.generate_username(),
vutils.generate_password()) vutils.generate_password())
@ -251,7 +251,7 @@ class ISCSITarget(driver.Target):
# Verify we haven't setup a CHAP creds file already # Verify we haven't setup a CHAP creds file already
# if DNE no big deal, we'll just create it # if DNE no big deal, we'll just create it
chap_auth = self._get_target_chap_auth(context, iscsi_name) chap_auth = self._get_target_chap_auth(context, volume)
if not chap_auth: if not chap_auth:
LOG.info(_LI("Skipping ensure_export. No iscsi_target " LOG.info(_LI("Skipping ensure_export. No iscsi_target "
"provision for volume: %s"), volume['id']) "provision for volume: %s"), volume['id'])
@ -321,17 +321,16 @@ class ISCSITarget(driver.Target):
if tid is None: if tid is None:
raise exception.NotFound() raise exception.NotFound()
def _get_target_chap_auth(self, context, iscsi_name): def _get_target_chap_auth(self, context, volume):
"""Get the current chap auth username and password.""" """Get the current chap auth username and password."""
try: try:
# 'iscsi_name': 'iqn.2010-10.org.openstack:volume-00000001' # Query DB to get latest state of volume
vol_id = iscsi_name.split(':volume-')[1] volume_info = self.db.volume_get(context, volume['id'])
volume_info = self.db.volume_get(context, vol_id)
# 'provider_auth': 'CHAP user_id password' # 'provider_auth': 'CHAP user_id password'
if volume_info['provider_auth']: if volume_info['provider_auth']:
return tuple(volume_info['provider_auth'].split(' ', 3)[1:]) return tuple(volume_info['provider_auth'].split(' ', 3)[1:])
except exception.NotFound: except exception.NotFound:
LOG.debug('Failed to get CHAP auth from DB for %s.', vol_id) LOG.debug('Failed to get CHAP auth from DB for %s.', volume['id'])
@abc.abstractmethod @abc.abstractmethod
def _get_target_and_lun(self, context, volume): def _get_target_and_lun(self, context, volume):
@ -392,7 +391,7 @@ class SanISCSITarget(ISCSITarget):
def _get_target_and_lun(self, context, volume): def _get_target_and_lun(self, context, volume):
pass pass
def _get_target_chap_auth(self, context, iscsi_name): def _get_target_chap_auth(self, context, volume):
pass pass
def create_iscsi_target(self, name, tid, lun, path, def create_iscsi_target(self, name, tid, lun, path,

27
cinder/volume/targets/scst.py
View File

@ -236,13 +236,22 @@ class SCSTAdm(iscsi.ISCSITarget):
return "%s:%s,%s %s %s" % (ip, self.configuration.iscsi_port, return "%s:%s,%s %s %s" % (ip, self.configuration.iscsi_port,
target, iqn, lun) target, iqn, lun)
def _get_iscsi_name(self, volume):
if self.target_name is None:
return "%s%s" % (self.configuration.iscsi_target_prefix,
volume['name'])
else:
return self.target_name
def _get_iscsi_target(self, context, vol_id): def _get_iscsi_target(self, context, vol_id):
# FIXME(jdg): Need to implement abc method # FIXME(jdg): Need to implement abc method
pass pass
def _get_target_chap_auth(self, context, iscsi_name): def _get_target_chap_auth(self, context, volume):
# FIXME(jdg): Need to implement abc method # FIXME(jdg): Need to implement abc method
iscsi_name = self._get_iscsi_name(volume)
if self._get_target(iscsi_name) is None: if self._get_target(iscsi_name) is None:
return None return None
(out, _err) = self.scst_execute('-list_tgt_attr', iscsi_name, (out, _err) = self.scst_execute('-list_tgt_attr', iscsi_name,
@ -262,16 +271,12 @@ class SCSTAdm(iscsi.ISCSITarget):
def ensure_export(self, context, volume, volume_path): def ensure_export(self, context, volume, volume_path):
iscsi_target, lun = self._get_target_and_lun(context, volume) iscsi_target, lun = self._get_target_and_lun(context, volume)
if self.target_name is None: iscsi_name = self._get_iscsi_name(volume)
iscsi_name = "%s%s" % (self.configuration.iscsi_target_prefix,
volume['name'])
else:
iscsi_name = self.target_name
if self.chap_username and self.chap_password: if self.chap_username and self.chap_password:
chap_auth = (self.chap_username, self.chap_password) chap_auth = (self.chap_username, self.chap_password)
else: else:
chap_auth = self._get_target_chap_auth(context, iscsi_name) chap_auth = self._get_target_chap_auth(context, volume)
self.create_iscsi_target(iscsi_name, volume['id'], iscsi_target, self.create_iscsi_target(iscsi_name, volume['id'], iscsi_target,
lun, volume_path, chap_auth) lun, volume_path, chap_auth)
@ -279,16 +284,12 @@ class SCSTAdm(iscsi.ISCSITarget):
def create_export(self, context, volume, volume_path): def create_export(self, context, volume, volume_path):
"""Creates an export for a logical volume.""" """Creates an export for a logical volume."""
iscsi_target, lun = self._get_target_and_lun(context, volume) iscsi_target, lun = self._get_target_and_lun(context, volume)
if self.target_name is None: iscsi_name = self._get_iscsi_name(volume)
iscsi_name = "%s%s" % (self.configuration.iscsi_target_prefix,
volume['name'])
else:
iscsi_name = self.target_name
if self.chap_username and self.chap_password: if self.chap_username and self.chap_password:
chap_auth = (self.chap_username, self.chap_password) chap_auth = (self.chap_username, self.chap_password)
else: else:
chap_auth = self._get_target_chap_auth(context, iscsi_name) chap_auth = self._get_target_chap_auth(context, volume)
if not chap_auth: if not chap_auth:
chap_auth = (vutils.generate_username(), chap_auth = (vutils.generate_username(),
vutils.generate_password()) vutils.generate_password())