From 891abdff2f56be724a272641790eea173e977686 Mon Sep 17 00:00:00 2001 From: Yingxin Date: Fri, 23 Nov 2018 20:58:08 +0800 Subject: [PATCH] Support RSD scenario of nvme connector * When execute `nvme connect`, "host-nqn" is needed in RSD environment. So add it as an optional parameter. * It is observed that the first `nvme list` can miss the newly connected nvme device, right after a success call of `nvme connect`. The solution is to add the retry&sleep logic back. * Sometimes there can be more than 10 nvme devices, e.g. "/dev/nvme10n10". Need to update the device re pattern. * In RSD environment, the connector needs additional information like "system uuid", to let cinder-volume know the nova-cpu service is running on which RSD node. * Add a new protocol "NVMEOF" mapping to allow both Cinder and Nova to identify and use the nvme connector. Implements: blueprint nvme-volume-driver Change-Id: I26e3dc140b2cf30a97665679fdc4d2f897cd4872 --- os_brick/initiator/__init__.py | 1 + os_brick/initiator/connector.py | 2 + os_brick/initiator/connectors/nvme.py | 67 ++++++++++--- .../tests/initiator/connectors/test_nvme.py | 98 ++++++++++++++++++- os_brick/tests/initiator/test_connector.py | 4 + .../nvme-rsd-support-d487afd77c534fa1.yaml | 4 + 6 files changed, 160 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/nvme-rsd-support-d487afd77c534fa1.yaml diff --git a/os_brick/initiator/__init__.py b/os_brick/initiator/__init__.py index 0f49a9747..d93e5ec18 100644 --- a/os_brick/initiator/__init__.py +++ b/os_brick/initiator/__init__.py @@ -63,3 +63,4 @@ GPFS = "GPFS" VERITAS_HYPERSCALE = "VERITAS_HYPERSCALE" STORPOOL = "STORPOOL" NVME = "NVME" +NVMEOF = "NVMEOF" diff --git a/os_brick/initiator/connector.py b/os_brick/initiator/connector.py index b25ec8d80..d9fb5d422 100644 --- a/os_brick/initiator/connector.py +++ b/os_brick/initiator/connector.py @@ -116,6 +116,8 @@ _connector_mapping_linux = { 'os_brick.initiator.connectors.storpool.StorPoolConnector', initiator.NVME: 'os_brick.initiator.connectors.nvme.NVMeConnector', + initiator.NVMEOF: + 'os_brick.initiator.connectors.nvme.NVMeConnector', } # Mapping for the S390X platform diff --git a/os_brick/initiator/connectors/nvme.py b/os_brick/initiator/connectors/nvme.py index 83951c6c1..c5fadeceb 100644 --- a/os_brick/initiator/connectors/nvme.py +++ b/os_brick/initiator/connectors/nvme.py @@ -43,10 +43,39 @@ class NVMeConnector(base.BaseLinuxConnector): device_scan_attempts=device_scan_attempts, *args, **kwargs) + def _get_system_uuid(self): + # RSD requires system_uuid to let Cinder RSD Driver identify + # Nova node for later RSD volume attachment. + try: + out, err = self._execute("dmidecode", + root_helper=self._root_helper, + run_as_root=True) + if err: + LOG.warning("dmidecode execute error: %s", err) + return "" + for line in out.split("\n"): + line = line.strip() + if line.startswith("UUID:"): + uuid = line.split(" ")[1] + LOG.debug("got system uuid: %s", uuid) + return uuid + LOG.warning("Cannot get system uuid from %s", out) + return "" + except putils.ProcessExecutionError as e: + LOG.warning("Unable to locate dmidecode. For Cinder RSD Backend, " + "please make sure it is installed: %s", e) + return "" + @staticmethod def get_connector_properties(root_helper, *args, **kwargs): """The NVMe connector properties.""" - return {} + nvme = NVMeConnector(root_helper=root_helper, + execute=kwargs.get('execute')) + uuid = nvme._get_system_uuid() + if uuid: + return {"system uuid": uuid} + else: + return {} def get_search_path(self): return '/dev' @@ -58,7 +87,8 @@ class NVMeConnector(base.BaseLinuxConnector): def _get_nvme_devices(self): nvme_devices = [] - pattern = r'/dev/nvme[0-9]n[0-9]' + # match nvme devices like /dev/nvme10n10 + pattern = r'/dev/nvme[0-9]+n[0-9]+' cmd = ['nvme', 'list'] for retry in range(1, self.device_scan_attempts + 1): try: @@ -83,6 +113,16 @@ class NVMeConnector(base.BaseLinuxConnector): "when running nvme list.") raise exception.CommandExecutionFailed(message=msg) + @utils.retry(exceptions=exception.VolumePathsNotFound) + def _get_device_path(self, current_nvme_devices): + all_nvme_devices = self._get_nvme_devices() + LOG.debug("all_nvme_devices are %(all_nvme_devices)s", + {'all_nvme_devices': all_nvme_devices}) + path = set(all_nvme_devices) - set(current_nvme_devices) + if not path: + raise exception.VolumePathsNotFound() + return list(path) + @utils.trace @synchronized('connect_volume') def connect_volume(self, connection_properties): @@ -105,12 +145,15 @@ class NVMeConnector(base.BaseLinuxConnector): 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') cmd = [ 'nvme', 'connect', '-t', nvme_transport_type, '-n', conn_nqn, '-a', target_portal, '-s', port] + if host_nqn: + cmd.extend(['-q', host_nqn]) try: self._execute(*cmd, root_helper=self._root_helper, run_as_root=True) @@ -120,14 +163,7 @@ class NVMeConnector(base.BaseLinuxConnector): "%(conn_nqn)s", {'conn_nqn': conn_nqn}) raise - all_nvme_devices = self._get_nvme_devices() - path = set(all_nvme_devices) - set(current_nvme_devices) - path = list(path) - if not path: - raise exception.TargetPortalNotFound(target_portal) - - LOG.debug("all_nvme_devices are %(all_nvme_devices)s", - {'all_nvme_devices': all_nvme_devices}) + path = self._get_device_path(current_nvme_devices) device_info['path'] = path[0] LOG.debug("NVMe device to be connected to is %(path)s", {'path': path[0]}) @@ -135,7 +171,8 @@ class NVMeConnector(base.BaseLinuxConnector): @utils.trace @synchronized('disconnect_volume') - def disconnect_volume(self, connection_properties, device_info): + def disconnect_volume(self, connection_properties, device_info, + force=False, ignore_errors=False): """Detach and flush the volume. :param connection_properties: The dictionary that describes all @@ -150,7 +187,10 @@ class NVMeConnector(base.BaseLinuxConnector): """ conn_nqn = connection_properties['nqn'] - device_path = connection_properties['device_path'] + if device_info and device_info.get('path'): + device_path = device_info.get('path') + else: + device_path = connection_properties['device_path'] or '' current_nvme_devices = self._get_nvme_devices() if device_path not in current_nvme_devices: LOG.warning("Trying to disconnect device %(device_path)s that " @@ -177,7 +217,8 @@ class NVMeConnector(base.BaseLinuxConnector): "Failed to disconnect from NVMe nqn " "%(conn_nqn)s with device_path %(device_path)s", {'conn_nqn': conn_nqn, 'device_path': device_path}) - raise + if not ignore_errors: + raise @utils.trace @synchronized('extend_volume') diff --git a/os_brick/tests/initiator/connectors/test_nvme.py b/os_brick/tests/initiator/connectors/test_nvme.py index 93d942917..0630acf33 100644 --- a/os_brick/tests/initiator/connectors/test_nvme.py +++ b/os_brick/tests/initiator/connectors/test_nvme.py @@ -26,6 +26,8 @@ Node SN Model \ - --------- -------------------------- ---------------- --------\n /dev/nvme0n1 67ff9467da6e5567 Linux \ 10 1.07 GB / 1.07 GB 512 B + 0 B 4.8.0-58\n +/dev/nvme11n12 fecc8e73584753d7 Linux \ + 1 3.22 GB / 3.22 GB 512 B + 0 B 4.8.0-56\n """ @@ -38,11 +40,28 @@ class NVMeConnectorTestCase(test_connector.ConnectorTestCase): self.connector = nvme.NVMeConnector(None, execute=self.fake_execute) + @mock.patch.object(nvme.NVMeConnector, '_execute') + def test_get_connector_properties_without_sysuuid( + self, mock_execute): + mock_execute.side_effect = putils.ProcessExecutionError + props = self.connector.get_connector_properties('sudo') + expected_props = {} + self.assertEqual(expected_props, props) + + @mock.patch.object(nvme.NVMeConnector, '_get_system_uuid') + def test_get_connector_properties_with_sysuuid( + self, mock_sysuuid): + mock_sysuuid.return_value = "9126E942-396D-11E7-B0B7-A81E84C186D1" + props = self.connector.get_connector_properties('sudo') + expected_props = { + "system uuid": "9126E942-396D-11E7-B0B7-A81E84C186D1"} + self.assertEqual(expected_props, props) + def _nvme_list_cmd(self, *args, **kwargs): return FAKE_NVME_LIST_OUTPUT, None def test__get_nvme_devices(self): - expected = ['/dev/nvme0n1'] + expected = ['/dev/nvme0n1', '/dev/nvme11n12'] self.connector._execute = self._nvme_list_cmd actual = self.connector._get_nvme_devices() self.assertEqual(expected, actual) @@ -83,6 +102,38 @@ class NVMeConnectorTestCase(test_connector.ConnectorTestCase): root_helper=None, run_as_root=True) + @mock.patch.object(nvme.NVMeConnector, '_get_nvme_devices') + @mock.patch.object(nvme.NVMeConnector, '_execute') + @mock.patch('time.sleep') + def test_connect_volume_hostnqn( + self, mock_sleep, mock_execute, mock_devices): + connection_properties = {'target_portal': 'portal', + 'target_port': 1, + 'nqn': 'nqn.volume_123', + 'device_path': '', + 'transport_type': 'rdma', + 'host_nqn': 'nqn.host_456'} + + mock_devices.side_effect = [ + ['/dev/nvme0n1'], ['/dev/nvme0n2']] + + device_info = self.connector.connect_volume( + connection_properties) + self.assertEqual('/dev/nvme0n2', device_info['path']) + self.assertEqual('block', device_info['type']) + + self.assertEqual(2, mock_devices.call_count) + + mock_execute.assert_called_once_with( + 'nvme', 'connect', + '-t', connection_properties['transport_type'], + '-n', connection_properties['nqn'], + '-a', connection_properties['target_portal'], + '-s', connection_properties['target_port'], + '-q', connection_properties['host_nqn'], + root_helper=None, + run_as_root=True) + @mock.patch.object(nvme.NVMeConnector, '_execute') @mock.patch('time.sleep') def test_connect_volume_raise(self, mock_sleep, mock_execute): @@ -109,14 +160,34 @@ class NVMeConnectorTestCase(test_connector.ConnectorTestCase): mock_devices.return_value = '/dev/nvme0n1' - self.assertRaises(exception.TargetPortalNotFound, + self.assertRaises(exception.VolumePathsNotFound, self.connector.connect_volume, connection_properties) @mock.patch.object(nvme.NVMeConnector, '_get_nvme_devices') @mock.patch.object(nvme.NVMeConnector, '_execute') @mock.patch('time.sleep') - def test_disconnect_volume(self, mock_sleep, mock_execute, mock_devices): + def test_connect_volume_nvmelist_retry_success( + self, mock_sleep, mock_execute, mock_devices): + connection_properties = {'target_portal': 'portal', + 'target_port': 1, + 'nqn': 'nqn.volume_123', + 'device_path': '', + 'transport_type': 'rdma'} + mock_devices.side_effect = [ + ['/dev/nvme0n1'], + ['/dev/nvme0n1'], + ['/dev/nvme0n1', '/dev/nvme0n2']] + device_info = self.connector.connect_volume( + connection_properties) + self.assertEqual('/dev/nvme0n2', device_info['path']) + self.assertEqual('block', device_info['type']) + + @mock.patch.object(nvme.NVMeConnector, '_get_nvme_devices') + @mock.patch.object(nvme.NVMeConnector, '_execute') + @mock.patch('time.sleep') + def test_disconnect_volume_nova( + self, mock_sleep, mock_execute, mock_devices): connection_properties = {'target_portal': 'portal', 'target_port': 1, 'nqn': 'nqn.volume_123', @@ -131,6 +202,27 @@ class NVMeConnectorTestCase(test_connector.ConnectorTestCase): root_helper=None, run_as_root=True) + @mock.patch.object(nvme.NVMeConnector, '_get_nvme_devices') + @mock.patch.object(nvme.NVMeConnector, '_execute') + @mock.patch('time.sleep') + def test_disconnect_volume_cinder( + self, mock_sleep, mock_execute, mock_devices): + connection_properties = {'target_portal': 'portal', + 'target_port': 1, + 'nqn': 'nqn.volume_123', + 'transport_type': 'rdma'} + device_info = {'path': '/dev/nvme0n1'} + mock_devices.return_value = '/dev/nvme0n1' + self.connector.disconnect_volume(connection_properties, + device_info, + ignore_errors=True) + + mock_execute.asert_called_once_with( + 'nvme', 'disconnect', '-n', + 'volume_123', + root_helper=None, + run_as_root=True) + @mock.patch.object(nvme.NVMeConnector, '_get_nvme_devices') @mock.patch.object(nvme.NVMeConnector, '_execute') @mock.patch('time.sleep') diff --git a/os_brick/tests/initiator/test_connector.py b/os_brick/tests/initiator/test_connector.py index c90d00f66..d518cab1e 100644 --- a/os_brick/tests/initiator/test_connector.py +++ b/os_brick/tests/initiator/test_connector.py @@ -23,6 +23,7 @@ from os_brick.initiator import connector from os_brick.initiator.connectors import base from os_brick.initiator.connectors import fake from os_brick.initiator.connectors import iscsi +from os_brick.initiator.connectors import nvme from os_brick.initiator import linuxfc from os_brick.privileged import rootwrap as priv_rootwrap from os_brick.tests import base as test_base @@ -33,6 +34,8 @@ FAKE_SCSI_WWN = '1234567890' class ConnectorUtilsTestCase(test_base.TestCase): + @mock.patch.object(nvme.NVMeConnector, '_get_system_uuid', + return_value=None) @mock.patch.object(iscsi.ISCSIConnector, 'get_initiator', return_value='fakeinitiator') @mock.patch.object(linuxfc.LinuxFibreChannel, 'get_fc_wwpns', @@ -46,6 +49,7 @@ class ConnectorUtilsTestCase(test_base.TestCase): multipath_result, mock_wwnns, mock_wwpns, mock_initiator, + mock_sysuuid, host='fakehost'): props_actual = connector.get_connector_properties('sudo', MY_IP, diff --git a/releasenotes/notes/nvme-rsd-support-d487afd77c534fa1.yaml b/releasenotes/notes/nvme-rsd-support-d487afd77c534fa1.yaml new file mode 100644 index 000000000..6c2d34653 --- /dev/null +++ b/releasenotes/notes/nvme-rsd-support-d487afd77c534fa1.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Extended nvme connector to support RSD with NVMe-oF.