openstack/os-brick
Code Issues Proposed changes

NVMe-oF: Improve hostnqn creation

Browse Source 
This patch improves the creation of the /etc/nvme/hostnqn file by using
the system UUID value we usually already know.

This saves us one or two calls to the nvme-cli command and it also
allows older nvme-cli versions that don't have the `show-hostnqn`
command or have it but can only read from file to generate the same
value every time, which may be useful when running inside a container
under some circumstances.

Change-Id: Ib250d213295695390dbdbb3506cb297a86e95218
This commit is contained in:
Gorka Eguileor 2023-09-14 18:38:59 +02:00
parent 7da3d82773
commit 088c676bf5
6 changed files with 91 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
os_brick/initiator/connectors/nvmeof.py
View File

@ -771,7 +771,7 @@ class NVMeOFConnector(base.BaseLinuxConnector):
uuid = nvmf._get_host_uuid()
suuid = priv_nvmeof.get_system_uuid()
if cls.nvme_present():
nqn = utils.get_host_nqn()
nqn = utils.get_host_nqn(suuid)
# Ensure /etc/nvme/hostid exists and defaults to the system uuid,
# or a random value.
hostid = utils.get_nvme_host_id(suuid)

60
os_brick/privileged/nvmeof.py
View File

@ -30,44 +30,54 @@ LOG = logging.getLogger(__name__)
@os_brick.privileged.default.entrypoint
def create_hostnqn() -> str:
def create_hostnqn(system_uuid: Optional[str] = None) -> str:
"""Create the hostnqn file to speed up finding out the nqn.
By having the /etc/nvme/hostnqn not only do we make sure that that value is
always used on this system, but we are also able to just open the file to
get the nqn on each get_connector_properties call instead of having to make
a call to nvme show-hostnqn command.
In newer nvme-cli versions calling show-hostnqn will not only try to
locate the file (which we know doesn't exist or this method wouldn't have
been called), but it will also generate one. In older nvme-cli versions
that is not the case.
"""
host_nqn = ''
try:
os.makedirs('/etc/nvme', mode=0o755, exist_ok=True)
# Try to get existing nqn generated from dmi or systemd
try:
host_nqn, err = rootwrap.custom_execute('nvme', 'show-hostnqn')
host_nqn = host_nqn.strip()
# If we have the system's unique uuid we can just write the file
if system_uuid:
host_nqn = 'nqn.2014-08.org.nvmexpress:uuid:' + system_uuid
else:
# Try to get existing nqn generated from dmi or systemd
try:
host_nqn, err = rootwrap.custom_execute('nvme', 'show-hostnqn')
host_nqn = host_nqn.strip()
# This is different from OSError's ENOENT, which is missing nvme
# command. This ENOENT is when nvme says there isn't an nqn.
except putils.ProcessExecutionError as e:
# nvme-cli's error are all over the place, so merge the output
err_msg = e.stdout + '\n' + e.stderr
msg = err_msg.casefold()
if 'error: invalid sub-command' in msg:
LOG.debug('Version too old cannot check current hostnqn.')
elif 'hostnqn is not available' in msg:
LOG.debug('Version too old to return hostnqn from non file '
'sources')
elif e.exit_code == errno.ENOENT:
LOG.debug('No nqn could be formed from dmi or systemd.')
else:
LOG.debug('Unknown error from nvme show-hostnqn: %s', err_msg)
raise
# This is different from OSError's ENOENT, which is missing nvme
# command. This ENOENT is when nvme says there isn't an nqn.
except putils.ProcessExecutionError as e:
# nvme-cli's error are all over the place, so merge the output
err_msg = e.stdout + '\n' + e.stderr
msg = err_msg.casefold()
if 'error: invalid sub-command' in msg:
LOG.debug('Version too old cannot check current hostnqn.')
elif 'hostnqn is not available' in msg:
LOG.debug('Version too old to return hostnqn from non '
'file sources')
elif e.exit_code == errno.ENOENT:
LOG.debug('No nqn could be formed from dmi or systemd.')
else:
LOG.debug('Unknown error from nvme show-hostnqn: %s',
err_msg)
raise
if not host_nqn:
LOG.debug('Generating nqn')
host_nqn, err = rootwrap.custom_execute('nvme', 'gen-hostnqn')
host_nqn = host_nqn.strip()
if not host_nqn:
LOG.debug('Generating nqn')
host_nqn, err = rootwrap.custom_execute('nvme', 'gen-hostnqn')
host_nqn = host_nqn.strip()
with open('/etc/nvme/hostnqn', 'w') as f:
LOG.debug('Writing hostnqn file')

26
os_brick/tests/initiator/connectors/test_nvmeof.py
View File

@ -945,6 +945,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
'nvme_hostid': SYS_UUID}
self.assertEqual(expected_props, props)
mock_get_host_id.assert_called_once_with(None)
mock_nqn.assert_called_once_with(None)
@mock.patch.object(utils, 'get_nvme_host_id',
return_value=SYS_UUID)
@ -971,6 +972,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
'nvme_hostid': SYS_UUID}
self.assertEqual(expected_props, props)
mock_get_host_id.assert_called_once_with(SYS_UUID)
mock_nqn.assert_called_once_with(SYS_UUID)
def test_get_volume_paths_device_info(self):
"""Device info path has highest priority."""
@ -1794,7 +1796,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
def test_get_host_nqn_file_available(self, mock_open):
mock_open.return_value.__enter__.return_value.read = (
lambda: HOST_NQN + "\n")
host_nqn = self._get_host_nqn()
host_nqn = utils.get_host_nqn()
mock_open.assert_called_once_with('/etc/nvme/hostnqn', 'r')
self.assertEqual(HOST_NQN, host_nqn)
@ -1805,7 +1807,17 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
mock_open.side_effect = IOError()
result = utils.get_host_nqn()
mock_open.assert_called_once_with('/etc/nvme/hostnqn', 'r')
mock_create.assert_called_once_with()
mock_create.assert_called_once_with(None)
self.assertEqual(mock.sentinel.nqn, result)
@mock.patch.object(utils.priv_nvme, 'create_hostnqn')
@mock.patch.object(builtins, 'open')
def test_get_host_nqn_io_err_sys_uuid(self, mock_open, mock_create):
mock_create.return_value = mock.sentinel.nqn
mock_open.side_effect = IOError()
result = utils.get_host_nqn(mock.sentinel.system_uuid)
mock_open.assert_called_once_with('/etc/nvme/hostnqn', 'r')
mock_create.assert_called_once_with(mock.sentinel.system_uuid)
self.assertEqual(mock.sentinel.nqn, result)
@mock.patch.object(utils.priv_nvme, 'create_hostnqn')
@ -1863,16 +1875,6 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
self.assertFalse(result)
mock_get_fs_type.assert_called_once_with(NVME_DEVICE_PATH)
def _get_host_nqn(self):
host_nqn = None
try:
with open('/etc/nvme/hostnqn', 'r') as f:
host_nqn = f.read().strip()
f.close()
except IOError:
host_nqn = HOST_NQN
return host_nqn
@ddt.data(True, False)
@mock.patch.object(nvmeof.NVMeOFConnector, 'native_multipath_supported',
None)

21
os_brick/tests/privileged/test_nvmeof.py
View File

@ -55,6 +55,27 @@ class PrivNVMeTestCase(base.TestCase):
mock_chmod.assert_called_once_with('/etc/nvme/hostnqn', 0o644)
self.assertEqual(stripped_hostnqn, res)
@mock.patch('os.chmod')
@mock.patch.object(builtins, 'open', new_callable=mock.mock_open)
@mock.patch('os.makedirs')
@mock.patch.object(rootwrap, 'custom_execute')
def test_create_hostnqn_from_system_uuid(self, mock_exec, mock_mkdirs,
mock_open, mock_chmod):
system_uuid = 'ea841a98-444c-4abb-bd99-092b20518542'
hostnqn = 'nqn.2014-08.org.nvmexpress:uuid:' + system_uuid
res = privsep_nvme.create_hostnqn(system_uuid)
mock_mkdirs.assert_called_once_with('/etc/nvme',
mode=0o755,
exist_ok=True)
mock_exec.assert_not_called()
mock_open.assert_called_once_with('/etc/nvme/hostnqn', 'w')
mock_open().write.assert_called_once_with(hostnqn)
mock_chmod.assert_called_once_with('/etc/nvme/hostnqn', 0o644)
self.assertEqual(hostnqn, res)
@mock.patch('os.chmod')
@mock.patch.object(builtins, 'open', new_callable=mock.mock_open)
@mock.patch('os.makedirs')

13
os_brick/utils.py
View File

@ -221,12 +221,21 @@ def convert_str(text: Union[bytes, str]) -> str:
return text
def get_host_nqn() -> Optional[str]:
def get_host_nqn(system_uuid: Optional[str] = None) -> Optional[str]:
"""Ensure that hostnqn exists, creating if necessary.
This method tries to return contents from /etc/nvme/hostnqn and if not
possible then creates the file calling create_hostnqn and passing provided
system_uuid and returns the contents of the newly created file.
Method create_hostnqn gives priority to the provided system_uuid parameter
for the contents of the file over other alternatives it has.
"""
try:
with open('/etc/nvme/hostnqn', 'r') as f:
host_nqn = f.read().strip()
except IOError:
host_nqn = priv_nvme.create_hostnqn()
host_nqn = priv_nvme.create_hostnqn(system_uuid)
except Exception:
host_nqn = None
return host_nqn

9
releasenotes/notes/nvmeof-faster-create-hostnqn-81a63844142858bf.yaml Normal file
View File

@ -0,0 +1,9 @@
---
features:
- |
NVMe-oF connector: Improved speed of creation of the ``/etc/nvme/hostnqn``
file.
- |
NVMe-oF connector: Always write the same value in the same system for the
``/etc/nvme/hostnqn`` file in older nvme-cli versions when system UUID can
be read from DMI.