diff --git a/os_brick/initiator/windows/fibre_channel.py b/os_brick/initiator/windows/fibre_channel.py index 3efd5130c..72aca33bf 100644 --- a/os_brick/initiator/windows/fibre_channel.py +++ b/os_brick/initiator/windows/fibre_channel.py @@ -16,10 +16,12 @@ import collections import time +from os_win import exceptions as os_win_exc from os_win import utilsfactory from oslo_log import log as logging from os_brick import exception +from os_brick.i18n import _ from os_brick.initiator.windows import base as win_conn_base from os_brick import utils @@ -29,6 +31,9 @@ LOG = logging.getLogger(__name__) class WindowsFCConnector(win_conn_base.BaseWindowsConnector): def __init__(self, *args, **kwargs): super(WindowsFCConnector, self).__init__(*args, **kwargs) + + self.use_multipath = kwargs.get('use_multipath', False) + self._fc_utils = utilsfactory.get_fc_utils() @staticmethod @@ -73,10 +78,14 @@ class WindowsFCConnector(win_conn_base.BaseWindowsConnector): # If multipath is used and the MPIO service is properly configured # to claim the disks, we'll still get a single device path, having # the same format, which will be used for all the IO operations. - disk_paths = set() + for attempt_num in range(self.device_scan_attempts): + disk_paths = set() + + if attempt_num: + time.sleep(self.device_scan_interval) - for attempt in range(self.device_scan_attempts): self._diskutils.rescan_disks() + volume_mappings = self._get_fc_volume_mappings( connection_properties) LOG.debug("Retrieved volume mappings %(vol_mappings)s " @@ -84,22 +93,41 @@ class WindowsFCConnector(win_conn_base.BaseWindowsConnector): dict(vol_mappings=volume_mappings, conn_props=connection_properties)) - # Because of MPIO, we may not be able to get the device name - # from a specific mapping if the disk was accessed through - # an other HBA at that moment. In that case, the device name - # will show up as an empty string. for mapping in volume_mappings: device_name = mapping['device_name'] if device_name: disk_paths.add(device_name) - if disk_paths: - break + if not disk_paths and volume_mappings: + fcp_lun = volume_mappings[0]['fcp_lun'] - time.sleep(self.device_scan_interval) + try: + disk_paths = self._get_disk_paths_by_scsi_id( + connection_properties, fcp_lun) + disk_paths = set(disk_paths or []) + except os_win_exc.OSWinException as ex: + LOG.debug("Failed to retrieve disk paths by SCSI ID. " + "Exception: %s", ex) - self._check_device_paths(disk_paths) - return list(disk_paths) + if not disk_paths: + LOG.debug("No disk path retrieved yet.") + continue + + if len(disk_paths) > 1: + LOG.debug("Multiple disk paths retrieved: %s This may happen " + "if MPIO did not claim them yet.", disk_paths) + continue + + dev_num = self._diskutils.get_device_number_from_device_name( + list(disk_paths)[0]) + if self.use_multipath and not self._diskutils.is_mpio_disk( + dev_num): + LOG.debug("Multipath was requested but the disk %s was not " + "claimed yet by the MPIO service.", dev_num) + continue + + return list(disk_paths) + return [] def _get_fc_volume_mappings(self, connection_properties): # Note(lpetrut): All the WWNs returned by os-win are upper case. @@ -125,6 +153,61 @@ class WindowsFCConnector(win_conn_base.BaseWindowsConnector): mappings[port['node_name']].append(port['port_name']) return mappings + def _get_disk_paths_by_scsi_id(self, connection_properties, fcp_lun): + for local_port_wwn, remote_port_wwns in connection_properties[ + 'initiator_target_map'].items(): + for remote_port_wwn in remote_port_wwns: + try: + dev_nums = self._get_dev_nums_by_scsi_id( + local_port_wwn, remote_port_wwn, fcp_lun) + + # This may raise a DiskNotFound exception if the disks + # are meanwhile claimed by the MPIO serivce. + disk_paths = [ + self._diskutils.get_device_name_by_device_number( + dev_num) + for dev_num in dev_nums] + return disk_paths + except os_win_exc.FCException as ex: + LOG.debug("Failed to retrieve volume paths by SCSI id. " + "Exception: %s", ex) + continue + return [] + + def _get_dev_nums_by_scsi_id(self, local_port_wwn, remote_port_wwn, + fcp_lun): + LOG.debug("Fetching SCSI Unique ID for FCP lun %(fcp_lun)s. " + "Port WWN: %(local_port_wwn)s. " + "Remote port WWN: %(remote_port_wwn)s.", + dict(fcp_lun=fcp_lun, + local_port_wwn=local_port_wwn, + remote_port_wwn=remote_port_wwn)) + + local_hba_wwn = self._get_fc_hba_wwn_for_port(local_port_wwn) + # This will return the SCSI identifiers in the order of precedence + # used by Windows. + identifiers = self._fc_utils.get_scsi_device_identifiers( + local_hba_wwn, local_port_wwn, + remote_port_wwn, fcp_lun) + + if identifiers: + identifier = identifiers[0] + dev_nums = self._diskutils.get_disk_numbers_by_unique_id( + unique_id=identifier['id'], + unique_id_format=identifier['type']) + return dev_nums + return [] + + def _get_fc_hba_wwn_for_port(self, port_wwn): + fc_hba_ports = self._fc_utils.get_fc_hba_ports() + for port in fc_hba_ports: + if port_wwn.upper() == port['port_name']: + return port['node_name'] + + err_msg = _("Could not find any FC HBA port " + "having WWN '%s'.") % port_wwn + raise exception.NotFound(err_msg) + @utils.trace def disconnect_volume(self, connection_properties, device_info=None, force=False, ignore_errors=False): diff --git a/os_brick/tests/windows/test_fibre_channel.py b/os_brick/tests/windows/test_fibre_channel.py index f8ae3e606..ee0e49d3a 100644 --- a/os_brick/tests/windows/test_fibre_channel.py +++ b/os_brick/tests/windows/test_fibre_channel.py @@ -15,6 +15,7 @@ import ddt import mock +from os_win import exceptions as os_win_exc from os_brick import exception from os_brick.initiator.windows import fibre_channel as fc @@ -87,20 +88,43 @@ class WindowsFCConnectorTestCase(test_base.WindowsConnectorTestBase): mock.sentinel.conn_props) @ddt.data({'volume_mappings': [], 'expected_paths': []}, - {'volume_mappings': [dict(device_name='')] * 3, + {'volume_mappings': [dict(device_name='', + fcp_lun=mock.sentinel.fcp_lun)] * 3, + 'scsi_id_side_eff': os_win_exc.OSWinException, 'expected_paths': []}, - {'volume_mappings': [dict(device_name=''), + {'volume_mappings': [dict(device_name='', + fcp_lun=mock.sentinel.fcp_lun), dict(device_name=mock.sentinel.disk_path)], - 'expected_paths': [mock.sentinel.disk_path]}) + 'expected_paths': [mock.sentinel.disk_path]}, + {'volume_mappings': [dict(device_name='', + fcp_lun=mock.sentinel.fcp_lun)], + 'scsi_id_side_eff': [[mock.sentinel.disk_path]], + 'expected_paths': [mock.sentinel.disk_path]}, + {'volume_mappings': [dict(device_name=mock.sentinel.disk_path)], + 'use_multipath': True, + 'is_mpio_disk': True, + 'expected_paths': [mock.sentinel.disk_path]}, + {'volume_mappings': [dict(device_name=mock.sentinel.disk_path)], + 'use_multipath': True, + 'is_mpio_disk': False, + 'expected_paths': []}) @ddt.unpack @mock.patch('time.sleep') @mock.patch.object(fc.WindowsFCConnector, '_get_fc_volume_mappings') - @mock.patch.object(fc.WindowsFCConnector, '_check_device_paths') - def test_get_volume_paths(self, mock_check_device_paths, + @mock.patch.object(fc.WindowsFCConnector, '_get_disk_paths_by_scsi_id') + def test_get_volume_paths(self, mock_get_disk_paths_by_scsi_id, mock_get_fc_mappings, mock_sleep, - volume_mappings, expected_paths): + volume_mappings, expected_paths, + scsi_id_side_eff=None, + use_multipath=False, + is_mpio_disk=False): + mock_get_dev_num = self._diskutils.get_device_number_from_device_name mock_get_fc_mappings.return_value = volume_mappings + mock_get_disk_paths_by_scsi_id.side_effect = scsi_id_side_eff + self._diskutils.is_mpio_disk.return_value = is_mpio_disk + + self._connector.use_multipath = use_multipath vol_paths = self._connector.get_volume_paths(mock.sentinel.conn_props) self.assertEqual(expected_paths, vol_paths) @@ -114,12 +138,23 @@ class WindowsFCConnectorTestCase(test_base.WindowsConnectorTestBase): [mock.call()] * expected_try_count) mock_get_fc_mappings.assert_has_calls( [mock.call(mock.sentinel.conn_props)] * expected_try_count) - mock_check_device_paths.assert_called_once_with( - set(vol_paths)) mock_sleep.assert_has_calls( [mock.call(mock.sentinel.rescan_interval)] * (expected_try_count - 1)) + dev_names = [mapping['device_name'] + for mapping in volume_mappings if mapping['device_name']] + if volume_mappings and not dev_names: + mock_get_disk_paths_by_scsi_id.assert_any_call( + mock.sentinel.conn_props, + volume_mappings[0]['fcp_lun']) + + if expected_paths and use_multipath: + mock_get_dev_num.assert_called_once_with(expected_paths[0]) + + self._diskutils.is_mpio_disk.assert_any_call( + mock_get_dev_num.return_value) + @mock.patch.object(fc.WindowsFCConnector, '_get_fc_hba_mappings') def test_get_fc_volume_mappings(self, mock_get_fc_hba_mappings): fake_target_wwpn = 'FAKE_TARGET_WWPN' @@ -158,3 +193,51 @@ class WindowsFCConnectorTestCase(test_base.WindowsConnectorTestBase): expected_mappings = { mock.sentinel.node_name: [mock.sentinel.port_name]} self.assertEqual(expected_mappings, resulted_mappings) + + @mock.patch.object(fc.WindowsFCConnector, '_get_dev_nums_by_scsi_id') + def test_get_disk_paths_by_scsi_id(self, mock_get_dev_nums): + remote_wwpns = [mock.sentinel.remote_wwpn_0, + mock.sentinel.remote_wwpn_1] + fake_init_target_map = {mock.sentinel.local_wwpn: remote_wwpns} + conn_props = dict(initiator_target_map=fake_init_target_map) + + mock_get_dev_nums.side_effect = [os_win_exc.FCException, + [mock.sentinel.dev_num]] + mock_get_dev_name = self._diskutils.get_device_name_by_device_number + mock_get_dev_name.return_value = mock.sentinel.dev_name + + disk_paths = self._connector._get_disk_paths_by_scsi_id( + conn_props, mock.sentinel.fcp_lun) + self.assertEqual([mock.sentinel.dev_name], disk_paths) + + mock_get_dev_nums.assert_has_calls([ + mock.call(mock.sentinel.local_wwpn, + remote_wwpn, + mock.sentinel.fcp_lun) + for remote_wwpn in remote_wwpns]) + mock_get_dev_name.assert_called_once_with(mock.sentinel.dev_num) + + @mock.patch.object(fc.WindowsFCConnector, '_get_fc_hba_wwn_for_port') + def test_get_dev_nums_by_scsi_id(self, mock_get_fc_hba_wwn): + fake_identifier = dict(id=mock.sentinel.id, + type=mock.sentinel.type) + + mock_get_fc_hba_wwn.return_value = mock.sentinel.local_wwnn + self._fc_utils.get_scsi_device_identifiers.return_value = [ + fake_identifier] + self._diskutils.get_disk_numbers_by_unique_id.return_value = ( + mock.sentinel.dev_nums) + + dev_nums = self._connector._get_dev_nums_by_scsi_id( + mock.sentinel.local_wwpn, + mock.sentinel.remote_wwpn, + mock.sentinel.fcp_lun) + self.assertEqual(mock.sentinel.dev_nums, dev_nums) + + mock_get_fc_hba_wwn.assert_called_once_with(mock.sentinel.local_wwpn) + self._fc_utils.get_scsi_device_identifiers.assert_called_once_with( + mock.sentinel.local_wwnn, mock.sentinel.local_wwpn, + mock.sentinel.remote_wwpn, mock.sentinel.fcp_lun) + self._diskutils.get_disk_numbers_by_unique_id.assert_called_once_with( + unique_id=mock.sentinel.id, + unique_id_format=mock.sentinel.type)