Merge "Enhance iSCSI multipath support"

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)

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)
+        LOG.debug("expected = %s." % expected_commands)

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)

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')

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')

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):

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,

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

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']
 

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

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

etc/cinder/rootwrap.d/volume.filters

@@ -113,6 +113,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