openstack/cinder
Code Issues Proposed changes

nvmeof: Support new connection properties

Browse Source 
The os-brick nvmeof connector supports  2 different connection
properties formats, the original one and a later one that added support
to multiple portals as well as replicated devices for a software RAID.

Targets that inherit from the nvmeof base target (spdk and nvmet)
supported the original connection properties format, but it was decided
that new features, such as multipathing, would only be added to os-brick
in the new format code path.

This patch adds support to the nvmeof target class (and specifically to
the nvmet target, though it should work for other as well) for the new
connection properties format to enable support for future features.

Support for the old connection properties has been maintained, and is
still the default, since we need an easy way to exert the old code path
in os-brick to ensure that the code still works.

Configuration option ``nvmeof_conn_info_version`` is used to select what
version of the connection properties the nvmet target should use. With
version 1 being the old format and version 2 the new format. Defaults to
the old format to preserve existing behavior.

Change-Id: If3f7f66a5cd23604cc81a6973304db9f9018fdb3
This commit is contained in:
Gorka Eguileor 2022-03-08 17:18:48 +01:00
parent d892637112
commit 8e9a347c0e
9 changed files with 175 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/opts.py
View File

@ -279,6 +279,7 @@ def list_opts():
[cinder_volume_api.az_cache_time_opt],
cinder_volume_driver.volume_opts,
cinder_volume_driver.iser_opts,
cinder_volume_driver.nvmeof_opts,
cinder_volume_driver.nvmet_opts,
cinder_volume_driver.scst_opts,
cinder_volume_driver.backup_opts,
@ -324,6 +325,7 @@ def list_opts():
itertools.chain(
cinder_volume_driver.volume_opts,
cinder_volume_driver.iser_opts,
cinder_volume_driver.nvmeof_opts,
cinder_volume_driver.nvmet_opts,
cinder_volume_driver.scst_opts,
cinder_volume_driver.image_opts,

41
cinder/tests/unit/targets/test_nvmeof_driver.py
View File

@ -12,6 +12,7 @@
from unittest import mock
import ddt
from oslo_utils import timeutils
from cinder import context
@ -25,15 +26,11 @@ class FakeNVMeOFDriver(nvmeof.NVMeOF):
def __init__(self, *args, **kwargs):
super(FakeNVMeOFDriver, self).__init__(*args, **kwargs)
def create_nvmeof_target(
self, target_name, target_ip, target_port,
transport_type, ns_id, volume_path):
pass
def delete_nvmeof_target(self, target_name):
pass
@ddt.ddt
class TestNVMeOFDriver(tf.TargetDriverFixture):
def setUp(self):
@ -75,16 +72,18 @@ class TestNVMeOFDriver(tf.TargetDriverFixture):
'created_at': timeutils.utcnow(),
'host': 'fake_host@lvm#lvm'})
def test_initialize_connection(self):
@mock.patch.object(nvmeof.NVMeOF, '_get_connection_properties')
def test_initialize_connection(self, mock_get_conn):
mock_connector = {'initiator': 'fake_init'}
mock_testvol = self.testvol
expected_return = {
'driver_volume_type': 'nvmeof',
'data': self.target._get_connection_properties(mock_testvol)
'data': mock_get_conn.return_value
}
self.assertEqual(expected_return,
self.target.initialize_connection(mock_testvol,
mock_connector))
mock_get_conn.assert_called_once_with(mock_testvol)
@mock.patch.object(FakeNVMeOFDriver, 'create_nvmeof_target')
def test_create_export(self, mock_create_nvme_target):
@ -109,7 +108,8 @@ class TestNVMeOFDriver(tf.TargetDriverFixture):
self.testvol
)
def test_get_connection_properties(self):
def test__get_connection_properties_old(self):
"""Test connection properties with the old NVMe-oF format."""
expected_return = {
'target_portal': self.target_ip,
'target_port': str(self.target_port),
@ -121,6 +121,31 @@ class TestNVMeOFDriver(tf.TargetDriverFixture):
self.assertEqual(expected_return,
self.target._get_connection_properties(self.testvol))
@ddt.data(('rdma', 'RoCEv2'), ('tcp', 'tcp'))
@ddt.unpack
@mock.patch.object(nvmeof.NVMeOF, '_get_nvme_uuid')
def test__get_connection_properties_new(self, transport,
expected_transport, mock_uuid):
"""Test connection properties with the new NVMe-oF format."""
nqn = f'ngn.{self.nvmet_subsystem_name}-{self.fake_volume_id}'
vol = self.testvol.copy()
vol['provider_location'] = self.target.get_nvmeof_location(
nqn, self.target_ip, self.target_port, transport, self.nvmet_ns_id)
self.configuration.nvmeof_conn_info_version = 2
expected_return = {
'target_nqn': nqn,
'vol_uuid': mock_uuid.return_value,
'ns_id': str(self.nvmet_ns_id),
'portals': [(self.target_ip,
str(self.target_port),
expected_transport)],
}
self.assertEqual(expected_return,
self.target._get_connection_properties(vol))
mock_uuid.assert_called_once_with(vol)
def test_validate_connector(self):
mock_connector = {'initiator': 'fake_init'}
self.assertTrue(self.target.validate_connector(mock_connector))

103
cinder/tests/unit/targets/test_nvmet_driver.py
View File

@ -34,96 +34,79 @@ class TestNVMETDriver(tf.TargetDriverFixture):
configuration=self.configuration)
fake_nvmet_lib.reset_mock()
@mock.patch.object(nvmet.NVMET, 'create_nvmeof_target')
def test_create_export(self, mock_create_target):
"""Test that the nvmeof class calls the nvmet method."""
res = self.target.create_export(mock.sentinel.ctxt,
self.testvol,
mock.sentinel.volume_path)
self.assertEqual(mock_create_target.return_value, res)
mock_create_target.assert_called_once_with(
self.testvol['id'],
self.target.configuration.target_prefix,
self.target.target_ip,
self.target.target_port,
self.target.nvme_transport_type,
self.target.nvmet_port_id,
self.target.nvmet_ns_id,
mock.sentinel.volume_path)
@mock.patch.object(nvmet.NVMET, '_get_nvme_uuid')
@mock.patch.object(nvmet.NVMET, 'get_nvmeof_location')
@mock.patch.object(nvmet.NVMET, '_ensure_port_exports')
@mock.patch.object(nvmet.NVMET, '_ensure_subsystem_exists')
@mock.patch.object(nvmet.NVMET, '_get_target_nqn')
def test_create_nvmeof_target(self, mock_nqn, mock_subsys, mock_port,
mock_location):
def test_create_export(self, mock_nqn, mock_subsys, mock_port,
mock_location, mock_uuid):
"""Normal create target execution."""
mock_nqn.return_value = mock.sentinel.nqn
mock_uuid.return_value = mock.sentinel.uuid
vol = mock.Mock()
res = self.target.create_nvmeof_target(mock.sentinel.vol_id,
mock.sentinel.target_prefix,
mock.sentinel.target_ip,
mock.sentinel.target_port,
mock.sentinel.transport_type,
mock.sentinel.port_id,
mock.sentinel.ns_id,
res = self.target.create_export(mock.sentinel.context,
vol,
mock.sentinel.volume_path)
self.assertEqual({'location': mock_location.return_value, 'auth': ''},
res)
mock_nqn.assert_called_once_with(mock.sentinel.vol_id)
mock_nqn.assert_called_once_with(vol.id)
mock_uuid.assert_called_once_with(vol)
mock_subsys.assert_called_once_with(mock.sentinel.nqn,
mock.sentinel.ns_id,
mock.sentinel.volume_path)
self.target.nvmet_ns_id,
mock.sentinel.volume_path,
mock.sentinel.uuid)
mock_port.assert_called_once_with(mock.sentinel.nqn,
mock.sentinel.target_ip,
mock.sentinel.target_port,
mock.sentinel.transport_type,
mock.sentinel.port_id)
self.target.target_ip,
self.target.target_port,
self.target.nvme_transport_type,
self.target.nvmet_port_id)
mock_location.assert_called_once_with(mock.sentinel.nqn,
mock.sentinel.target_ip,
mock.sentinel.target_port,
mock.sentinel.transport_type,
mock.sentinel.ns_id)
self.target.target_ip,
self.target.target_port,
self.target.nvme_transport_type,
self.target.nvmet_ns_id)
@ddt.data((ValueError, None), (None, IndexError))
@ddt.unpack
@mock.patch.object(nvmet.NVMET, '_get_nvme_uuid')
@mock.patch.object(nvmet.NVMET, 'get_nvmeof_location')
@mock.patch.object(nvmet.NVMET, '_ensure_port_exports')
@mock.patch.object(nvmet.NVMET, '_ensure_subsystem_exists')
@mock.patch.object(nvmet.NVMET, '_get_target_nqn')
def test_create_nvmeof_target_error(self, subsys_effect, port_effect,
def test_create_export_error(self, subsys_effect, port_effect,
mock_nqn, mock_subsys, mock_port,
mock_location):
mock_location, mock_uuid):
"""Failing create target executing subsystem or port creation."""
mock_subsys.side_effect = subsys_effect
mock_port.side_effect = port_effect
mock_nqn.return_value = mock.sentinel.nqn
mock_uuid.return_value = mock.sentinel.uuid
vol = mock.Mock()
self.assertRaises(nvmet.NVMETTargetAddError,
self.target.create_nvmeof_target,
mock.sentinel.vol_id,
mock.sentinel.target_prefix,
mock.sentinel.target_ip,
mock.sentinel.target_port,
mock.sentinel.transport_type,
mock.sentinel.port_id,
mock.sentinel.ns_id,
self.target.create_export,
mock.sentinel.context,
vol,
mock.sentinel.volume_path)
mock_nqn.assert_called_once_with(mock.sentinel.vol_id)
mock_nqn.assert_called_once_with(vol.id)
mock_uuid.assert_called_once_with(vol)
mock_subsys.assert_called_once_with(mock.sentinel.nqn,
mock.sentinel.ns_id,
mock.sentinel.volume_path)
self.target.nvmet_ns_id,
mock.sentinel.volume_path,
mock.sentinel.uuid)
if subsys_effect:
mock_port.assert_not_called()
else:
mock_port.assert_called_once_with(mock.sentinel.nqn,
mock.sentinel.target_ip,
mock.sentinel.target_port,
mock.sentinel.transport_type,
mock.sentinel.port_id)
self.target.target_ip,
self.target.target_port,
self.target.nvme_transport_type,
self.target.nvmet_port_id)
mock_location.assert_not_called()
@mock.patch.object(priv_nvmet, 'Subsystem')
@ -131,7 +114,8 @@ class TestNVMETDriver(tf.TargetDriverFixture):
"""Skip subsystem creation if already exists."""
nqn = 'nqn.nvme-subsystem-1-uuid'
self.target._ensure_subsystem_exists(nqn, mock.sentinel.ns_id,
mock.sentinel.vol_path)
mock.sentinel.vol_path,
mock.sentinel.uuid)
mock_subsys.assert_called_once_with(nqn)
mock_subsys.setup.assert_not_called()
@ -143,12 +127,14 @@ class TestNVMETDriver(tf.TargetDriverFixture):
mock_uuid.return_value = 'uuid'
nqn = 'nqn.nvme-subsystem-1-uuid'
self.target._ensure_subsystem_exists(nqn, mock.sentinel.ns_id,
mock.sentinel.vol_path)
mock.sentinel.vol_path,
mock.sentinel.uuid)
mock_subsys.assert_called_once_with(nqn)
expected_section = {
'allowed_hosts': [],
'attr': {'allow_any_host': '1'},
'namespaces': [{'device': {'nguid': 'uuid',
'uuid': mock.sentinel.uuid,
'path': mock.sentinel.vol_path},
'enable': 1,
'nsid': mock.sentinel.ns_id}],
@ -265,3 +251,8 @@ class TestNVMETDriver(tf.TargetDriverFixture):
def test__get_target_nqn(self):
res = self.target._get_target_nqn('volume_id')
self.assertEqual('nqn.nvme-subsystem-1-volume_id', res)
def test__get_nvme_uuid(self):
vol = mock.Mock()
res = self.target._get_nvme_uuid(vol)
self.assertEqual(vol.name_id, res)

1
cinder/tests/unit/volume/drivers/test_spdk.py
View File

@ -504,6 +504,7 @@ class SpdkDriverTestCase(test.TestCase):
self.configuration.target_ip_address = "192.168.0.1"
self.configuration.target_port = 4420
self.configuration.target_prefix = "nqn.2014-08.io.spdk"
self.configuration.nvmeof_conn_info_version = 1
self.configuration.nvmet_port_id = "1"
self.configuration.nvmet_ns_id = "fake_id"
self.configuration.nvmet_subsystem_name = "2014-08.io.spdk"

13
cinder/volume/driver.py
View File

@ -263,6 +263,16 @@ iser_opts = [
help='The name of the iSER target user-land tool to use'),
]
nvmeof_opts = [
cfg.IntOpt('nvmeof_conn_info_version',
default=1,
min=1, max=2,
help='NVMe os-brick connector has 2 different connection info '
'formats, this allows some NVMe-oF drivers that use the '
'original format (version 1), such as spdk and LVM-nvmet, '
'to send the newer format.'),
]
nvmet_opts = [
cfg.PortOpt('nvmet_port_id',
default=1,
@ -348,11 +358,13 @@ fqdn_opts = [
CONF = cfg.CONF
CONF.register_opts(volume_opts, group=configuration.SHARED_CONF_GROUP)
CONF.register_opts(iser_opts, group=configuration.SHARED_CONF_GROUP)
CONF.register_opts(nvmeof_opts, group=configuration.SHARED_CONF_GROUP)
CONF.register_opts(nvmet_opts, group=configuration.SHARED_CONF_GROUP)
CONF.register_opts(scst_opts, group=configuration.SHARED_CONF_GROUP)
CONF.register_opts(image_opts, group=configuration.SHARED_CONF_GROUP)
CONF.register_opts(volume_opts)
CONF.register_opts(iser_opts)
CONF.register_opts(nvmeof_opts)
CONF.register_opts(nvmet_opts)
CONF.register_opts(scst_opts)
CONF.register_opts(backup_opts)
@ -414,6 +426,7 @@ class BaseVD(object, metaclass=abc.ABCMeta):
if self.configuration:
self.configuration.append_config_values(volume_opts)
self.configuration.append_config_values(iser_opts)
self.configuration.append_config_values(nvmeof_opts)
self.configuration.append_config_values(nvmet_opts)
self.configuration.append_config_values(scst_opts)
self.configuration.append_config_values(backup_opts)

2
cinder/volume/drivers/lvm.py
View File

@ -67,7 +67,7 @@ volume_opts = [
cfg.BoolOpt('lvm_suppress_fd_warnings',
default=False,
help='Suppress leaked file descriptor warnings in LVM '
'commands.')
'commands.'),
]
CONF = cfg.CONF

60
cinder/volume/targets/nvmeof.py
View File

@ -87,28 +87,68 @@ class NVMeOF(driver.Target):
def _get_connection_properties(self, volume):
"""Gets NVMeOF connection configuration.
:return: dictionary of the following keys:
:target_portal: NVMe target IP address
:target_port: NVMe target port
:nqn: NQN of the NVMe target
:transport_type: Network fabric being used for an
NVMe-over-Fabrics network
:ns_id: namespace id associated with the subsystem
"""
For nvmeof_conn_info_version set to 1 (default) the old format will
be sent:
{
'target_portal': NVMe target IP address
'target_port': NVMe target port
'nqn': NQN of the NVMe target
'transport_type': Network fabric being used for an NVMe-oF network
One of: tcp, rdma
'ns_id': namespace id associated with the subsystem
}
For nvmeof_conn_info_version set to 2 the new format will be sent:
{
'target_nqn': NQN of the NVMe target
'vol_uuid': NVMe-oF UUID of the volume. May be different than Cinder
volume id and may be None if ns_id is provided.
'portals': [(target_address, target_port, transport_type) ... ]
'ns_id': namespace id associated with the subsystem, in case target
doesn't provide the volume_uuid.
}
Unlike the old format the transport_type can be one of RoCEv2 and tcp
:return: dictionary with the connection properties using one of the 2
existing formats depending on the nvmeof_conn_info_version
configuration option.
"""
location = volume['provider_location']
target_connection, nvme_transport_type, nqn, nvmet_ns_id = (
location.split(' '))
target_portal, target_port = target_connection.split(':')
# NVMe-oF Connection Information Version 2
if self.configuration.nvmeof_conn_info_version == 2:
uuid = self._get_nvme_uuid(volume)
if nvme_transport_type == 'rdma':
nvme_transport_type = 'RoCEv2'
return {
'target_nqn': nqn,
'vol_uuid': uuid,
'portals': [(target_portal, target_port, nvme_transport_type)],
'ns_id': nvmet_ns_id,
}
# NVMe-oF Connection Information Version 1
return {
'target_portal': target_portal,
'target_port': target_port,
'nqn': nqn,
'transport_type': nvme_transport_type,
'ns_id': nvmet_ns_id
'ns_id': nvmet_ns_id,
}
def _get_nvme_uuid(self, volume):
"""Return the NVMe uuid of a given volume.
Targets that want to support the nvmeof_conn_info_version=2 option need
to override this method and return the NVMe uuid of the given volume.
"""
return None
def get_nvmeof_location(self, nqn, target_ip, target_port,
nvme_transport_type, nvmet_ns_id):
"""Serializes driver data into single line string."""
@ -150,7 +190,6 @@ class NVMeOF(driver.Target):
missing='initiator')
return True
@abc.abstractmethod
def create_nvmeof_target(self,
volume_id,
subsystem_name,
@ -160,6 +199,7 @@ class NVMeOF(driver.Target):
nvmet_port_id,
ns_id,
volume_path):
"""Targets that don't override create_export must implement this."""
pass
@abc.abstractmethod

39
cinder/volume/targets/nvmet.py
View File

@ -37,36 +37,33 @@ class NVMET(nvmeof.NVMeOF):
self._nvmet_root = nvmet.Root()
@utils.synchronized('nvmetcli', external=True)
def create_nvmeof_target(self,
volume_id,
subsystem_name, # Ignoring this, using config
target_ip,
target_port,
transport_type,
nvmet_port_id,
ns_id,
volume_path):
def create_export(self, context, volume, volume_path):
# Create NVME subsystem for previously created LV
nqn = self._get_target_nqn(volume_id)
nqn = self._get_target_nqn(volume.id)
try:
self._ensure_subsystem_exists(nqn, ns_id, volume_path)
self._ensure_port_exports(nqn, target_ip, target_port,
transport_type, nvmet_port_id)
uuid = self._get_nvme_uuid(volume)
self._ensure_subsystem_exists(nqn, self.nvmet_ns_id, volume_path,
uuid)
self._ensure_port_exports(nqn, self.target_ip, self.target_port,
self.nvme_transport_type,
self.nvmet_port_id)
except Exception:
LOG.error('Failed to add subsystem: %s', nqn)
raise NVMETTargetAddError(subsystem=nqn)
LOG.info('Subsystem %s now exported on port %s', nqn, target_port)
LOG.info('Subsystem %s now exported on port %s', nqn, self.target_port)
return {
'location': self.get_nvmeof_location(
nqn,
target_ip,
target_port,
transport_type,
ns_id),
self.target_ip,
self.target_port,
self.nvme_transport_type,
self.nvmet_ns_id),
'auth': ''}
def _ensure_subsystem_exists(self, nqn, nvmet_ns_id, volume_path):
def _ensure_subsystem_exists(self, nqn, nvmet_ns_id, volume_path, uuid):
# Assume if subsystem exists, it has the right configuration
try:
nvmet.Subsystem(nqn)
@ -84,6 +81,7 @@ class NVMET(nvmeof.NVMeOF):
{
"device": {
"nguid": str(uuidutils.generate_uuid()),
"uuid": uuid,
"path": volume_path,
},
"enable": 1,
@ -95,6 +93,9 @@ class NVMET(nvmeof.NVMeOF):
nvmet.Subsystem.setup(subsystem_section) # privsep
LOG.debug('Added subsystem: %s', nqn)
def _get_nvme_uuid(self, volume):
return volume.name_id
def _ensure_port_exports(self, nqn, addr, port, transport_type, port_id):
# Assume if port exists, it has the right configuration
try:

6
releasenotes/notes/lvm-nvmet-new-conn_props-25320e34d6ca6ac7.yaml Normal file
View File

@ -0,0 +1,6 @@
---
features:
- |
LVM nvmet target: Added support for new nvmeof connection properties
format (version 2). Controlled with ``nvmeof_conn_info_version``
configuration option.