diff --git a/os_brick/initiator/connectors/nvmeof.py b/os_brick/initiator/connectors/nvmeof.py index 2e10766f8..154b42337 100644 --- a/os_brick/initiator/connectors/nvmeof.py +++ b/os_brick/initiator/connectors/nvmeof.py @@ -196,10 +196,38 @@ class NVMeOFConnector(base.BaseLinuxConnector): raise exception.VolumePathsNotFound() return list(path) + @utils.retry(exception.VolumeDeviceNotFound) + def _get_device_path_by_nguid(self, nguid): + device_path = os.path.join(DEV_SEARCH_PATH, + 'disk', + 'by-id', + 'nvme-eui.%s' % nguid) + LOG.debug("Try to retrieve symlink to %(device_path)s.", + {"device_path": device_path}) + try: + paths, _err = self._execute('readlink', + '-e', + device_path, + run_as_root=True, + root_helper=self._root_helper) + if not paths: + raise exception.VolumePathsNotFound() + return paths.split()[0] + except putils.ProcessExecutionError as e: + LOG.exception(e) + raise exception.VolumeDeviceNotFound(device=device_path) + @utils.retry(putils.ProcessExecutionError) def _try_connect_nvme(self, cmd): - self._execute(*cmd, root_helper=self._root_helper, - run_as_root=True) + try: + self._execute(*cmd, root_helper=self._root_helper, + run_as_root=True) + except putils.ProcessExecutionError as e: + # Idempotent connection to target. + # Exit code 70 means that target is already connected. + if e.exit_code == 70: + return + raise def _get_nvme_subsys(self): # Example output: @@ -228,12 +256,25 @@ class NVMeOFConnector(base.BaseLinuxConnector): return ret_val + @staticmethod + def _filter_nvme_devices(current_nvme_devices, nvme_controller): + LOG.debug("Filter NVMe devices belonging to controller " + "%(nvme_controller)s.", + {"nvme_controller": nvme_controller}) + nvme_name_pattern = "/dev/%sn[0-9]+" % nvme_controller + nvme_devices_filtered = list( + filter( + lambda device: re.match(nvme_name_pattern, device), + current_nvme_devices + ) + ) + return nvme_devices_filtered + @utils.retry(exception.NotFound, retries=5) def _is_nvme_available(self, nvme_name): - nvme_name_pattern = "/dev/%sn[0-9]+" % nvme_name - for nvme_dev_name in self._get_nvme_devices(): - if re.match(nvme_name_pattern, nvme_dev_name): - return True + current_nvme_devices = self._get_nvme_devices() + if self._filter_nvme_devices(current_nvme_devices, nvme_name): + return True else: LOG.error("Failed to find nvme device") raise exception.NotFound() @@ -275,25 +316,6 @@ class NVMeOFConnector(base.BaseLinuxConnector): # Wait until nvme will be available in kernel return self._is_nvme_available(nvme_name) - def _try_disconnect_volume(self, conn_nqn, ignore_errors=False): - cmd = [ - 'nvme', - 'disconnect', - '-n', - conn_nqn] - try: - 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", {'conn_nqn': conn_nqn}) - if not ignore_errors: - raise - @utils.trace @synchronized('connect_volume') def connect_volume(self, connection_properties): @@ -312,13 +334,13 @@ class NVMeOFConnector(base.BaseLinuxConnector): return self._connect_volume_replicated(connection_properties) current_nvme_devices = self._get_nvme_devices() - device_info = {'type': 'block'} conn_nqn = connection_properties['nqn'] target_portal = connection_properties['target_portal'] port = connection_properties['target_port'] nvme_transport_type = connection_properties['transport_type'] host_nqn = connection_properties.get('host_nqn') + device_nguid = connection_properties.get('volume_nguid') cmd = [ 'nvme', 'connect', '-t', nvme_transport_type, @@ -334,20 +356,27 @@ class NVMeOFConnector(base.BaseLinuxConnector): target_portal, port) except exception.NotFound: LOG.error("Waiting for nvme failed") - self._try_disconnect_volume(conn_nqn, True) raise exception.NotFound(message="nvme connect: NVMe device " "not found") - path = self._get_device_path(current_nvme_devices) - device_info['path'] = path[0] + if device_nguid: + path = self._get_device_path_by_nguid(device_nguid) + else: + path = self._get_device_path(current_nvme_devices) + device_info['path'] = path LOG.debug("NVMe device to be connected to is %(path)s", - {'path': path[0]}) + {'path': path}) return device_info @utils.trace @synchronized('connect_volume') def disconnect_volume(self, connection_properties, device_info, force=False, ignore_errors=False): - """Detach and flush the volume. + """Flush the volume. + + Disconnect of volumes happens on storage system side. Here we could + remove connections to subsystems if no volumes are left. But new + volumes can pop up asynchronously in the meantime. So the only thing + left is flushing or disassembly of a correspondng RAID device. :param connection_properties: The dictionary that describes all of the target volume attributes. @@ -376,32 +405,11 @@ 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): + try: self._linuxscsi.flush_device_io(device_path) - - LOG.debug( - "Trying to disconnect from device %(device_path)s with " - "subnqn %(conn_nqn)s", - {'device_path': device_path, 'conn_nqn': conn_nqn}) - cmd = [ - 'nvme', - 'disconnect', - '-n', - conn_nqn] - 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) - - if exc: - LOG.warning('There were errors removing %s, leftovers may remain ' - 'in the system', device_path) + except putils.ProcessExecutionError: if not ignore_errors: - raise exc + raise @utils.trace @synchronized('extend_volume') @@ -413,9 +421,11 @@ class NVMeOFConnector(base.BaseLinuxConnector): """ if connection_properties.get('vol_uuid'): # compatibility return self._extend_volume_replicated(connection_properties) - volume_paths = self.get_volume_paths(connection_properties) if volume_paths: + if connection_properties.get('volume_nguid'): + for path in volume_paths: + return self._linuxscsi.get_device_size(path) return self._linuxscsi.extend_volume( volume_paths, use_multipath=self.use_multipath) else: diff --git a/os_brick/tests/initiator/connectors/test_nvmeof.py b/os_brick/tests/initiator/connectors/test_nvmeof.py index b06c0f962..8731b897c 100644 --- a/os_brick/tests/initiator/connectors/test_nvmeof.py +++ b/os_brick/tests/initiator/connectors/test_nvmeof.py @@ -31,6 +31,7 @@ EXECUTOR = executor.Executor(None) VOL_UUID = 'c20aba21-6ef6-446b-b374-45733b4883ba' NVME_DEVICE_PATH = '/dev/nvme1' NVME_NS_PATH = '/dev/nvme1n1' +NVME_DEVICE_NGUID = '4941ef7595b8ee978ccf096800f205c6' SYS_UUID = '9126E942-396D-11E7-B0B7-A81E84C186D1' HOST_UUID = 'c20aba21-6ef6-446b-b374-45733b4883ba' HOST_NQN = 'nqn.2014-08.org.nvmexpress:uuid:' \ @@ -48,6 +49,51 @@ connection_properties = { } fake_portal = ('fake', 'portal', 'tcp') +nvme_list_subsystems_stdout = """ + { + "Subsystems" : [ + { + "Name" : "nvme-subsys0", + "NQN" : "nqn.2016-06.io.spdk:cnode1" + }, + { + "Paths" : [ + { + "Name" : "nvme0", + "Transport" : "tcp", + "Address" : "traddr=10.0.2.15 trsvcid=4420" + } + ] + }, + { + "Name" : "nvme-subsys1", + "NQN" : "nqn.2016-06.io.spdk:cnode2" + }, + { + "Paths" : [ + { + "Name" : "nvme1", + "Transport" : "rdma", + "Address" : "traddr=10.0.2.16 trsvcid=4420" + }, + { + "Name" : "nvme2", + "Transport" : "rdma", + "Address" : "traddr=10.0.2.17 trsvcid=4420" + } + ] + } + ] + } +""" + +nvme_list_stdout = """ +Node SN Model Namespace Usage Format FW Rev +------------- ------- ----- --------- ---------------- ----------- ------- +/dev/nvme0n1 AB12345 s123 12682 0.00 B / 2.15 GB 512 B + 0 B 2.1.0.0 +/dev/nvme0n2 AB12345 s123 12683 0.00 B / 1.07 GB 512 B + 0 B 2.1.0.0 +""" + @ddt.ddt class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @@ -153,6 +199,41 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): connection_properties), [connection_properties['device_path']]) + @mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True) + def test__try_connect_nvme_idempotent(self, mock_execute): + cmd = [ + 'nvme', 'connect', + '-t', 'tcp', + '-n', TARGET_NQN, + '-a', 'portal', + '-s', 4420] + mock_execute.side_effect = putils.ProcessExecutionError(exit_code=70) + self.connector._try_connect_nvme(cmd) + mock_execute.assert_called_once_with(self.connector, + *cmd, + root_helper=None, + run_as_root=True) + + @mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True) + def test__get_device_path_by_nguid(self, mock_execute): + mock_execute.return_value = '/dev/nvme0n1\n', None + res = self.connector._get_device_path_by_nguid(NVME_DEVICE_NGUID) + self.assertEqual(res, '/dev/nvme0n1') + + @mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True) + def test__get_device_path_by_nguid_empty_response(self, mock_execute): + mock_execute.return_value = None, None + self.assertRaises(exception.VolumePathsNotFound, + self.connector._get_device_path_by_nguid, + NVME_DEVICE_NGUID) + + @mock.patch.object(nvmeof.NVMeOFConnector, '_execute', autospec=True) + def test__get_device_path_by_nguid_exception(self, mock_execute): + mock_execute.side_effect = putils.ProcessExecutionError() + self.assertRaises(exception.VolumeDeviceNotFound, + self.connector._get_device_path_by_nguid, + NVME_DEVICE_NGUID) + @mock.patch.object(nvmeof.NVMeOFConnector, '_connect_target_volume') def test_connect_volume_single_rep( self, mock_connect_target_volume): @@ -217,10 +298,10 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): @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): + def test_disconnect_volume_nova(self, mock_sleep, + mock_devices, + mock_flush): device = '/dev/nvme0n1' connection_properties = {'target_portal': 'portal', 'target_port': 1, @@ -228,23 +309,17 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): '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): + def test_disconnect_volume_cinder(self, mock_sleep, + mock_devices, + mock_flush): device = '/dev/nvme0n1' connection_properties = {'target_portal': 'portal', 'target_port': 1, @@ -258,55 +333,17 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): 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_devices, mock_flush): device = '/dev/nvme0n1' mock_flush.side_effect = putils.ProcessExecutionError - mock_execute.side_effect = putils.ProcessExecutionError - mock_devices.return_value = device + mock_devices.return_value = [device] connection_properties = {'target_portal': 'portal', 'target_port': 1, 'nqn': 'nqn.volume_123', @@ -319,11 +356,6 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): 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): @@ -424,6 +456,20 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): self.connector, ['mdadm', '--grow', '--size', 'max', device_path]) mock_device_size.assert_called_with(device_path) + @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size') + def test_extend_volume_with_nguid(self, mock_device_size): + device_path = '/dev/nvme0n1' + connection_properties = { + 'volume_nguid': NVME_DEVICE_NGUID, + 'device_path': device_path, + } + mock_device_size.return_value = 100 + self.assertEqual( + self.connector.extend_volume(connection_properties), + 100 + ) + mock_device_size.assert_called_with(device_path) + @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan') @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path') def test__connect_target_volume_with_connected_device( @@ -930,6 +976,86 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase): self.assertEqual(args[4], cmd[4]) self.assertEqual(args[5], cmd[5]) + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') + def test__is_nvme_available(self, mock_nvme_devices): + mock_nvme_devices.return_value = {'/dev/nvme0n1', + '/dev/nvme2n1', + '/dev/nvme2n2', + '/dev/nvme3n1'} + result = self.connector._is_nvme_available('nvme2') + self.assertTrue(result) + + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') + def test__is_nvme_available_wrong_name(self, mock_nvme_devices): + mock_nvme_devices.return_value = {'/dev/nvme0n1', + '/dev/nvme2n1', + '/dev/nvme2n2', + '/dev/nvme3n1'} + self.assertRaises(exception.NotFound, + self.connector._is_nvme_available, + 'nvme1') + + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') + def test__is_nvme_available_no_devices(self, mock_nvme_devices): + mock_nvme_devices.return_value = [] + self.assertRaises(exception.NotFound, + self.connector._is_nvme_available, + 'nvme1') + + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_devices') + def test__is_nvme_available_fail_to_get_devices(self, mock_nvme_devices): + mock_nvme_devices.side_effect = exception.CommandExecutionFailed() + self.assertRaises(exception.CommandExecutionFailed, + self.connector._is_nvme_available, + 'nvme1') + + @mock.patch.object(executor.Executor, '_execute') + def test__get_nvme_devices(self, mock_execute): + mock_execute.return_value = nvme_list_stdout, None + res = self.connector._get_nvme_devices() + self.assertEqual(set(res), {'/dev/nvme0n1', '/dev/nvme0n2'}) + + @mock.patch.object(executor.Executor, '_execute') + def test__get_nvme_devices_failed(self, mock_execute): + mock_execute.side_effect = putils.ProcessExecutionError() + self.assertRaises(exception.CommandExecutionFailed, + self.connector._get_nvme_devices) + + @mock.patch.object(nvmeof.NVMeOFConnector, '_is_nvme_available') + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_subsys') + def test__wait_for_blk(self, mock_nvme_subsys, mock_nvme_avail): + mock_nvme_subsys.return_value = nvme_list_subsystems_stdout, None + mock_nvme_avail.return_value = True + result = self.connector._wait_for_blk('rdma', + 'nqn.2016-06.io.spdk:cnode2', + '10.0.2.16', '4420') + self.assertTrue(result) + + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_subsys') + def test__wait_for_blk_cli_exception(self, mock_nvme_subsys): + mock_nvme_subsys.side_effect = putils.ProcessExecutionError() + self.assertRaises(putils.ProcessExecutionError, + self.connector._wait_for_blk, + 'rdma', + 'nqn.2016-06.io.spdk:cnode2', + '10.0.2.16', '4420') + + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_subsys') + def test__wait_for_blk_bad_json(self, mock_nvme_subsys): + mock_nvme_subsys.return_value = ".", None + result = self.connector._wait_for_blk('rdma', + 'nqn.2016-06.io.spdk:cnode2', + '10.0.2.16', '4420') + self.assertFalse(result) + + @mock.patch.object(nvmeof.NVMeOFConnector, '_get_nvme_subsys') + def test__wait_for_blk_ip_not_found(self, mock_nvme_subsys): + mock_nvme_subsys.return_value = nvme_list_subsystems_stdout, None + result = self.connector._wait_for_blk('rdma', + 'nqn.2016-06.io.spdk:cnode2', + '10.0.2.18', '4420') + self.assertFalse(result) + def _get_host_nqn(self): try: with open('/etc/nvme/hostnqn', 'r') as f: diff --git a/releasenotes/notes/nvmeof-multiple-volumes-within-subsystem-support-05879c1c3bdf52c9.yaml b/releasenotes/notes/nvmeof-multiple-volumes-within-subsystem-support-05879c1c3bdf52c9.yaml new file mode 100644 index 000000000..ebecd5d4b --- /dev/null +++ b/releasenotes/notes/nvmeof-multiple-volumes-within-subsystem-support-05879c1c3bdf52c9.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + NVMe-OF connector: Added support for storage systems presenting + multiple volumes within one NVMe subsystem. +