From: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
Date: Thu, 11 Dec 2014 23:23:18 +0000 (-0500)
Subject: Enhance iSCSI multipath support
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=ca97a7461f0f56835a0f2dceb9afaebac756f7c3;p=openstack-build%2Fcinder-build.git

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

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