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
This commit is contained in:
parent
1b2e229542
commit
85dbd0f5d7
|
@ -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