From ca97a7461f0f56835a0f2dceb9afaebac756f7c3 Mon Sep 17 00:00:00 2001 From: Tomoki Sekiyama Date: Thu, 11 Dec 2014 18:23:18 -0500 Subject: [PATCH] Enhance iSCSI multipath support Currently, nova-compute and brick support multipath for iSCSI volume data path. It depends on response to targets discovery of the main iSCSI portal, expecting multiple portal addresses to be contained. However, some arrays only respond to discovery with a single portal address, even if secondary portals are available. In this case, the connector cannot know secondary portals and corresponding iSCSI target IQN, so cannot establish multiple sessions for the target(s). To enable the connector to login to secondary portals, cinder should tell all the portal addresses and corresponding target iqns/luns. With this patch initialize_connection API will return connection_info with multiple portal addresses/iqns/luns when multipath=True is specified in the connector info. For example: {"driver_volume_type": "iscsi", "data": {"target_portals": ["10.0.2.15:3260", "10.0.3.15:3260"], "target_iqns": ["iqn.2010-10.org.example:target-01", "iqn.2010-10.org.example:target-02"], "target_luns": [1, 2], ...}} This also modify brick/iscsi/connector.py to make the parameter recognized and to login to every target. In addition, this adds an config option 'iscsi_secondary_ip_addresses' to specify a list of secondary portal addresses for LVM iSCSI Driver. Change-Id: I2a650c8ae70189b738937eaea7e5cd28f896257c Implements: blueprint iscsi-multipath-enhancement --- cinder/brick/initiator/connector.py | 170 +++++++++++++------ cinder/tests/brick/test_brick_connector.py | 182 ++++++++++++++++++++- cinder/tests/targets/test_tgt_driver.py | 8 + cinder/tests/test_coraid.py | 3 +- cinder/tests/test_utils.py | 3 +- cinder/tests/test_volume.py | 17 +- cinder/utils.py | 13 +- cinder/volume/driver.py | 65 ++++++-- cinder/volume/targets/iscsi.py | 27 ++- cinder/volume/targets/lio.py | 4 +- cinder/volume/targets/tgt.py | 17 +- etc/cinder/rootwrap.d/volume.filters | 1 + 12 files changed, 431 insertions(+), 79 deletions(-) diff --git a/cinder/brick/initiator/connector.py b/cinder/brick/initiator/connector.py index 56d6c91ba..ffed9c66c 100644 --- a/cinder/brick/initiator/connector.py +++ b/cinder/brick/initiator/connector.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import copy import os import socket import time @@ -36,8 +37,31 @@ synchronized = lockutils.synchronized_with_prefix('brick-') DEVICE_SCAN_ATTEMPTS_DEFAULT = 3 -def get_connector_properties(root_helper, my_ip): - """Get the connection properties for all protocols.""" +def _check_multipathd_running(root_helper, enforce_multipath): + try: + putils.execute('multipathd', 'show', 'status', + run_as_root=True, root_helper=root_helper) + except putils.ProcessExecutionError as err: + LOG.error(_LE('multipathd is not running: exit code %(err)s'), + {'err': err.exit_code}) + if enforce_multipath: + raise + return False + + return True + + +def get_connector_properties(root_helper, my_ip, multipath, enforce_multipath): + """Get the connection properties for all protocols. + + When the connector wants to use multipath, multipath=True should be + specified. If enforce_multipath=True is specified too, an exception is + thrown when multipathd is not running. Otherwise, it falls back to + multipath=False and only the first path shown up is used. + For the compatibility reason, even if multipath=False is specified, + some cinder storage drivers may export the target for multipath, which + can be found via sendtargets discovery. + """ iscsi = ISCSIConnector(root_helper=root_helper) fc = linuxfc.LinuxFibreChannel(root_helper=root_helper) @@ -54,7 +78,9 @@ def get_connector_properties(root_helper, my_ip): wwnns = fc.get_fc_wwnns() if wwnns: props['wwnns'] = wwnns - + props['multipath'] = (multipath and + _check_multipathd_running(root_helper, + enforce_multipath)) return props @@ -191,65 +217,97 @@ class ISCSIConnector(InitiatorConnector): super(ISCSIConnector, self).set_execute(execute) self._linuxscsi.set_execute(execute) + def _iterate_multiple_targets(self, connection_properties, ips_iqns_luns): + for ip, iqn, lun in ips_iqns_luns: + props = copy.deepcopy(connection_properties) + props['target_portal'] = ip + props['target_iqn'] = iqn + props['target_lun'] = lun + yield props + + def _multipath_targets(self, connection_properties): + return zip(connection_properties.get('target_portals', []), + connection_properties.get('target_iqns', []), + connection_properties.get('target_luns', [])) + + def _discover_iscsi_portals(self, connection_properties): + if all([key in connection_properties for key in ('target_portals', + 'target_iqns')]): + # Use targets specified by connection_properties + return zip(connection_properties['target_portals'], + connection_properties['target_iqns']) + + # Discover and return every available target + out = self._run_iscsiadm_bare(['-m', + 'discovery', + '-t', + 'sendtargets', + '-p', + connection_properties['target_portal']], + check_exit_code=[0, 255])[0] \ + or "" + + return self._get_target_portals_from_iscsiadm_output(out) + @synchronized('connect_volume') def connect_volume(self, connection_properties): """Attach the volume to instance_name. connection_properties for iSCSI must include: - target_portal - ip and optional port - target_iqn - iSCSI Qualified Name - target_lun - LUN id of the volume + target_portal(s) - ip and optional port + target_iqn(s) - iSCSI Qualified Name + target_lun(s) - LUN id of the volume + Note that plural keys may be used when use_multipath=True """ device_info = {'type': 'block'} if self.use_multipath: #multipath installed, discovering other targets if available - target_portal = connection_properties['target_portal'] - out = self._run_iscsiadm_bare(['-m', - 'discovery', - '-t', - 'sendtargets', - '-p', - target_portal], - check_exit_code=[0, 255])[0] \ - or "" - - for ip, iqn in self._get_target_portals_from_iscsiadm_output(out): - props = connection_properties.copy() + for ip, iqn in self._discover_iscsi_portals(connection_properties): + props = copy.deepcopy(connection_properties) props['target_portal'] = ip props['target_iqn'] = iqn self._connect_to_iscsi_portal(props) self._rescan_iscsi() + host_devices = self._get_device_path(connection_properties) else: self._connect_to_iscsi_portal(connection_properties) - - host_device = self._get_device_path(connection_properties) + host_devices = self._get_device_path(connection_properties) # The /dev/disk/by-path/... node is not always present immediately # TODO(justinsb): This retry-with-delay is a pattern, move to utils? tries = 0 - while not os.path.exists(host_device): + # Loop until at least 1 path becomes available + while all(map(lambda x: not os.path.exists(x), host_devices)): if tries >= self.device_scan_attempts: - raise exception.VolumeDeviceNotFound(device=host_device) + raise exception.VolumeDeviceNotFound(device=host_devices) - LOG.warn(_LW("ISCSI volume not yet found at: %(host_device)s. " + LOG.warn(_LW("ISCSI volume not yet found at: %(host_devices)s. " "Will rescan & retry. Try number: %(tries)s"), - {'host_device': host_device, + {'host_devices': host_devices, 'tries': tries}) # The rescan isn't documented as being necessary(?), but it helps - self._run_iscsiadm(connection_properties, ("--rescan",)) + if self.use_multipath: + self._rescan_iscsi() + else: + self._run_iscsiadm(connection_properties, ("--rescan",)) tries = tries + 1 - if not os.path.exists(host_device): + if all(map(lambda x: not os.path.exists(x), host_devices)): time.sleep(tries ** 2) + else: + break if tries != 0: - LOG.debug("Found iSCSI node %(host_device)s " + LOG.debug("Found iSCSI node %(host_devices)s " "(after %(tries)s rescans)", - {'host_device': host_device, 'tries': tries}) + {'host_devices': host_devices, 'tries': tries}) + + # Choose an accessible host device + host_device = next(dev for dev in host_devices if os.path.exists(dev)) if self.use_multipath: #we use the multipath device instead of the single path device @@ -266,9 +324,9 @@ class ISCSIConnector(InitiatorConnector): """Detach the volume from instance_name. connection_properties for iSCSI must include: - target_portal - IP and optional port - target_iqn - iSCSI Qualified Name - target_lun - LUN id of the volume + target_portal(s) - IP and optional port + target_iqn(s) - iSCSI Qualified Name + target_lun(s) - LUN id of the volume """ # Moved _rescan_iscsi and _rescan_multipath # from _disconnect_volume_multipath_iscsi to here. @@ -276,19 +334,43 @@ class ISCSIConnector(InitiatorConnector): # but before logging out, the removed devices under /dev/disk/by-path # will reappear after rescan. self._rescan_iscsi() - host_device = self._get_device_path(connection_properties) - multipath_device = None if self.use_multipath: self._rescan_multipath() - multipath_device = self._get_multipath_device_name(host_device) + host_device = multipath_device = None + host_devices = self._get_device_path(connection_properties) + # Choose an accessible host device + for dev in host_devices: + if os.path.exists(dev): + host_device = dev + multipath_device = self._get_multipath_device_name(dev) + if multipath_device: + break + if not host_device: + LOG.error(_LE("No accessible volume device: %(host_devices)s"), + {'host_devices': host_devices}) + raise exception.VolumeDeviceNotFound(device=host_devices) + if multipath_device: device_realpath = os.path.realpath(host_device) self._linuxscsi.remove_multipath_device(device_realpath) return self._disconnect_volume_multipath_iscsi( connection_properties, multipath_device) + # When multiple portals/iqns/luns are specified, we need to remove + # unused devices created by logging into other LUNs' session. + ips_iqns_luns = self._multipath_targets(connection_properties) + if not ips_iqns_luns: + ips_iqns_luns = [[connection_properties['target_portal'], + connection_properties['target_iqn'], + connection_properties.get('target_lun', 0)]] + for props in self._iterate_multiple_targets(connection_properties, + ips_iqns_luns): + self._disconnect_volume_iscsi(props) + + def _disconnect_volume_iscsi(self, connection_properties): # remove the device from the scsi subsystem # this eliminates any stale entries until logout + host_device = self._get_device_path(connection_properties)[0] dev_name = self._linuxscsi.get_name_from_path(host_device) if dev_name: self._linuxscsi.remove_scsi_device(dev_name) @@ -306,11 +388,16 @@ class ISCSIConnector(InitiatorConnector): self._disconnect_from_iscsi_portal(connection_properties) def _get_device_path(self, connection_properties): + multipath_targets = self._multipath_targets(connection_properties) + if multipath_targets: + return ["/dev/disk/by-path/ip-%s-iscsi-%s-lun-%s" % x for x in + multipath_targets] + path = ("/dev/disk/by-path/ip-%(portal)s-iscsi-%(iqn)s-lun-%(lun)s" % {'portal': connection_properties['target_portal'], 'iqn': connection_properties['target_iqn'], 'lun': connection_properties.get('target_lun', 0)}) - return path + return [path] def get_initiator(self): """Secure helper to read file as root.""" @@ -370,16 +457,7 @@ class ISCSIConnector(InitiatorConnector): # Do a discovery to find all targets. # Targets for multiple paths for the same multipath device # may not be the same. - out = self._run_iscsiadm_bare(['-m', - 'discovery', - '-t', - 'sendtargets', - '-p', - connection_properties['target_portal']], - check_exit_code=[0, 255])[0] \ - or "" - - ips_iqns = self._get_target_portals_from_iscsiadm_output(out) + ips_iqns = self._discover_iscsi_portals(connection_properties) if not devices: # disconnect if no other multipath devices @@ -499,7 +577,7 @@ class ISCSIConnector(InitiatorConnector): def _disconnect_mpath(self, connection_properties, ips_iqns): for ip, iqn in ips_iqns: - props = connection_properties.copy() + props = copy.deepcopy(connection_properties) props['target_portal'] = ip props['target_iqn'] = iqn self._disconnect_from_iscsi_portal(props) diff --git a/cinder/tests/brick/test_brick_connector.py b/cinder/tests/brick/test_brick_connector.py index bdb52e491..931a9ec95 100644 --- a/cinder/tests/brick/test_brick_connector.py +++ b/cinder/tests/brick/test_brick_connector.py @@ -13,21 +13,76 @@ # under the License. import os.path +import socket import string import tempfile import time +import mock from oslo_concurrency import processutils as putils +from oslo_config import cfg from cinder.brick import exception from cinder.brick.initiator import connector from cinder.brick.initiator import host_driver +from cinder.brick.initiator import linuxfc from cinder.i18n import _LE from cinder.openstack.common import log as logging from cinder.openstack.common import loopingcall from cinder import test LOG = logging.getLogger(__name__) +CONF = cfg.CONF + + +class ConnectorUtilsTestCase(test.TestCase): + + @mock.patch.object(socket, 'gethostname', return_value='fakehost') + @mock.patch.object(connector.ISCSIConnector, 'get_initiator', + return_value='fakeinitiator') + @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_wwpns', + return_value=None) + @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_wwnns', + return_value=None) + def _test_brick_get_connector_properties(self, multipath, + enforce_multipath, + multipath_result, + mock_wwnns, mock_wwpns, + mock_initiator, mock_gethostname): + props_actual = connector.get_connector_properties('sudo', + CONF.my_ip, + multipath, + enforce_multipath) + props = {'initiator': 'fakeinitiator', + 'host': 'fakehost', + 'ip': CONF.my_ip, + 'multipath': multipath_result} + self.assertEqual(props, props_actual) + + def test_brick_get_connector_properties(self): + self._test_brick_get_connector_properties(False, False, False) + + @mock.patch.object(putils, 'execute') + def test_brick_get_connector_properties_multipath(self, mock_execute): + self._test_brick_get_connector_properties(True, True, True) + mock_execute.assert_called_once_with('multipathd', 'show', 'status', + run_as_root=True, + root_helper='sudo') + + @mock.patch.object(putils, 'execute', + side_effect=putils.ProcessExecutionError) + def test_brick_get_connector_properties_fallback(self, mock_execute): + self._test_brick_get_connector_properties(True, False, False) + mock_execute.assert_called_once_with('multipathd', 'show', 'status', + run_as_root=True, + root_helper='sudo') + + @mock.patch.object(putils, 'execute', + side_effect=putils.ProcessExecutionError) + def test_brick_get_connector_properties_raise(self, mock_execute): + self.assertRaises(putils.ProcessExecutionError, + self._test_brick_get_connector_properties, + True, True, None) class ConnectorTestCase(test.TestCase): @@ -117,6 +172,8 @@ class ISCSIConnectorTestCase(ConnectorTestCase): super(ISCSIConnectorTestCase, self).setUp() self.connector = connector.ISCSIConnector( None, execute=self.fake_execute, use_multipath=False) + self.connector_with_multipath = connector.ISCSIConnector( + None, execute=self.fake_execute, use_multipath=True) self.stubs.Set(self.connector._linuxscsi, 'get_name_from_path', lambda x: "/dev/sdb") @@ -131,6 +188,17 @@ class ISCSIConnectorTestCase(ConnectorTestCase): } } + def iscsi_connection_multipath(self, volume, locations, iqns, luns): + return { + 'driver_volume_type': 'iscsi', + 'data': { + 'volume_id': volume['id'], + 'target_portals': locations, + 'target_iqns': iqns, + 'target_luns': luns, + } + } + def test_get_initiator(self): def initiator_no_file(*args, **kwargs): raise putils.ProcessExecutionError('No file') @@ -200,8 +268,6 @@ class ISCSIConnectorTestCase(ConnectorTestCase): vol = {'id': 1, 'name': name} connection_properties = self.iscsi_connection(vol, location, iqn) - self.connector_with_multipath =\ - connector.ISCSIConnector(None, use_multipath=True) self.stubs.Set(self.connector_with_multipath, '_run_iscsiadm_bare', lambda *args, **kwargs: "%s %s" % (location, iqn)) @@ -227,6 +293,116 @@ class ISCSIConnectorTestCase(ConnectorTestCase): 'type': 'block'} self.assertEqual(result, expected_result) + @mock.patch.object(os.path, 'exists', return_value=True) + @mock.patch.object(host_driver.HostDriver, 'get_all_block_devices') + @mock.patch.object(connector.ISCSIConnector, '_rescan_multipath') + @mock.patch.object(connector.ISCSIConnector, '_run_multipath') + @mock.patch.object(connector.ISCSIConnector, '_get_multipath_device_name') + @mock.patch.object(connector.ISCSIConnector, '_get_multipath_iqn') + def test_connect_volume_with_multiple_portals( + self, mock_get_iqn, mock_device_name, mock_run_multipath, + mock_rescan_multipath, mock_devices, mock_exists): + location1 = '10.0.2.15:3260' + location2 = '10.0.3.15:3260' + name1 = 'volume-00000001-1' + name2 = 'volume-00000001-2' + iqn1 = 'iqn.2010-10.org.openstack:%s' % name1 + iqn2 = 'iqn.2010-10.org.openstack:%s' % name2 + fake_multipath_dev = '/dev/mapper/fake-multipath-dev' + vol = {'id': 1, 'name': name1} + connection_properties = self.iscsi_connection_multipath( + vol, [location1, location2], [iqn1, iqn2], [1, 2]) + devs = ['/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location1, iqn1), + '/dev/disk/by-path/ip-%s-iscsi-%s-lun-2' % (location2, iqn2)] + mock_devices.return_value = devs + mock_device_name.return_value = fake_multipath_dev + mock_get_iqn.return_value = [iqn1, iqn2] + + result = self.connector_with_multipath.connect_volume( + connection_properties['data']) + expected_result = {'path': fake_multipath_dev, 'type': 'block'} + cmd_format = 'iscsiadm -m node -T %s -p %s --%s' + expected_commands = [cmd_format % (iqn1, location1, 'login'), + cmd_format % (iqn2, location2, 'login')] + self.assertEqual(expected_result, result) + for command in expected_commands: + self.assertIn(command, self.cmds) + mock_device_name.assert_called_once_with(devs[0]) + + self.cmds = [] + self.connector_with_multipath.disconnect_volume( + connection_properties['data'], result) + expected_commands = [cmd_format % (iqn1, location1, 'logout'), + cmd_format % (iqn2, location2, 'logout')] + for command in expected_commands: + self.assertIn(command, self.cmds) + + @mock.patch.object(os.path, 'exists') + @mock.patch.object(host_driver.HostDriver, 'get_all_block_devices') + @mock.patch.object(connector.ISCSIConnector, '_rescan_multipath') + @mock.patch.object(connector.ISCSIConnector, '_run_multipath') + @mock.patch.object(connector.ISCSIConnector, '_get_multipath_device_name') + @mock.patch.object(connector.ISCSIConnector, '_get_multipath_iqn') + @mock.patch.object(connector.ISCSIConnector, '_run_iscsiadm') + def test_connect_volume_with_multiple_portals_primary_error( + self, mock_iscsiadm, mock_get_iqn, mock_device_name, + mock_run_multipath, mock_rescan_multipath, mock_devices, + mock_exists): + location1 = '10.0.2.15:3260' + location2 = '10.0.3.15:3260' + name1 = 'volume-00000001-1' + name2 = 'volume-00000001-2' + iqn1 = 'iqn.2010-10.org.openstack:%s' % name1 + iqn2 = 'iqn.2010-10.org.openstack:%s' % name2 + fake_multipath_dev = '/dev/mapper/fake-multipath-dev' + vol = {'id': 1, 'name': name1} + connection_properties = self.iscsi_connection_multipath( + vol, [location1, location2], [iqn1, iqn2], [1, 2]) + dev1 = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location1, iqn1) + dev2 = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-2' % (location2, iqn2) + + def fake_run_iscsiadm(iscsi_properties, iscsi_command, **kwargs): + if iscsi_properties['target_portal'] == location1: + if iscsi_command == ('--login',): + raise putils.ProcessExecutionError(None, None, 21) + return mock.DEFAULT + + mock_exists.side_effect = lambda x: x != dev1 + mock_devices.return_value = [dev2] + mock_device_name.return_value = fake_multipath_dev + mock_get_iqn.return_value = [iqn2] + mock_iscsiadm.side_effect = fake_run_iscsiadm + + props = connection_properties['data'].copy() + result = self.connector_with_multipath.connect_volume( + connection_properties['data']) + + expected_result = {'path': fake_multipath_dev, 'type': 'block'} + self.assertEqual(expected_result, result) + mock_device_name.assert_called_once_with(dev2) + props['target_portal'] = location1 + props['target_iqn'] = iqn1 + mock_iscsiadm.assert_any_call(props, ('--login',), + check_exit_code=[0, 255]) + props['target_portal'] = location2 + props['target_iqn'] = iqn2 + mock_iscsiadm.assert_any_call(props, ('--login',), + check_exit_code=[0, 255]) + + mock_iscsiadm.reset_mock() + self.connector_with_multipath.disconnect_volume( + connection_properties['data'], result) + + props = connection_properties['data'].copy() + props['target_portal'] = location1 + props['target_iqn'] = iqn1 + mock_iscsiadm.assert_any_call(props, ('--logout',), + check_exit_code=[0, 21, 255]) + props['target_portal'] = location2 + props['target_iqn'] = iqn2 + mock_iscsiadm.assert_any_call(props, ('--logout',), + check_exit_code=[0, 21, 255]) + def test_connect_volume_with_not_found_device(self): self.stubs.Set(os.path, 'exists', lambda x: False) self.stubs.Set(time, 'sleep', lambda x: None) @@ -841,4 +1017,4 @@ class HuaweiStorHyperConnectorTestCase(ConnectorTestCase): ' -v volume-b2911673-863c-4380-a5f2-e1729eecfe3f'] LOG.debug("self.cmds = %s." % self.cmds) - LOG.debug("expected = %s." % expected_commands) \ No newline at end of file + LOG.debug("expected = %s." % expected_commands) diff --git a/cinder/tests/targets/test_tgt_driver.py b/cinder/tests/targets/test_tgt_driver.py index 462dfc75e..efbf599d1 100644 --- a/cinder/tests/targets/test_tgt_driver.py +++ b/cinder/tests/targets/test_tgt_driver.py @@ -300,3 +300,11 @@ class TestTgtAdmDriver(test.TestCase): iscsi_write_cache='on', check_exit_code=False, old_name=None) + + def test_iscsi_location(self): + location = self.target._iscsi_location('portal', 1, 'target', 2) + self.assertEqual('portal:3260,1 target 2', location) + + location = self.target._iscsi_location('portal', 1, 'target', 2, + ['portal2']) + self.assertEqual('portal:3260;portal2:3260,1 target 2', location) diff --git a/cinder/tests/test_coraid.py b/cinder/tests/test_coraid.py index 9bbc7ec41..8a46bfe86 100644 --- a/cinder/tests/test_coraid.py +++ b/cinder/tests/test_coraid.py @@ -243,6 +243,7 @@ class CoraidDriverTestCase(test.TestCase): configuration.snapshot_name_template = "snapshot-%s" configuration.coraid_repository_key = fake_coraid_repository_key configuration.use_multipath_for_image_xfer = False + configuration.enforce_multipath_for_image_xfer = False configuration.num_volume_device_scan_tries = 3 configuration.volume_dd_blocksize = '1M' self.fake_rpc = FakeRpc() @@ -815,7 +816,7 @@ class CoraidDriverImageTestCases(CoraidDriverTestCase): self.mox.StubOutWithMock(connector, 'get_connector_properties') connector.get_connector_properties(root_helper, - CONF.my_ip).\ + CONF.my_ip, False, False).\ AndReturn({}) self.mox.StubOutWithMock(utils, 'brick_get_connector') diff --git a/cinder/tests/test_utils.py b/cinder/tests/test_utils.py index ecaa497ef..3429f4070 100644 --- a/cinder/tests/test_utils.py +++ b/cinder/tests/test_utils.py @@ -1241,7 +1241,8 @@ class BrickUtils(test.TestCase): mock_conf.my_ip = '1.2.3.4' output = utils.brick_get_connector_properties() mock_helper.assert_called_once_with() - mock_get.assert_called_once_with(mock_helper.return_value, '1.2.3.4') + mock_get.assert_called_once_with(mock_helper.return_value, '1.2.3.4', + False, False) self.assertEqual(mock_get.return_value, output) @mock.patch('cinder.brick.initiator.connector.InitiatorConnector.factory') diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 9f23a17ba..4a3756afa 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -3680,7 +3680,7 @@ class GenericVolumeDriverTestCase(DriverTestCase): self.volume.driver.db.volume_get(self.context, vol['id']).\ AndReturn(vol) cinder.brick.initiator.connector.\ - get_connector_properties(root_helper, CONF.my_ip).\ + get_connector_properties(root_helper, CONF.my_ip, False, False).\ AndReturn(properties) self.volume.driver._attach_volume(self.context, vol, properties).\ AndReturn(attach_info) @@ -3713,7 +3713,7 @@ class GenericVolumeDriverTestCase(DriverTestCase): self.mox.StubOutWithMock(self.volume.driver, 'terminate_connection') cinder.brick.initiator.connector.\ - get_connector_properties(root_helper, CONF.my_ip).\ + get_connector_properties(root_helper, CONF.my_ip, False, False).\ AndReturn(properties) self.volume.driver._attach_volume(self.context, vol, properties).\ AndReturn(attach_info) @@ -4086,6 +4086,19 @@ class ISCSITestCase(DriverTestCase): self.assertEqual(result["target_iqn"], "iqn:iqn") self.assertEqual(result["target_lun"], 0) + def test_get_iscsi_properties_multiple_portals(self): + volume = {"provider_location": '1.1.1.1:3260;2.2.2.2:3261,1 iqn:iqn 0', + "id": "0", + "provider_auth": "a b c", + "attached_mode": "rw"} + iscsi_driver = \ + cinder.volume.targets.tgt.TgtAdm(configuration=self.configuration) + result = iscsi_driver._get_iscsi_properties(volume, multipath=True) + self.assertEqual(["1.1.1.1:3260", "2.2.2.2:3261"], + result["target_portals"]) + self.assertEqual(["iqn:iqn", "iqn:iqn"], result["target_iqns"]) + self.assertEqual([0, 0], result["target_luns"]) + def test_get_volume_stats(self): def _fake_get_all_physical_volumes(obj, root_helper, vg_name): diff --git a/cinder/utils.py b/cinder/utils.py index b1f1a44d8..d1bd2e5ee 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -574,14 +574,23 @@ def get_root_helper(): return 'sudo cinder-rootwrap %s' % CONF.rootwrap_config -def brick_get_connector_properties(): +def brick_get_connector_properties(multipath=False, enforce_multipath=False): """wrapper for the brick calls to automatically set the root_helper needed for cinder. + + :param multipath: A boolean indicating whether the connector can + support multipath. + :param enforce_multipath: If True, it raises exception when multipath=True + is specified but multipathd is not running. + If False, it falls back to multipath=False + when multipathd is not running. """ root_helper = get_root_helper() return connector.get_connector_properties(root_helper, - CONF.my_ip) + CONF.my_ip, + multipath, + enforce_multipath) def brick_get_connector(protocol, driver=None, diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index dc7286e52..ed4baea37 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -50,6 +50,9 @@ volume_opts = [ cfg.StrOpt('iscsi_ip_address', default='$my_ip', help='The IP address that the iSCSI daemon is listening on'), + cfg.ListOpt('iscsi_secondary_ip_addresses', + default=[], + help='The list of secondary IP addresses of the iSCSI daemon'), cfg.IntOpt('iscsi_port', default=3260, help='The port that the iSCSI daemon is listening on'), @@ -65,6 +68,11 @@ volume_opts = [ default=False, help='Do we attach/detach volumes in cinder using multipath ' 'for volume to image and image to volume transfers?'), + cfg.BoolOpt('enforce_multipath_for_image_xfer', + default=False, + help='If this is set to True, attachment of volumes for ' + 'image transfer will be aborted when multipathd is not ' + 'running. Otherwise, it will fallback to single path.'), cfg.StrOpt('volume_clear', default='zero', help='Method used to wipe old volumes (valid options are: ' @@ -397,7 +405,10 @@ class VolumeDriver(object): LOG.debug(('copy_data_between_volumes %(src)s -> %(dest)s.') % {'src': src_vol['name'], 'dest': dest_vol['name']}) - properties = utils.brick_get_connector_properties() + use_multipath = self.configuration.use_multipath_for_image_xfer + enforce_multipath = self.configuration.enforce_multipath_for_image_xfer + properties = utils.brick_get_connector_properties(use_multipath, + enforce_multipath) dest_remote = True if remote in ['dest', 'both'] else False dest_orig_status = dest_vol['status'] try: @@ -453,7 +464,10 @@ class VolumeDriver(object): """Fetch the image from image_service and write it to the volume.""" LOG.debug(('copy_image_to_volume %s.') % volume['name']) - properties = utils.brick_get_connector_properties() + use_multipath = self.configuration.use_multipath_for_image_xfer + enforce_multipath = self.configuration.enforce_multipath_for_image_xfer + properties = utils.brick_get_connector_properties(use_multipath, + enforce_multipath) attach_info = self._attach_volume(context, volume, properties) try: @@ -470,7 +484,10 @@ class VolumeDriver(object): """Copy the volume to the specified image.""" LOG.debug(('copy_volume_to_image %s.') % volume['name']) - properties = utils.brick_get_connector_properties() + use_multipath = self.configuration.use_multipath_for_image_xfer + enforce_multipath = self.configuration.enforce_multipath_for_image_xfer + properties = utils.brick_get_connector_properties(use_multipath, + enforce_multipath) attach_info = self._attach_volume(context, volume, properties) try: @@ -580,7 +597,10 @@ class VolumeDriver(object): LOG.debug(('Creating a new backup for volume %s.') % volume['name']) - properties = utils.brick_get_connector_properties() + use_multipath = self.configuration.use_multipath_for_image_xfer + enforce_multipath = self.configuration.enforce_multipath_for_image_xfer + properties = utils.brick_get_connector_properties(use_multipath, + enforce_multipath) attach_info = self._attach_volume(context, volume, properties) try: @@ -605,7 +625,10 @@ class VolumeDriver(object): {'backup': backup['id'], 'volume': volume['name']}) - properties = utils.brick_get_connector_properties() + use_multipath = self.configuration.use_multipath_for_image_xfer + enforce_multipath = self.configuration.enforce_multipath_for_image_xfer + properties = utils.brick_get_connector_properties(use_multipath, + enforce_multipath) attach_info = self._attach_volume(context, volume, properties) try: @@ -958,7 +981,7 @@ class ISCSIDriver(VolumeDriver): return target return None - def _get_iscsi_properties(self, volume): + def _get_iscsi_properties(self, volume, multipath=False): """Gets iscsi configuration We ideally get saved information in the volume entity, but fall back @@ -983,6 +1006,11 @@ class ISCSIDriver(VolumeDriver): :access_mode: the volume access mode allow client used ('rw' or 'ro' currently supported) + + In some of drivers, When multipath=True is specified, :target_iqn, + :target_portal, :target_lun may be replaced with :target_iqns, + :target_portals, :target_luns, which contain lists of multiple values. + In this case, the initiator should establish sessions to all the path. """ properties = {} @@ -1004,19 +1032,30 @@ class ISCSIDriver(VolumeDriver): properties['target_discovered'] = True results = location.split(" ") - properties['target_portal'] = results[0].split(",")[0] - properties['target_iqn'] = results[1] + portals = results[0].split(",")[0].split(";") + iqn = results[1] + nr_portals = len(portals) + try: - properties['target_lun'] = int(results[2]) + lun = int(results[2]) except (IndexError, ValueError): if (self.configuration.volume_driver in ['cinder.volume.drivers.lvm.LVMISCSIDriver', 'cinder.volume.drivers.lvm.LVMISERDriver', 'cinder.volume.drivers.lvm.ThinLVMVolumeDriver'] and self.configuration.iscsi_helper in ('tgtadm', 'iseradm')): - properties['target_lun'] = 1 + lun = 1 else: - properties['target_lun'] = 0 + lun = 0 + + if multipath: + properties['target_portals'] = portals + properties['target_iqns'] = [iqn] * nr_portals + properties['target_luns'] = [lun] * nr_portals + else: + properties['target_portal'] = portals[0] + properties['target_iqn'] = iqn + properties['target_lun'] = lun properties['volume_id'] = volume['id'] @@ -1089,7 +1128,9 @@ class ISCSIDriver(VolumeDriver): # drivers, for now leaving it as there are 3'rd party # drivers that don't use target drivers, but inherit from # this base class and use this init data - iscsi_properties = self._get_iscsi_properties(volume) + iscsi_properties = self._get_iscsi_properties(volume, + connector.get( + 'multipath')) return { 'driver_volume_type': 'iscsi', 'data': iscsi_properties diff --git a/cinder/volume/targets/iscsi.py b/cinder/volume/targets/iscsi.py index 3bf390eec..669a32f61 100644 --- a/cinder/volume/targets/iscsi.py +++ b/cinder/volume/targets/iscsi.py @@ -38,7 +38,7 @@ class ISCSITarget(driver.Target): self.configuration.safe_get('iscsi_protocol') self.protocol = 'iSCSI' - def _get_iscsi_properties(self, volume): + def _get_iscsi_properties(self, volume, multipath=False): """Gets iscsi configuration We ideally get saved information in the volume entity, but fall back @@ -65,6 +65,11 @@ class ISCSITarget(driver.Target): :access_mode: the volume access mode allow client used ('rw' or 'ro' currently supported) + + When multipath=True is specified, :target_iqn, :target_portal, + :target_lun may be replaced with :target_iqns, :target_portals, + :target_luns, which contain lists of multiple values. + In this case, the initiator should establish sessions to all the path. """ properties = {} @@ -86,10 +91,11 @@ class ISCSITarget(driver.Target): properties['target_discovered'] = True results = location.split(" ") - properties['target_portal'] = results[0].split(",")[0] - properties['target_iqn'] = results[1] + portals = results[0].split(",")[0].split(";") + iqn = results[1] + nr_portals = len(portals) try: - properties['target_lun'] = int(results[2]) + lun = int(results[2]) except (IndexError, ValueError): # NOTE(jdg): The following is carried over from the existing # code. The trick here is that different targets use different @@ -99,9 +105,18 @@ class ISCSITarget(driver.Target): ['cinder.volume.drivers.lvm.LVMISCSIDriver', 'cinder.volume.drivers.lvm.ThinLVMVolumeDriver'] and self.configuration.iscsi_helper == 'tgtadm'): - properties['target_lun'] = 1 + lun = 1 else: - properties['target_lun'] = 0 + lun = 0 + + if multipath: + properties['target_portals'] = portals + properties['target_iqns'] = [iqn] * nr_portals + properties['target_luns'] = [lun] * nr_portals + else: + properties['target_portal'] = portals[0] + properties['target_iqn'] = iqn + properties['target_lun'] = lun properties['volume_id'] = volume['id'] diff --git a/cinder/volume/targets/lio.py b/cinder/volume/targets/lio.py index 348ad2254..12ccc7791 100644 --- a/cinder/volume/targets/lio.py +++ b/cinder/volume/targets/lio.py @@ -173,7 +173,9 @@ class LioAdm(TgtAdm): raise exception.ISCSITargetAttachFailed( volume_id=volume['id']) - iscsi_properties = self._get_iscsi_properties(volume) + iscsi_properties = self._get_iscsi_properties(volume, + connector.get( + 'multipath')) # FIXME(jdg): For LIO the target_lun is 0, other than that all data # is the same as it is for tgtadm, just modify it here diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py index a63b51137..9f8878a03 100644 --- a/cinder/volume/targets/tgt.py +++ b/cinder/volume/targets/tgt.py @@ -116,9 +116,13 @@ class TgtAdm(iscsi.ISCSITarget): LOG.debug('StdOut from recreate backing lun: %s' % out) LOG.debug('StdErr from recreate backing lun: %s' % err) - def _iscsi_location(self, ip, target, iqn, lun=None): - return "%s:%s,%s %s %s" % (ip, self.configuration.iscsi_port, - target, iqn, lun) + def _iscsi_location(self, ip, target, iqn, lun=None, ip_secondary=None): + ip_secondary = ip_secondary or [] + port = self.configuration.iscsi_port + portals = map(lambda x: "%s:%s" % (x, port), [ip] + ip_secondary) + return ("%(portals)s,%(target)s %(iqn)s %(lun)s" + % ({'portals': ";".join(portals), + 'target': target, 'iqn': iqn, 'lun': lun})) def _get_iscsi_target(self, context, vol_id): return 0 @@ -321,7 +325,8 @@ class TgtAdm(iscsi.ISCSITarget): iscsi_write_cache=iscsi_write_cache) data = {} data['location'] = self._iscsi_location( - self.configuration.iscsi_ip_address, tid, iscsi_name, lun) + self.configuration.iscsi_ip_address, tid, iscsi_name, lun, + self.configuration.iscsi_secondary_ip_addresses) LOG.debug('Set provider_location to: %s', data['location']) data['auth'] = self._iscsi_authentication( 'CHAP', chap_username, chap_password) @@ -353,7 +358,9 @@ class TgtAdm(iscsi.ISCSITarget): self.remove_iscsi_target(iscsi_target, 0, volume['id'], volume['name']) def initialize_connection(self, volume, connector): - iscsi_properties = self._get_iscsi_properties(volume) + iscsi_properties = self._get_iscsi_properties(volume, + connector.get( + 'multipath')) return { 'driver_volume_type': self.iscsi_protocol, 'data': iscsi_properties diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters index a90f8ded1..14484c76d 100644 --- a/etc/cinder/rootwrap.d/volume.filters +++ b/etc/cinder/rootwrap.d/volume.filters @@ -107,6 +107,7 @@ ssc: CommandFilter, ssc, root ls: CommandFilter, ls, root tee: CommandFilter, tee, root multipath: CommandFilter, multipath, root +multipathd: CommandFilter, multipathd, root systool: CommandFilter, systool, root # cinder/volume/drivers/block_device.py -- 2.45.2