diff --git a/os_brick/initiator/connectors/fibre_channel.py b/os_brick/initiator/connectors/fibre_channel.py index 0e70f614c..50f404b27 100644 --- a/os_brick/initiator/connectors/fibre_channel.py +++ b/os_brick/initiator/connectors/fibre_channel.py @@ -220,8 +220,8 @@ class FibreChannelConnector(base.BaseLinuxConnector): {'device': device}) if os.path.exists(device) and self.check_valid_device(device): self.host_device = device - # get the /dev/sdX device. This is used - # to find the multipath device. + # get the /dev/sdX device. This variable is maintained to + # keep the same log output. self.device_name = os.path.realpath(device) raise loopingcall.LoopingCallDone() @@ -255,8 +255,11 @@ class FibreChannelConnector(base.BaseLinuxConnector): # see if the new drive is part of a multipath # device. If so, we'll use the multipath device. if self.use_multipath: + # Pass a symlink, not a real path, otherwise we'll get a real path + # back if we don't find a multipath and we'll return that to the + # caller, breaking Nova's encryption which requires a symlink. (device_path, multipath_id) = self._discover_mpath_device( - device_wwn, connection_properties, self.device_name) + device_wwn, connection_properties, self.host_device) if multipath_id: # only set the multipath_id if we found one device_info['multipath_id'] = multipath_id @@ -339,8 +342,8 @@ class FibreChannelConnector(base.BaseLinuxConnector): mpath_path = self._linuxscsi.find_multipath_device_path(wwn) if mpath_path: self._linuxscsi.flush_multipath_device(mpath_path) - device_info = self._linuxscsi.get_device_info(real_path) - devices.append(device_info) + dev_info = self._linuxscsi.get_device_info(real_path) + devices.append(dev_info) LOG.debug("devices to remove = %s", devices) self._remove_devices(connection_properties, devices, device_info) @@ -351,7 +354,15 @@ class FibreChannelConnector(base.BaseLinuxConnector): # all of them path_used = self._linuxscsi.get_dev_path(connection_properties, device_info) - was_multipath = '/pci-' not in path_used + # NOTE: Due to bug #1897787 device_info may have a real path for some + # single paths instead of a symlink as it should have, so it'll only + # be a multipath if it was a symlink (not real path) and it wasn't a + # single path symlink (those have filenames starting with pci-) + # We don't use os.path.islink in case the file is no longer there. + was_symlink = path_used.count(os.sep) > 2 + # We check for /pci because that's the value we return for single + # paths, whereas for multipaths we have multiple link formats. + was_multipath = '/pci-' not in path_used and was_symlink for device in devices: device_path = device['device'] flush = self._linuxscsi.requires_flush(device_path, diff --git a/os_brick/initiator/linuxscsi.py b/os_brick/initiator/linuxscsi.py index 3edf452be..c1498e5cb 100644 --- a/os_brick/initiator/linuxscsi.py +++ b/os_brick/initiator/linuxscsi.py @@ -262,16 +262,23 @@ class LinuxSCSI(executor.Executor): """ # No used path happens on failed attachs, when we don't care about # individual flushes. - # When Nova used a multipath we don't need to do individual flushes. - if not path_used or was_multipath: + if not path_used: return False - # We need to flush the single path that was used. - # For encrypted volumes the symlink has been replaced, so realpath - # won't return device under /dev but under /dev/disk/... path = os.path.realpath(path) path_used = os.path.realpath(path_used) - return path_used == path or '/dev' != os.path.split(path_used)[0] + + # Need to flush this device if we used this specific path. We check + # this before checking if it's multipath in case we don't detect it + # being multipath correctly (as in bug #1897787). + if path_used == path: + return True + + # We flush individual path if Nova didn't use a multipath and we + # replaced the symlink to a real device with a link to the decrypted + # DM. We know we replaced it because it doesn't link to /dev/XYZ, + # instead it maps to /dev/mapped/crypt-XYZ + return not was_multipath and '/dev' != os.path.split(path_used)[0] def remove_connection(self, devices_names, is_multipath, force=False, exc=None, path_used=None, was_multipath=False): diff --git a/os_brick/tests/initiator/connectors/test_fibre_channel.py b/os_brick/tests/initiator/connectors/test_fibre_channel.py index 7a01c6bee..fec678ece 100644 --- a/os_brick/tests/initiator/connectors/test_fibre_channel.py +++ b/os_brick/tests/initiator/connectors/test_fibre_channel.py @@ -474,6 +474,9 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase): find_mp_dev_mock, 'rw', False) self.assertNotIn('multipathd_id', connection_info['data']) + # Ensure we don't call it with the real path + device_name = discover_mp_dev_mock.call_args[0][-1] + self.assertNotEqual(realpath_mock.return_value, device_name) @mock.patch.object(fibre_channel.FibreChannelConnector, 'get_volume_paths') def test_extend_volume_no_path(self, mock_volume_paths): @@ -731,6 +734,8 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase): @ddt.data(('/dev/mapper/', True), ('/dev/mapper/mpath0', True), + # Check real devices are properly detected as non multipaths + ('/dev/sda', False), ('/dev/disk/by-path/pci-1-fc-1-lun-1', False)) @ddt.unpack @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.remove_scsi_device') diff --git a/os_brick/tests/initiator/test_linuxscsi.py b/os_brick/tests/initiator/test_linuxscsi.py index b4c02da24..32ee94e9b 100644 --- a/os_brick/tests/initiator/test_linuxscsi.py +++ b/os_brick/tests/initiator/test_linuxscsi.py @@ -1126,8 +1126,15 @@ loop0 0""" def test_get_dev_path_no_path(self, con_props, dev_info): self.assertEqual('', self.linuxscsi.get_dev_path(con_props, dev_info)) - @ddt.data(('/dev/sda', '/dev/sda', True, False, None), + @ddt.data(('/dev/sda', '/dev/sda', False, True, None), + # This checks that we ignore the was_multipath parameter if it + # doesn't make sense (because the used path is the one we are + # asking about) + ('/dev/sda', '/dev/sda', True, True, None), ('/dev/sda', '', True, False, None), + # Check for encrypted volume + ('/dev/link_sda', '/dev/disk/by-path/pci-XYZ', False, True, + ('/dev/sda', '/dev/mapper/crypt-pci-XYZ')), ('/dev/link_sda', '/dev/link_sdb', False, False, ('/dev/sda', '/dev/sdb')), ('/dev/link_sda', '/dev/link2_sda', False, True, ('/dev/sda', diff --git a/releasenotes/notes/fc-flush-single-path-22ed6cc7b56a6d9b.yaml b/releasenotes/notes/fc-flush-single-path-22ed6cc7b56a6d9b.yaml new file mode 100644 index 000000000..a26856d1b --- /dev/null +++ b/releasenotes/notes/fc-flush-single-path-22ed6cc7b56a6d9b.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + `Bug #1897787 `_: Fix + Fibre Channel not flushing volumes on detach when a multipath connection + was requested on their attach, but one was not found.