NVMe-oF: Flush on disconnect
The current NVMe-oF connector code in os-brick doesn't flush on
disconnect, as it just calls
nvme disconnect -n <conn_nqn>
Which doesn't flush on the `nvme` code or the kernel code.
This patch adds the normal block device ioctl flush call:
blockdev --flushbufs /dev/nvme0n1
The patch also adds and extends disconnect unit tests that were removed
when we added the new NVMe connector code (were not removed when we
added old code back).
Closes-Bug: #1903032
Change-Id: Ie6c8bdf1f3a629206da019405aee11c802a6a9bb
(cherry picked from commit 85dbd0f5d7)
This commit is contained in:
Gorka Eguileor
parent
0cd58a9b0a
commit
0c7f5fcfe5
|
|
@ -370,6 +370,10 @@ class NVMeOFConnector(base.BaseLinuxConnector):
|
|||
{'device_path': device_path, 'conn_nqn': conn_nqn})
|
||||
return
|
||||
|
||||
exc = exception.ExceptionChainer()
|
||||
with exc.context(force, 'Flushing %s failed', device_path):
|
||||
self._linuxscsi.flush_device_io(device_path)
|
||||
|
||||
LOG.debug(
|
||||
"Trying to disconnect from device %(device_path)s with "
|
||||
"subnqn %(conn_nqn)s",
|
||||
|
|
@ -379,19 +383,19 @@ class NVMeOFConnector(base.BaseLinuxConnector):
|
|||
'disconnect',
|
||||
'-n',
|
||||
conn_nqn]
|
||||
try:
|
||||
with exc.context(force, "Failed to disconnect from NVMe nqn "
|
||||
"%(conn_nqn)s with device_path %(device_path)s",
|
||||
{'conn_nqn': conn_nqn, 'device_path': device_path}):
|
||||
self._execute(
|
||||
*cmd,
|
||||
root_helper=self._root_helper,
|
||||
run_as_root=True)
|
||||
|
||||
except putils.ProcessExecutionError:
|
||||
LOG.error(
|
||||
"Failed to disconnect from NVMe nqn "
|
||||
"%(conn_nqn)s with device_path %(device_path)s",
|
||||
{'conn_nqn': conn_nqn, 'device_path': device_path})
|
||||
if exc:
|
||||
LOG.warning('There were errors removing %s, leftovers may remain '
|
||||
'in the system', device_path)
|
||||
if not ignore_errors:
|
||||
raise
|
||||
raise exc
|
||||
|
||||
@utils.trace
|
||||
@synchronized('extend_volume')
|
||||
|
|
|
|||
|
|
@ -192,6 +192,117 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
|
|||
self.assertRaises(exception.VolumeDeviceNotFound,
|
||||
self.connector.connect_volume, connection_properties)
|
||||
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'flush_device_io', autospec=True)
|
||||
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices',
|
||||
autospec=True)
|
||||
@mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True)
|
||||
@mock.patch('os_brick.utils._time_sleep')
|
||||
def test_disconnect_volume_nova(self, mock_sleep, mock_execute,
|
||||
mock_devices, mock_flush):
|
||||
device = '/dev/nvme0n1'
|
||||
connection_properties = {'target_portal': 'portal',
|
||||
'target_port': 1,
|
||||
'nqn': 'nqn.volume_123',
|
||||
'device_path': device,
|
||||
'transport_type': 'rdma'}
|
||||
mock_devices.return_value = [device]
|
||||
|
||||
self.connector.disconnect_volume(connection_properties, None)
|
||||
|
||||
mock_flush.assert_called_once_with(mock.ANY, device)
|
||||
mock_execute.assert_called_once_with(
|
||||
self.connector,
|
||||
'nvme', 'disconnect', '-n', 'nqn.volume_123',
|
||||
root_helper=None,
|
||||
run_as_root=True)
|
||||
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'flush_device_io', autospec=True)
|
||||
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices',
|
||||
autospec=True)
|
||||
@mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True)
|
||||
@mock.patch('os_brick.utils._time_sleep')
|
||||
def test_disconnect_volume_cinder(self, mock_sleep, mock_execute,
|
||||
mock_devices, mock_flush):
|
||||
device = '/dev/nvme0n1'
|
||||
connection_properties = {'target_portal': 'portal',
|
||||
'target_port': 1,
|
||||
'nqn': 'nqn.volume_123',
|
||||
'transport_type': 'rdma'}
|
||||
device_info = {'path': device}
|
||||
mock_devices.return_value = [device]
|
||||
|
||||
self.connector.disconnect_volume(connection_properties,
|
||||
device_info,
|
||||
ignore_errors=True)
|
||||
|
||||
mock_flush.assert_called_once_with(mock.ANY, device)
|
||||
mock_execute.assert_called_once_with(
|
||||
self.connector,
|
||||
'nvme', 'disconnect', '-n', 'nqn.volume_123',
|
||||
root_helper=None,
|
||||
run_as_root=True)
|
||||
|
||||
@ddt.data({'force': False, 'expected': putils.ProcessExecutionError},
|
||||
{'force': True, 'expected': exception.ExceptionChainer})
|
||||
@ddt.unpack
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'flush_device_io', autospec=True)
|
||||
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices',
|
||||
autospec=True)
|
||||
@mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True)
|
||||
@mock.patch('os_brick.utils._time_sleep')
|
||||
def test_disconnect_volume_raise(self, mock_sleep, mock_execute,
|
||||
mock_devices, mock_flush,
|
||||
force, expected):
|
||||
device = '/dev/nvme0n1'
|
||||
mock_execute.side_effect = putils.ProcessExecutionError
|
||||
mock_devices.return_value = device
|
||||
connection_properties = {'target_portal': 'portal',
|
||||
'target_port': 1,
|
||||
'nqn': 'nqn.volume_123',
|
||||
'device_path': device,
|
||||
'transport_type': 'rdma'}
|
||||
|
||||
self.assertRaises(expected,
|
||||
self.connector.disconnect_volume,
|
||||
connection_properties,
|
||||
None, force)
|
||||
mock_flush.assert_called_once_with(mock.ANY, device)
|
||||
mock_execute.assert_called_once_with(
|
||||
self.connector,
|
||||
'nvme', 'disconnect', '-n', 'nqn.volume_123',
|
||||
root_helper=None,
|
||||
run_as_root=True)
|
||||
|
||||
@mock.patch.object(linuxscsi.LinuxSCSI, 'flush_device_io', autospec=True)
|
||||
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices',
|
||||
autospec=True)
|
||||
@mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True)
|
||||
@mock.patch('os_brick.utils._time_sleep')
|
||||
def test_disconnect_volume_force_ignore_errors(self, mock_sleep,
|
||||
mock_execute, mock_devices,
|
||||
mock_flush):
|
||||
device = '/dev/nvme0n1'
|
||||
mock_flush.side_effect = putils.ProcessExecutionError
|
||||
mock_execute.side_effect = putils.ProcessExecutionError
|
||||
mock_devices.return_value = device
|
||||
connection_properties = {'target_portal': 'portal',
|
||||
'target_port': 1,
|
||||
'nqn': 'nqn.volume_123',
|
||||
'device_path': device,
|
||||
'transport_type': 'rdma'}
|
||||
|
||||
res = self.connector.disconnect_volume(connection_properties,
|
||||
None,
|
||||
force=True,
|
||||
ignore_errors=True)
|
||||
self.assertIsNone(res)
|
||||
mock_flush.assert_called_once_with(mock.ANY, device)
|
||||
mock_execute.assert_called_once_with(
|
||||
self.connector,
|
||||
'nvme', 'disconnect', '-n', 'nqn.volume_123',
|
||||
root_helper=None,
|
||||
run_as_root=True)
|
||||
|
||||
@mock.patch.object(nvmeof.NVMeOFConnector, '_get_fs_type')
|
||||
def test_disconnect_unreplicated_volume_nova(self, mock_get_fs_type):
|
||||
connection_properties = {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
NVMe-oF connector `bug #1903032
|
||||
<https://bugs.launchpad.net/os-brick/+bug/1903032>`_: Fixed not flushing
|
||||
device on disconnect.
|
||||
Loading…
Reference in New Issue