FC: Fix not flushing on detach
When doing multipathed Fibre Channel we may end up not flushing the volume when disconnecting it if didn't find a multipath when doing the connection. The issue is caused by the `connect_volume` call returning a real path instead of a symlink and us passing the wrong device_info to the _remove_devices method, which makes the disconnect code incorrectly identify when a multipath was used to decide whether to do a flush or not. This patch fixes these things in order to flush the individual path when necessary. Closes-Bug: #1897787 Change-Id: Ie3266c81b3c6bb5a2c213a12410539d404d1febe
This commit is contained in:
parent
165c8017e4
commit
1432c369fb
|
@ -225,8 +225,8 @@ class FibreChannelConnector(base.BaseLinuxConnector):
|
||||||
{'device': device})
|
{'device': device})
|
||||||
if os.path.exists(device) and self.check_valid_device(device):
|
if os.path.exists(device) and self.check_valid_device(device):
|
||||||
self.host_device = device
|
self.host_device = device
|
||||||
# get the /dev/sdX device. This is used
|
# get the /dev/sdX device. This variable is maintained to
|
||||||
# to find the multipath device.
|
# keep the same log output.
|
||||||
self.device_name = os.path.realpath(device)
|
self.device_name = os.path.realpath(device)
|
||||||
raise loopingcall.LoopingCallDone()
|
raise loopingcall.LoopingCallDone()
|
||||||
|
|
||||||
|
@ -260,8 +260,11 @@ class FibreChannelConnector(base.BaseLinuxConnector):
|
||||||
# see if the new drive is part of a multipath
|
# see if the new drive is part of a multipath
|
||||||
# device. If so, we'll use the multipath device.
|
# device. If so, we'll use the multipath device.
|
||||||
if self.use_multipath:
|
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_path, multipath_id) = self._discover_mpath_device(
|
||||||
device_wwn, connection_properties, self.device_name)
|
device_wwn, connection_properties, self.host_device)
|
||||||
if multipath_id:
|
if multipath_id:
|
||||||
# only set the multipath_id if we found one
|
# only set the multipath_id if we found one
|
||||||
device_info['multipath_id'] = multipath_id
|
device_info['multipath_id'] = multipath_id
|
||||||
|
@ -344,8 +347,8 @@ class FibreChannelConnector(base.BaseLinuxConnector):
|
||||||
mpath_path = self._linuxscsi.find_multipath_device_path(wwn)
|
mpath_path = self._linuxscsi.find_multipath_device_path(wwn)
|
||||||
if mpath_path:
|
if mpath_path:
|
||||||
self._linuxscsi.flush_multipath_device(mpath_path)
|
self._linuxscsi.flush_multipath_device(mpath_path)
|
||||||
device_info = self._linuxscsi.get_device_info(real_path)
|
dev_info = self._linuxscsi.get_device_info(real_path)
|
||||||
devices.append(device_info)
|
devices.append(dev_info)
|
||||||
|
|
||||||
LOG.debug("devices to remove = %s", devices)
|
LOG.debug("devices to remove = %s", devices)
|
||||||
self._remove_devices(connection_properties, devices, device_info)
|
self._remove_devices(connection_properties, devices, device_info)
|
||||||
|
@ -356,7 +359,15 @@ class FibreChannelConnector(base.BaseLinuxConnector):
|
||||||
# all of them
|
# all of them
|
||||||
path_used = self._linuxscsi.get_dev_path(connection_properties,
|
path_used = self._linuxscsi.get_dev_path(connection_properties,
|
||||||
device_info)
|
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:
|
for device in devices:
|
||||||
device_path = device['device']
|
device_path = device['device']
|
||||||
flush = self._linuxscsi.requires_flush(device_path,
|
flush = self._linuxscsi.requires_flush(device_path,
|
||||||
|
|
|
@ -270,16 +270,23 @@ class LinuxSCSI(executor.Executor):
|
||||||
"""
|
"""
|
||||||
# No used path happens on failed attachs, when we don't care about
|
# No used path happens on failed attachs, when we don't care about
|
||||||
# individual flushes.
|
# individual flushes.
|
||||||
# When Nova used a multipath we don't need to do individual flushes.
|
if not path_used:
|
||||||
if not path_used or was_multipath:
|
|
||||||
return False
|
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 = os.path.realpath(path)
|
||||||
path_used = os.path.realpath(path_used)
|
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,
|
def remove_connection(self, devices_names, is_multipath, force=False,
|
||||||
exc=None, path_used=None, was_multipath=False):
|
exc=None, path_used=None, was_multipath=False):
|
||||||
|
|
|
@ -475,6 +475,9 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
|
||||||
find_mp_dev_mock, 'rw', False)
|
find_mp_dev_mock, 'rw', False)
|
||||||
|
|
||||||
self.assertNotIn('multipathd_id', connection_info['data'])
|
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')
|
@mock.patch.object(fibre_channel.FibreChannelConnector, 'get_volume_paths')
|
||||||
def test_extend_volume_no_path(self, mock_volume_paths):
|
def test_extend_volume_no_path(self, mock_volume_paths):
|
||||||
|
@ -732,6 +735,8 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
|
||||||
|
|
||||||
@ddt.data(('/dev/mapper/<WWN>', True),
|
@ddt.data(('/dev/mapper/<WWN>', True),
|
||||||
('/dev/mapper/mpath0', 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))
|
('/dev/disk/by-path/pci-1-fc-1-lun-1', False))
|
||||||
@ddt.unpack
|
@ddt.unpack
|
||||||
@mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.remove_scsi_device')
|
@mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.remove_scsi_device')
|
||||||
|
|
|
@ -1138,8 +1138,15 @@ loop0 0"""
|
||||||
def test_get_dev_path_no_path(self, con_props, dev_info):
|
def test_get_dev_path_no_path(self, con_props, dev_info):
|
||||||
self.assertEqual('', self.linuxscsi.get_dev_path(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),
|
('/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/link_sda', '/dev/link_sdb', False, False, ('/dev/sda',
|
||||||
'/dev/sdb')),
|
'/dev/sdb')),
|
||||||
('/dev/link_sda', '/dev/link2_sda', False, True, ('/dev/sda',
|
('/dev/link_sda', '/dev/link2_sda', False, True, ('/dev/sda',
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
`Bug #1897787 <https://bugs.launchpad.net/cinder/+bug/1897787>`_: Fix
|
||||||
|
Fibre Channel not flushing volumes on detach when a multipath connection
|
||||||
|
was requested on their attach, but one was not found.
|
Loading…
Reference in New Issue