Skip to content

Commit

Permalink
LIO: Handle initiator IQNs as case insensitive
Browse files Browse the repository at this point in the history 
If there is a mismatch in case between the stored
initiator IQN in the LIO ACL vs. the IQN provided
in a later call, rtstool would fail to find the
existing ACL.

Compare IQNs in a case insensitive manner to ensure
they will always match as intended.  (Refer to RFC
3720 section 3.2.6.1 for details on this.)

Closes-Bug: #1522053

Change-Id: I6f535f3f4fbfcbbb49da30cffb08d17b3cac778a
(cherry picked from commit c3d8c0d)
  • Loading branch information
@eharney
eharney committed Dec 3, 2015
1 parent 6c9b7a4 commit 6686b89
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 10 deletions.
4 changes: 2 additions & 2 deletions cinder/cmd/rtstool.py
View file
Expand Up @@ -128,7 +128,7 @@ def add_initiator(target_iqn, initiator_iqn, userid, password):
tpg = next(target.tpgs) # get the first one
for acl in tpg.node_acls:
# See if this ACL configuration already exists
if acl.node_wwn == initiator_iqn:
if acl.node_wwn.lower() == initiator_iqn.lower():
# No further action required
return

Expand All @@ -143,7 +143,7 @@ def delete_initiator(target_iqn, initiator_iqn):
target = _lookup_target(target_iqn, initiator_iqn)
tpg = next(target.tpgs) # get the first one
for acl in tpg.node_acls:
if acl.node_wwn == initiator_iqn:
if acl.node_wwn.lower() == initiator_iqn.lower():
acl.delete()
return
raise RtstoolError(_('Could not find ACL %(acl)s in target %(target)s')
Expand Down
102 changes: 94 additions & 8 deletions cinder/tests/unit/test_cmd.py
View file
Expand Up @@ -816,6 +816,9 @@ def setUp(self):
sys.argv = ['cinder-rtstool']
CONF(sys.argv[1:], project='cinder', version=version.version_string())

self.INITIATOR_IQN = 'iqn.2015.12.com.example.openstack.i:UNIT1'
self.TARGET_IQN = 'iqn.2015.12.com.example.openstack.i:TARGET1'

def tearDown(self):
super(TestCinderRtstoolCmd, self).tearDown()

Expand Down Expand Up @@ -986,7 +989,7 @@ def test_add_initiator_rtslib_error(self, rtsroot):
self.assertRaises(rtslib_fb.utils.RTSLibError,
cinder_rtstool.add_initiator,
mock.sentinel.target_iqn,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)

Expand All @@ -997,7 +1000,7 @@ def test_add_initiator_rtstool_error(self, rtsroot):
self.assertRaises(cinder_rtstool.RtstoolError,
cinder_rtstool.add_initiator,
mock.sentinel.target_iqn,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)

Expand All @@ -1007,15 +1010,64 @@ def test_add_initiator_rtstool_error(self, rtsroot):
def test_add_initiator_acl_exists(self, rtsroot, node_acl, mapped_lun):
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': mock.sentinel.initiator_iqn}]
acl = mock.MagicMock(node_wwn=mock.sentinel.initiator_iqn)
[{'node_acls': self.INITIATOR_IQN}]
acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=self.TARGET_IQN)
rtsroot.return_value = mock.MagicMock(targets=[target])

cinder_rtstool.add_initiator(self.TARGET_IQN,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
self.assertFalse(node_acl.called)
self.assertFalse(mapped_lun.called)

@mock.patch.object(rtslib_fb, 'MappedLUN')
@mock.patch.object(rtslib_fb, 'NodeACL')
@mock.patch.object(rtslib_fb.root, 'RTSRoot')
def test_add_initiator_acl_exists_case_1(self,
rtsroot,
node_acl,
mapped_lun):
"""Ensure initiator iqns are handled in a case-insensitive manner."""
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': self.INITIATOR_IQN.lower()}]
acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
rtsroot.return_value = mock.MagicMock(targets=[target])

cinder_rtstool.add_initiator(target_iqn,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
self.assertFalse(node_acl.called)
self.assertFalse(mapped_lun.called)

@mock.patch.object(rtslib_fb, 'MappedLUN')
@mock.patch.object(rtslib_fb, 'NodeACL')
@mock.patch.object(rtslib_fb.root, 'RTSRoot')
def test_add_initiator_acl_exists_case_2(self,
rtsroot,
node_acl,
mapped_lun):
"""Ensure initiator iqns are handled in a case-insensitive manner."""
iqn_lower = self.INITIATOR_IQN.lower()
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': self.INITIATOR_IQN}]
acl = mock.MagicMock(node_wwn=iqn_lower)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
rtsroot.return_value = mock.MagicMock(targets=[target])

cinder_rtstool.add_initiator(target_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
self.assertFalse(node_acl.called)
Expand All @@ -1027,7 +1079,7 @@ def test_add_initiator_acl_exists(self, rtsroot, node_acl, mapped_lun):
def test_add_initiator(self, rtsroot, node_acl, mapped_lun):
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': mock.sentinel.initiator_iqn}]
[{'node_acls': self.INITIATOR_IQN}]
tpg = mock.MagicMock()
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
Expand All @@ -1038,11 +1090,11 @@ def test_add_initiator(self, rtsroot, node_acl, mapped_lun):
node_acl.return_value = acl_new

cinder_rtstool.add_initiator(target_iqn,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mock.sentinel.userid,
mock.sentinel.password)
node_acl.assert_called_once_with(tpg,
mock.sentinel.initiator_iqn,
self.INITIATOR_IQN,
mode='create')
mapped_lun.assert_called_once_with(acl_new, 0, tpg_lun=0)

Expand Down Expand Up @@ -1071,6 +1123,40 @@ def test_delete(self, rtsroot):
target.delete.assert_called_once_with()
storage_object.delete.assert_called_once_with()

@mock.patch.object(rtslib_fb, 'MappedLUN')
@mock.patch.object(rtslib_fb, 'NodeACL')
@mock.patch.object(rtslib_fb.root, 'RTSRoot')
def test_delete_initiator(self, rtsroot, node_acl, mapped_lun):
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': self.INITIATOR_IQN}]
acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
rtsroot.return_value = mock.MagicMock(targets=[target])

cinder_rtstool.delete_initiator(target_iqn,
self.INITIATOR_IQN)

@mock.patch.object(rtslib_fb, 'MappedLUN')
@mock.patch.object(rtslib_fb, 'NodeACL')
@mock.patch.object(rtslib_fb.root, 'RTSRoot')
def test_delete_initiator_case(self, rtsroot, node_acl, mapped_lun):
"""Ensure iqns are handled in a case-insensitive manner."""
initiator_iqn_lower = self.INITIATOR_IQN.lower()
target_iqn = mock.MagicMock()
target_iqn.tpgs.return_value = \
[{'node_acls': initiator_iqn_lower}]
acl = mock.MagicMock(node_wwn=self.INITIATOR_IQN)
tpg = mock.MagicMock(node_acls=[acl])
tpgs = iter([tpg])
target = mock.MagicMock(tpgs=tpgs, wwn=target_iqn)
rtsroot.return_value = mock.MagicMock(targets=[target])

cinder_rtstool.delete_initiator(target_iqn,
self.INITIATOR_IQN)

@mock.patch.object(cinder_rtstool, 'os', autospec=True)
@mock.patch.object(cinder_rtstool, 'rtslib_fb', autospec=True)
def test_save_with_filename(self, mock_rtslib, mock_os):
Expand Down

0 comments on commit 6686b89

Please sign in to comment.