Add support for --interface option in iscsiadm

Enables use of the libvirt parameter iscsi_iface that can be used to
specify an iSCSI iface, which used in conjunction with the
--interface parameter provides offloaded iSCSI support.

Brings os-brick on par with with nova support for offload transports.

DocImpact
Closes-Bug: 1370226
Implements: blueprint brick-add-open-iscsi-transport-support

Change-Id: I74c3e50c0304a9aeeac18e5ba7a12dda201fb627

os_brick/initiator/connector.py

@@ -22,6 +22,7 @@
 
 import abc
 import copy
+import glob
 import json
 import os
 import platform
@@ -159,20 +160,17 @@ def factory(protocol, root_helper, driver=None,
         LOG.debug("Factory for %(protocol)s on %(arch)s",
                   {'protocol': protocol, 'arch': arch})
         protocol = protocol.upper()
-        if protocol == ISCSI:
+        if protocol in [ISCSI, ISER]:
+            # override transport kwarg for requests not comming
+            # from the nova LibvirtISERVolumeDriver
+            if protocol == ISER:
+                kwargs.update({'transport': 'iser'})
             return ISCSIConnector(root_helper=root_helper,
                                   driver=driver,
                                   execute=execute,
                                   use_multipath=use_multipath,
                                   device_scan_attempts=device_scan_attempts,
                                   *args, **kwargs)
-        elif protocol == ISER:
-            return ISERConnector(root_helper=root_helper,
-                                 driver=driver,
-                                 execute=execute,
-                                 use_multipath=use_multipath,
-                                 device_scan_attempts=device_scan_attempts,
-                                 *args, **kwargs)
         elif protocol == FIBRE_CHANNEL:
             if arch in (S390, S390X):
                 return FibreChannelConnectorS390X(
@@ -294,23 +292,64 @@ def disconnect_volume(self, connection_properties, device_info):
 
 class ISCSIConnector(InitiatorConnector):
     """Connector class to attach/detach iSCSI volumes."""
+    supported_transports = ['be2iscsi', 'bnx2i', 'cxgb3i', 'default',
+                            'cxgb4i', 'qla4xxx', 'ocs', 'iser']
 
     def __init__(self, root_helper, driver=None,
                  execute=putils.execute, use_multipath=False,
                  device_scan_attempts=DEVICE_SCAN_ATTEMPTS_DEFAULT,
-                 *args, **kwargs):
+                 transport='default', *args, **kwargs):
         self._linuxscsi = linuxscsi.LinuxSCSI(root_helper, execute)
         super(ISCSIConnector, self).__init__(
             root_helper, driver=driver,
             execute=execute,
             device_scan_attempts=device_scan_attempts,
-            *args, **kwargs)
+            transport=transport, *args, **kwargs)
         self.use_multipath = use_multipath
+        self.transport = self._validate_iface_transport(transport)
 
     def set_execute(self, execute):
         super(ISCSIConnector, self).set_execute(execute)
         self._linuxscsi.set_execute(execute)
 
+    def _validate_iface_transport(self, transport_iface):
+        """Check that given iscsi_iface uses only supported transports
+
+        Accepted transport names for provided iface param are
+        be2iscsi, bnx2i, cxgb3i, cxgb4i, default, qla4xxx, ocs or iser.
+        Note the difference between transport and iface;
+        unlike default(iscsi_tcp)/iser, this is not one and the same for
+        offloaded transports, where the default format is
+        transport_name.hwaddress
+        """
+        # Note that default(iscsi_tcp) and iser do not require a separate
+        # iface file, just the transport is enough and do not need to be
+        # validated. This is not the case for the other entries in
+        # supported_transports array.
+        if transport_iface in ['default', 'iser']:
+            return transport_iface
+        # Will return (6) if iscsi_iface file was not found, or (2) if iscsid
+        # could not be contacted
+        out = self._run_iscsiadm_bare(['-m',
+                                       'iface',
+                                       '-I',
+                                       transport_iface],
+                                      check_exit_code=[0, 2, 6])[0] or ""
+        LOG.debug("iscsiadm %(iface)s configuration: stdout=%(out)s.",
+                  {'iface': transport_iface, 'out': out})
+        for data in [line.split() for line in out.splitlines()]:
+            if data[0] == 'iface.transport_name':
+                if data[2] in self.supported_transports:
+                    return transport_iface
+
+        LOG.warn(_LW("No useable transport found for iscsi iface %s. "
+                     "Falling back to default transport."),
+                 transport_iface)
+        return 'default'
+
+    def _get_transport(self):
+        return self.transport
+
     def _iterate_all_targets(self, connection_properties):
         for ip, iqn, lun in self._get_all_targets(connection_properties):
             props = copy.deepcopy(connection_properties)
@@ -422,6 +461,8 @@ def connect_volume(self, connection_properties):
             if self.use_multipath:
                 self._rescan_iscsi()
             else:
+                if (tries):
+                    host_devices = self._get_device_path(target_props)
                 self._run_iscsiadm(target_props, ("--rescan",))
 
             tries = tries + 1
@@ -487,7 +528,13 @@ def disconnect_volume(self, connection_properties, device_info):
     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]
+        host_devices = self._get_device_path(connection_properties)
+
+        if host_devices:
+            host_device = host_devices[0]
+        else:
+            return
+
         dev_name = self._linuxscsi.get_name_from_path(host_device)
         if dev_name:
             self._linuxscsi.remove_scsi_device(dev_name)
@@ -501,19 +548,31 @@ def _disconnect_volume_iscsi(self, connection_properties):
 
         # NOTE(vish): Only disconnect from the target if no luns from the
         #             target are in use.
-        device_prefix = ("/dev/disk/by-path/ip-%(portal)s-iscsi-%(iqn)s-lun-" %
+        device_byname = ("ip-%(portal)s-iscsi-%(iqn)s-lun-" %
                          {'portal': connection_properties['target_portal'],
                           'iqn': connection_properties['target_iqn']})
         devices = self.driver.get_all_block_devices()
-        devices = [dev for dev in devices if dev.startswith(device_prefix)
+        devices = [dev for dev in devices if (device_byname in dev
+                                              and
+                                              dev.startswith(
+                                                  '/dev/disk/by-path/'))
                    and os.path.exists(dev)]
-
         if not devices:
             self._disconnect_from_iscsi_portal(connection_properties)
 
     def _get_device_path(self, connection_properties):
-        return ["/dev/disk/by-path/ip-%s-iscsi-%s-lun-%s" % x for x in
-                self._get_all_targets(connection_properties)]
+        if self._get_transport() == "default":
+            return ["/dev/disk/by-path/ip-%s-iscsi-%s-lun-%s" % x for x in
+                    self._get_all_targets(connection_properties)]
+        else:
+            # we are looking for paths in the format :
+            # /dev/disk/by-path/pci-XXXX:XX:XX.X-ip-PORTAL:PORT-iscsi-IQN-lun-LUN_ID
+            device_list = []
+            for x in self._get_all_targets(connection_properties):
+                look_for_device = glob.glob('/dev/disk/by-path/*ip-%s-iscsi-%s-lun-%s' % x)  # noqa
+                if look_for_device:
+                    device_list.extend(look_for_device)
+            return device_list
 
     def get_initiator(self):
         """Secure helper to read file as root."""
@@ -640,7 +699,9 @@ def _connect_to_iscsi_portal(self, connection_properties):
         except putils.ProcessExecutionError as exc:
             # iscsiadm returns 21 for "No records found" after version 2.0-871
             if exc.exit_code in [21, 255]:
-                self._run_iscsiadm(connection_properties, ('--op', 'new'))
+                self._run_iscsiadm(connection_properties,
+                                   ('--interface', self._get_transport(),
+                                    '--op', 'new'))
             else:
                 raise
 
@@ -721,7 +782,12 @@ def _get_iscsi_devices(self):
             devices = list(os.walk('/dev/disk/by-path'))[0][-1]
         except IndexError:
             return []
-        return [entry for entry in devices if entry.startswith("ip-")]
+        # For iSCSI HBAs, look at an offset of len('pci-0000:00:00.0')
+        return [entry for entry in devices if (entry.startswith("ip-")
+                                               or (entry.startswith("pci-")
+                                                   and
+                                                   entry.find("ip-", 16, 21)
+                                                   >= 16))]
 
     def _disconnect_mpath(self, connection_properties, ips_iqns):
         for ip, iqn in ips_iqns:
@@ -795,15 +861,6 @@ def _rescan_multipath(self):
         self._run_multipath(['-r'], check_exit_code=[0, 1, 21])
 
 
-class ISERConnector(ISCSIConnector):
-
-    def _get_device_path(self, iser_properties):
-        return ("/dev/disk/by-path/ip-%s-iser-%s-lun-%s" %
-                (iser_properties['target_portal'],
-                 iser_properties['target_iqn'],
-                 iser_properties.get('target_lun', 0)))
-
-
 class FibreChannelConnector(InitiatorConnector):
     """Connector class to attach/detach Fibre Channel volumes."""
 

os_brick/tests/initiator/test_connector.py

@@ -17,6 +17,7 @@
 import tempfile
 import time
 
+import glob
 import json
 import mock
 from oslo_concurrency import processutils as putils
@@ -138,9 +139,6 @@ def test_factory(self):
         obj = connector.InitiatorConnector.factory('iscsi', None)
         self.assertEqual(obj.__class__.__name__, "ISCSIConnector")
 
-        obj = connector.InitiatorConnector.factory('iser', None)
-        self.assertEqual(obj.__class__.__name__, "ISERConnector")
-
         obj = connector.InitiatorConnector.factory('fibre_channel', None)
         self.assertEqual(obj.__class__.__name__, "FibreChannelConnector")
 
@@ -210,6 +208,13 @@ def setUp(self):
                           return_value="/dev/sdb").start()
         self.addCleanup(mock.patch.stopall)
 
+    def generate_device(self, location, iqn, transport=None, lun=1):
+        dev_format = "ip-%s-iscsi-%s-lun-%s" % (location, iqn, lun)
+        if transport:
+            dev_format = "pci-0000:00:00.0-" + dev_format
+        fake_dev_path = "/dev/disk/by-path/" + dev_format
+        return fake_dev_path
+
     def iscsi_connection(self, volume, location, iqn):
         return {
             'driver_volume_type': 'iscsi',
@@ -256,8 +261,30 @@ def initiator_get_text(*arg, **kwargs):
         initiator = self.connector.get_initiator()
         self.assertEqual(initiator, 'iqn.1234-56.foo.bar:01:23456789abc')
 
+    @mock.patch.object(connector.ISCSIConnector, '_run_iscsiadm_bare')
+    def test_brick_iscsi_validate_transport(self, mock_iscsiadm):
+        sample_output = ('# BEGIN RECORD 2.0-872\n'
+                         'iface.iscsi_ifacename = %s.fake_suffix\n'
+                         'iface.net_ifacename = <empty>\n'
+                         'iface.ipaddress = <empty>\n'
+                         'iface.hwaddress = 00:53:00:00:53:00\n'
+                         'iface.transport_name = %s\n'
+                         'iface.initiatorname = <empty>\n'
+                         '# END RECORD')
+        for tport in self.connector.supported_transports:
+            mock_iscsiadm.return_value = (sample_output % (tport, tport), '')
+            self.assertEqual(tport + '.fake_suffix',
+                             self.connector._validate_iface_transport(
+                                 tport + '.fake_suffix'))
+
+        mock_iscsiadm.return_value = ("", 'iscsiadm: Could not '
+                                      'read iface fake_transport (6)')
+        self.assertEqual('default',
+                         self.connector._validate_iface_transport(
+                             'fake_transport'))
+
     def _test_connect_volume(self, extra_props, additional_commands,
-                             disconnect_mock=None):
+                             transport=None, disconnect_mock=None):
         # for making sure the /dev/disk/by-path is gone
         exists_mock = mock.Mock()
         exists_mock.return_value = True
@@ -270,8 +297,14 @@ def _test_connect_volume(self, extra_props, additional_commands,
         connection_info = self.iscsi_connection(vol, location, iqn)
         for key, value in extra_props.items():
             connection_info['data'][key] = value
-        device = self.connector.connect_volume(connection_info['data'])
-        dev_str = '/dev/disk/by-path/ip-%s-iscsi-%s-lun-1' % (location, iqn)
+        if transport is not None:
+            dev_list = self.generate_device(location, iqn, transport)
+            with mock.patch.object(glob, 'glob', return_value=[dev_list]):
+                device = self.connector.connect_volume(connection_info['data'])
+        else:
+            device = self.connector.connect_volume(connection_info['data'])
+
+        dev_str = self.generate_device(location, iqn, transport)
         self.assertEqual(device['type'], 'block')
         self.assertEqual(device['path'], dev_str)
 
@@ -289,7 +322,15 @@ def mock_exists_effect(*args, **kwargs):
 
         with mock.patch.object(os.path, 'exists',
                                side_effect=disconnect_mock):
-            self.connector.disconnect_volume(connection_info['data'], device)
+            if transport is not None:
+                dev_list = self.generate_device(location, iqn, transport)
+                with mock.patch.object(glob, 'glob', return_value=[dev_list]):
+                    self.connector.disconnect_volume(connection_info['data'],
+                                                     device)
+            else:
+                self.connector.disconnect_volume(connection_info['data'],
+                                                 device)
+
             expected_commands = [
                 ('iscsiadm -m node -T %s -p %s' % (iqn, location)),
                 ('iscsiadm -m session'),
@@ -315,6 +356,13 @@ def mock_exists_effect(*args, **kwargs):
     def test_connect_volume(self):
         self._test_connect_volume({}, [])
 
+    @testtools.skipUnless(os.path.exists('/dev/disk/by-path'),
+                          'Test requires /dev/disk/by-path')
+    @mock.patch.object(connector.ISCSIConnector, '_get_transport')
+    def test_connect_volume_with_transport(self, mock_transport):
+        mock_transport.return_value = 'fake_transport'
+        self._test_connect_volume({}, [], 'fake_transport')
+
     @testtools.skipUnless(os.path.exists('/dev/disk/by-path'),
                           'Test requires /dev/disk/by-path')
     def test_connect_volume_with_alternative_targets(self):

os_brick/tests/initiator/test_linuxscsi.py

@@ -50,7 +50,11 @@ def test_get_name_from_path(self, realpath_mock):
         disk_path = ("/dev/disk/by-path/ip-10.10.220.253:3260-"
                      "iscsi-iqn.2000-05.com.3pardata:21810002ac00383d-lun-0")
         name = self.linuxscsi.get_name_from_path(disk_path)
-        self.assertEqual(name, device_name)
+        self.assertEqual(device_name, name)
+        disk_path = ("/dev/disk/by-path/pci-0000:00:00.0-ip-10.9.8.7:3260-"
+                     "iscsi-iqn.2000-05.com.openstack:2180002ac00383d-lun-0")
+        name = self.linuxscsi.get_name_from_path(disk_path)
+        self.assertEqual(device_name, name)
         realpath_mock.return_value = "bogus"
         name = self.linuxscsi.get_name_from_path(disk_path)
         self.assertIsNone(name)