openstack/cinder
Code Issues Proposed changes

Merge "nvmeof: Support new connection properties"

Browse Source
This commit is contained in:
Zuul
2023-02-18 19:18:20 +00:00
committed by Gerrit Code Review
parent 9bdf1a5135 8e9a347c0e
commit e6637b94fa
9 changed files with 175 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/opts.py
View File

@@ -283,6 +283,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,
@@ -329,6 +330,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))

107
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,
mock.sentinel.volume_path)
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,
mock_nqn, mock_subsys, mock_port,
mock_location):
def test_create_export_error(self, subsys_effect, port_effect,
mock_nqn, mock_subsys, mock_port,
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.