diff --git a/os_brick/initiator/connectors/nvmeof.py b/os_brick/initiator/connectors/nvmeof.py index 5d3259b9e..844f80608 100644 --- a/os_brick/initiator/connectors/nvmeof.py +++ b/os_brick/initiator/connectors/nvmeof.py @@ -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') diff --git a/os_brick/tests/initiator/connectors/test_nvmeof.py b/os_brick/tests/initiator/connectors/test_nvmeof.py index 573137563..e5de73f03 100644 --- a/os_brick/tests/initiator/connectors/test_nvmeof.py +++ b/os_brick/tests/initiator/connectors/test_nvmeof.py @@ -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 = { diff --git a/releasenotes/notes/nvme-flush-f31ab337224e5d3d.yaml b/releasenotes/notes/nvme-flush-f31ab337224e5d3d.yaml new file mode 100644 index 000000000..7f034f04d --- /dev/null +++ b/releasenotes/notes/nvme-flush-f31ab337224e5d3d.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + NVMe-oF connector `bug #1903032 + `_: Fixed not flushing + device on disconnect.