Add support for multiple volumes within subsystem to NVMe-OF connector

NVMe-OF connector has several issues with utilizing storages
which present multiple volumes within one NVMe subsystem.
Found issues are described in blueprint with more details.
This patch tries to address all of them without breaking
the existing functionality.

Co-Authored-By: Vladislav Belogrudov <vladislav.belogrudov@dell.com>
Implements: blueprint nvmeof-support-multiple-volumes-within-subsystem
Change-Id: I5f73ec4287f6ab2a0a5ae83748947fa56a0eeb9d
This commit is contained in:
Ivan Pchelintsev 2021-07-08 14:09:46 +03:00 committed by Vladislav Belogrudov
parent 13516073d5
commit 4edb7c641e
3 changed files with 255 additions and 113 deletions

View File

@ -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:

View File

@ -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:

View File

@ -0,0 +1,6 @@
---
features:
- |
NVMe-OF connector: Added support for storage systems presenting
multiple volumes within one NVMe subsystem.