Fixes Hyper-V iSCSI target login method
Hyper-V is trying to log in to portals/targets every time a volume is attached, even if the portal/target was already connected. This is not an issue with most volume backends, but it causes the attach to fail when using Storwize as a Cinder backend. This patch fixes the issue by checking if the target or portal is already connected, and simply perform an update in this case, instead of attempting to create a new connection. Also, a retry loop is introduced for the case when the iSCSI target login fails. As in this loop it is checked whether or not the operation completed successfully, this method does not rely anymore only on a sleep interval to ensure that the connection to the target has been made. Even so, on V2 volume utilities, the sleep interval is still needed to avoid having a new connection attempt before the last one finished. The method which gets the according mounted disk must raise an exception when the device number is -1, which signifies that the disk has not been mounted properly. Co-Authored-By: Jay Bryant <jsbryant@us.ibm.com> Closes-bug: #1221525 Change-Id: I92d2a586704fa3e857f46432bb571c81e86d183b
This commit is contained in:
Lucian Petrut
@@ -1720,9 +1720,11 @@ class VolumeOpsTestCase(HyperVAPIBaseTestCase):
|
||||
self.assertEqual(disk, 'disk_path')
|
||||
|
||||
def test_get_mounted_disk_from_lun_failure(self):
|
||||
self.flags(mounted_disk_query_retry_count=1, group='hyperv')
|
||||
|
||||
with mock.patch.object(self.volumeops._volutils,
|
||||
'get_device_number_for_target') as m_device_num:
|
||||
m_device_num.return_value = None
|
||||
m_device_num.side_effect = [None, -1]
|
||||
|
||||
block_device_info = db_fakes.get_fake_block_device_info(
|
||||
self._volume_target_portal, self._volume_id)
|
||||
@@ -1732,6 +1734,7 @@ class VolumeOpsTestCase(HyperVAPIBaseTestCase):
|
||||
target_lun = data['target_lun']
|
||||
target_iqn = data['target_iqn']
|
||||
|
||||
self.assertRaises(exception.NotFound,
|
||||
self.volumeops._get_mounted_disk_from_lun,
|
||||
target_iqn, target_lun)
|
||||
for attempt in xrange(1):
|
||||
self.assertRaises(exception.NotFound,
|
||||
self.volumeops._get_mounted_disk_from_lun,
|
||||
target_iqn, target_lun)
|
||||
|
||||
134
nova/tests/virt/hyperv/test_volumeutils.py
Normal file
134
nova/tests/virt/hyperv/test_volumeutils.py
Normal file
@@ -0,0 +1,134 @@
|
||||
# Copyright 2014 Cloudbase Solutions Srl
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import mock
|
||||
from oslo.config import cfg
|
||||
|
||||
from nova import test
|
||||
from nova.virt.hyperv import vmutils
|
||||
from nova.virt.hyperv import volumeutils
|
||||
|
||||
CONF = cfg.CONF
|
||||
CONF.import_opt('volume_attach_retry_count', 'nova.virt.hyperv.volumeops',
|
||||
'hyperv')
|
||||
|
||||
|
||||
class VolumeUtilsTestCase(test.NoDBTestCase):
|
||||
"""Unit tests for the Hyper-V VolumeUtils class."""
|
||||
|
||||
_FAKE_PORTAL_ADDR = '10.1.1.1'
|
||||
_FAKE_PORTAL_PORT = '3260'
|
||||
_FAKE_LUN = 0
|
||||
_FAKE_TARGET = 'iqn.2010-10.org.openstack:fake_target'
|
||||
|
||||
def setUp(self):
|
||||
super(VolumeUtilsTestCase, self).setUp()
|
||||
self._volutils = volumeutils.VolumeUtils()
|
||||
self._volutils._conn_wmi = mock.MagicMock()
|
||||
self.flags(volume_attach_retry_count=4, group='hyperv')
|
||||
self.flags(volume_attach_retry_interval=0, group='hyperv')
|
||||
|
||||
def _test_login_target_portal(self, portal_connected):
|
||||
fake_portal = '%s:%s' % (self._FAKE_PORTAL_ADDR,
|
||||
self._FAKE_PORTAL_PORT)
|
||||
|
||||
self._volutils.execute = mock.MagicMock()
|
||||
if portal_connected:
|
||||
exec_output = 'Address and Socket: %s %s' % (
|
||||
self._FAKE_PORTAL_ADDR, self._FAKE_PORTAL_PORT)
|
||||
else:
|
||||
exec_output = ''
|
||||
|
||||
self._volutils.execute.return_value = exec_output
|
||||
|
||||
self._volutils._login_target_portal(fake_portal)
|
||||
|
||||
call_list = self._volutils.execute.call_args_list
|
||||
all_call_args = [arg for call in call_list for arg in call[0]]
|
||||
|
||||
if portal_connected:
|
||||
self.assertIn('RefreshTargetPortal', all_call_args)
|
||||
else:
|
||||
self.assertIn('AddTargetPortal', all_call_args)
|
||||
|
||||
def test_login_connected_portal(self):
|
||||
self._test_login_target_portal(True)
|
||||
|
||||
def test_login_new_portal(self):
|
||||
self._test_login_target_portal(False)
|
||||
|
||||
def _test_login_target(self, target_connected, raise_exception=False):
|
||||
fake_portal = '%s:%s' % (self._FAKE_PORTAL_ADDR,
|
||||
self._FAKE_PORTAL_PORT)
|
||||
self._volutils.execute = mock.MagicMock()
|
||||
self._volutils._login_target_portal = mock.MagicMock()
|
||||
|
||||
if target_connected:
|
||||
self._volutils.execute.return_value = self._FAKE_TARGET
|
||||
elif raise_exception:
|
||||
self._volutils.execute.return_value = ''
|
||||
else:
|
||||
self._volutils.execute.side_effect = (
|
||||
['', '', '', self._FAKE_TARGET, ''])
|
||||
|
||||
if raise_exception:
|
||||
self.assertRaises(vmutils.HyperVException,
|
||||
self._volutils.login_storage_target,
|
||||
self._FAKE_LUN, self._FAKE_TARGET, fake_portal)
|
||||
else:
|
||||
self._volutils.login_storage_target(self._FAKE_LUN,
|
||||
self._FAKE_TARGET,
|
||||
fake_portal)
|
||||
|
||||
call_list = self._volutils.execute.call_args_list
|
||||
all_call_args = [arg for call in call_list for arg in call[0]]
|
||||
|
||||
if target_connected:
|
||||
self.assertNotIn('qlogintarget', all_call_args)
|
||||
else:
|
||||
self.assertIn('qlogintarget', all_call_args)
|
||||
|
||||
def test_login_connected_target(self):
|
||||
self._test_login_target(True)
|
||||
|
||||
def test_login_disconncted_target(self):
|
||||
self._test_login_target(False)
|
||||
|
||||
def test_login_target_exception(self):
|
||||
self._test_login_target(False, True)
|
||||
|
||||
def _test_execute_wrapper(self, raise_exception):
|
||||
fake_cmd = ('iscsicli.exe', 'ListTargetPortals')
|
||||
|
||||
if raise_exception:
|
||||
output = 'fake error'
|
||||
else:
|
||||
output = 'The operation completed successfully'
|
||||
|
||||
with mock.patch('nova.utils.execute') as fake_execute:
|
||||
fake_execute.return_value = (output, None)
|
||||
|
||||
if raise_exception:
|
||||
self.assertRaises(vmutils.HyperVException,
|
||||
self._volutils.execute,
|
||||
*fake_cmd)
|
||||
else:
|
||||
ret_val = self._volutils.execute(*fake_cmd)
|
||||
self.assertEqual(output, ret_val)
|
||||
|
||||
def test_execute_raise_exception(self):
|
||||
self._test_execute_wrapper(True)
|
||||
|
||||
def test_execute_exception(self):
|
||||
self._test_execute_wrapper(False)
|
||||
112
nova/tests/virt/hyperv/test_volumeutilsv2.py
Normal file
112
nova/tests/virt/hyperv/test_volumeutilsv2.py
Normal file
@@ -0,0 +1,112 @@
|
||||
# Copyright 2014 Cloudbase Solutions Srl
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import mock
|
||||
from oslo.config import cfg
|
||||
|
||||
from nova import test
|
||||
from nova.virt.hyperv import vmutils
|
||||
from nova.virt.hyperv import volumeutilsv2
|
||||
|
||||
CONF = cfg.CONF
|
||||
CONF.import_opt('volume_attach_retry_count', 'nova.virt.hyperv.volumeops',
|
||||
'hyperv')
|
||||
|
||||
|
||||
class VolumeUtilsV2TestCase(test.NoDBTestCase):
|
||||
"""Unit tests for the Hyper-V VolumeUtilsV2 class."""
|
||||
|
||||
_FAKE_PORTAL_ADDR = '10.1.1.1'
|
||||
_FAKE_PORTAL_PORT = '3260'
|
||||
_FAKE_LUN = 0
|
||||
_FAKE_TARGET = 'iqn.2010-10.org.openstack:fake_target'
|
||||
|
||||
def setUp(self):
|
||||
super(VolumeUtilsV2TestCase, self).setUp()
|
||||
self._volutilsv2 = volumeutilsv2.VolumeUtilsV2()
|
||||
self._volutilsv2._conn_storage = mock.MagicMock()
|
||||
self.flags(volume_attach_retry_count=4, group='hyperv')
|
||||
self.flags(volume_attach_retry_interval=0, group='hyperv')
|
||||
|
||||
def _test_login_target_portal(self, portal_connected):
|
||||
fake_portal = '%s:%s' % (self._FAKE_PORTAL_ADDR,
|
||||
self._FAKE_PORTAL_PORT)
|
||||
fake_portal_object = mock.MagicMock()
|
||||
_query = self._volutilsv2._conn_storage.query
|
||||
self._volutilsv2._conn_storage.MSFT_iSCSITargetPortal = (
|
||||
fake_portal_object)
|
||||
|
||||
if portal_connected:
|
||||
_query.return_value = [fake_portal_object]
|
||||
else:
|
||||
_query.return_value = None
|
||||
|
||||
self._volutilsv2._login_target_portal(fake_portal)
|
||||
|
||||
if portal_connected:
|
||||
fake_portal_object.Update.assert_called_once_with()
|
||||
else:
|
||||
fake_portal_object.New.assert_called_once_with(
|
||||
TargetPortalAddress=self._FAKE_PORTAL_ADDR,
|
||||
TargetPortalPortNumber=self._FAKE_PORTAL_PORT)
|
||||
|
||||
def test_login_connected_portal(self):
|
||||
self._test_login_target_portal(True)
|
||||
|
||||
def test_login_new_portal(self):
|
||||
self._test_login_target_portal(False)
|
||||
|
||||
def _test_login_target(self, target_connected, raise_exception=False):
|
||||
fake_portal = '%s:%s' % (self._FAKE_PORTAL_ADDR,
|
||||
self._FAKE_PORTAL_PORT)
|
||||
|
||||
fake_target_object = mock.MagicMock()
|
||||
|
||||
if target_connected:
|
||||
fake_target_object.IsConnected = True
|
||||
elif not raise_exception:
|
||||
type(fake_target_object).IsConnected = mock.PropertyMock(
|
||||
side_effect=[False, True])
|
||||
else:
|
||||
fake_target_object.IsConnected = False
|
||||
|
||||
_query = self._volutilsv2._conn_storage.query
|
||||
_query.return_value = [fake_target_object]
|
||||
|
||||
self._volutilsv2._conn_storage.MSFT_iSCSITarget = (
|
||||
fake_target_object)
|
||||
|
||||
if raise_exception:
|
||||
self.assertRaises(vmutils.HyperVException,
|
||||
self._volutilsv2.login_storage_target,
|
||||
self._FAKE_LUN, self._FAKE_TARGET, fake_portal)
|
||||
else:
|
||||
self._volutilsv2.login_storage_target(self._FAKE_LUN,
|
||||
self._FAKE_TARGET,
|
||||
fake_portal)
|
||||
|
||||
if target_connected:
|
||||
fake_target_object.Update.assert_called_with()
|
||||
else:
|
||||
fake_target_object.Connect.assert_called_once_with(
|
||||
IsPersistent=True, NodeAddress=self._FAKE_TARGET)
|
||||
|
||||
def test_login_connected_target(self):
|
||||
self._test_login_target(True)
|
||||
|
||||
def test_login_disconncted_target(self):
|
||||
self._test_login_target(False)
|
||||
|
||||
def test_login_target_exception(self):
|
||||
self._test_login_target(False, True)
|
||||
@@ -201,8 +201,8 @@ class VolumeOps(object):
|
||||
# be avoided by adding a retry.
|
||||
for i in xrange(CONF.hyperv.mounted_disk_query_retry_count):
|
||||
device_number = self._volutils.get_device_number_for_target(
|
||||
target_iqn, target_lun)
|
||||
if device_number is None:
|
||||
target_iqn, target_lun)
|
||||
if device_number in (None, -1):
|
||||
attempt = i + 1
|
||||
LOG.debug(_('Attempt %d to get device_number '
|
||||
'from get_device_number_for_target failed. '
|
||||
@@ -211,7 +211,7 @@ class VolumeOps(object):
|
||||
else:
|
||||
break
|
||||
|
||||
if device_number is None:
|
||||
if device_number in (None, -1):
|
||||
raise exception.NotFound(_('Unable to find a mounted disk for '
|
||||
'target_iqn: %s') % target_iqn)
|
||||
LOG.debug(_('Device number: %(device_number)s, '
|
||||
|
||||
@@ -23,15 +23,19 @@ documentation can be retrieved at:
|
||||
http://www.microsoft.com/en-us/download/details.aspx?id=34750
|
||||
"""
|
||||
|
||||
import re
|
||||
import time
|
||||
|
||||
from oslo.config import cfg
|
||||
|
||||
from nova.openstack.common.gettextutils import _
|
||||
from nova.openstack.common import log as logging
|
||||
from nova import utils
|
||||
from nova.virt.hyperv import basevolumeutils
|
||||
from nova.virt.hyperv import vmutils
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
CONF = cfg.CONF
|
||||
|
||||
|
||||
@@ -46,25 +50,60 @@ class VolumeUtils(basevolumeutils.BaseVolumeUtils):
|
||||
raise vmutils.HyperVException(_('An error has occurred when '
|
||||
'calling the iscsi initiator: %s')
|
||||
% stdout_value)
|
||||
return stdout_value
|
||||
|
||||
def login_storage_target(self, target_lun, target_iqn, target_portal):
|
||||
"""Add target portal, list targets and logins to the target."""
|
||||
def _login_target_portal(self, target_portal):
|
||||
(target_address,
|
||||
target_port) = utils.parse_server_string(target_portal)
|
||||
|
||||
#Adding target portal to iscsi initiator. Sending targets
|
||||
self.execute('iscsicli.exe', 'AddTargetPortal',
|
||||
target_address, target_port,
|
||||
'*', '*', '*', '*', '*', '*', '*', '*', '*', '*', '*',
|
||||
'*', '*')
|
||||
output = self.execute('iscsicli.exe', 'ListTargetPortals')
|
||||
pattern = r'Address and Socket *: (.*)'
|
||||
portals = [addr.split() for addr in re.findall(pattern, output)]
|
||||
LOG.debug("Ensuring connection to portal: %s" % target_portal)
|
||||
if [target_address, str(target_port)] in portals:
|
||||
self.execute('iscsicli.exe', 'RefreshTargetPortal',
|
||||
target_address, target_port)
|
||||
else:
|
||||
#Adding target portal to iscsi initiator. Sending targets
|
||||
self.execute('iscsicli.exe', 'AddTargetPortal',
|
||||
target_address, target_port,
|
||||
'*', '*', '*', '*', '*', '*', '*', '*', '*', '*', '*',
|
||||
'*', '*')
|
||||
|
||||
def login_storage_target(self, target_lun, target_iqn, target_portal):
|
||||
"""Ensure that the target is logged in."""
|
||||
|
||||
self._login_target_portal(target_portal)
|
||||
#Listing targets
|
||||
self.execute('iscsicli.exe', 'ListTargets')
|
||||
#Sending login
|
||||
self.execute('iscsicli.exe', 'qlogintarget', target_iqn)
|
||||
#Waiting the disk to be mounted.
|
||||
#TODO(pnavarro): Check for the operation to end instead of
|
||||
#relying on a timeout
|
||||
time.sleep(CONF.hyperv.volume_attach_retry_interval)
|
||||
|
||||
retry_count = CONF.hyperv.volume_attach_retry_count
|
||||
|
||||
# If the target is not connected, at least two iterations are needed:
|
||||
# one for performing the login and another one for checking if the
|
||||
# target was logged in successfully.
|
||||
if retry_count < 2:
|
||||
retry_count = 2
|
||||
|
||||
for attempt in xrange(retry_count):
|
||||
try:
|
||||
session_info = self.execute('iscsicli.exe', 'SessionList')
|
||||
if session_info.find(target_iqn) == -1:
|
||||
# Sending login
|
||||
self.execute('iscsicli.exe', 'qlogintarget', target_iqn)
|
||||
else:
|
||||
return
|
||||
except vmutils.HyperVException as exc:
|
||||
LOG.debug(_("Attempt %(attempt)d to connect to target "
|
||||
"%(target_iqn)s failed. Retrying. "
|
||||
"Exceptipn: %(exc)s ") %
|
||||
{'target_iqn': target_iqn,
|
||||
'exc': exc,
|
||||
'attempt': attempt})
|
||||
time.sleep(CONF.hyperv.volume_attach_retry_interval)
|
||||
|
||||
raise vmutils.HyperVException(_('Failed to login target %s') %
|
||||
target_iqn)
|
||||
|
||||
def logout_storage_target(self, target_iqn):
|
||||
"""Logs out storage target through its session id."""
|
||||
|
||||
@@ -26,9 +26,13 @@ if sys.platform == 'win32':
|
||||
|
||||
from oslo.config import cfg
|
||||
|
||||
from nova.openstack.common.gettextutils import _
|
||||
from nova.openstack.common import log as logging
|
||||
from nova import utils
|
||||
from nova.virt.hyperv import basevolumeutils
|
||||
from nova.virt.hyperv import vmutils
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
CONF = cfg.CONF
|
||||
|
||||
|
||||
@@ -40,23 +44,62 @@ class VolumeUtilsV2(basevolumeutils.BaseVolumeUtils):
|
||||
if sys.platform == 'win32':
|
||||
self._conn_storage = wmi.WMI(moniker=storage_namespace)
|
||||
|
||||
def login_storage_target(self, target_lun, target_iqn, target_portal):
|
||||
"""Add target portal, list targets and logins to the target."""
|
||||
def _login_target_portal(self, target_portal):
|
||||
(target_address,
|
||||
target_port) = utils.parse_server_string(target_portal)
|
||||
|
||||
#Adding target portal to iscsi initiator. Sending targets
|
||||
portal = self._conn_storage.MSFT_iSCSITargetPortal
|
||||
portal.New(TargetPortalAddress=target_address,
|
||||
TargetPortalPortNumber=target_port)
|
||||
#Connecting to the target
|
||||
target = self._conn_storage.MSFT_iSCSITarget
|
||||
target.Connect(NodeAddress=target_iqn,
|
||||
IsPersistent=True)
|
||||
#Waiting the disk to be mounted.
|
||||
#TODO(pnavarro): Check for the operation to end instead of
|
||||
#relying on a timeout
|
||||
time.sleep(CONF.hyperv.volume_attach_retry_interval)
|
||||
# Checking if the portal is already connected.
|
||||
portal = self._conn_storage.query("SELECT * FROM "
|
||||
"MSFT_iSCSITargetPortal "
|
||||
"WHERE TargetPortalAddress='%s' "
|
||||
"AND TargetPortalPortNumber='%s'"
|
||||
% (target_address, target_port))
|
||||
if portal:
|
||||
portal[0].Update()
|
||||
else:
|
||||
# Adding target portal to iscsi initiator. Sending targets
|
||||
portal = self._conn_storage.MSFT_iSCSITargetPortal
|
||||
portal.New(TargetPortalAddress=target_address,
|
||||
TargetPortalPortNumber=target_port)
|
||||
|
||||
def login_storage_target(self, target_lun, target_iqn, target_portal):
|
||||
"""Ensure that the target is logged in."""
|
||||
|
||||
self._login_target_portal(target_portal)
|
||||
|
||||
retry_count = CONF.hyperv.volume_attach_retry_count
|
||||
|
||||
# If the target is not connected, at least two iterations are needed:
|
||||
# one for performing the login and another one for checking if the
|
||||
# target was logged in successfully.
|
||||
if retry_count < 2:
|
||||
retry_count = 2
|
||||
|
||||
for attempt in xrange(retry_count):
|
||||
target = self._conn_storage.query("SELECT * FROM MSFT_iSCSITarget "
|
||||
"WHERE NodeAddress='%s' " %
|
||||
target_iqn)
|
||||
|
||||
if target and target[0].IsConnected:
|
||||
if attempt == 0:
|
||||
# The target was already connected but an update may be
|
||||
# required
|
||||
target[0].Update()
|
||||
return
|
||||
try:
|
||||
target = self._conn_storage.MSFT_iSCSITarget
|
||||
target.Connect(NodeAddress=target_iqn,
|
||||
IsPersistent=True)
|
||||
time.sleep(CONF.hyperv.volume_attach_retry_interval)
|
||||
except wmi.x_wmi as exc:
|
||||
LOG.debug("Attempt %(attempt)d to connect to target "
|
||||
"%(target_iqn)s failed. Retrying. "
|
||||
"WMI exception: %(exc)s " %
|
||||
{'target_iqn': target_iqn,
|
||||
'exc': exc,
|
||||
'attempt': attempt})
|
||||
raise vmutils.HyperVException(_('Failed to login target %s') %
|
||||
target_iqn)
|
||||
|
||||
def logout_storage_target(self, target_iqn):
|
||||
"""Logs out storage target through its session id."""
|
||||
|
||||
Reference in New Issue
Block a user