From 5c4f2858b446cad19960f0cee12d04d9e281dbe0 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 19 Oct 2026 14:03:07 +0000 Subject: [PATCH 1/7] nvme: Fix byte order of the sanitize overwrite pattern Command dwords of passthru commands are passed in CPU byte order, the kernel takes care of the conversion. Converting the pattern to little endian beforehand swapped it twice on big endian architectures. Co-Authored-By: Claude Opus 5.5 --- src/plugins/nvme/nvme-op.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/nvme/nvme-op.c b/src/plugins/nvme/nvme-op.c index 96272222..b42d4eda 100644 --- a/src/plugins/nvme/nvme-op.c +++ b/src/plugins/nvme/nvme-op.c @@ -363,7 +363,7 @@ gboolean bd_nvme_sanitize (const gchar *device, BDNVMESanitizeAction action, gbo .owpass = overwrite_pass_count, .oipbp = overwrite_invert_pattern, .nodas = no_dealloc, - .ovrpat = GUINT32_TO_LE (overwrite_pattern), + .ovrpat = overwrite_pattern, }; switch (action) { From dd990caec790abab779a2156bc2d4dae04dcbf2c Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 19 Oct 2026 14:10:04 +0000 Subject: [PATCH 2/7] tests: Look up NVMe devices in sysfs The 'nvme list' JSON output differs between nvme-cli versions and changed again in nvme-cli 3.0. Read the controllers and namespaces of a subsystem from sysfs instead, which works independent of the installed nvme-cli version. The kernel scans namespaces asynchronously after the controller went live, allow waiting for the expected number of namespaces to show up. Co-Authored-By: Claude Opus 5.5 --- tests/nvme_test.py | 2 +- tests/utils.py | 113 ++++++++++++++++++--------------------------- 2 files changed, 46 insertions(+), 69 deletions(-) diff --git a/tests/nvme_test.py b/tests/nvme_test.py index a785ec82..349b4a0e 100644 --- a/tests/nvme_test.py +++ b/tests/nvme_test.py @@ -431,7 +431,7 @@ def test_connect_multiple_ns(self): for c in ctrls: self.assertTrue(re.match(r'/dev/nvme[0-9]+', c)) self.assertTrue(os.path.exists(c)) - namespaces = find_nvme_ns_devs_for_subnqn(self.SUBNQN) + namespaces = find_nvme_ns_devs_for_subnqn(self.SUBNQN, num_expected=NUM_NS) self.assertEqual(len(namespaces), NUM_NS) for ns in namespaces: self.assertTrue(re.match(r'/dev/nvme[0-9]+n[0-9]+', ns)) diff --git a/tests/utils.py b/tests/utils.py index 847415fc..14d1e052 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -10,7 +10,6 @@ import unittest import time import sys -import json import uuid from contextlib import contextmanager from enum import Enum @@ -313,95 +312,73 @@ def find_nvme_ctrl_devs_for_subnqn(subnqn, wait_for_ready=True): :param bool wait_for_ready: whether to wait for controllers to be ready (default: True) """ - def _check_subsys(subsys, dev_paths): - if subsys['SubsystemNQN'] == subnqn: - for ctrl in subsys['Controllers']: - if ctrl['Transport'] != 'loop': - continue - path = os.path.join('/dev/', ctrl['Controller']) - try: - st = os.lstat(path) - # nvme controller node is a character device - if stat.S_ISCHR(st.st_mode): - dev_paths += [path] - except: - pass - # Wait for controllers to be ready if requested if wait_for_ready: _wait_for_nvme_controllers_ready(subnqn) - ret, out, err = run_command("nvme list --output-format=json --verbose") - if ret != 0: - raise RuntimeError("Error getting NVMe list: '%s %s'" % (out, err)) - - decoder = json.JSONDecoder() - decoded = decoder.decode(out) - if not decoded or 'Devices' not in decoded: - return [] + # read sysfs directly, the 'nvme list' JSON output differs between nvme-cli versions dev_paths = [] - for dev in decoded['Devices']: - # nvme-cli 2.x - if 'Subsystems' in dev: - for subsys in dev['Subsystems']: - _check_subsys(subsys, dev_paths) - # nvme-cli 1.x - if 'SubsystemNQN' in dev: - _check_subsys(dev, dev_paths) + for ctrl_path in sorted(glob.glob("/sys/class/nvme/nvme*")): + try: + if read_file(os.path.join(ctrl_path, "subsysnqn")).strip() != subnqn: + continue + if read_file(os.path.join(ctrl_path, "transport")).strip() != "loop": + continue + path = os.path.join('/dev/', os.path.basename(ctrl_path)) + # nvme controller node is a character device + if stat.S_ISCHR(os.lstat(path).st_mode): + dev_paths += [path] + except: + pass return dev_paths -def find_nvme_ns_devs_for_subnqn(subnqn, wait_for_ready=True): +def find_nvme_ns_devs_for_subnqn(subnqn, wait_for_ready=True, num_expected=None, timeout=5): """ Find NVMe namespace block devices for the specified subsystem nqn :param str subnqn: subsystem nqn :param bool wait_for_ready: whether to wait for controllers to be ready (default: True) + :param int num_expected: wait until this many namespaces show up (default: don't wait) + :param int timeout: timeout in seconds when waiting for @num_expected namespaces (default: 5) """ - def _check_namespaces(node, ns_dev_paths): - for ns in node['Namespaces']: - path = os.path.join('/dev/', ns['NameSpace']) - try: - st = os.lstat(path) - if stat.S_ISBLK(st.st_mode): - ns_dev_paths += [path] - except: - pass - - def _check_subsys(subsys, ns_dev_paths): - if subsys['SubsystemNQN'] == subnqn: - if 'Namespaces' in subsys: - _check_namespaces(subsys, ns_dev_paths) - # kernel 4.18 - if 'Controllers' in subsys: - for ctrl in subsys['Controllers']: - if 'Namespaces' in ctrl: - _check_namespaces(ctrl, ns_dev_paths) - if wait_for_ready: _wait_for_nvme_controllers_ready(subnqn) - ret, out, err = run_command("nvme list --output-format=json --verbose") - if ret != 0: - raise RuntimeError("Error getting NVMe list: '%s %s'" % (out, err)) - decoder = json.JSONDecoder() - decoded = decoder.decode(out) - if not decoded or 'Devices' not in decoded: - return [] + # the kernel scans namespaces asynchronously after the controller went live + start_time = time.time() + ns_dev_paths = _find_nvme_ns_devs_for_subnqn(subnqn) + while num_expected is not None and len(ns_dev_paths) < num_expected and time.time() - start_time < timeout: + time.sleep(0.5) + os.system("udevadm settle") + ns_dev_paths = _find_nvme_ns_devs_for_subnqn(subnqn) + + return ns_dev_paths + +def _find_nvme_ns_devs_for_subnqn(subnqn): + # namespaces are listed under the subsystem with native NVMe multipath + # and under the controllers otherwise + parents = glob.glob("/sys/class/nvme-subsystem/nvme-subsys*") + glob.glob("/sys/class/nvme/nvme*") ns_dev_paths = [] - for dev in decoded['Devices']: - # nvme-cli 2.x - if 'Subsystems' in dev: - for subsys in dev['Subsystems']: - _check_subsys(subsys, ns_dev_paths) - # nvme-cli 1.x - if 'SubsystemNQN' in dev: - _check_subsys(dev, ns_dev_paths) + for parent in parents: + try: + if read_file(os.path.join(parent, "subsysnqn")).strip() != subnqn: + continue + except: + continue + for ns_path in glob.glob(os.path.join(parent, "nvme*n*")): + path = os.path.join('/dev/', os.path.basename(ns_path)) + try: + # skips the hidden per-path nvmeXcYnZ nodes + if stat.S_ISBLK(os.lstat(path).st_mode) and path not in ns_dev_paths: + ns_dev_paths += [path] + except: + pass - return ns_dev_paths + return sorted(ns_dev_paths) def get_nvme_hostnqn(): From 1b779d56e9d07dc50fa40838abe7ee27118baecd Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 19 Oct 2026 14:12:53 +0000 Subject: [PATCH 3/7] nvme: Move all libnvme calls behind an internal backend API libnvme-1.x is replaced by libnvme3 shipped with nvme-cli 3.x, which comes with a reworked API. As libblockdev is often backported to older distributions, both libraries need to be supported for a while. Instead of sprinkling #ifdefs over the plugin, introduce an internal backend API (nvme-backend.h) and move all the libnvme calls to a libnvme-1.x implementation of it (nvme-libnvme1.c). The generic code only uses the NVMe specification types, constants and static inline helpers from the libnvme headers, which are compatible between both libraries. The few constants renamed in libnvme3 to follow the NVMe specification are mapped in nvme-libnvme1.h. The backend returns negative errno values on failure, as libnvme3 does, instead of -1 with errno set. The extra arguments of bd_nvme_connect() are parsed by the generic code, and the topology is passed as a snapshot to the generic lookup functions. No functional change. Co-Authored-By: Claude Opus 5.5 --- src/plugins/nvme/Makefile.am | 3 + src/plugins/nvme/nvme-backend.h | 175 +++++++++++++ src/plugins/nvme/nvme-error.c | 20 +- src/plugins/nvme/nvme-fabrics.c | 402 +++++++++++------------------ src/plugins/nvme/nvme-info.c | 150 +++++------ src/plugins/nvme/nvme-libnvme1.c | 428 +++++++++++++++++++++++++++++++ src/plugins/nvme/nvme-libnvme1.h | 39 +++ src/plugins/nvme/nvme-op.c | 152 +++++------ src/plugins/nvme/nvme-private.h | 6 +- src/plugins/nvme/nvme.c | 2 - 10 files changed, 936 insertions(+), 441 deletions(-) create mode 100644 src/plugins/nvme/nvme-backend.h create mode 100644 src/plugins/nvme/nvme-libnvme1.c create mode 100644 src/plugins/nvme/nvme-libnvme1.h diff --git a/src/plugins/nvme/Makefile.am b/src/plugins/nvme/Makefile.am index a1e8d5c9..1f29f959 100644 --- a/src/plugins/nvme/Makefile.am +++ b/src/plugins/nvme/Makefile.am @@ -11,6 +11,9 @@ libbd_nvme_la_SOURCES = \ nvme.h \ nvme.c \ nvme-private.h \ + nvme-backend.h \ + nvme-libnvme1.h \ + nvme-libnvme1.c \ nvme-info.c \ nvme-error.c \ nvme-op.c \ diff --git a/src/plugins/nvme/nvme-backend.h b/src/plugins/nvme/nvme-backend.h new file mode 100644 index 00000000..93754390 --- /dev/null +++ b/src/plugins/nvme/nvme-backend.h @@ -0,0 +1,175 @@ +/* + * Copyright (C) 2026 SUSE LLC + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + * + * Author: Daniel Wagner + */ + +/* + * Internal API between the NVMe plugin and the libnvme library backend. + * + * The plugin is built against exactly one libnvme flavour. The generic code + * (nvme-info.c, nvme-op.c, nvme-fabrics.c, nvme-error.c) only uses the NVMe + * specification types, constants and static inline helpers provided by the + * libnvme headers. Every call into the library itself goes through the + * functions declared here, implemented by nvme-libnvme1.c. + * + * Unless stated otherwise, functions returning int return 0 on success, + * a negative errno value on failure or a positive NVMe status value. + */ + +#include +#include + +#include "nvme-libnvme1.h" + +#ifndef BD_NVME_BACKEND +#define BD_NVME_BACKEND + +/* opaque handle of an open NVMe controller or namespace device */ +typedef struct _NVMeDev NVMeDev; + +/* device access */ +G_GNUC_INTERNAL +NVMeDev *_nvme_be_open (const gchar *device, GError **error); +G_GNUC_INTERNAL +void _nvme_be_close (NVMeDev *dev); +G_GNUC_INTERNAL +int _nvme_be_get_nsid (NVMeDev *dev, __u32 *nsid); + +/* admin commands */ +G_GNUC_INTERNAL +int _nvme_be_identify_ctrl (NVMeDev *dev, struct nvme_id_ctrl *ctrl_id); +G_GNUC_INTERNAL +int _nvme_be_identify_ns (NVMeDev *dev, __u32 nsid, struct nvme_id_ns *ns_info); +G_GNUC_INTERNAL +int _nvme_be_identify_ns_descs (NVMeDev *dev, __u32 nsid, struct nvme_ns_id_desc *descs); +G_GNUC_INTERNAL +int _nvme_be_identify_independent_ns (NVMeDev *dev, __u32 nsid, struct nvme_id_independent_id_ns *ns_info); +G_GNUC_INTERNAL +int _nvme_be_get_log_smart (NVMeDev *dev, struct nvme_smart_log *smart_log); +G_GNUC_INTERNAL +int _nvme_be_get_log_error (NVMeDev *dev, guint entries, struct nvme_error_log_page *err_log); +G_GNUC_INTERNAL +int _nvme_be_get_log_self_test (NVMeDev *dev, struct nvme_self_test_log *self_test_log); +G_GNUC_INTERNAL +int _nvme_be_get_log_sanitize (NVMeDev *dev, struct nvme_sanitize_log_page *sanitize_log); +G_GNUC_INTERNAL +int _nvme_be_self_test (NVMeDev *dev, __u32 nsid, __u8 stc); +G_GNUC_INTERNAL +int _nvme_be_format (NVMeDev *dev, __u32 nsid, __u8 lbaf, __u8 ses); +G_GNUC_INTERNAL +int _nvme_be_sanitize (NVMeDev *dev, __u8 sanact, gboolean ause, __u8 owpass, gboolean oipbp, gboolean nodas, __u32 ovrpat); + +/* kernel notifications after a format */ +G_GNUC_INTERNAL +int _nvme_be_rescan_ns (NVMeDev *dev); +G_GNUC_INTERNAL +int _nvme_be_update_block_size (NVMeDev *dev, int block_size); +G_GNUC_INTERNAL +int _nvme_be_reread_partitions (NVMeDev *dev); + +/* strings */ +G_GNUC_INTERNAL +const gchar *_nvme_be_status_to_string (int status, gboolean fabrics); +G_GNUC_INTERNAL +const gchar *_nvme_be_fabrics_errno_to_string (int err); +G_GNUC_INTERNAL +gchar *_nvme_be_uuid_to_string (unsigned char uuid[NVME_UUID_LEN]); + +/* host identity, all return newly allocated strings or %NULL when not available */ +G_GNUC_INTERNAL +gchar *_nvme_be_read_hostnqn (void); +G_GNUC_INTERNAL +gchar *_nvme_be_read_hostid (void); +G_GNUC_INTERNAL +gchar *_nvme_be_generate_hostnqn (void); + +/* topology snapshot */ +typedef struct { + gchar *name; /* kernel name, e.g. "nvme0" */ + gchar *sysfs_dir; + gchar **ns_sysfs_dirs; /* private namespaces of the controller */ +} NVMeTopoCtrl; + +typedef struct { + gchar *host_nqn; + gchar *host_id; + gchar *subsysnqn; + gchar **ns_sysfs_dirs; /* shared (multipath) namespaces of the subsystem */ + NVMeTopoCtrl **ctrls; /* %NULL-terminated */ +} NVMeTopoSubsys; + +/* Returns a %NULL-terminated array of subsystems, as seen by all hosts. + * A topology scan failure is logged and results in an incomplete array. */ +G_GNUC_INTERNAL +NVMeTopoSubsys **_nvme_be_scan_topology (void); +G_GNUC_INTERNAL +void _nvme_topo_free (NVMeTopoSubsys **topo); + +/* fabrics */ +typedef struct { + gboolean set; + gint64 val; +} NVMeOptInt; + +typedef struct { + gboolean set; + gboolean val; +} NVMeOptBool; + +typedef struct { + const gchar *subsysnqn; + const gchar *transport; + const gchar *transport_addr; + const gchar *transport_svcid; + const gchar *host_traddr; + const gchar *host_iface; + const gchar *host_nqn; + const gchar *host_id; + + /* the extra arguments, %NULL when not specified */ + gboolean use_config; + const gchar *config_file; /* %NULL for the libnvme default */ + const gchar *hostkey; + const gchar *ctrlkey; + const gchar *hostsymname; + const gchar *keyring; + const gchar *tls_key; + NVMeOptInt nr_io_queues; + NVMeOptInt nr_write_queues; + NVMeOptInt nr_poll_queues; + NVMeOptInt queue_size; + NVMeOptInt keep_alive_tmo; + NVMeOptInt reconnect_delay; + NVMeOptInt ctrl_loss_tmo; + NVMeOptInt fast_io_fail_tmo; + NVMeOptInt tos; + NVMeOptBool duplicate_connect; + NVMeOptBool disable_sqflow; + NVMeOptBool hdr_digest; + NVMeOptBool data_digest; + NVMeOptBool tls; +} NVMeConnectParams; + +G_GNUC_INTERNAL +gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error); + +/* Disconnects all controllers matching @subsysnqn and/or the kernel controller @name. + * Sets @found when at least one matching controller has been disconnected. */ +G_GNUC_INTERNAL +gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, gboolean *found, GError **error); + +#endif /* BD_NVME_BACKEND */ diff --git a/src/plugins/nvme/nvme-error.c b/src/plugins/nvme/nvme-error.c index 4bd4d771..f06b4c31 100644 --- a/src/plugins/nvme/nvme-error.c +++ b/src/plugins/nvme/nvme-error.c @@ -27,12 +27,11 @@ #include #include -#include - #include #include #include "nvme.h" #include "nvme-private.h" +#include "nvme-backend.h" /** @@ -55,8 +54,8 @@ void _nvme_status_to_error (gint status, gboolean fabrics, GError **error) } if (status < 0) { - /* generic errno errors */ - switch (errno) { + /* generic errno errors, returned as negative values */ + switch (-status) { case EWOULDBLOCK: code = BD_NVME_ERROR_BUSY; break; @@ -64,7 +63,7 @@ void _nvme_status_to_error (gint status, gboolean fabrics, GError **error) code = BD_NVME_ERROR_FAILED; } g_set_error_literal (error, BD_NVME_ERROR, code, - strerror_l (errno, _C_LOCALE)); + strerror_l (-status, _C_LOCALE)); return; } else { /* NVMe status codes */ @@ -87,15 +86,16 @@ void _nvme_status_to_error (gint status, gboolean fabrics, GError **error) default: code = BD_NVME_ERROR_SC_GENERIC; } - g_set_error_literal (error, BD_NVME_ERROR, code, nvme_status_to_string (status, fabrics)); + g_set_error_literal (error, BD_NVME_ERROR, code, _nvme_be_status_to_string (status, fabrics)); return; } g_warn_if_reached (); } -void _nvme_fabrics_errno_to_gerror (int result, int _errno, GError **error) +void _nvme_fabrics_errno_to_gerror (int result, GError **error) { gint code; + int _errno = -result; if (error == NULL) return; @@ -143,17 +143,17 @@ void _nvme_fabrics_errno_to_gerror (int result, int _errno, GError **error) default: code = BD_NVME_ERROR_CONNECT; } - g_set_error_literal (error, BD_NVME_ERROR, code, nvme_errno_to_string (_errno)); + g_set_error_literal (error, BD_NVME_ERROR, code, _nvme_be_fabrics_errno_to_string (_errno)); return; } else { - switch (errno) { + switch (_errno) { case EWOULDBLOCK: code = BD_NVME_ERROR_BUSY; break; default: code = BD_NVME_ERROR_FAILED; } - g_set_error_literal (error, BD_NVME_ERROR, code, strerror_l (errno, _C_LOCALE)); + g_set_error_literal (error, BD_NVME_ERROR, code, strerror_l (_errno, _C_LOCALE)); return; } g_warn_if_reached (); diff --git a/src/plugins/nvme/nvme-fabrics.c b/src/plugins/nvme/nvme-fabrics.c index 796b6bfc..772edd26 100644 --- a/src/plugins/nvme/nvme-fabrics.c +++ b/src/plugins/nvme/nvme-fabrics.c @@ -29,20 +29,37 @@ #include #include -#include - #include #include #include "nvme.h" #include "nvme-private.h" +#include "nvme-backend.h" -/* nvme-cli defaults */ -#define PATH_NVMF_CONFIG "/etc/nvme/config.json" -#define MAX_DISC_RETRIES 10 +void _nvme_topo_free (NVMeTopoSubsys **topo) { + NVMeTopoSubsys **s; + NVMeTopoCtrl **c; + if (!topo) + return; + for (s = topo; *s; s++) { + for (c = (*s)->ctrls; *c; c++) { + g_free ((*c)->name); + g_free ((*c)->sysfs_dir); + g_strfreev ((*c)->ns_sysfs_dirs); + g_free (*c); + } + g_free ((*s)->ctrls); + g_free ((*s)->host_nqn); + g_free ((*s)->host_id); + g_free ((*s)->subsysnqn); + g_strfreev ((*s)->ns_sysfs_dirs); + g_free (*s); + } + g_free (topo); +} -static void parse_extra_args (const BDExtraArg **extra, struct nvme_fabrics_config *cfg, const gchar **config_file, const gchar **hostkey, const gchar **ctrlkey, const gchar **hostsymname) { +static void parse_extra_args (const BDExtraArg **extra, NVMeConnectParams *params) { const BDExtraArg **extra_i; if (!extra) @@ -53,97 +70,92 @@ static void parse_extra_args (const BDExtraArg **extra, struct nvme_fabrics_conf gchar *endptr = NULL; \ \ v = g_ascii_strtoll ((*extra_i)->val, &endptr, 0); \ - if (endptr != (*extra_i)->val) \ - target = v; \ + if (endptr != (*extra_i)->val) { \ + target.val = v; \ + target.set = TRUE; \ + } \ } #define SAFE_BOOL_CONV(target) { \ if (g_ascii_strcasecmp ((*extra_i)->val, "on") == 0 || \ g_ascii_strcasecmp ((*extra_i)->val, "1") == 0 || \ - g_ascii_strcasecmp ((*extra_i)->val, "true") == 0) \ - target = TRUE; \ - else \ + g_ascii_strcasecmp ((*extra_i)->val, "true") == 0) { \ + target.val = TRUE; \ + target.set = TRUE; \ + } else \ if (g_ascii_strcasecmp ((*extra_i)->val, "off") == 0 || \ g_ascii_strcasecmp ((*extra_i)->val, "0") == 0 || \ - g_ascii_strcasecmp ((*extra_i)->val, "false") == 0) \ - target = FALSE; \ + g_ascii_strcasecmp ((*extra_i)->val, "false") == 0) { \ + target.val = FALSE; \ + target.set = TRUE; \ + } \ } for (extra_i = extra; *extra_i; extra_i++) { - if (g_strcmp0 ((*extra_i)->opt, "config") == 0 && config_file) { - if (g_ascii_strcasecmp ((*extra_i)->val, "none") == 0) - *config_file = NULL; - else - *config_file = (*extra_i)->val; + if (g_strcmp0 ((*extra_i)->opt, "config") == 0) { + if (g_ascii_strcasecmp ((*extra_i)->val, "none") == 0) { + params->use_config = FALSE; + params->config_file = NULL; + } else { + params->use_config = TRUE; + params->config_file = (*extra_i)->val; + } } else - if (g_strcmp0 ((*extra_i)->opt, "dhchap_key") == 0 && hostkey) - *hostkey = (*extra_i)->val; + if (g_strcmp0 ((*extra_i)->opt, "dhchap_key") == 0) + params->hostkey = (*extra_i)->val; + else + if (g_strcmp0 ((*extra_i)->opt, "dhchap_ctrl_key") == 0) + params->ctrlkey = (*extra_i)->val; else - if (g_strcmp0 ((*extra_i)->opt, "dhchap_ctrl_key") == 0 && ctrlkey) - *ctrlkey = (*extra_i)->val; + if (g_strcmp0 ((*extra_i)->opt, "hostsymname") == 0) + params->hostsymname = (*extra_i)->val; else - if (g_strcmp0 ((*extra_i)->opt, "hostsymname") == 0 && hostsymname) - *hostsymname = (*extra_i)->val; + if (g_strcmp0 ((*extra_i)->opt, "keyring") == 0) + params->keyring = (*extra_i)->val; + else + if (g_strcmp0 ((*extra_i)->opt, "tls_key") == 0) + params->tls_key = (*extra_i)->val; else if (g_strcmp0 ((*extra_i)->opt, "nr_io_queues") == 0) - SAFE_INT_CONV (cfg->nr_io_queues) + SAFE_INT_CONV (params->nr_io_queues) else if (g_strcmp0 ((*extra_i)->opt, "nr_write_queues") == 0) - SAFE_INT_CONV (cfg->nr_write_queues) + SAFE_INT_CONV (params->nr_write_queues) else if (g_strcmp0 ((*extra_i)->opt, "nr_poll_queues") == 0) - SAFE_INT_CONV (cfg->nr_poll_queues) + SAFE_INT_CONV (params->nr_poll_queues) else if (g_strcmp0 ((*extra_i)->opt, "queue_size") == 0) - SAFE_INT_CONV (cfg->queue_size) + SAFE_INT_CONV (params->queue_size) else if (g_strcmp0 ((*extra_i)->opt, "keep_alive_tmo") == 0) - SAFE_INT_CONV (cfg->keep_alive_tmo) + SAFE_INT_CONV (params->keep_alive_tmo) else if (g_strcmp0 ((*extra_i)->opt, "reconnect_delay") == 0) - SAFE_INT_CONV (cfg->reconnect_delay) + SAFE_INT_CONV (params->reconnect_delay) else if (g_strcmp0 ((*extra_i)->opt, "ctrl_loss_tmo") == 0) - SAFE_INT_CONV (cfg->ctrl_loss_tmo) + SAFE_INT_CONV (params->ctrl_loss_tmo) else if (g_strcmp0 ((*extra_i)->opt, "fast_io_fail_tmo") == 0) - SAFE_INT_CONV (cfg->fast_io_fail_tmo) + SAFE_INT_CONV (params->fast_io_fail_tmo) else if (g_strcmp0 ((*extra_i)->opt, "tos") == 0) - SAFE_INT_CONV (cfg->tos) + SAFE_INT_CONV (params->tos) else if (g_strcmp0 ((*extra_i)->opt, "duplicate_connect") == 0) - SAFE_BOOL_CONV (cfg->duplicate_connect) + SAFE_BOOL_CONV (params->duplicate_connect) else if (g_strcmp0 ((*extra_i)->opt, "disable_sqflow") == 0) - SAFE_BOOL_CONV (cfg->disable_sqflow) + SAFE_BOOL_CONV (params->disable_sqflow) else if (g_strcmp0 ((*extra_i)->opt, "hdr_digest") == 0) - SAFE_BOOL_CONV (cfg->hdr_digest) + SAFE_BOOL_CONV (params->hdr_digest) else if (g_strcmp0 ((*extra_i)->opt, "data_digest") == 0) - SAFE_BOOL_CONV (cfg->data_digest) + SAFE_BOOL_CONV (params->data_digest) else if (g_strcmp0 ((*extra_i)->opt, "tls") == 0) - SAFE_BOOL_CONV (cfg->tls) -#ifdef HAVE_LIBNVME_1_4 - else - if (g_strcmp0 ((*extra_i)->opt, "keyring") == 0) { - int keyring; - - keyring = nvme_lookup_keyring ((*extra_i)->val); - if (keyring) { - cfg->keyring = keyring; - nvme_set_keyring (cfg->keyring); - } - } else - if (g_strcmp0 ((*extra_i)->opt, "tls_key") == 0) { - int key; - - key = nvme_lookup_key ("psk", (*extra_i)->val); - if (key) - cfg->tls_key = key; - } -#endif + SAFE_BOOL_CONV (params->tls) } #undef SAFE_INT_CONV @@ -216,17 +228,7 @@ static void parse_extra_args (const BDExtraArg **extra, struct nvme_fabrics_conf * Tech category: %BD_NVME_TECH_FABRICS-%BD_NVME_TECH_MODE_INITIATOR */ gboolean bd_nvme_connect (const gchar *subsysnqn, const gchar *transport, const gchar *transport_addr, const gchar *transport_svcid, const gchar *host_traddr, const gchar *host_iface, const gchar *host_nqn, const gchar *host_id, const BDExtraArg **extra, GError **error) { - int ret; - const gchar *config_file = PATH_NVMF_CONFIG; - gchar *host_nqn_val; - gchar *host_id_val; - const gchar *hostkey = NULL; - const gchar *ctrlkey = NULL; - const gchar *hostsymname = NULL; - nvme_root_t root; - nvme_host_t host; - nvme_ctrl_t ctrl; - struct nvme_fabrics_config cfg; + NVMeConnectParams params = ZERO_INIT; if (subsysnqn == NULL) { g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, @@ -244,61 +246,6 @@ gboolean bd_nvme_connect (const gchar *subsysnqn, const gchar *transport, const return FALSE; } - /* HostNQN checks */ - host_nqn_val = g_strdup (host_nqn); - host_id_val = g_strdup (host_id); - if (host_nqn_val == NULL) - host_nqn_val = nvmf_hostnqn_from_file (); - if (host_id_val == NULL) - host_id_val = nvmf_hostid_from_file (); - if (host_nqn_val == NULL) - host_nqn_val = nvmf_hostnqn_generate (); - if (host_nqn_val == NULL) { - g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, - "Could not determine HostNQN"); - g_free (host_nqn_val); - g_free (host_id_val); - return FALSE; - } - if (host_id_val == NULL) { - /* derive hostid from hostnqn, newer kernels refuse empty hostid */ - host_id_val = g_strrstr (host_nqn_val, "uuid:"); - if (host_id_val) - host_id_val = g_strdup (host_id_val + strlen ("uuid:")); - /* TODO: in theory generating arbitrary uuid might work as a fallback */ - } - if (host_id_val == NULL) { - g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, - "Could not determine HostID value from HostNQN '%s'", - host_nqn_val); - g_free (host_nqn_val); - return FALSE; - } - - /* parse extra arguments */ - nvmf_default_config (&cfg); - parse_extra_args (extra, &cfg, &config_file, &hostkey, &ctrlkey, &hostsymname); - - root = nvme_scan (config_file); - g_assert (root != NULL); - nvme_init_logging (root, -1, false, false); - host = nvme_lookup_host (root, host_nqn_val, host_id_val); - if (host == NULL) { - g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, - "Unable to lookup host for HostNQN '%s' and HostID '%s'", - host_nqn_val, host_id_val); - g_free (host_nqn_val); - g_free (host_id_val); - nvme_free_tree (root); - return FALSE; - } - g_free (host_nqn_val); - g_free (host_id_val); - if (hostkey) - nvme_host_set_dhchap_key (host, hostkey); - if (hostsymname) - nvme_host_set_hostsymname (host, hostsymname); - /* tr_svcid defaults */ if (!transport_svcid) { if (g_strcmp0 (transport, "tcp") == 0) { @@ -311,69 +258,20 @@ gboolean bd_nvme_connect (const gchar *subsysnqn, const gchar *transport, const transport_svcid = G_STRINGIFY (NVME_RDMA_IP_PORT); } - ctrl = nvme_create_ctrl (root, subsysnqn, transport, transport_addr, host_traddr, host_iface, transport_svcid); - if (ctrl == NULL) { - _nvme_fabrics_errno_to_gerror (-1, errno, error); - g_prefix_error (error, "Error creating the controller: "); - nvme_free_tree (root); - return FALSE; - } - if (ctrlkey) - nvme_ctrl_set_dhchap_key (ctrl, ctrlkey); - - ret = nvmf_add_ctrl (host, ctrl, &cfg); - if (ret != 0) { - _nvme_fabrics_errno_to_gerror (ret, errno, error); - g_prefix_error (error, "Error connecting the controller: "); - nvme_free_ctrl (ctrl); - nvme_free_tree (root); - return FALSE; - } - nvme_free_ctrl (ctrl); - nvme_free_tree (root); + params.subsysnqn = subsysnqn; + params.transport = transport; + params.transport_addr = transport_addr; + params.transport_svcid = transport_svcid; + params.host_traddr = host_traddr; + params.host_iface = host_iface; + params.host_nqn = host_nqn; + params.host_id = host_id; - return TRUE; -} + /* parse extra arguments */ + params.use_config = TRUE; + parse_extra_args (extra, ¶ms); -static gboolean _disconnect (const gchar *subsysnqn, const gchar *path, GError **error, gboolean *found) { - nvme_root_t root; - nvme_host_t host; - nvme_subsystem_t subsys; - nvme_ctrl_t ctrl; - int ret; - - root = nvme_create_root (NULL, -1); - if (root == NULL) { - g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, - "Failed to create topology root: %s", - strerror_l (errno, _C_LOCALE)); - return FALSE; - } - ret = nvme_scan_topology (root, NULL, NULL); - if (ret < 0) { - g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, - "Failed to scan topology: %s", - strerror_l (errno, _C_LOCALE)); - nvme_free_tree (root); - return FALSE; - } - nvme_for_each_host (root, host) - nvme_for_each_subsystem (host, subsys) - if (!subsysnqn || g_strcmp0 (nvme_subsystem_get_nqn (subsys), subsysnqn) == 0) - nvme_subsystem_for_each_ctrl (subsys, ctrl) - if (!path || g_strcmp0 (nvme_ctrl_get_name (ctrl), path) == 0) { - ret = nvme_disconnect_ctrl (ctrl); - if (ret != 0) { - g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, - "Error disconnecting the controller: %s", - strerror_l (errno, _C_LOCALE)); - nvme_free_tree (root); - return FALSE; - } - *found = TRUE; - } - nvme_free_tree (root); - return TRUE; + return _nvme_be_connect (¶ms, error); } /** @@ -393,7 +291,7 @@ static gboolean _disconnect (const gchar *subsysnqn, const gchar *path, GError * gboolean bd_nvme_disconnect (const gchar *subsysnqn, GError **error) { gboolean found = FALSE; - if (!_disconnect (subsysnqn, NULL, error, &found)) + if (!_nvme_be_disconnect (subsysnqn, NULL, &found, error)) return FALSE; if (!found) { @@ -425,7 +323,7 @@ gboolean bd_nvme_disconnect_by_path (const gchar *path, GError **error) { if (g_str_has_prefix (p, "/dev/")) p += 5; - if (!_disconnect (NULL, p, error, &found)) + if (!_nvme_be_disconnect (NULL, p, &found, error)) return FALSE; if (!found) { @@ -455,11 +353,10 @@ gboolean bd_nvme_disconnect_by_path (const gchar *path, GError **error) { */ gchar ** bd_nvme_find_ctrls_for_ns (const gchar *ns_sysfs_path, const gchar *subsysnqn, const gchar *host_nqn, const gchar *host_id, GError **error G_GNUC_UNUSED) { GPtrArray *ptr_array; - nvme_root_t root; - nvme_host_t h; - nvme_subsystem_t s; - nvme_ctrl_t c; - nvme_ns_t n; + NVMeTopoSubsys **topo; + NVMeTopoSubsys **s; + NVMeTopoCtrl **c; + gchar **n; char realp[PATH_MAX]; gchar *subsysnqn_p; @@ -470,49 +367,45 @@ gchar ** bd_nvme_find_ctrls_for_ns (const gchar *ns_sysfs_path, const gchar *sub ptr_array = g_ptr_array_new (); - root = nvme_scan (NULL); - g_warn_if_fail (root != NULL); + topo = _nvme_be_scan_topology (); + + for (s = topo; *s; s++) { + gboolean found = FALSE; - nvme_for_each_host (root, h) { - if (host_nqn && g_strcmp0 (nvme_host_get_hostnqn (h), host_nqn) != 0) + if (host_nqn && g_strcmp0 ((*s)->host_nqn, host_nqn) != 0) continue; - if (host_id && g_strcmp0 (nvme_host_get_hostid (h), host_id) != 0) + if (host_id && g_strcmp0 ((*s)->host_id, host_id) != 0) + continue; + if (subsysnqn && g_strcmp0 ((*s)->subsysnqn, subsysnqn_p) != 0) continue; - nvme_for_each_subsystem (h, s) { - gboolean found = FALSE; - - if (subsysnqn && g_strcmp0 (nvme_subsystem_get_nqn (s), subsysnqn_p) != 0) - continue; - - nvme_subsystem_for_each_ctrl (s, c) - nvme_ctrl_for_each_ns (c, n) - if (realpath (nvme_ns_get_sysfs_dir (n), realp) && - g_strcmp0 (realp, ns_sysfs_path) == 0) { - if (realpath (nvme_ctrl_get_sysfs_dir (c), realp)) { - g_ptr_array_add (ptr_array, g_strdup (realp)); - break; - } - } - - nvme_subsystem_for_each_ns (s, n) - if (realpath (nvme_ns_get_sysfs_dir (n), realp) && + for (c = (*s)->ctrls; *c; c++) + for (n = (*c)->ns_sysfs_dirs; *n; n++) + if (realpath (*n, realp) && g_strcmp0 (realp, ns_sysfs_path) == 0) { - found = TRUE; - /* at least one of the namespaces match, don't care about the rest */ - break; - } - - if (found) - /* add all controllers in the subsystem */ - nvme_subsystem_for_each_ctrl (s, c) { - if (realpath (nvme_ctrl_get_sysfs_dir (c), realp)) { + if ((*c)->sysfs_dir && realpath ((*c)->sysfs_dir, realp)) { g_ptr_array_add (ptr_array, g_strdup (realp)); + break; } } - } + + for (n = (*s)->ns_sysfs_dirs; *n; n++) + if (realpath (*n, realp) && + g_strcmp0 (realp, ns_sysfs_path) == 0) { + found = TRUE; + /* at least one of the namespaces match, don't care about the rest */ + break; + } + + if (found) + /* add all controllers in the subsystem */ + for (c = (*s)->ctrls; *c; c++) { + if ((*c)->sysfs_dir && realpath ((*c)->sysfs_dir, realp)) { + g_ptr_array_add (ptr_array, g_strdup (realp)); + } + } } - nvme_free_tree (root); + _nvme_topo_free (topo); g_free (subsysnqn_p); g_ptr_array_add (ptr_array, NULL); /* trailing NULL element */ @@ -538,11 +431,10 @@ gchar ** bd_nvme_find_ctrls_for_ns (const gchar *ns_sysfs_path, const gchar *sub */ gchar ** bd_nvme_find_namespaces_for_ctrl (const gchar *ctrl_sysfs_path, const gchar *subsysnqn, const gchar *host_nqn, const gchar *host_id, GError **error G_GNUC_UNUSED) { GPtrArray *ptr_array; - nvme_root_t root; - nvme_host_t h; - nvme_subsystem_t s; - nvme_ctrl_t c; - nvme_ns_t n; + NVMeTopoSubsys **topo; + NVMeTopoSubsys **s; + NVMeTopoCtrl **c; + gchar **n; char realp[PATH_MAX]; gchar *subsysnqn_p; @@ -553,40 +445,36 @@ gchar ** bd_nvme_find_namespaces_for_ctrl (const gchar *ctrl_sysfs_path, const g ptr_array = g_ptr_array_new (); - root = nvme_scan (NULL); - g_warn_if_fail (root != NULL); + topo = _nvme_be_scan_topology (); - nvme_for_each_host (root, h) { - if (host_nqn && g_strcmp0 (nvme_host_get_hostnqn (h), host_nqn) != 0) + for (s = topo; *s; s++) { + if (host_nqn && g_strcmp0 ((*s)->host_nqn, host_nqn) != 0) + continue; + if (host_id && g_strcmp0 ((*s)->host_id, host_id) != 0) continue; - if (host_id && g_strcmp0 (nvme_host_get_hostid (h), host_id) != 0) + if (subsysnqn && g_strcmp0 ((*s)->subsysnqn, subsysnqn_p) != 0) continue; - nvme_for_each_subsystem (h, s) { - if (subsysnqn && g_strcmp0 (nvme_subsystem_get_nqn (s), subsysnqn_p) != 0) - continue; - - nvme_subsystem_for_each_ctrl (s, c) { - if (realpath (nvme_ctrl_get_sysfs_dir (c), realp) && - g_strcmp0 (realp, ctrl_sysfs_path) == 0) { - /* Found matching controller, add all its namespaces */ - nvme_ctrl_for_each_ns (c, n) { - if (realpath (nvme_ns_get_sysfs_dir (n), realp)) { - g_ptr_array_add (ptr_array, g_strdup (realp)); - } + for (c = (*s)->ctrls; *c; c++) { + if ((*c)->sysfs_dir && realpath ((*c)->sysfs_dir, realp) && + g_strcmp0 (realp, ctrl_sysfs_path) == 0) { + /* Found matching controller, add all its namespaces */ + for (n = (*c)->ns_sysfs_dirs; *n; n++) { + if (realpath (*n, realp)) { + g_ptr_array_add (ptr_array, g_strdup (realp)); } - /* Also check subsystem-level namespaces */ - nvme_subsystem_for_each_ns (s, n) { - if (realpath (nvme_ns_get_sysfs_dir (n), realp)) { - g_ptr_array_add (ptr_array, g_strdup (realp)); - } + } + /* Also check subsystem-level namespaces */ + for (n = (*s)->ns_sysfs_dirs; *n; n++) { + if (realpath (*n, realp)) { + g_ptr_array_add (ptr_array, g_strdup (realp)); } - break; } + break; } } } - nvme_free_tree (root); + _nvme_topo_free (topo); g_free (subsysnqn_p); g_ptr_array_add (ptr_array, NULL); /* trailing NULL element */ @@ -609,7 +497,7 @@ gchar * bd_nvme_get_host_nqn (G_GNUC_UNUSED GError **error) { char *hostnqn; /* FIXME: libnvme SYSCONFDIR might be different from PACKAGE_SYSCONF_DIR */ - hostnqn = nvmf_hostnqn_from_file (); + hostnqn = _nvme_be_read_hostnqn (); return hostnqn ? hostnqn : g_strdup (""); } @@ -628,7 +516,7 @@ gchar * bd_nvme_get_host_nqn (G_GNUC_UNUSED GError **error) { gchar * bd_nvme_generate_host_nqn (GError **error) { char *nqn; - nqn = nvmf_hostnqn_generate (); + nqn = _nvme_be_generate_hostnqn (); if (!nqn) g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, "Unable to generate Host NQN."); @@ -650,7 +538,7 @@ gchar * bd_nvme_generate_host_nqn (GError **error) { gchar * bd_nvme_get_host_id (G_GNUC_UNUSED GError **error) { char *hostid; - hostid = nvmf_hostid_from_file (); + hostid = _nvme_be_read_hostid (); return hostid ? hostid : g_strdup (""); } diff --git a/src/plugins/nvme/nvme-info.c b/src/plugins/nvme/nvme-info.c index b36a644b..4705b1cd 100644 --- a/src/plugins/nvme/nvme-info.c +++ b/src/plugins/nvme/nvme-info.c @@ -27,12 +27,11 @@ #include #include -#include - #include #include #include "nvme.h" #include "nvme-private.h" +#include "nvme-backend.h" /** @@ -389,19 +388,6 @@ static guint64 int128_to_guint64 (__u8 data[16]) return result; } -gint _open_dev (const gchar *device, GError **error) { - int fd; - - fd = open (device, O_RDONLY); - if (fd < 0) { - _nvme_status_to_error (-1, FALSE, error); - g_prefix_error (error, "Failed to open device '%s': ", device); - return -1; - } - - return fd; -} - /* backported from nvme-cli: https://github.com/linux-nvme/nvme-cli/pull/2051 */ #define ROUND_UP(N, S) ((((N) + (S) - 1) / (S)) * (S)) @@ -438,14 +424,6 @@ static gchar *decode_nvme_rev (guint32 ver) { return g_strdup_printf ("%u.%u.%u", mjr, mnr, ter); } -static gchar *_uuid_to_str (unsigned char uuid[NVME_UUID_LEN]) { - gchar uuid_buf[NVME_UUID_LEN_STRING] = ZERO_INIT; - - if (nvme_uuid_to_string (uuid, uuid_buf) == 0) - return g_strdup (uuid_buf); - return NULL; -} - static gboolean _nvme_a_is_zero (const __u8 a[], int len) { int i; @@ -469,30 +447,30 @@ static gboolean _nvme_a_is_zero (const __u8 a[], int len) { */ BDNVMEControllerInfo * bd_nvme_get_controller_info (const gchar *device, GError **error) { int ret; - int fd; + NVMeDev *dev; struct nvme_id_ctrl *ctrl_id; BDNVMEControllerInfo *info; /* open the block device */ - fd = _open_dev (device, error); - if (fd < 0) + dev = _nvme_be_open (device, error); + if (!dev) return NULL; ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); if (!ctrl_id) { - close (fd); + _nvme_be_close (dev); return NULL; } /* send the NVME_IDENTIFY_CNS_CTRL ioctl */ - ret = nvme_identify_ctrl (fd, ctrl_id); + ret = _nvme_be_identify_ctrl (dev, ctrl_id); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Identify Controller command error: "); - close (fd); + _nvme_be_close (dev); free (ctrl_id); return NULL; } - close (fd); + _nvme_be_close (dev); info = g_new0 (BDNVMEControllerInfo, 1); if ((ctrl_id->cmic & NVME_CTRL_CMIC_MULTI_PORT) == NVME_CTRL_CMIC_MULTI_PORT) @@ -509,13 +487,13 @@ BDNVMEControllerInfo * bd_nvme_get_controller_info (const gchar *device, GError info->features |= BD_NVME_CTRL_FEAT_ENCLOSURE; if ((ctrl_id->mec & NVME_CTRL_MEC_PCIEME) == NVME_CTRL_MEC_PCIEME) info->features |= BD_NVME_CTRL_FEAT_MGMT_PCIE; - if ((ctrl_id->mec & NVME_CTRL_MEC_SMBUSME) == NVME_CTRL_MEC_SMBUSME) + if ((ctrl_id->mec & BE_NVME_CTRL_MEC_TWPME) == BE_NVME_CTRL_MEC_TWPME) info->features |= BD_NVME_CTRL_FEAT_MGMT_SMBUS; info->pci_vendor_id = GUINT16_FROM_LE (ctrl_id->vid); info->pci_subsys_vendor_id = GUINT16_FROM_LE (ctrl_id->ssvid); info->ctrl_id = GUINT16_FROM_LE (ctrl_id->cntlid); if (!_nvme_a_is_zero (ctrl_id->fguid, sizeof (ctrl_id->fguid))) - info->fguid = _uuid_to_str (ctrl_id->fguid); + info->fguid = _nvme_be_uuid_to_string (ctrl_id->fguid); info->model_number = g_strndup (ctrl_id->mn, sizeof (ctrl_id->mn)); g_strstrip (info->model_number); info->serial_number = g_strndup (ctrl_id->sn, sizeof (ctrl_id->sn)); @@ -524,11 +502,11 @@ BDNVMEControllerInfo * bd_nvme_get_controller_info (const gchar *device, GError g_strstrip (info->firmware_ver); info->nvme_ver = decode_nvme_rev (GUINT32_FROM_LE (ctrl_id->ver)); /* TODO: vwci: VPD Write Cycle Information */ - if ((ctrl_id->oacs & NVME_CTRL_OACS_FORMAT) == NVME_CTRL_OACS_FORMAT) + if ((ctrl_id->oacs & BE_NVME_CTRL_OACS_FNVMS) == BE_NVME_CTRL_OACS_FNVMS) info->features |= BD_NVME_CTRL_FEAT_FORMAT; - if ((ctrl_id->oacs & NVME_CTRL_OACS_NS_MGMT) == NVME_CTRL_OACS_NS_MGMT) + if ((ctrl_id->oacs & BE_NVME_CTRL_OACS_NMS) == BE_NVME_CTRL_OACS_NMS) info->features |= BD_NVME_CTRL_FEAT_NS_MGMT; - if ((ctrl_id->oacs & NVME_CTRL_OACS_SELF_TEST) == NVME_CTRL_OACS_SELF_TEST) + if ((ctrl_id->oacs & BE_NVME_CTRL_OACS_DSTS) == BE_NVME_CTRL_OACS_DSTS) info->features |= BD_NVME_CTRL_FEAT_SELFTEST; switch (ctrl_id->cntrltype) { case NVME_CTRL_CNTRLTYPE_IO: @@ -548,7 +526,7 @@ BDNVMEControllerInfo * bd_nvme_get_controller_info (const gchar *device, GError info->size_total = int128_to_guint64 (ctrl_id->tnvmcap); info->size_unalloc = int128_to_guint64 (ctrl_id->unvmcap); info->selftest_ext_time = GUINT16_FROM_LE (ctrl_id->edstt); - /* TODO: lpa: Log Page Attributes - NVME_CTRL_LPA_PERSETENT_EVENT: Persistent Event log */ + /* TODO: lpa: Log Page Attributes - Persistent Event log */ if ((ctrl_id->dsto & NVME_CTRL_DSTO_ONE_DST) == NVME_CTRL_DSTO_ONE_DST) info->features |= BD_NVME_CTRL_FEAT_SELFTEST_SINGLE; if ((ctrl_id->sanicap & NVME_CTRL_SANICAP_CES) == NVME_CTRL_SANICAP_CES) @@ -595,7 +573,7 @@ BDNVMENamespaceInfo *bd_nvme_get_namespace_info (const gchar *device, GError **e int ret_ctrl; int ret_desc = -1; int ret_ns_ind = -1; - int fd; + NVMeDev *dev; __u32 nsid = 0; struct nvme_id_ctrl *ctrl_id; struct nvme_id_ns *ns_info; @@ -609,30 +587,30 @@ BDNVMENamespaceInfo *bd_nvme_get_namespace_info (const gchar *device, GError **e GPtrArray *ptr_array; /* open the block device */ - fd = _open_dev (device, error); - if (fd < 0) + dev = _nvme_be_open (device, error); + if (!dev) return NULL; /* get Namespace Identifier (NSID) for the @device (NVME_IOCTL_ID) */ - ret = nvme_get_nsid (fd, &nsid); + ret = _nvme_be_get_nsid (dev, &nsid); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "Error getting Namespace Identifier (NSID): "); - close (fd); + _nvme_be_close (dev); return NULL; } /* send the NVME_IDENTIFY_CNS_NS ioctl */ ns_info = _nvme_alloc (sizeof (struct nvme_id_ns), error); if (!ns_info) { - close (fd); + _nvme_be_close (dev); return NULL; } - ret = nvme_identify_ns (fd, nsid, ns_info); + ret = _nvme_be_identify_ns (dev, nsid, ns_info); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Identify Namespace command error: "); - close (fd); + _nvme_be_close (dev); free (ns_info); return NULL; } @@ -640,26 +618,26 @@ BDNVMENamespaceInfo *bd_nvme_get_namespace_info (const gchar *device, GError **e /* send the NVME_IDENTIFY_CNS_CTRL ioctl */ ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); if (!ctrl_id) { - close (fd); + _nvme_be_close (dev); free (ns_info); return NULL; } - ret_ctrl = nvme_identify_ctrl (fd, ctrl_id); + ret_ctrl = _nvme_be_identify_ctrl (dev, ctrl_id); /* send the NVME_IDENTIFY_CNS_NS_DESC_LIST ioctl, NVMe 1.3 */ if (ret_ctrl == 0 && GUINT32_FROM_LE (ctrl_id->ver) >= 0x10300) { descs = _nvme_alloc (NVME_IDENTIFY_DATA_SIZE, NULL); if (descs != NULL) - ret_desc = nvme_identify_ns_descs (fd, nsid, descs); + ret_desc = _nvme_be_identify_ns_descs (dev, nsid, descs); } /* send the NVME_IDENTIFY_CNS_CSI_INDEPENDENT_ID_NS ioctl, NVMe 2.0 */ if (ret_ctrl == 0 && GUINT32_FROM_LE (ctrl_id->ver) >= 0x20000) { ns_info_ind = _nvme_alloc (sizeof (struct nvme_id_independent_id_ns), NULL); if (ns_info_ind != NULL) - ret_ns_ind = nvme_identify_independent_identify_ns (fd, nsid, ns_info_ind); + ret_ns_ind = _nvme_be_identify_independent_ns (dev, nsid, ns_info_ind); } - close (fd); + _nvme_be_close (dev); info = g_new0 (BDNVMENamespaceInfo, 1); info->nsid = nsid; @@ -699,7 +677,7 @@ BDNVMENamespaceInfo *bd_nvme_get_namespace_info (const gchar *device, GError **e break; case NVME_NIDT_UUID: g_free (info->uuid); - info->uuid = _uuid_to_str (d->nid); + info->uuid = _nvme_be_uuid_to_string (d->nid); break; case NVME_NIDT_CSI: /* unused */ @@ -764,28 +742,28 @@ BDNVMENamespaceInfo *bd_nvme_get_namespace_info (const gchar *device, GError **e BDNVMESmartLog * bd_nvme_get_smart_log (const gchar *device, GError **error) { int ret; int ret_identify; - int fd; + NVMeDev *dev; struct nvme_id_ctrl *ctrl_id; struct nvme_smart_log *smart_log; BDNVMESmartLog *log; guint i; /* open the block device */ - fd = _open_dev (device, error); - if (fd < 0) + dev = _nvme_be_open (device, error); + if (!dev) return NULL; /* send the NVME_IDENTIFY_CNS_CTRL ioctl */ ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); if (!ctrl_id) { - close (fd); + _nvme_be_close (dev); return NULL; } - ret_identify = nvme_identify_ctrl (fd, ctrl_id); + ret_identify = _nvme_be_identify_ctrl (dev, ctrl_id); if (ret_identify != 0) { _nvme_status_to_error (ret_identify, FALSE, error); g_prefix_error (error, "NVMe Identify Controller command error: "); - close (fd); + _nvme_be_close (dev); free (ctrl_id); return NULL; } @@ -793,20 +771,20 @@ BDNVMESmartLog * bd_nvme_get_smart_log (const gchar *device, GError **error) { /* send the NVME_LOG_LID_SMART ioctl */ smart_log = _nvme_alloc (sizeof (struct nvme_smart_log), error); if (!smart_log) { - close (fd); + _nvme_be_close (dev); free (ctrl_id); return NULL; } - ret = nvme_get_log_smart (fd, NVME_NSID_ALL, FALSE /* rae */, smart_log); + ret = _nvme_be_get_log_smart (dev, smart_log); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Get Log Page - SMART / Health Information Log command error: "); - close (fd); + _nvme_be_close (dev); free (ctrl_id); free (smart_log); return NULL; } - close (fd); + _nvme_be_close (dev); log = g_new0 (BDNVMESmartLog, 1); if ((smart_log->critical_warning & NVME_SMART_CRIT_SPARE) == NVME_SMART_CRIT_SPARE) @@ -875,7 +853,7 @@ BDNVMESmartLog * bd_nvme_get_smart_log (const gchar *device, GError **error) { */ BDNVMEErrorLogEntry ** bd_nvme_get_error_log_entries (const gchar *device, GError **error) { int ret; - int fd; + NVMeDev *dev; guint elpe; struct nvme_id_ctrl *ctrl_id; struct nvme_error_log_page *err_log; @@ -883,21 +861,21 @@ BDNVMEErrorLogEntry ** bd_nvme_get_error_log_entries (const gchar *device, GErro guint i; /* open the block device */ - fd = _open_dev (device, error); - if (fd < 0) + dev = _nvme_be_open (device, error); + if (!dev) return NULL; /* find out the maximum number of error log entries as reported by the controller */ ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); if (!ctrl_id) { - close (fd); + _nvme_be_close (dev); return NULL; } - ret = nvme_identify_ctrl (fd, ctrl_id); + ret = _nvme_be_identify_ctrl (dev, ctrl_id); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Identify Controller command error: "); - close (fd); + _nvme_be_close (dev); free (ctrl_id); return NULL; } @@ -908,18 +886,18 @@ BDNVMEErrorLogEntry ** bd_nvme_get_error_log_entries (const gchar *device, GErro /* send the NVME_LOG_LID_ERROR ioctl */ err_log = _nvme_alloc (sizeof (struct nvme_error_log_page) * elpe, error); if (!err_log) { - close (fd); + _nvme_be_close (dev); return NULL; } - ret = nvme_get_log_error (fd, elpe, FALSE /* rae */, err_log); + ret = _nvme_be_get_log_error (dev, elpe, err_log); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Get Log Page - Error Information Log Entry command error: "); - close (fd); + _nvme_be_close (dev); free (err_log); return NULL; } - close (fd); + _nvme_be_close (dev); /* parse the log */ ptr_array = g_ptr_array_new (); @@ -963,32 +941,32 @@ BDNVMEErrorLogEntry ** bd_nvme_get_error_log_entries (const gchar *device, GErro */ BDNVMESelfTestLog * bd_nvme_get_self_test_log (const gchar *device, GError **error) { int ret; - int fd; + NVMeDev *dev; struct nvme_self_test_log *self_test_log; BDNVMESelfTestLog *log; GPtrArray *ptr_array; guint i; /* open the block device */ - fd = _open_dev (device, error); - if (fd < 0) + dev = _nvme_be_open (device, error); + if (!dev) return NULL; /* send the NVME_LOG_LID_DEVICE_SELF_TEST ioctl */ self_test_log = _nvme_alloc (sizeof (struct nvme_self_test_log), error); if (!self_test_log) { - close (fd); + _nvme_be_close (dev); return NULL; } - ret = nvme_get_log_device_self_test (fd, self_test_log); + ret = _nvme_be_get_log_self_test (dev, self_test_log); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Get Log Page - Device Self-test Log command error: "); - close (fd); + _nvme_be_close (dev); free (self_test_log); return NULL; } - close (fd); + _nvme_be_close (dev); log = g_new0 (BDNVMESelfTestLog, 1); switch (self_test_log->current_operation & NVME_ST_CURR_OP_MASK) { @@ -1111,36 +1089,36 @@ BDNVMESelfTestLog * bd_nvme_get_self_test_log (const gchar *device, GError **err */ BDNVMESanitizeLog * bd_nvme_get_sanitize_log (const gchar *device, GError **error) { int ret; - int fd; + NVMeDev *dev; struct nvme_sanitize_log_page *sanitize_log; BDNVMESanitizeLog *log; __u16 sstat; /* open the block device */ - fd = _open_dev (device, error); - if (fd < 0) + dev = _nvme_be_open (device, error); + if (!dev) return NULL; /* send the NVME_LOG_LID_SANITIZE ioctl */ sanitize_log = _nvme_alloc (sizeof (struct nvme_sanitize_log_page), error); if (!sanitize_log) { - close (fd); + _nvme_be_close (dev); return NULL; } - ret = nvme_get_log_sanitize (fd, FALSE /* rae */, sanitize_log); + ret = _nvme_be_get_log_sanitize (dev, sanitize_log); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Get Log Page - Sanitize Status Log command error: "); - close (fd); + _nvme_be_close (dev); free (sanitize_log); return NULL; } - close (fd); + _nvme_be_close (dev); log = g_new0 (BDNVMESanitizeLog, 1); log->sanitize_progress = 0; sstat = GUINT16_FROM_LE (sanitize_log->sstat); - if ((sstat & NVME_SANITIZE_SSTAT_STATUS_MASK) == NVME_SANITIZE_SSTAT_STATUS_IN_PROGESS) + if ((sstat & NVME_SANITIZE_SSTAT_STATUS_MASK) == BE_NVME_SANITIZE_SSTAT_STATUS_IN_PROGRESS) log->sanitize_progress = ((gdouble) GUINT16_FROM_LE (sanitize_log->sprog) * 100) / 0x10000; log->global_data_erased = sstat & NVME_SANITIZE_SSTAT_GLOBAL_DATA_ERASED; log->overwrite_passes = (sstat >> NVME_SANITIZE_SSTAT_COMPLETED_PASSES_SHIFT) & NVME_SANITIZE_SSTAT_COMPLETED_PASSES_MASK; @@ -1149,7 +1127,7 @@ BDNVMESanitizeLog * bd_nvme_get_sanitize_log (const gchar *device, GError **erro case NVME_SANITIZE_SSTAT_STATUS_COMPLETE_SUCCESS: log->sanitize_status = BD_NVME_SANITIZE_STATUS_SUCCESS; break; - case NVME_SANITIZE_SSTAT_STATUS_IN_PROGESS: + case BE_NVME_SANITIZE_SSTAT_STATUS_IN_PROGRESS: log->sanitize_status = BD_NVME_SANITIZE_STATUS_IN_PROGRESS; break; case NVME_SANITIZE_SSTAT_STATUS_COMPLETED_FAILED: diff --git a/src/plugins/nvme/nvme-libnvme1.c b/src/plugins/nvme/nvme-libnvme1.c new file mode 100644 index 00000000..8325db0f --- /dev/null +++ b/src/plugins/nvme/nvme-libnvme1.c @@ -0,0 +1,428 @@ +/* + * Copyright (C) 2014-2021 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + * + * Author: Tomas Bzatek + */ + +/* libnvme-1.x backend, see nvme-backend.h */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include "nvme.h" +#include "nvme-private.h" +#include "nvme-backend.h" + + +/* nvme-cli defaults */ +#define PATH_NVMF_CONFIG "/etc/nvme/config.json" + +struct _NVMeDev { + int fd; +}; + +/* libnvme-1.x returns -1 with errno set on failure */ +static int _ret (int ret) { + return ret < 0 ? -errno : ret; +} + +NVMeDev *_nvme_be_open (const gchar *device, GError **error) { + NVMeDev *dev; + int fd; + + fd = open (device, O_RDONLY); + if (fd < 0) { + _nvme_status_to_error (-errno, FALSE, error); + g_prefix_error (error, "Failed to open device '%s': ", device); + return NULL; + } + + dev = g_new0 (NVMeDev, 1); + dev->fd = fd; + return dev; +} + +void _nvme_be_close (NVMeDev *dev) { + close (dev->fd); + g_free (dev); +} + +int _nvme_be_get_nsid (NVMeDev *dev, __u32 *nsid) { + return _ret (nvme_get_nsid (dev->fd, nsid)); +} + +int _nvme_be_identify_ctrl (NVMeDev *dev, struct nvme_id_ctrl *ctrl_id) { + return _ret (nvme_identify_ctrl (dev->fd, ctrl_id)); +} + +int _nvme_be_identify_ns (NVMeDev *dev, __u32 nsid, struct nvme_id_ns *ns_info) { + return _ret (nvme_identify_ns (dev->fd, nsid, ns_info)); +} + +int _nvme_be_identify_ns_descs (NVMeDev *dev, __u32 nsid, struct nvme_ns_id_desc *descs) { + return _ret (nvme_identify_ns_descs (dev->fd, nsid, descs)); +} + +int _nvme_be_identify_independent_ns (NVMeDev *dev, __u32 nsid, struct nvme_id_independent_id_ns *ns_info) { + return _ret (nvme_identify_independent_identify_ns (dev->fd, nsid, ns_info)); +} + +int _nvme_be_get_log_smart (NVMeDev *dev, struct nvme_smart_log *smart_log) { + return _ret (nvme_get_log_smart (dev->fd, NVME_NSID_ALL, FALSE /* rae */, smart_log)); +} + +int _nvme_be_get_log_error (NVMeDev *dev, guint entries, struct nvme_error_log_page *err_log) { + return _ret (nvme_get_log_error (dev->fd, entries, FALSE /* rae */, err_log)); +} + +int _nvme_be_get_log_self_test (NVMeDev *dev, struct nvme_self_test_log *self_test_log) { + return _ret (nvme_get_log_device_self_test (dev->fd, self_test_log)); +} + +int _nvme_be_get_log_sanitize (NVMeDev *dev, struct nvme_sanitize_log_page *sanitize_log) { + return _ret (nvme_get_log_sanitize (dev->fd, FALSE /* rae */, sanitize_log)); +} + +int _nvme_be_self_test (NVMeDev *dev, __u32 nsid, __u8 stc) { + struct nvme_dev_self_test_args args = { + .args_size = sizeof(args), + .fd = dev->fd, + .result = NULL, + .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, + .nsid = nsid, + .stc = stc, + }; + + return _ret (nvme_dev_self_test (&args)); +} + +int _nvme_be_format (NVMeDev *dev, __u32 nsid, __u8 lbaf, __u8 ses) { + struct nvme_format_nvm_args args = { + .args_size = sizeof(args), + .fd = dev->fd, + .result = NULL, + .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, + .nsid = nsid, + .mset = NVME_FORMAT_MSET_SEPARATE /* 0 */, + .pi = NVME_FORMAT_PI_DISABLE /* 0 */, + .pil = NVME_FORMAT_PIL_LAST /* 0 */, + .ses = ses, + .lbaf = lbaf, + }; + + return _ret (nvme_format_nvm (&args)); +} + +int _nvme_be_sanitize (NVMeDev *dev, __u8 sanact, gboolean ause, __u8 owpass, gboolean oipbp, gboolean nodas, __u32 ovrpat) { + struct nvme_sanitize_nvm_args args = { + .args_size = sizeof(args), + .fd = dev->fd, + .result = NULL, + .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, + .sanact = sanact, + .ause = ause, + .owpass = owpass, + .oipbp = oipbp, + .nodas = nodas, + .ovrpat = ovrpat, + }; + + return _ret (nvme_sanitize_nvm (&args)); +} + +int _nvme_be_rescan_ns (NVMeDev *dev) { + return _ret (ioctl (dev->fd, NVME_IOCTL_RESCAN)); +} + +int _nvme_be_update_block_size (NVMeDev *dev, int block_size) { + return _ret (ioctl (dev->fd, BLKBSZSET, &block_size)); +} + +int _nvme_be_reread_partitions (NVMeDev *dev) { + return _ret (ioctl (dev->fd, BLKRRPART)); +} + +const gchar *_nvme_be_status_to_string (int status, gboolean fabrics) { + return nvme_status_to_string (status, fabrics); +} + +const gchar *_nvme_be_fabrics_errno_to_string (int err) { + return nvme_errno_to_string (err); +} + +gchar *_nvme_be_uuid_to_string (unsigned char uuid[NVME_UUID_LEN]) { + gchar uuid_buf[NVME_UUID_LEN_STRING] = ZERO_INIT; + + if (nvme_uuid_to_string (uuid, uuid_buf) == 0) + return g_strdup (uuid_buf); + return NULL; +} + +gchar *_nvme_be_read_hostnqn (void) { + return nvmf_hostnqn_from_file (); +} + +gchar *_nvme_be_read_hostid (void) { + return nvmf_hostid_from_file (); +} + +gchar *_nvme_be_generate_hostnqn (void) { + return nvmf_hostnqn_generate (); +} + +static void _add_sysfs_dir (GPtrArray *ptr_array, const char *dir) { + if (dir) + g_ptr_array_add (ptr_array, g_strdup (dir)); +} + +NVMeTopoSubsys **_nvme_be_scan_topology (void) { + GPtrArray *subsystems; + nvme_root_t root; + nvme_host_t h; + nvme_subsystem_t s; + nvme_ctrl_t c; + nvme_ns_t n; + + subsystems = g_ptr_array_new (); + + root = nvme_scan (NULL); + g_warn_if_fail (root != NULL); + if (root == NULL) { + g_ptr_array_add (subsystems, NULL); + return (NVMeTopoSubsys **) g_ptr_array_free (subsystems, FALSE); + } + + nvme_for_each_host (root, h) + nvme_for_each_subsystem (h, s) { + NVMeTopoSubsys *subsys; + GPtrArray *ctrls; + GPtrArray *nss; + + subsys = g_new0 (NVMeTopoSubsys, 1); + subsys->host_nqn = g_strdup (nvme_host_get_hostnqn (h)); + subsys->host_id = g_strdup (nvme_host_get_hostid (h)); + subsys->subsysnqn = g_strdup (nvme_subsystem_get_nqn (s)); + + nss = g_ptr_array_new (); + nvme_subsystem_for_each_ns (s, n) + _add_sysfs_dir (nss, nvme_ns_get_sysfs_dir (n)); + g_ptr_array_add (nss, NULL); + subsys->ns_sysfs_dirs = (gchar **) g_ptr_array_free (nss, FALSE); + + ctrls = g_ptr_array_new (); + nvme_subsystem_for_each_ctrl (s, c) { + NVMeTopoCtrl *ctrl; + + ctrl = g_new0 (NVMeTopoCtrl, 1); + ctrl->name = g_strdup (nvme_ctrl_get_name (c)); + ctrl->sysfs_dir = g_strdup (nvme_ctrl_get_sysfs_dir (c)); + nss = g_ptr_array_new (); + nvme_ctrl_for_each_ns (c, n) + _add_sysfs_dir (nss, nvme_ns_get_sysfs_dir (n)); + g_ptr_array_add (nss, NULL); + ctrl->ns_sysfs_dirs = (gchar **) g_ptr_array_free (nss, FALSE); + g_ptr_array_add (ctrls, ctrl); + } + g_ptr_array_add (ctrls, NULL); + subsys->ctrls = (NVMeTopoCtrl **) g_ptr_array_free (ctrls, FALSE); + + g_ptr_array_add (subsystems, subsys); + } + nvme_free_tree (root); + + g_ptr_array_add (subsystems, NULL); + return (NVMeTopoSubsys **) g_ptr_array_free (subsystems, FALSE); +} + +gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { + int ret; + const gchar *config_file; + gchar *host_nqn_val; + gchar *host_id_val; + nvme_root_t root; + nvme_host_t host; + nvme_ctrl_t ctrl; + struct nvme_fabrics_config cfg; + + /* HostNQN checks */ + host_nqn_val = g_strdup (params->host_nqn); + host_id_val = g_strdup (params->host_id); + if (host_nqn_val == NULL) + host_nqn_val = nvmf_hostnqn_from_file (); + if (host_id_val == NULL) + host_id_val = nvmf_hostid_from_file (); + if (host_nqn_val == NULL) + host_nqn_val = nvmf_hostnqn_generate (); + if (host_nqn_val == NULL) { + g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, + "Could not determine HostNQN"); + g_free (host_nqn_val); + g_free (host_id_val); + return FALSE; + } + if (host_id_val == NULL) { + /* derive hostid from hostnqn, newer kernels refuse empty hostid */ + host_id_val = g_strrstr (host_nqn_val, "uuid:"); + if (host_id_val) + host_id_val = g_strdup (host_id_val + strlen ("uuid:")); + /* TODO: in theory generating arbitrary uuid might work as a fallback */ + } + if (host_id_val == NULL) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, + "Could not determine HostID value from HostNQN '%s'", + host_nqn_val); + g_free (host_nqn_val); + return FALSE; + } + + /* apply extra arguments */ + nvmf_default_config (&cfg); +#define APPLY_OPT(opt) { \ + if (params->opt.set) \ + cfg.opt = params->opt.val; \ + } + APPLY_OPT (nr_io_queues); + APPLY_OPT (nr_write_queues); + APPLY_OPT (nr_poll_queues); + APPLY_OPT (queue_size); + APPLY_OPT (keep_alive_tmo); + APPLY_OPT (reconnect_delay); + APPLY_OPT (ctrl_loss_tmo); + APPLY_OPT (fast_io_fail_tmo); + APPLY_OPT (tos); + APPLY_OPT (duplicate_connect); + APPLY_OPT (disable_sqflow); + APPLY_OPT (hdr_digest); + APPLY_OPT (data_digest); + APPLY_OPT (tls); +#undef APPLY_OPT +#ifdef HAVE_LIBNVME_1_4 + if (params->keyring) { + int keyring; + + keyring = nvme_lookup_keyring (params->keyring); + if (keyring) { + cfg.keyring = keyring; + nvme_set_keyring (cfg.keyring); + } + } + if (params->tls_key) { + int key; + + key = nvme_lookup_key ("psk", params->tls_key); + if (key) + cfg.tls_key = key; + } +#endif + + config_file = NULL; + if (params->use_config) + config_file = params->config_file ? params->config_file : PATH_NVMF_CONFIG; + + root = nvme_scan (config_file); + g_assert (root != NULL); + nvme_init_logging (root, -1, false, false); + host = nvme_lookup_host (root, host_nqn_val, host_id_val); + if (host == NULL) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Unable to lookup host for HostNQN '%s' and HostID '%s'", + host_nqn_val, host_id_val); + g_free (host_nqn_val); + g_free (host_id_val); + nvme_free_tree (root); + return FALSE; + } + g_free (host_nqn_val); + g_free (host_id_val); + if (params->hostkey) + nvme_host_set_dhchap_key (host, params->hostkey); + if (params->hostsymname) + nvme_host_set_hostsymname (host, params->hostsymname); + + ctrl = nvme_create_ctrl (root, params->subsysnqn, params->transport, params->transport_addr, + params->host_traddr, params->host_iface, params->transport_svcid); + if (ctrl == NULL) { + _nvme_fabrics_errno_to_gerror (-errno, error); + g_prefix_error (error, "Error creating the controller: "); + nvme_free_tree (root); + return FALSE; + } + if (params->ctrlkey) + nvme_ctrl_set_dhchap_key (ctrl, params->ctrlkey); + + ret = nvmf_add_ctrl (host, ctrl, &cfg); + if (ret != 0) { + _nvme_fabrics_errno_to_gerror (-errno, error); + g_prefix_error (error, "Error connecting the controller: "); + nvme_free_ctrl (ctrl); + nvme_free_tree (root); + return FALSE; + } + nvme_free_ctrl (ctrl); + nvme_free_tree (root); + + return TRUE; +} + +gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, gboolean *found, GError **error) { + nvme_root_t root; + nvme_host_t host; + nvme_subsystem_t subsys; + nvme_ctrl_t ctrl; + int ret; + + root = nvme_create_root (NULL, -1); + if (root == NULL) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Failed to create topology root: %s", + strerror_l (errno, _C_LOCALE)); + return FALSE; + } + ret = nvme_scan_topology (root, NULL, NULL); + if (ret < 0) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Failed to scan topology: %s", + strerror_l (errno, _C_LOCALE)); + nvme_free_tree (root); + return FALSE; + } + nvme_for_each_host (root, host) + nvme_for_each_subsystem (host, subsys) + if (!subsysnqn || g_strcmp0 (nvme_subsystem_get_nqn (subsys), subsysnqn) == 0) + nvme_subsystem_for_each_ctrl (subsys, ctrl) + if (!name || g_strcmp0 (nvme_ctrl_get_name (ctrl), name) == 0) { + ret = nvme_disconnect_ctrl (ctrl); + if (ret != 0) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Error disconnecting the controller: %s", + strerror_l (errno, _C_LOCALE)); + nvme_free_tree (root); + return FALSE; + } + *found = TRUE; + } + nvme_free_tree (root); + return TRUE; +} diff --git a/src/plugins/nvme/nvme-libnvme1.h b/src/plugins/nvme/nvme-libnvme1.h new file mode 100644 index 00000000..d6a96f7d --- /dev/null +++ b/src/plugins/nvme/nvme-libnvme1.h @@ -0,0 +1,39 @@ +/* + * Copyright (C) 2026 SUSE LLC + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + * + * Author: Daniel Wagner + */ + +/* + * NVMe specification types for the libnvme-1.x backend. + * + * A few constants have been renamed to the NVMe specification names in libnvme3, while + * newer libnvme-1.x releases use some of the new names for accessor macros. The generic + * plugin code uses the BE_ prefixed names defined by each backend header instead. + */ + +#include + +#ifndef BD_NVME_LIBNVME1 +#define BD_NVME_LIBNVME1 + +#define BE_NVME_CTRL_MEC_TWPME NVME_CTRL_MEC_SMBUSME +#define BE_NVME_CTRL_OACS_FNVMS NVME_CTRL_OACS_FORMAT +#define BE_NVME_CTRL_OACS_NMS NVME_CTRL_OACS_NS_MGMT +#define BE_NVME_CTRL_OACS_DSTS NVME_CTRL_OACS_SELF_TEST +#define BE_NVME_SANITIZE_SSTAT_STATUS_IN_PROGRESS NVME_SANITIZE_SSTAT_STATUS_IN_PROGESS + +#endif /* BD_NVME_LIBNVME1 */ diff --git a/src/plugins/nvme/nvme-op.c b/src/plugins/nvme/nvme-op.c index b42d4eda..3428a5fa 100644 --- a/src/plugins/nvme/nvme-op.c +++ b/src/plugins/nvme/nvme-op.c @@ -26,14 +26,11 @@ #include #include #include -#include - -#include - #include #include #include "nvme.h" #include "nvme-private.h" +#include "nvme-backend.h" /** @@ -57,25 +54,22 @@ */ gboolean bd_nvme_device_self_test (const gchar *device, BDNVMESelfTestAction action, GError **error) { int ret; - struct nvme_dev_self_test_args args = { - .args_size = sizeof(args), - .result = NULL, - .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .nsid = 0xffffffff, - }; + NVMeDev *dev; + __u32 nsid = 0xffffffff; + __u8 stc; switch (action) { case BD_NVME_SELF_TEST_ACTION_SHORT: - args.stc = NVME_DST_STC_SHORT; + stc = NVME_DST_STC_SHORT; break; case BD_NVME_SELF_TEST_ACTION_EXTENDED: - args.stc = NVME_DST_STC_LONG; + stc = NVME_DST_STC_LONG; break; case BD_NVME_SELF_TEST_ACTION_VENDOR_SPECIFIC: - args.stc = NVME_DST_STC_VS; + stc = NVME_DST_STC_VS; break; case BD_NVME_SELF_TEST_ACTION_ABORT: - args.stc = NVME_DST_STC_ABORT; + stc = NVME_DST_STC_ABORT; break; default: g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, @@ -84,37 +78,37 @@ gboolean bd_nvme_device_self_test (const gchar *device, BDNVMESelfTestAction act } /* open the block device */ - args.fd = _open_dev (device, error); - if (args.fd < 0) + dev = _nvme_be_open (device, error); + if (!dev) return FALSE; /* get Namespace Identifier (NSID) for the @device (NVME_IOCTL_ID) */ - ret = nvme_get_nsid (args.fd, &args.nsid); - if (ret < 0 && errno == ENOTTY) + ret = _nvme_be_get_nsid (dev, &nsid); + if (ret == -ENOTTY) /* not a block device, assuming controller character device */ - args.nsid = 0xffffffff; + nsid = 0xffffffff; else if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "Error getting Namespace Identifier (NSID): "); - close (args.fd); + _nvme_be_close (dev); return FALSE; } - ret = nvme_dev_self_test (&args); + ret = _nvme_be_self_test (dev, nsid, stc); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Device Self-test command error: "); - close (args.fd); + _nvme_be_close (dev); return FALSE; } - close (args.fd); + _nvme_be_close (dev); return TRUE; } /* returns 0xff in case of error (the NVMe standard defines total of 16 flba records) */ -static __u8 find_lbaf_for_size (int fd, __u32 nsid, guint16 lba_data_size, guint16 metadata_size, GError **error) { +static __u8 find_lbaf_for_size (NVMeDev *dev, __u32 nsid, guint16 lba_data_size, guint16 metadata_size, GError **error) { int ret; struct nvme_id_ns *ns_info; __u8 flbas = 0; @@ -124,7 +118,7 @@ static __u8 find_lbaf_for_size (int fd, __u32 nsid, guint16 lba_data_size, guint ns_info = _nvme_alloc (sizeof (struct nvme_id_ns), error); if (!ns_info) return 0xff; - ret = nvme_identify_ns (fd, nsid == 0xffffffff ? 1 : nsid, ns_info); + ret = _nvme_be_identify_ns (dev, nsid == 0xffffffff ? 1 : nsid, ns_info); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Identify Namespace command error: "); @@ -185,31 +179,25 @@ gboolean bd_nvme_format (const gchar *device, guint16 lba_data_size, guint16 met int ret; gboolean ctrl_device = FALSE; struct nvme_id_ctrl *ctrl_id; - struct nvme_format_nvm_args args = { - .args_size = sizeof(args), - .result = NULL, - .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .nsid = 0xffffffff, - .mset = NVME_FORMAT_MSET_SEPARATE /* 0 */, - .pi = NVME_FORMAT_PI_DISABLE /* 0 */, - .pil = NVME_FORMAT_PIL_LAST /* 0 */, - .ses = NVME_FORMAT_SES_NONE, - }; + NVMeDev *dev; + __u32 nsid = 0xffffffff; + __u8 lbaf; + __u8 ses; /* open the block device */ - args.fd = _open_dev (device, error); - if (args.fd < 0) + dev = _nvme_be_open (device, error); + if (!dev) return FALSE; - ret = nvme_get_nsid (args.fd, &args.nsid); - if (ret < 0 && errno == ENOTTY) { + ret = _nvme_be_get_nsid (dev, &nsid); + if (ret == -ENOTTY) { /* not a block device, assuming controller character device */ - args.nsid = 0xffffffff; + nsid = 0xffffffff; ctrl_device = TRUE; } else if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "Error getting Namespace Identifier (NSID): "); - close (args.fd); + _nvme_be_close (dev); return FALSE; } @@ -217,14 +205,14 @@ gboolean bd_nvme_format (const gchar *device, guint16 lba_data_size, guint16 met if (! ctrl_device) { ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); if (!ctrl_id) { - close (args.fd); + _nvme_be_close (dev); return FALSE; } - ret = nvme_identify_ctrl (args.fd, ctrl_id); + ret = _nvme_be_identify_ctrl (dev, ctrl_id); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "NVMe Identify Controller command error: "); - close (args.fd); + _nvme_be_close (dev); free (ctrl_id); return FALSE; } @@ -238,7 +226,7 @@ gboolean bd_nvme_format (const gchar *device, guint16 lba_data_size, guint16 met * should be called on a controller device instead */ g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_WOULD_FORMAT_ALL_NS, "The NVMe controller indicates it would format all namespaces."); - close (args.fd); + _nvme_be_close (dev); free (ctrl_id); return FALSE; } @@ -246,38 +234,39 @@ gboolean bd_nvme_format (const gchar *device, guint16 lba_data_size, guint16 met } /* find out the desired LBA data format index */ - args.lbaf = find_lbaf_for_size (args.fd, args.nsid, lba_data_size, metadata_size, error); - if (args.lbaf == 0xff) { - close (args.fd); + lbaf = find_lbaf_for_size (dev, nsid, lba_data_size, metadata_size, error); + if (lbaf == 0xff) { + _nvme_be_close (dev); return FALSE; } switch (secure_erase) { case BD_NVME_FORMAT_SECURE_ERASE_USER_DATA: - args.ses = NVME_FORMAT_SES_USER_DATA_ERASE; + ses = NVME_FORMAT_SES_USER_DATA_ERASE; break; case BD_NVME_FORMAT_SECURE_ERASE_CRYPTO: - args.ses = NVME_FORMAT_SES_CRYPTO_ERASE; + ses = NVME_FORMAT_SES_CRYPTO_ERASE; break; case BD_NVME_FORMAT_SECURE_ERASE_NONE: default: - args.ses = NVME_FORMAT_SES_NONE; + ses = NVME_FORMAT_SES_NONE; } - ret = nvme_format_nvm (&args); + ret = _nvme_be_format (dev, nsid, lbaf, ses); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "Format NVM command error: "); - close (args.fd); + _nvme_be_close (dev); return FALSE; } /* rescan the namespaces if block size has changed */ if (ctrl_device) { - if (ioctl (args.fd, NVME_IOCTL_RESCAN) < 0) { + ret = _nvme_be_rescan_ns (dev); + if (ret < 0) { g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, - "Failed to rescan namespaces after format: %s", strerror_l (errno, _C_LOCALE)); - close (args.fd); + "Failed to rescan namespaces after format: %s", strerror_l (-ret, _C_LOCALE)); + _nvme_be_close (dev); return FALSE; } } else { @@ -289,23 +278,25 @@ gboolean bd_nvme_format (const gchar *device, guint16 lba_data_size, guint16 met */ int block_size = lba_data_size; - if (ioctl (args.fd, BLKBSZSET, &block_size) < 0) { + ret = _nvme_be_update_block_size (dev, block_size); + if (ret < 0) { g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, - "Failed to set block size to %d after format: %s", block_size, strerror_l (errno, _C_LOCALE)); - close (args.fd); + "Failed to set block size to %d after format: %s", block_size, strerror_l (-ret, _C_LOCALE)); + _nvme_be_close (dev); return FALSE; } - if (ioctl (args.fd, BLKRRPART) < 0) { + ret = _nvme_be_reread_partitions (dev); + if (ret < 0) { g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, - "Failed to re-read partition table after format: %s", strerror_l (errno, _C_LOCALE)); - close (args.fd); + "Failed to re-read partition table after format: %s", strerror_l (-ret, _C_LOCALE)); + _nvme_be_close (dev); return FALSE; } } } - close (args.fd); + _nvme_be_close (dev); return TRUE; } @@ -355,29 +346,21 @@ gboolean bd_nvme_format (const gchar *device, guint16 lba_data_size, guint16 met */ gboolean bd_nvme_sanitize (const gchar *device, BDNVMESanitizeAction action, gboolean no_dealloc, gint overwrite_pass_count, guint32 overwrite_pattern, gboolean overwrite_invert_pattern, GError **error) { int ret; - struct nvme_sanitize_nvm_args args = { - .args_size = sizeof(args), - .result = NULL, - .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .ause = TRUE, - .owpass = overwrite_pass_count, - .oipbp = overwrite_invert_pattern, - .nodas = no_dealloc, - .ovrpat = overwrite_pattern, - }; + NVMeDev *dev; + __u8 sanact; switch (action) { case BD_NVME_SANITIZE_ACTION_EXIT_FAILURE: - args.sanact = NVME_SANITIZE_SANACT_EXIT_FAILURE; + sanact = NVME_SANITIZE_SANACT_EXIT_FAILURE; break; case BD_NVME_SANITIZE_ACTION_BLOCK_ERASE: - args.sanact = NVME_SANITIZE_SANACT_START_BLOCK_ERASE; + sanact = NVME_SANITIZE_SANACT_START_BLOCK_ERASE; break; case BD_NVME_SANITIZE_ACTION_OVERWRITE: - args.sanact = NVME_SANITIZE_SANACT_START_OVERWRITE; + sanact = NVME_SANITIZE_SANACT_START_OVERWRITE; break; case BD_NVME_SANITIZE_ACTION_CRYPTO_ERASE: - args.sanact = NVME_SANITIZE_SANACT_START_CRYPTO_ERASE; + sanact = NVME_SANITIZE_SANACT_START_CRYPTO_ERASE; break; default: g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, @@ -386,18 +369,23 @@ gboolean bd_nvme_sanitize (const gchar *device, BDNVMESanitizeAction action, gbo } /* open the block device */ - args.fd = _open_dev (device, error); - if (args.fd < 0) + dev = _nvme_be_open (device, error); + if (!dev) return FALSE; - ret = nvme_sanitize_nvm (&args); + ret = _nvme_be_sanitize (dev, sanact, + TRUE /* ause */, + overwrite_pass_count, + overwrite_invert_pattern, + no_dealloc, + overwrite_pattern); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); g_prefix_error (error, "Sanitize command error: "); - close (args.fd); + _nvme_be_close (dev); return FALSE; } - close (args.fd); + _nvme_be_close (dev); return TRUE; } diff --git a/src/plugins/nvme/nvme-private.h b/src/plugins/nvme/nvme-private.h index 01656d80..e3a97c92 100644 --- a/src/plugins/nvme/nvme-private.h +++ b/src/plugins/nvme/nvme-private.h @@ -15,16 +15,14 @@ /* "C" locale to get the locale-agnostic error messages */ #define _C_LOCALE (locale_t) 0 -/* nvme-error.c */ +/* nvme-error.c, both functions take 0, a negative errno value or a positive status/error code */ G_GNUC_INTERNAL void _nvme_status_to_error (gint status, gboolean fabrics, GError **error); G_GNUC_INTERNAL -void _nvme_fabrics_errno_to_gerror (int result, int _errno, GError **error); +void _nvme_fabrics_errno_to_gerror (int result, GError **error); /* nvme-info.c */ G_GNUC_INTERNAL -gint _open_dev (const gchar *device, GError **error); -G_GNUC_INTERNAL void *_nvme_alloc (size_t len, GError **error); #endif /* BD_NVME_PRIVATE */ diff --git a/src/plugins/nvme/nvme.c b/src/plugins/nvme/nvme.c index 3641fc81..16d8e8fb 100644 --- a/src/plugins/nvme/nvme.c +++ b/src/plugins/nvme/nvme.c @@ -27,8 +27,6 @@ #include #include -#include - #include #include #include "nvme.h" From b77b955a10b0b46f7b29d0d21ef726ead3864d04 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 19 Oct 2026 14:18:33 +0000 Subject: [PATCH 4/7] nvme: Add a libnvme3 backend nvme-cli 3.0 merged libnvme into its tree and ships it as the new libnvme3 library with a reworked API. Devices are accessed through transport handles, commands are built with the nvme_init_*() helpers and errors are returned as negative errno values. Add a libnvme3 implementation of the backend API. The libnvme flavour is selected at build time with the new --with-libnvme configure option, by default libnvme3 is used when available. libnvme3 requires 3.2 or later. The Identify Controller bit field constants use the NVMe spec names since nvme-cli 3.2, e.g. NVME_CTRL_MEC_TWPME, NVME_CTRL_OACS_FNVMS, NVME_CTRL_OACS_NMS and NVME_CTRL_OACS_DSTS. Since 3.2, libnvme3.pc reports the nvme-cli release version, earlier releases report 1.0.0, the library ABI version. Following the DH-HMAC-CHAP to KX-HMAC-CHAP rename in nvme-cli, the "dhchap_key" and "dhchap_ctrl_key" extra arguments are now called "kxchap_secret" and "kxchap_ctrl_secret". The old names are kept as deprecated aliases. The configuration file is not supported with libnvme3 yet, libnvme3 dropped support for the JSON configuration file. Also note libnvme3 refuses a second identical connection to an already connected controller unless "duplicate_connect" is set. Resolves: https://github.com/storaged-project/libblockdev/issues/1211 Co-Authored-By: Claude Opus 5.5 --- configure.ac | 21 +- src/lib/plugin_apis/nvme.api | 20 +- src/plugins/nvme/Makefile.am | 8 +- src/plugins/nvme/nvme-backend.h | 7 +- src/plugins/nvme/nvme-fabrics.c | 24 +- src/plugins/nvme/nvme-libnvme3.c | 451 +++++++++++++++++++++++++++++++ src/plugins/nvme/nvme-libnvme3.h | 37 +++ tests/nvme_test.py | 5 +- 8 files changed, 545 insertions(+), 28 deletions(-) create mode 100644 src/plugins/nvme/nvme-libnvme3.c create mode 100644 src/plugins/nvme/nvme-libnvme3.h diff --git a/configure.ac b/configure.ac index 2f00ba27..4ea10ed1 100644 --- a/configure.ac +++ b/configure.ac @@ -150,6 +150,11 @@ AC_ARG_WITH([nvme], [], [with_nvme=yes]) +AC_ARG_WITH([libnvme], + AS_HELP_STRING([--with-libnvme=auto|libnvme3|libnvme1], [libnvme flavour to use for the nvme plugin @<:@default=auto@:>@]), + [], + [with_libnvme=auto]) + AC_SUBST([WITH_NVME], [0]) AM_CONDITIONAL(WITH_NVME, test "x$with_nvme" != "xno") AS_IF([test "x$with_nvme" != "xno"], @@ -279,11 +284,19 @@ AS_IF([test "x$with_nvdimm" != "xno"], []) AS_IF([test "x$with_nvme" != "xno"], - [LIBBLOCKDEV_PKG_CHECK_MODULES([NVME], [libnvme >= 1.3]) - AS_IF([$PKG_CONFIG --atleast-version=1.4 libnvme], - [AC_DEFINE([HAVE_LIBNVME_1_4])], []) + [AS_IF([test "x$with_libnvme" = "xauto"], + [PKG_CHECK_EXISTS([libnvme3 >= 3.2], [with_libnvme=libnvme3], [with_libnvme=libnvme1])]) + AS_CASE([$with_libnvme], + [libnvme3], [LIBBLOCKDEV_PKG_CHECK_MODULES([NVME], [libnvme3 >= 3.2]) + AC_DEFINE([HAVE_LIBNVME3], [], [Define if the nvme plugin uses libnvme3])], + [libnvme1], [LIBBLOCKDEV_PKG_CHECK_MODULES([NVME], [libnvme >= 1.3]) + AS_IF([$PKG_CONFIG --atleast-version=1.4 libnvme], + [AC_DEFINE([HAVE_LIBNVME_1_4])], [])], + [AC_MSG_ERROR([Invalid value '$with_libnvme' for --with-libnvme, use auto, libnvme3 or libnvme1])]) ], []) +AM_CONDITIONAL(WITH_LIBNVME3, test "x$with_nvme" != "xno" -a "x$with_libnvme" = "xlibnvme3") +AS_IF([test "x$with_nvme" != "xno"], [nvme_info="yes ($with_libnvme)"], [nvme_info="no"]) AS_IF([test "x$with_smart" != "xno"], [LIBBLOCKDEV_PKG_CHECK_MODULES([SMART], [libatasmart >= 0.17]) @@ -393,7 +406,7 @@ echo " MDRAID plugin: ${with_mdraid} MPath plugin ${with_mpath} NVDIMM plugin (deprecated): ${with_nvdimm} - NVMe plugin: ${with_nvme} + NVMe plugin: ${nvme_info} Part plugin: ${with_part} S390 plugin: ${s390_info} SMART plugin (libatasmart): ${with_smart} diff --git a/src/lib/plugin_apis/nvme.api b/src/lib/plugin_apis/nvme.api index 85bbe2bb..251abb01 100644 --- a/src/lib/plugin_apis/nvme.api +++ b/src/lib/plugin_apis/nvme.api @@ -1277,7 +1277,7 @@ gboolean bd_nvme_set_host_id (const gchar *host_id, GError **error); * @host_iface: (nullable): The network interface used on the host to connect to the Controller (e.g. IP `eth1`, `enp2s0`). This forces the connection to be made on a specific interface instead of letting the system decide. * @host_nqn: (nullable): Overrides the default Host NQN that identifies the NVMe Host. If this option is %NULL, the default is read from `/etc/nvme/hostnqn` first. * If that does not exist, the autogenerated NQN value from the NVMe Host kernel module is used next. The Host NQN uniquely identifies the NVMe Host. - * @host_id: (nullable): User-defined host UUID or %NULL to use default (as defined in `/etc/nvme/hostid`) + * @host_id: (nullable): User-defined host UUID or %NULL to use default (as defined in `/etc/nvme/hostid`). * @extra: (nullable) (array zero-terminated=1): Additional arguments. * @error: (out) (nullable): Place to store error (if any). * @@ -1293,12 +1293,14 @@ gboolean bd_nvme_set_host_id (const gchar *host_id, GError **error); * In addition to the primary options it's possible to supply @extra arguments: * - `"config"`: Use the specified JSON configuration file instead of the default file (see below) or * specify `"none"` to avoid reading any configuration file. - * - `"dhchap_key"`: NVMe In-band authentication secret in ASCII format as described - * in the NVMe 2.0 specification. When not specified, the secret is by default read - * from `/etc/nvme/hostkey`. In case that file does not exist no in-band authentication - * is attempted. - * - `"dhchap_ctrl_key"`: NVMe In-band authentication controller secret for bi-directional authentication. - * When not specified, no bi-directional authentication is attempted. + * - `"kxchap_secret"`: NVMe In-band authentication (KX-HMAC-CHAP, formerly DH-HMAC-CHAP) host secret + * in ASCII format as described in the NVMe specification. When not specified, + * the secret is taken from the configuration file or, with libnvme-1.x, read from + * `/etc/nvme/hostkey`, if present. Otherwise no in-band authentication is attempted. + * - `"kxchap_ctrl_secret"`: NVMe In-band authentication controller secret for bi-directional authentication. + * When not specified, the secret is taken from the configuration file, if present. + * Otherwise no bi-directional authentication is attempted. + * - `"dhchap_key"`, `"dhchap_ctrl_key"`: Deprecated aliases for `"kxchap_secret"` and `"kxchap_ctrl_secret"`. * - `"nr_io_queues"`: The number of I/O queues. * - `"nr_write_queues"`: Number of additional queues that will be used for write I/O. * - `"nr_poll_queues"`: Number of additional queues that will be used for polling latency sensitive I/O. @@ -1320,8 +1322,8 @@ gboolean bd_nvme_set_host_id (const gchar *host_id, GError **error); * Boolean values can be expressed by "0"/"1", "on"/"off" or "True"/"False" case-insensitive * strings. Failed numerical or boolean string conversions will result in the option being ignored. * - * By default additional options are read from the default configuration file `/etc/nvme/config.json`. - * This follows the default behaviour of `nvme-cli`. Use the @extra `"config"` argument + * With libnvme-1.x, additional options are by default read from the configuration file `/etc/nvme/config.json`. + * This follows the default behaviour of `nvme-cli` 2.x. Configuration files are not supported with libnvme3 yet. Use the @extra `"config"` argument * to either specify a different config file or disable use of it. The JSON configuration * file format is documented in [https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json](https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json). * As a rule @extra key names are kept consistent with the JSON config file schema. diff --git a/src/plugins/nvme/Makefile.am b/src/plugins/nvme/Makefile.am index 1f29f959..dcfc8216 100644 --- a/src/plugins/nvme/Makefile.am +++ b/src/plugins/nvme/Makefile.am @@ -12,8 +12,6 @@ libbd_nvme_la_SOURCES = \ nvme.c \ nvme-private.h \ nvme-backend.h \ - nvme-libnvme1.h \ - nvme-libnvme1.c \ nvme-info.c \ nvme-error.c \ nvme-op.c \ @@ -21,5 +19,11 @@ libbd_nvme_la_SOURCES = \ ../check_deps.c \ ../check_deps.h +if WITH_LIBNVME3 +libbd_nvme_la_SOURCES += nvme-libnvme3.h nvme-libnvme3.c +else +libbd_nvme_la_SOURCES += nvme-libnvme1.h nvme-libnvme1.c +endif + libbd_nvmeincludedir = $(includedir)/blockdev libbd_nvmeinclude_HEADERS = nvme.h diff --git a/src/plugins/nvme/nvme-backend.h b/src/plugins/nvme/nvme-backend.h index 93754390..8f173b84 100644 --- a/src/plugins/nvme/nvme-backend.h +++ b/src/plugins/nvme/nvme-backend.h @@ -24,7 +24,8 @@ * (nvme-info.c, nvme-op.c, nvme-fabrics.c, nvme-error.c) only uses the NVMe * specification types, constants and static inline helpers provided by the * libnvme headers. Every call into the library itself goes through the - * functions declared here, implemented by nvme-libnvme1.c. + * functions declared here, implemented by either nvme-libnvme1.c or + * nvme-libnvme3.c as selected by configure. * * Unless stated otherwise, functions returning int return 0 on success, * a negative errno value on failure or a positive NVMe status value. @@ -33,7 +34,11 @@ #include #include +#ifdef HAVE_LIBNVME3 +#include "nvme-libnvme3.h" +#else #include "nvme-libnvme1.h" +#endif #ifndef BD_NVME_BACKEND #define BD_NVME_BACKEND diff --git a/src/plugins/nvme/nvme-fabrics.c b/src/plugins/nvme/nvme-fabrics.c index 772edd26..b78dd833 100644 --- a/src/plugins/nvme/nvme-fabrics.c +++ b/src/plugins/nvme/nvme-fabrics.c @@ -100,10 +100,12 @@ static void parse_extra_args (const BDExtraArg **extra, NVMeConnectParams *param params->config_file = (*extra_i)->val; } } else - if (g_strcmp0 ((*extra_i)->opt, "dhchap_key") == 0) + if (g_strcmp0 ((*extra_i)->opt, "kxchap_secret") == 0 || + g_strcmp0 ((*extra_i)->opt, "dhchap_key") == 0 /* deprecated */) params->hostkey = (*extra_i)->val; else - if (g_strcmp0 ((*extra_i)->opt, "dhchap_ctrl_key") == 0) + if (g_strcmp0 ((*extra_i)->opt, "kxchap_ctrl_secret") == 0 || + g_strcmp0 ((*extra_i)->opt, "dhchap_ctrl_key") == 0 /* deprecated */) params->ctrlkey = (*extra_i)->val; else if (g_strcmp0 ((*extra_i)->opt, "hostsymname") == 0) @@ -189,12 +191,14 @@ static void parse_extra_args (const BDExtraArg **extra, NVMeConnectParams *param * In addition to the primary options it's possible to supply @extra arguments: * - `"config"`: Use the specified JSON configuration file instead of the default file (see below) or * specify `"none"` to avoid reading any configuration file. - * - `"dhchap_key"`: NVMe In-band authentication secret in ASCII format as described - * in the NVMe 2.0 specification. When not specified, the secret is by default read - * from `/etc/nvme/hostkey`. In case that file does not exist no in-band authentication - * is attempted. - * - `"dhchap_ctrl_key"`: NVMe In-band authentication controller secret for bi-directional authentication. - * When not specified, no bi-directional authentication is attempted. + * - `"kxchap_secret"`: NVMe In-band authentication (KX-HMAC-CHAP, formerly DH-HMAC-CHAP) host secret + * in ASCII format as described in the NVMe specification. When not specified, + * the secret is taken from the configuration file or, with libnvme-1.x, read from + * `/etc/nvme/hostkey`, if present. Otherwise no in-band authentication is attempted. + * - `"kxchap_ctrl_secret"`: NVMe In-band authentication controller secret for bi-directional authentication. + * When not specified, the secret is taken from the configuration file, if present. + * Otherwise no bi-directional authentication is attempted. + * - `"dhchap_key"`, `"dhchap_ctrl_key"`: Deprecated aliases for `"kxchap_secret"` and `"kxchap_ctrl_secret"`. * - `"nr_io_queues"`: The number of I/O queues. * - `"nr_write_queues"`: Number of additional queues that will be used for write I/O. * - `"nr_poll_queues"`: Number of additional queues that will be used for polling latency sensitive I/O. @@ -216,8 +220,8 @@ static void parse_extra_args (const BDExtraArg **extra, NVMeConnectParams *param * Boolean values can be expressed by "0"/"1", "on"/"off" or "True"/"False" case-insensitive * strings. Failed numerical or boolean string conversions will result in the option being ignored. * - * By default additional options are read from the default configuration file `/etc/nvme/config.json`. - * This follows the default behaviour of `nvme-cli`. Use the @extra `"config"` argument + * With libnvme-1.x, additional options are by default read from the configuration file `/etc/nvme/config.json`. + * This follows the default behaviour of `nvme-cli` 2.x. Configuration files are not supported with libnvme3 yet. Use the @extra `"config"` argument * to either specify a different config file or disable use of it. The JSON configuration * file format is documented in [https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json](https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json). * As a rule @extra key names are kept consistent with the JSON config file schema. diff --git a/src/plugins/nvme/nvme-libnvme3.c b/src/plugins/nvme/nvme-libnvme3.c new file mode 100644 index 00000000..1c1207c6 --- /dev/null +++ b/src/plugins/nvme/nvme-libnvme3.c @@ -0,0 +1,451 @@ +/* + * Copyright (C) 2026 SUSE LLC + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + * + * Author: Daniel Wagner + */ + +/* libnvme3 backend, see nvme-backend.h */ + +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include "nvme.h" +#include "nvme-private.h" +#include "nvme-backend.h" + + +struct _NVMeDev { + struct libnvme_global_ctx *ctx; + struct libnvme_transport_handle *hdl; +}; + +NVMeDev *_nvme_be_open (const gchar *device, GError **error) { + NVMeDev *dev; + gchar *realp; + int ret; + + /* libnvme_open() only takes the kernel device name, resolve any symlinks first */ + realp = realpath (device, NULL); + if (!realp) { + _nvme_status_to_error (-errno, FALSE, error); + g_prefix_error (error, "Failed to open device '%s': ", device); + return NULL; + } + + dev = g_new0 (NVMeDev, 1); + dev->ctx = libnvme_create_global_ctx (); + if (!dev->ctx) { + g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Failed to create libnvme context"); + free (realp); + g_free (dev); + return NULL; + } + + ret = libnvme_open (dev->ctx, realp, O_RDONLY, &dev->hdl); + free (realp); + if (ret != 0) { + _nvme_status_to_error (ret, FALSE, error); + g_prefix_error (error, "Failed to open device '%s': ", device); + libnvme_free_global_ctx (dev->ctx); + g_free (dev); + return NULL; + } + + return dev; +} + +void _nvme_be_close (NVMeDev *dev) { + libnvme_close (dev->hdl); + libnvme_free_global_ctx (dev->ctx); + g_free (dev); +} + +int _nvme_be_get_nsid (NVMeDev *dev, __u32 *nsid) { + return libnvme_get_nsid (dev->hdl, nsid); +} + +int _nvme_be_identify_ctrl (NVMeDev *dev, struct nvme_id_ctrl *ctrl_id) { + struct libnvme_passthru_cmd cmd; + + nvme_init_identify_ctrl (&cmd, ctrl_id); + return libnvme_exec_admin_passthru (dev->hdl, &cmd); +} + +int _nvme_be_identify_ns (NVMeDev *dev, __u32 nsid, struct nvme_id_ns *ns_info) { + struct libnvme_passthru_cmd cmd; + + nvme_init_identify_ns (&cmd, nsid, ns_info); + return libnvme_exec_admin_passthru (dev->hdl, &cmd); +} + +int _nvme_be_identify_ns_descs (NVMeDev *dev, __u32 nsid, struct nvme_ns_id_desc *descs) { + struct libnvme_passthru_cmd cmd; + + nvme_init_identify_ns_descs_list (&cmd, nsid, descs); + return libnvme_exec_admin_passthru (dev->hdl, &cmd); +} + +int _nvme_be_identify_independent_ns (NVMeDev *dev, __u32 nsid, struct nvme_id_independent_id_ns *ns_info) { + struct libnvme_passthru_cmd cmd; + + nvme_init_identify_csi_independent_identify_id_ns (&cmd, nsid, ns_info); + return libnvme_exec_admin_passthru (dev->hdl, &cmd); +} + +int _nvme_be_get_log_smart (NVMeDev *dev, struct nvme_smart_log *smart_log) { + struct libnvme_passthru_cmd cmd; + + nvme_init_get_log_smart (&cmd, NVME_NSID_ALL, smart_log); + return libnvme_get_log (dev->hdl, &cmd, FALSE /* rae */, NVME_LOG_PAGE_PDU_SIZE); +} + +int _nvme_be_get_log_error (NVMeDev *dev, guint entries, struct nvme_error_log_page *err_log) { + struct libnvme_passthru_cmd cmd; + + nvme_init_get_log_error (&cmd, entries, err_log); + return libnvme_get_log (dev->hdl, &cmd, FALSE /* rae */, NVME_LOG_PAGE_PDU_SIZE); +} + +int _nvme_be_get_log_self_test (NVMeDev *dev, struct nvme_self_test_log *self_test_log) { + struct libnvme_passthru_cmd cmd; + + nvme_init_get_log_device_self_test (&cmd, self_test_log); + return libnvme_get_log (dev->hdl, &cmd, FALSE /* rae */, NVME_LOG_PAGE_PDU_SIZE); +} + +int _nvme_be_get_log_sanitize (NVMeDev *dev, struct nvme_sanitize_log_page *sanitize_log) { + struct libnvme_passthru_cmd cmd; + + nvme_init_get_log_sanitize (&cmd, sanitize_log); + return libnvme_get_log (dev->hdl, &cmd, FALSE /* rae */, NVME_LOG_PAGE_PDU_SIZE); +} + +int _nvme_be_self_test (NVMeDev *dev, __u32 nsid, __u8 stc) { + struct libnvme_passthru_cmd cmd; + + nvme_init_dev_self_test (&cmd, nsid, stc); + return libnvme_exec_admin_passthru (dev->hdl, &cmd); +} + +int _nvme_be_format (NVMeDev *dev, __u32 nsid, __u8 lbaf, __u8 ses) { + struct libnvme_passthru_cmd cmd; + + nvme_init_format_nvm (&cmd, nsid, lbaf, + NVME_FORMAT_MSET_SEPARATE /* 0 */, + NVME_FORMAT_PI_DISABLE /* 0 */, + NVME_FORMAT_PIL_LAST /* 0 */, + ses); + return libnvme_exec_admin_passthru (dev->hdl, &cmd); +} + +int _nvme_be_sanitize (NVMeDev *dev, __u8 sanact, gboolean ause, __u8 owpass, gboolean oipbp, gboolean nodas, __u32 ovrpat) { + struct libnvme_passthru_cmd cmd; + + nvme_init_sanitize_nvm (&cmd, sanact, ause, owpass, oipbp, nodas, + FALSE /* emvs */, + FALSE /* preq */, + ovrpat); + return libnvme_exec_admin_passthru (dev->hdl, &cmd); +} + +int _nvme_be_rescan_ns (NVMeDev *dev) { + return libnvme_rescan_ns (dev->hdl); +} + +int _nvme_be_update_block_size (NVMeDev *dev, int block_size) { + return libnvme_update_block_size (dev->hdl, block_size); +} + +int _nvme_be_reread_partitions (NVMeDev *dev) { + return libnvme_reread_partitions (dev->hdl); +} + +const gchar *_nvme_be_status_to_string (int status, gboolean fabrics) { + return libnvme_status_to_string (status, fabrics); +} + +const gchar *_nvme_be_fabrics_errno_to_string (int err) { + return libnvme_errno_to_string (err); +} + +gchar *_nvme_be_uuid_to_string (unsigned char uuid[NVME_UUID_LEN]) { + gchar uuid_buf[NVME_UUID_LEN_STRING] = ZERO_INIT; + + if (libnvme_uuid_to_string (uuid, uuid_buf) == 0) + return g_strdup (uuid_buf); + return NULL; +} + +gchar *_nvme_be_read_hostnqn (void) { + struct libnvme_global_ctx *ctx; + gchar *hostnqn; + + ctx = libnvme_create_global_ctx (); + if (ctx == NULL) + return NULL; + /* FIXME: libnvme SYSCONFDIR might be different from PACKAGE_SYSCONF_DIR */ + hostnqn = libnvmf_read_hostnqn (ctx); + libnvme_free_global_ctx (ctx); + return hostnqn; +} + +gchar *_nvme_be_read_hostid (void) { + struct libnvme_global_ctx *ctx; + gchar *hostid; + + ctx = libnvme_create_global_ctx (); + if (ctx == NULL) + return NULL; + hostid = libnvmf_read_hostid (ctx); + libnvme_free_global_ctx (ctx); + return hostid; +} + +gchar *_nvme_be_generate_hostnqn (void) { + struct libnvme_global_ctx *ctx; + gchar *nqn; + + ctx = libnvme_create_global_ctx (); + if (ctx == NULL) + return NULL; + nqn = libnvmf_generate_hostnqn (ctx); + libnvme_free_global_ctx (ctx); + return nqn; +} + +static void _add_sysfs_dir (GPtrArray *ptr_array, const char *dir) { + if (dir) + g_ptr_array_add (ptr_array, g_strdup (dir)); +} + +NVMeTopoSubsys **_nvme_be_scan_topology (void) { + GPtrArray *subsystems; + struct libnvme_global_ctx *ctx; + struct libnvme_host *h; + struct libnvme_subsystem *s; + struct libnvme_ctrl *c; + struct libnvme_ns *n; + int ret; + + subsystems = g_ptr_array_new (); + + ctx = libnvme_create_global_ctx (); + if (ctx == NULL) { + bd_utils_log_format (BD_UTILS_LOG_WARNING, "Failed to create libnvme context"); + g_ptr_array_add (subsystems, NULL); + return (NVMeTopoSubsys **) g_ptr_array_free (subsystems, FALSE); + } + ret = libnvme_scan_topology (ctx, NULL, NULL); + if (ret < 0) + bd_utils_log_format (BD_UTILS_LOG_WARNING, "Failed to scan topology: %s", + strerror_l (-ret, _C_LOCALE)); + + libnvme_for_each_host (ctx, h) + libnvme_for_each_subsystem (h, s) { + NVMeTopoSubsys *subsys; + GPtrArray *ctrls; + GPtrArray *nss; + + subsys = g_new0 (NVMeTopoSubsys, 1); + subsys->host_nqn = g_strdup (libnvme_host_get_hostnqn (h)); + subsys->host_id = g_strdup (libnvme_host_get_hostid (h)); + subsys->subsysnqn = g_strdup (libnvme_subsystem_get_subsysnqn (s)); + + nss = g_ptr_array_new (); + libnvme_subsystem_for_each_ns (s, n) + _add_sysfs_dir (nss, libnvme_ns_get_sysfs_dir (n)); + g_ptr_array_add (nss, NULL); + subsys->ns_sysfs_dirs = (gchar **) g_ptr_array_free (nss, FALSE); + + ctrls = g_ptr_array_new (); + libnvme_subsystem_for_each_ctrl (s, c) { + NVMeTopoCtrl *ctrl; + + ctrl = g_new0 (NVMeTopoCtrl, 1); + ctrl->name = g_strdup (libnvme_ctrl_get_name (c)); + ctrl->sysfs_dir = g_strdup (libnvme_ctrl_get_sysfs_dir (c)); + nss = g_ptr_array_new (); + libnvme_ctrl_for_each_ns (c, n) + _add_sysfs_dir (nss, libnvme_ns_get_sysfs_dir (n)); + g_ptr_array_add (nss, NULL); + ctrl->ns_sysfs_dirs = (gchar **) g_ptr_array_free (nss, FALSE); + g_ptr_array_add (ctrls, ctrl); + } + g_ptr_array_add (ctrls, NULL); + subsys->ctrls = (NVMeTopoCtrl **) g_ptr_array_free (ctrls, FALSE); + + g_ptr_array_add (subsystems, subsys); + } + libnvme_free_global_ctx (ctx); + + g_ptr_array_add (subsystems, NULL); + return (NVMeTopoSubsys **) g_ptr_array_free (subsystems, FALSE); +} + +static int _apply_params (struct libnvmf_context *fctx, const NVMeConnectParams *params) { +#define APPLY_OPT(opt) { \ + if (params->opt.set) \ + libnvmf_context_set_##opt (fctx, params->opt.val); \ + } + APPLY_OPT (nr_io_queues); + APPLY_OPT (nr_write_queues); + APPLY_OPT (nr_poll_queues); + APPLY_OPT (queue_size); + APPLY_OPT (keep_alive_tmo); + APPLY_OPT (reconnect_delay); + APPLY_OPT (ctrl_loss_tmo); + APPLY_OPT (fast_io_fail_tmo); + APPLY_OPT (tos); + APPLY_OPT (duplicate_connect); + APPLY_OPT (disable_sqflow); + APPLY_OPT (hdr_digest); + APPLY_OPT (data_digest); + APPLY_OPT (tls); +#undef APPLY_OPT + + if (params->hostkey || params->ctrlkey || params->keyring || params->tls_key) + return libnvmf_context_set_crypto (fctx, params->hostkey, params->ctrlkey, + params->keyring, params->tls_key, NULL); + return 0; +} + +gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { + int ret; + gchar *host_nqn_val = NULL; + gchar *host_id_val = NULL; + struct libnvme_global_ctx *ctx; + struct libnvme_host *host; + struct libnvmf_context *fctx = NULL; + gboolean success = FALSE; + + ctx = libnvme_create_global_ctx (); + if (ctx == NULL) { + g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Failed to create libnvme context"); + return FALSE; + } + + /* HostNQN and HostID, read from /etc/nvme/ or generated when not specified */ + ret = libnvmf_host_get_ids (ctx, params->host_nqn, params->host_id, &host_nqn_val, &host_id_val); + if (ret != 0 || host_nqn_val == NULL || host_id_val == NULL) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, + "Could not determine HostNQN or HostID: %s", + ret != 0 ? libnvme_strerror (-ret) : "unknown error"); + goto out; + } + + /* existing controllers need to be known to detect duplicate connections */ + libnvme_skip_namespaces (ctx); + ret = libnvme_scan_topology (ctx, NULL, NULL); + if (ret < 0) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Failed to scan topology: %s", + strerror_l (-ret, _C_LOCALE)); + goto out; + } + + ret = libnvmf_context_create (ctx, NULL, NULL, NULL, NULL, &fctx); + if (ret == 0) + ret = libnvmf_context_set_connection (fctx, params->subsysnqn, params->transport, + params->transport_addr, params->transport_svcid, + params->host_traddr, params->host_iface); + if (ret == 0) + ret = libnvmf_context_set_hostnqn (fctx, host_nqn_val, host_id_val); + if (ret == 0) + ret = _apply_params (fctx, params); + if (ret != 0) { + _nvme_fabrics_errno_to_gerror (ret, error); + g_prefix_error (error, "Error creating the controller: "); + goto out; + } + + if (params->hostsymname) { + ret = libnvme_get_host (ctx, host_nqn_val, host_id_val, &host); + if (ret != 0) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Unable to lookup host for HostNQN '%s' and HostID '%s': %s", + host_nqn_val, host_id_val, libnvme_strerror (-ret)); + goto out; + } + libnvme_host_set_hostsymname (host, params->hostsymname); + } + + ret = libnvmf_connect (ctx, fctx); + if (ret != 0) { + _nvme_fabrics_errno_to_gerror (ret, error); + g_prefix_error (error, "Error connecting the controller: "); + goto out; + } + + success = TRUE; + +out: + libnvmf_context_free (fctx); + libnvme_free_global_ctx (ctx); + free (host_nqn_val); + free (host_id_val); + + return success; +} + +gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, gboolean *found, GError **error) { + struct libnvme_global_ctx *ctx; + struct libnvme_host *host; + struct libnvme_subsystem *subsys; + struct libnvme_ctrl *ctrl; + int ret; + + ctx = libnvme_create_global_ctx (); + if (ctx == NULL) { + g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Failed to create libnvme context"); + return FALSE; + } + ret = libnvme_scan_topology (ctx, NULL, NULL); + if (ret < 0) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Failed to scan topology: %s", + strerror_l (-ret, _C_LOCALE)); + libnvme_free_global_ctx (ctx); + return FALSE; + } + libnvme_for_each_host (ctx, host) + libnvme_for_each_subsystem (host, subsys) + if (!subsysnqn || g_strcmp0 (libnvme_subsystem_get_subsysnqn (subsys), subsysnqn) == 0) + libnvme_subsystem_for_each_ctrl (subsys, ctrl) + if (!name || g_strcmp0 (libnvme_ctrl_get_name (ctrl), name) == 0) { + ret = libnvmf_disconnect_ctrl (ctrl); + if (ret != 0) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Error disconnecting the controller: %s", + strerror_l (-ret, _C_LOCALE)); + libnvme_free_global_ctx (ctx); + return FALSE; + } + *found = TRUE; + } + libnvme_free_global_ctx (ctx); + return TRUE; +} diff --git a/src/plugins/nvme/nvme-libnvme3.h b/src/plugins/nvme/nvme-libnvme3.h new file mode 100644 index 00000000..9ebe334c --- /dev/null +++ b/src/plugins/nvme/nvme-libnvme3.h @@ -0,0 +1,37 @@ +/* + * Copyright (C) 2026 SUSE LLC + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + * + * Author: Daniel Wagner + */ + +/* + * NVMe specification types for the libnvme3 backend. + * + * See nvme-libnvme1.h for the BE_ prefixed names used by the generic plugin code. + */ + +#include + +#ifndef BD_NVME_LIBNVME3 +#define BD_NVME_LIBNVME3 + +#define BE_NVME_CTRL_MEC_TWPME NVME_CTRL_MEC_TWPME +#define BE_NVME_CTRL_OACS_FNVMS NVME_CTRL_OACS_FNVMS +#define BE_NVME_CTRL_OACS_NMS NVME_CTRL_OACS_NMS +#define BE_NVME_CTRL_OACS_DSTS NVME_CTRL_OACS_DSTS +#define BE_NVME_SANITIZE_SSTAT_STATUS_IN_PROGRESS NVME_SANITIZE_SSTAT_STATUS_IN_PROGRESS + +#endif /* BD_NVME_LIBNVME3 */ diff --git a/tests/nvme_test.py b/tests/nvme_test.py index 349b4a0e..267cc62e 100644 --- a/tests/nvme_test.py +++ b/tests/nvme_test.py @@ -381,8 +381,9 @@ def test_connect_single_ns(self): self.assertTrue(re.match(r'/dev/nvme[0-9]+n[0-9]+', ns)) self.assertTrue(os.path.exists(ns)) - # make a duplicate connection - ret = BlockDev.nvme_connect(self.SUBNQN, 'loop', None, None, None, None, None, None) + # make a duplicate connection, libnvme3 refuses it unless explicitly requested + ret = BlockDev.nvme_connect(self.SUBNQN, 'loop', None, None, None, None, None, None, + [BlockDev.ExtraArg.new("duplicate_connect", "True")]) self.assertTrue(ret) # should see two controllers now From 207c04a8bc46a868542df6e26f21840e415f941d Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 19 Oct 2026 14:24:29 +0000 Subject: [PATCH 5/7] nvme: Support the libnvme3 fabrics configuration file libnvme3 replaced the JSON configuration with an INI based /etc/nvme/nvme-fabrics.conf. Parameters of the first connection entry matching the bd_nvme_connect() arguments are applied, extra arguments still take precedence. Legacy JSON configuration files are not supported with libnvme3. nvme-cli converts an existing /etc/nvme/config.json to the INI format on its first fabrics command, but libnvme3 doesn't, and libblockdev doesn't either. Log a warning pointing to 'nvme config-convert' when the default configuration is used and an unconverted legacy file is found, as its options and keys would be silently ignored otherwise. A JSON file specified via the "config" extra argument fails to parse, mention the conversion in the error. libnvme3 treats a missing configuration file as an empty one, which is fine for the default file only. A missing file specified via the "config" extra argument is reported as an error. The libnvme-1.x backend keeps reading the JSON configuration file. Co-Authored-By: Claude Opus 5.5 --- src/lib/plugin_apis/nvme.api | 23 +++-- src/plugins/nvme/nvme-fabrics.c | 23 +++-- src/plugins/nvme/nvme-libnvme3.c | 143 +++++++++++++++++++++++++++++-- tests/nvme_test.py | 65 ++++++++++++++ tests/utils.py | 7 +- 5 files changed, 239 insertions(+), 22 deletions(-) diff --git a/src/lib/plugin_apis/nvme.api b/src/lib/plugin_apis/nvme.api index 251abb01..e2f14266 100644 --- a/src/lib/plugin_apis/nvme.api +++ b/src/lib/plugin_apis/nvme.api @@ -1291,7 +1291,7 @@ gboolean bd_nvme_set_host_id (const gchar *host_id, GError **error); * - `"loop"`: A NVMe over Fabrics target on the local host. * * In addition to the primary options it's possible to supply @extra arguments: - * - `"config"`: Use the specified JSON configuration file instead of the default file (see below) or + * - `"config"`: Use the specified configuration file instead of the default file (see below) or * specify `"none"` to avoid reading any configuration file. * - `"kxchap_secret"`: NVMe In-band authentication (KX-HMAC-CHAP, formerly DH-HMAC-CHAP) host secret * in ASCII format as described in the NVMe specification. When not specified, @@ -1322,11 +1322,22 @@ gboolean bd_nvme_set_host_id (const gchar *host_id, GError **error); * Boolean values can be expressed by "0"/"1", "on"/"off" or "True"/"False" case-insensitive * strings. Failed numerical or boolean string conversions will result in the option being ignored. * - * With libnvme-1.x, additional options are by default read from the configuration file `/etc/nvme/config.json`. - * This follows the default behaviour of `nvme-cli` 2.x. Configuration files are not supported with libnvme3 yet. Use the @extra `"config"` argument - * to either specify a different config file or disable use of it. The JSON configuration - * file format is documented in [https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json](https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json). - * As a rule @extra key names are kept consistent with the JSON config file schema. + * By default additional options are read from the default configuration file, following the + * default behaviour of `nvme-cli`. Use the @extra `"config"` argument to either specify a different + * config file or disable use of it. The configuration file format depends on the libnvme flavour + * libblockdev has been built with: + * - libnvme3: The INI configuration file `/etc/nvme/nvme-fabrics.conf` (and its `/etc/nvme/nvme-fabrics.conf.d/` + * drop-in directory). Parameters of the first connection entry matching the @subsysnqn, @transport, + * @transport_addr, @transport_svcid, @host_traddr and @host_iface arguments are applied, along with + * its host identity when @host_nqn or @host_id are not specified. The format is documented + * in [https://github.com/linux-nvme/nvme-cli/blob/master/libnvme/design/CONFIG.md](https://github.com/linux-nvme/nvme-cli/blob/master/libnvme/design/CONFIG.md), + * legacy JSON configuration files are not supported. Unlike `nvme-cli`, libblockdev doesn't convert + * an existing `/etc/nvme/config.json` automatically, a warning is logged instead. Use `nvme config-convert` + * to convert it. + * - libnvme-1.x: The JSON configuration file `/etc/nvme/config.json`. The format is documented + * in [https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json](https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json). + * As a rule @extra key names are kept consistent with the JSON config file schema. + * * Any @extra option generally overrides particular option specified in a configuration file. * * Returns: %TRUE if the subsystem was connected successfully, %FALSE otherwise with @error set. diff --git a/src/plugins/nvme/nvme-fabrics.c b/src/plugins/nvme/nvme-fabrics.c index b78dd833..570d922e 100644 --- a/src/plugins/nvme/nvme-fabrics.c +++ b/src/plugins/nvme/nvme-fabrics.c @@ -189,7 +189,7 @@ static void parse_extra_args (const BDExtraArg **extra, NVMeConnectParams *param * - `"loop"`: A NVMe over Fabrics target on the local host. * * In addition to the primary options it's possible to supply @extra arguments: - * - `"config"`: Use the specified JSON configuration file instead of the default file (see below) or + * - `"config"`: Use the specified configuration file instead of the default file (see below) or * specify `"none"` to avoid reading any configuration file. * - `"kxchap_secret"`: NVMe In-band authentication (KX-HMAC-CHAP, formerly DH-HMAC-CHAP) host secret * in ASCII format as described in the NVMe specification. When not specified, @@ -220,11 +220,22 @@ static void parse_extra_args (const BDExtraArg **extra, NVMeConnectParams *param * Boolean values can be expressed by "0"/"1", "on"/"off" or "True"/"False" case-insensitive * strings. Failed numerical or boolean string conversions will result in the option being ignored. * - * With libnvme-1.x, additional options are by default read from the configuration file `/etc/nvme/config.json`. - * This follows the default behaviour of `nvme-cli` 2.x. Configuration files are not supported with libnvme3 yet. Use the @extra `"config"` argument - * to either specify a different config file or disable use of it. The JSON configuration - * file format is documented in [https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json](https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json). - * As a rule @extra key names are kept consistent with the JSON config file schema. + * By default additional options are read from the default configuration file, following the + * default behaviour of `nvme-cli`. Use the @extra `"config"` argument to either specify a different + * config file or disable use of it. The configuration file format depends on the libnvme flavour + * libblockdev has been built with: + * - libnvme3: The INI configuration file `/etc/nvme/nvme-fabrics.conf` (and its `/etc/nvme/nvme-fabrics.conf.d/` + * drop-in directory). Parameters of the first connection entry matching the @subsysnqn, @transport, + * @transport_addr, @transport_svcid, @host_traddr and @host_iface arguments are applied, along with + * its host identity when @host_nqn or @host_id are not specified. The format is documented + * in [https://github.com/linux-nvme/nvme-cli/blob/master/libnvme/design/CONFIG.md](https://github.com/linux-nvme/nvme-cli/blob/master/libnvme/design/CONFIG.md), + * legacy JSON configuration files are not supported. Unlike `nvme-cli`, libblockdev doesn't convert + * an existing `/etc/nvme/config.json` automatically, a warning is logged instead. Use `nvme config-convert` + * to convert it. + * - libnvme-1.x: The JSON configuration file `/etc/nvme/config.json`. The format is documented + * in [https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json](https://raw.githubusercontent.com/linux-nvme/libnvme/master/doc/config-schema.json). + * As a rule @extra key names are kept consistent with the JSON config file schema. + * * Any @extra option generally overrides particular option specified in a configuration file. * * Returns: %TRUE if the subsystem was connected successfully, %FALSE otherwise with @error set. diff --git a/src/plugins/nvme/nvme-libnvme3.c b/src/plugins/nvme/nvme-libnvme3.c index 1c1207c6..66d10b7a 100644 --- a/src/plugins/nvme/nvme-libnvme3.c +++ b/src/plugins/nvme/nvme-libnvme3.c @@ -304,7 +304,59 @@ NVMeTopoSubsys **_nvme_be_scan_topology (void) { return (NVMeTopoSubsys **) g_ptr_array_free (subsystems, FALSE); } -static int _apply_params (struct libnvmf_context *fctx, const NVMeConnectParams *params) { +struct config_match_data { + const NVMeConnectParams *params; + const struct libnvmf_config_conn *conn; +}; + +static void config_match_conn (const struct libnvmf_config_conn *conn, void *user_data) { + struct config_match_data *data = user_data; + const NVMeConnectParams *params = data->params; + const gchar *val; + + /* first match wins */ + if (data->conn) + return; + + if (g_strcmp0 (libnvmf_config_conn_get_subsysnqn (conn), params->subsysnqn) != 0 || + g_strcmp0 (libnvmf_config_conn_get_transport (conn), params->transport) != 0 || + g_strcmp0 (libnvmf_config_conn_get_traddr (conn), params->transport_addr) != 0 || + g_strcmp0 (libnvmf_config_conn_get_host_traddr (conn), params->host_traddr) != 0 || + g_strcmp0 (libnvmf_config_conn_get_host_iface (conn), params->host_iface) != 0) + return; + + /* unset values in the config file match anything */ + val = libnvmf_config_conn_get_trsvcid (conn); + if (val && params->transport_svcid && g_strcmp0 (val, params->transport_svcid) != 0) + return; + val = libnvmf_config_conn_get_hostnqn (conn); + if (val && params->host_nqn && g_strcmp0 (val, params->host_nqn) != 0) + return; + + data->conn = conn; +} + +static const gchar *get_param_nonempty (const struct libnvmf_config_conn *conn, const gchar *key) { + const gchar *val; + + if (!conn) + return NULL; + val = libnvmf_params_get (libnvmf_config_conn_get_params (conn), key); + if (val && *val == '\0') + return NULL; + return val; +} + +/* applies the extra arguments on top of the configuration file entry @conn */ +static int _apply_params (struct libnvmf_context *fctx, const NVMeConnectParams *params, const struct libnvmf_config_conn *conn) { + int ret; + + if (conn) { + ret = libnvmf_context_apply_params (fctx, libnvmf_config_conn_get_params (conn)); + if (ret != 0) + return ret; + } + #define APPLY_OPT(opt) { \ if (params->opt.set) \ libnvmf_context_set_##opt (fctx, params->opt.val); \ @@ -326,18 +378,48 @@ static int _apply_params (struct libnvmf_context *fctx, const NVMeConnectParams #undef APPLY_OPT if (params->hostkey || params->ctrlkey || params->keyring || params->tls_key) - return libnvmf_context_set_crypto (fctx, params->hostkey, params->ctrlkey, - params->keyring, params->tls_key, NULL); + return libnvmf_context_set_crypto (fctx, + params->hostkey ? params->hostkey : get_param_nonempty (conn, "kxchap-secret"), + params->ctrlkey ? params->ctrlkey : get_param_nonempty (conn, "kxchap-ctrl-secret"), + params->keyring ? params->keyring : get_param_nonempty (conn, "keyring"), + params->tls_key /* NULL keeps the current key */, + get_param_nonempty (conn, "tls-key-identity")); return 0; } +/* Warns about a legacy JSON configuration that nvme-cli would convert to the INI format + * on its first fabrics command. libnvme3 doesn't read it and libblockdev doesn't convert it, + * its options and keys would be silently ignored otherwise. */ +static void _warn_legacy_config (void) { + /* FIXME: libnvme SYSCONFDIR might be different from PACKAGE_SYSCONF_DIR */ + gchar *ini = g_build_filename (PACKAGE_SYSCONF_DIR, "nvme", "nvme-fabrics.conf", NULL); + gchar *json = g_build_filename (PACKAGE_SYSCONF_DIR, "nvme", "config.json", NULL); + gchar *marker = g_strconcat (json, ".converted", NULL); + + if (!g_file_test (ini, G_FILE_TEST_EXISTS) && + g_file_test (json, G_FILE_TEST_EXISTS) && + !g_file_test (marker, G_FILE_TEST_EXISTS)) + bd_utils_log_format (BD_UTILS_LOG_WARNING, + "The legacy NVMe-oF configuration file %s is not supported by libnvme3 and is ignored, " + "run 'nvme config-convert' to convert it to %s", json, ini); + + g_free (ini); + g_free (json); + g_free (marker); +} + gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { int ret; + const gchar *host_nqn; + const gchar *host_id; + const gchar *hostsymname; gchar *host_nqn_val = NULL; gchar *host_id_val = NULL; struct libnvme_global_ctx *ctx; struct libnvme_host *host; struct libnvmf_context *fctx = NULL; + struct libnvmf_config *config = NULL; + struct config_match_data match = ZERO_INIT; gboolean success = FALSE; ctx = libnvme_create_global_ctx (); @@ -347,8 +429,54 @@ gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { return FALSE; } + /* look up the connection in the configuration file */ + if (params->use_config) { + /* libnvme treats a missing file as an empty configuration, which is only fine for the default */ + if (params->config_file && !g_file_test (params->config_file, G_FILE_TEST_EXISTS)) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, + "Failed to read configuration file '%s': %s", + params->config_file, strerror_l (ENOENT, _C_LOCALE)); + goto out; + } + if (!params->config_file) + _warn_legacy_config (); + /* NULL means the libnvme default configuration file */ + ret = libnvmf_config_read (ctx, params->config_file, &config); + if (ret != 0) { + if (params->config_file) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, + "Failed to read configuration file '%s': %s%s", + params->config_file, libnvme_strerror (-ret), + g_str_has_suffix (params->config_file, ".json") ? + " (legacy JSON configuration files are not supported with libnvme3, " + "convert it with 'nvme config-convert')" : ""); + goto out; + } + /* not fatal for the default configuration file */ + bd_utils_log_format (BD_UTILS_LOG_WARNING, + "Failed to read the default NVMe-oF configuration file: %s", + libnvme_strerror (-ret)); + config = NULL; + } + } + host_nqn = params->host_nqn; + host_id = params->host_id; + hostsymname = params->hostsymname; + if (config) { + match.params = params; + libnvmf_config_conn_for_each (config, config_match_conn, &match); + if (match.conn) { + if (!host_nqn) + host_nqn = libnvmf_config_conn_get_hostnqn (match.conn); + if (!host_id) + host_id = libnvmf_config_conn_get_hostid (match.conn); + if (!hostsymname) + hostsymname = libnvmf_config_conn_get_hostsymname (match.conn); + } + } + /* HostNQN and HostID, read from /etc/nvme/ or generated when not specified */ - ret = libnvmf_host_get_ids (ctx, params->host_nqn, params->host_id, &host_nqn_val, &host_id_val); + ret = libnvmf_host_get_ids (ctx, host_nqn, host_id, &host_nqn_val, &host_id_val); if (ret != 0 || host_nqn_val == NULL || host_id_val == NULL) { g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, "Could not determine HostNQN or HostID: %s", @@ -374,14 +502,14 @@ gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { if (ret == 0) ret = libnvmf_context_set_hostnqn (fctx, host_nqn_val, host_id_val); if (ret == 0) - ret = _apply_params (fctx, params); + ret = _apply_params (fctx, params, match.conn); if (ret != 0) { _nvme_fabrics_errno_to_gerror (ret, error); g_prefix_error (error, "Error creating the controller: "); goto out; } - if (params->hostsymname) { + if (hostsymname) { ret = libnvme_get_host (ctx, host_nqn_val, host_id_val, &host); if (ret != 0) { g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, @@ -389,7 +517,7 @@ gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { host_nqn_val, host_id_val, libnvme_strerror (-ret)); goto out; } - libnvme_host_set_hostsymname (host, params->hostsymname); + libnvme_host_set_hostsymname (host, hostsymname); } ret = libnvmf_connect (ctx, fctx); @@ -403,6 +531,7 @@ gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { out: libnvmf_context_free (fctx); + libnvmf_config_free (config); libnvme_free_global_ctx (ctx); free (host_nqn_val); free (host_id_val); diff --git a/tests/nvme_test.py b/tests/nvme_test.py index 267cc62e..25af8a10 100644 --- a/tests/nvme_test.py +++ b/tests/nvme_test.py @@ -12,6 +12,12 @@ from gi.repository import GLib, BlockDev +def plugin_uses_libnvme3(): + """ Whether the loaded nvme plugin has been built against libnvme3 """ + with open("/proc/self/maps") as f: + return "libnvme3.so" in f.read() + + @required_plugins(("nvme", "loop")) class NVMeTest(unittest.TestCase): requested_plugins = BlockDev.plugin_specs_from_names(("nvme", "loop")) @@ -624,6 +630,65 @@ def test_host_nqn(self): self._safe_unlink(HOSTID_PATH) + @tag_test(TestTags.CORE) + def test_connect_config(self): + """Test connecting with a libnvme3 fabrics configuration file""" + + if not plugin_uses_libnvme3(): + self.skipTest("the INI configuration file is only supported with libnvme3") + + # the configuration file requires a valid NQN and a transport address + subnqn = "nqn.2014-08.org.storaged:libblockdev-nvme-config" + traddr = "1" + self.addCleanup(self._clean_up) + self.dev_files += [create_sparse_tempfile("nvmeof_test0", 1024**3, dir=self.TMPDIR)] + setup_nvme_target(self.dev_files, subnqn, traddr) + os.unlink(self.dev_files[0]) + + config = os.path.join(self.TMPDIR, "libblockdev-nvme-fabrics.conf") + write_file(config, "[Subsystem]\n" + "nqn = %s\n" + "keep-alive-tmo = 17\n" + "controller = transport=loop;traddr=%s\n" % (subnqn, traddr)) + self.addCleanup(self._safe_unlink, config) + + def connect_get_kato(extra): + ret = BlockDev.nvme_connect(subnqn, 'loop', traddr, None, None, None, None, None, extra) + self.assertTrue(ret) + self.addCleanup(self._nvme_disconnect, subnqn, ignore_errors=True) + ctrls = find_nvme_ctrl_devs_for_subnqn(subnqn) + self.assertEqual(len(ctrls), 1) + kato = read_file("/sys/class/nvme/%s/kato" % os.path.basename(ctrls[0])).strip() + BlockDev.nvme_disconnect(subnqn) + return kato + + # parameters of the matching connection are applied + self.assertEqual(connect_get_kato([BlockDev.ExtraArg.new("config", config)]), "17") + + # extra arguments take precedence + self.assertEqual(connect_get_kato([BlockDev.ExtraArg.new("config", config), + BlockDev.ExtraArg.new("keep_alive_tmo", "23")]), "23") + + # no configuration file + self.assertNotEqual(connect_get_kato([BlockDev.ExtraArg.new("config", "none")]), "17") + + # a missing configuration file is an error when explicitly requested + with self.assertRaisesRegex(GLib.GError, r"Failed to read configuration file"): + BlockDev.nvme_connect(subnqn, 'loop', traddr, None, None, None, None, None, + [BlockDev.ExtraArg.new("config", "/nonexistent/nvme-fabrics.conf")]) + ctrls = find_nvme_ctrl_devs_for_subnqn(subnqn) + self.assertEqual(len(ctrls), 0) + + # legacy JSON configuration files are not supported anymore + json_config = os.path.join(self.TMPDIR, "libblockdev-nvme-config.json") + write_file(json_config, '[{"hostnqn": "%s", "subsystems": []}]\n' % self.hostnqn) + self.addCleanup(self._safe_unlink, json_config) + with self.assertRaisesRegex(GLib.GError, r"legacy JSON configuration files are not supported"): + BlockDev.nvme_connect(subnqn, 'loop', traddr, None, None, None, None, None, + [BlockDev.ExtraArg.new("config", json_config)]) + ctrls = find_nvme_ctrl_devs_for_subnqn(subnqn) + self.assertEqual(len(ctrls), 0) + @tag_test(TestTags.CORE) def test_persistent_dc(self): """Test connecting a persistent Discovery Controller""" diff --git a/tests/utils.py b/tests/utils.py index 14d1e052..1f45ffe4 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -401,13 +401,14 @@ def get_nvme_hostnqn(): return hostnqn.strip() -def setup_nvme_target(dev_paths, subnqn): +def setup_nvme_target(dev_paths, subnqn, traddr=""): """ Sets up a new NVMe target loop device (using nvmetcli) on top of the :param:`dev_paths` backing block devices. :param set dev_paths: set of backing block device paths :param str subnqn: Subsystem NQN + :param str traddr: transport address of the loop port, matched by the host when specified """ # modprobe required nvme target modules @@ -436,7 +437,7 @@ def setup_nvme_target(dev_paths, subnqn): { "addr": { "adrfam": "", - "traddr": "", + "traddr": "%s", "treq": "not specified", "trsvcid": "", "trtype": "loop" @@ -461,7 +462,7 @@ def setup_nvme_target(dev_paths, subnqn): ] } """ - tmp.write(json_str % (subnqn, namespaces, subnqn)) + tmp.write(json_str % (traddr, subnqn, namespaces, subnqn)) # export the loop device on the target ret, out, err = run_command("nvmetcli restore %s" % tcli_json_file) From 37fab767ea57201fb6f720453e99b91282f660f1 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 19 Oct 2026 14:38:19 +0000 Subject: [PATCH 6/7] nvme: Only disconnect controllers owned by libblockdev with libnvme3 libnvme3 keeps track of the owner of each connected NVMe-oF controller, e.g. boot volumes connected from the NBFT are owned by "nbft". Previously bd_nvme_disconnect() removed all matching controllers, including the ones other components depend on. With libnvme3, controllers connected by bd_nvme_connect() are now registered as owned by "libblockdev" (or by the new "owner" extra argument) and bd_nvme_disconnect() and bd_nvme_disconnect_by_path() only remove controllers owned by libblockdev. An already connected controller is never claimed. The new bd_nvme_disconnect_full() function allows to disconnect controllers of a different owner or, with BD_NVME_DISCONNECT_FORCE, regardless of their owner. libnvme-1.x has no ownership registry, the libnvme-1.x backend keeps disconnecting all matching controllers and ignores the owner. Co-Authored-By: Claude Opus 5.5 --- docs/libblockdev-sections.txt | 2 + src/lib/plugin_apis/nvme.api | 61 +++++++++++ src/plugins/nvme/nvme-backend.h | 17 ++- src/plugins/nvme/nvme-fabrics.c | 127 ++++++++++++++++++----- src/plugins/nvme/nvme-libnvme1.c | 4 +- src/plugins/nvme/nvme-libnvme3.c | 76 +++++++++++--- src/plugins/nvme/nvme.h | 17 +++ tests/nvme_test.py | 172 +++++++++++++++++++++++++++++++ 8 files changed, 434 insertions(+), 42 deletions(-) diff --git a/docs/libblockdev-sections.txt b/docs/libblockdev-sections.txt index 4a8f538d..2afaf5d3 100644 --- a/docs/libblockdev-sections.txt +++ b/docs/libblockdev-sections.txt @@ -835,6 +835,7 @@ bd_nvme_get_sanitize_log bd_nvme_sanitize_log_free bd_nvme_sanitize_log_copy BDNVMESanitizeAction +BDNVMEDisconnectFlags bd_nvme_sanitize bd_nvme_get_host_nqn bd_nvme_get_host_id @@ -844,6 +845,7 @@ bd_nvme_set_host_id bd_nvme_connect bd_nvme_disconnect bd_nvme_disconnect_by_path +bd_nvme_disconnect_full bd_nvme_find_ctrls_for_ns bd_nvme_find_namespaces_for_ctrl diff --git a/src/lib/plugin_apis/nvme.api b/src/lib/plugin_apis/nvme.api index e2f14266..a08547ec 100644 --- a/src/lib/plugin_apis/nvme.api +++ b/src/lib/plugin_apis/nvme.api @@ -32,6 +32,7 @@ GQuark bd_nvme_error_quark (void) { * @BD_NVME_ERROR_CONNECT_NODEV: Invalid interface. * @BD_NVME_ERROR_CONNECT_OPNOTSUPP: Operation not supported. * @BD_NVME_ERROR_CONNECT_REFUSED: Connection refused. + * @BD_NVME_ERROR_OWNED: The controller is not owned by the caller (see bd_nvme_disconnect()). */ /* BpG-skip-end */ typedef enum { @@ -53,6 +54,7 @@ typedef enum { BD_NVME_ERROR_CONNECT_NODEV, BD_NVME_ERROR_CONNECT_OPNOTSUPP, BD_NVME_ERROR_CONNECT_REFUSED, + BD_NVME_ERROR_OWNED, } BDNVMEError; typedef enum { @@ -1004,6 +1006,18 @@ typedef enum { BD_NVME_SANITIZE_ACTION_CRYPTO_ERASE = 3, } BDNVMESanitizeAction; +/* BpG-skip */ +/** + * BDNVMEDisconnectFlags: + * @BD_NVME_DISCONNECT_NONE: No flags. + * @BD_NVME_DISCONNECT_FORCE: Disconnect controllers regardless of their owner in the libnvme controller ownership registry. + */ +/* BpG-skip-end */ +typedef enum { + BD_NVME_DISCONNECT_NONE = 0, + BD_NVME_DISCONNECT_FORCE = 1 << 0, +} BDNVMEDisconnectFlags; + /** * bd_nvme_get_controller_info: @@ -1316,6 +1330,11 @@ gboolean bd_nvme_set_host_id (const gchar *host_id, GError **error); * - `"data_digest"`: Generates/verifies data digest (TCP). Boolean value. * - `"tls"`: Enable TLS encryption (TCP). Boolean value. * - `"hostsymname"`: TP8010: NVMe host symbolic name. + * - `"owner"`: Register a newly created controller with this owner in the libnvme controller ownership + * registry (`/run/nvme/registry/`) instead of the default `"libblockdev"`, e.g. `"udisks"`. + * An empty string leaves the controller without an owner. An already connected controller + * keeps its owner. See bd_nvme_disconnect() and bd_nvme_disconnect_full(). + * Only supported with libnvme3, ignored with libnvme-1.x. * - `"keyring"`: Keyring to store and lookup keys. String value. * - `"tls_key"`: TLS PSK for the connection. String value. * @@ -1355,8 +1374,17 @@ gboolean bd_nvme_connect (const gchar *subsysnqn, const gchar *transport, const * This may disconnect multiple controllers with matching @subsysnqn and %TRUE * is only returned when all controllers were disconnected successfully. * + * With libnvme3, only controllers owned by libblockdev according to the libnvme controller + * ownership registry are disconnected, i.e. the ones connected by bd_nvme_connect() without + * a specific `"owner"`. Controllers connected by other components, e.g. boot volumes + * connected via NBFT or connections managed by a discovery daemon, are left untouched. + * See bd_nvme_disconnect_full() to disconnect controllers of a different owner. + * libnvme-1.x has no ownership registry, all matching controllers are disconnected. + * * Returns: %TRUE if all matching controllers were disconnected successfully, %FALSE with @error * set in case of a disconnect error or when no matching controllers were found. + * The #BD_NVME_ERROR_OWNED error is returned when none of the matching controllers + * are owned by libblockdev. * * Tech category: %BD_NVME_TECH_FABRICS-%BD_NVME_TECH_MODE_INITIATOR */ @@ -1370,6 +1398,9 @@ gboolean bd_nvme_disconnect (const gchar *subsysnqn, GError **error); * Disconnects and removes a NVMe over Fabrics controller represented * by a block device path. * + * With libnvme3, only a controller owned by libblockdev according to the libnvme controller + * ownership registry is disconnected, see bd_nvme_disconnect() and bd_nvme_disconnect_full(). + * * Returns: %TRUE if the controller was disconnected successfully, * %FALSE otherwise with @error set. * @@ -1377,6 +1408,36 @@ gboolean bd_nvme_disconnect (const gchar *subsysnqn, GError **error); */ gboolean bd_nvme_disconnect_by_path (const gchar *path, GError **error); +/** + * bd_nvme_disconnect_full: + * @subsysnqn: (nullable): The name of the NVMe subsystem to disconnect. + * @path: (nullable): NVMe controller device to disconnect (e.g. `/dev/nvme0`). + * @owner: (nullable): The owner of the controllers to disconnect or %NULL for libblockdev. + * @flags: Flags to control the disconnect. + * @error: (out) (nullable): Place to store error (if any). + * + * Disconnects and removes existing NVMe over Fabrics controllers matching @subsysnqn + * or the controller represented by @path. Exactly one of @subsysnqn and @path has to + * be specified. + * + * Only controllers registered to @owner in the libnvme controller ownership registry + * (`/run/nvme/registry/`) are disconnected, see the `"owner"` argument of bd_nvme_connect(). + * With the #BD_NVME_DISCONNECT_FORCE flag all matching controllers are disconnected + * regardless of their owner, including controllers without any owner. Use with care, + * this may disconnect devices other components depend on, e.g. the root filesystem. + * + * libnvme-1.x has no ownership registry, @owner and @flags are ignored and all matching + * controllers are disconnected, same as with bd_nvme_disconnect(). + * + * Returns: %TRUE if all matching controllers were disconnected successfully, %FALSE with @error + * set in case of a disconnect error or when no matching controllers were found. + * The #BD_NVME_ERROR_OWNED error is returned when none of the matching controllers + * are owned by @owner. + * + * Tech category: %BD_NVME_TECH_FABRICS-%BD_NVME_TECH_MODE_INITIATOR + */ +gboolean bd_nvme_disconnect_full (const gchar *subsysnqn, const gchar *path, const gchar *owner, BDNVMEDisconnectFlags flags, GError **error); + /** * bd_nvme_find_ctrls_for_ns: * @ns_sysfs_path: NVMe namespace sysfs path. diff --git a/src/plugins/nvme/nvme-backend.h b/src/plugins/nvme/nvme-backend.h index 8f173b84..cb27cf5f 100644 --- a/src/plugins/nvme/nvme-backend.h +++ b/src/plugins/nvme/nvme-backend.h @@ -153,6 +153,7 @@ typedef struct { const gchar *hostsymname; const gchar *keyring; const gchar *tls_key; + const gchar *owner; /* %NULL for the default owner, "" for no owner */ NVMeOptInt nr_io_queues; NVMeOptInt nr_write_queues; NVMeOptInt nr_poll_queues; @@ -172,9 +173,19 @@ typedef struct { G_GNUC_INTERNAL gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error); -/* Disconnects all controllers matching @subsysnqn and/or the kernel controller @name. - * Sets @found when at least one matching controller has been disconnected. */ +/* owner registered for new connections unless specified otherwise */ +#define NVME_DEFAULT_OWNER "libblockdev" + +/* Disconnects all controllers matching @subsysnqn and/or the kernel controller @name + * that are owned by @owner, or all matching controllers when @force is set. + * Sets @found when at least one controller matches and @disconnected when at least + * one has been disconnected. @skipped_owner is set to the owner of a matching + * controller that has been skipped, if any, free with g_free(). + * + * Backends without an ownership registry (libnvme-1.x) ignore @owner and @force + * and disconnect all matching controllers. */ G_GNUC_INTERNAL -gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, gboolean *found, GError **error); +gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, const gchar *owner, gboolean force, + gboolean *found, gboolean *disconnected, gchar **skipped_owner, GError **error); #endif /* BD_NVME_BACKEND */ diff --git a/src/plugins/nvme/nvme-fabrics.c b/src/plugins/nvme/nvme-fabrics.c index 570d922e..29fd7e30 100644 --- a/src/plugins/nvme/nvme-fabrics.c +++ b/src/plugins/nvme/nvme-fabrics.c @@ -117,6 +117,9 @@ static void parse_extra_args (const BDExtraArg **extra, NVMeConnectParams *param if (g_strcmp0 ((*extra_i)->opt, "tls_key") == 0) params->tls_key = (*extra_i)->val; else + if (g_strcmp0 ((*extra_i)->opt, "owner") == 0) + params->owner = (*extra_i)->val; + else if (g_strcmp0 ((*extra_i)->opt, "nr_io_queues") == 0) SAFE_INT_CONV (params->nr_io_queues) else @@ -214,6 +217,11 @@ static void parse_extra_args (const BDExtraArg **extra, NVMeConnectParams *param * - `"data_digest"`: Generates/verifies data digest (TCP). Boolean value. * - `"tls"`: Enable TLS encryption (TCP). Boolean value. * - `"hostsymname"`: TP8010: NVMe host symbolic name. + * - `"owner"`: Register a newly created controller with this owner in the libnvme controller ownership + * registry (`/run/nvme/registry/`) instead of the default `"libblockdev"`, e.g. `"udisks"`. + * An empty string leaves the controller without an owner. An already connected controller + * keeps its owner. See bd_nvme_disconnect() and bd_nvme_disconnect_full(). + * Only supported with libnvme3, ignored with libnvme-1.x. * - `"keyring"`: Keyring to store and lookup keys. String value. * - `"tls_key"`: TLS PSK for the connection. String value. * @@ -289,6 +297,52 @@ gboolean bd_nvme_connect (const gchar *subsysnqn, const gchar *transport, const return _nvme_be_connect (¶ms, error); } +static gboolean _disconnect (const gchar *subsysnqn, const gchar *path, const gchar *owner, BDNVMEDisconnectFlags flags, GError **error) { + const gchar *name = NULL; + gboolean found = FALSE; + gboolean disconnected = FALSE; + gchar *skipped_owner = NULL; + gchar *what; + + if (path) { + name = path; + if (g_str_has_prefix (name, "/dev/")) + name += 5; + } + if (!owner) + owner = NVME_DEFAULT_OWNER; + + if (!_nvme_be_disconnect (subsysnqn, name, owner, flags & BD_NVME_DISCONNECT_FORCE, + &found, &disconnected, &skipped_owner, error)) + return FALSE; + + if (!found) { + if (path) + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_NO_MATCH, + "No controllers matching the %s device name found.", path); + else + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_NO_MATCH, + "No subsystems matching '%s' NQN found.", subsysnqn); + return FALSE; + } + if (!disconnected) { + what = path ? g_strdup_printf ("The %s controller is", path) + : g_strdup_printf ("Controllers of the '%s' subsystem are", subsysnqn); + if (skipped_owner) + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_OWNED, + "%s owned by '%s' instead of '%s', not disconnecting.", + what, skipped_owner, owner); + else + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_OWNED, + "%s not owned by '%s', not disconnecting.", what, owner); + g_free (what); + g_free (skipped_owner); + return FALSE; + } + g_free (skipped_owner); + return TRUE; +} + /** * bd_nvme_disconnect: * @subsysnqn: The name of the NVMe subsystem to disconnect. @@ -298,23 +352,22 @@ gboolean bd_nvme_connect (const gchar *subsysnqn, const gchar *transport, const * This may disconnect multiple controllers with matching @subsysnqn and %TRUE * is only returned when all controllers were disconnected successfully. * + * With libnvme3, only controllers owned by libblockdev according to the libnvme controller + * ownership registry are disconnected, i.e. the ones connected by bd_nvme_connect() without + * a specific `"owner"`. Controllers connected by other components, e.g. boot volumes + * connected via NBFT or connections managed by a discovery daemon, are left untouched. + * See bd_nvme_disconnect_full() to disconnect controllers of a different owner. + * libnvme-1.x has no ownership registry, all matching controllers are disconnected. + * * Returns: %TRUE if all matching controllers were disconnected successfully, %FALSE with @error * set in case of a disconnect error or when no matching controllers were found. + * The #BD_NVME_ERROR_OWNED error is returned when none of the matching controllers + * are owned by libblockdev. * * Tech category: %BD_NVME_TECH_FABRICS-%BD_NVME_TECH_MODE_INITIATOR */ gboolean bd_nvme_disconnect (const gchar *subsysnqn, GError **error) { - gboolean found = FALSE; - - if (!_nvme_be_disconnect (subsysnqn, NULL, &found, error)) - return FALSE; - - if (!found) { - g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_NO_MATCH, - "No subsystems matching '%s' NQN found.", subsysnqn); - return FALSE; - } - return TRUE; + return _disconnect (subsysnqn, NULL, NULL, BD_NVME_DISCONNECT_NONE, error); } /** @@ -325,28 +378,54 @@ gboolean bd_nvme_disconnect (const gchar *subsysnqn, GError **error) { * Disconnects and removes a NVMe over Fabrics controller represented * by a block device path. * + * With libnvme3, only a controller owned by libblockdev according to the libnvme controller + * ownership registry is disconnected, see bd_nvme_disconnect() and bd_nvme_disconnect_full(). + * * Returns: %TRUE if the controller was disconnected successfully, * %FALSE otherwise with @error set. * * Tech category: %BD_NVME_TECH_FABRICS-%BD_NVME_TECH_MODE_INITIATOR */ gboolean bd_nvme_disconnect_by_path (const gchar *path, GError **error) { - const gchar *p; - gboolean found = FALSE; - - p = path; - if (g_str_has_prefix (p, "/dev/")) - p += 5; - - if (!_nvme_be_disconnect (NULL, p, &found, error)) - return FALSE; + return _disconnect (NULL, path, NULL, BD_NVME_DISCONNECT_NONE, error); +} - if (!found) { - g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_NO_MATCH, - "No controllers matching the %s device name found.", path); +/** + * bd_nvme_disconnect_full: + * @subsysnqn: (nullable): The name of the NVMe subsystem to disconnect. + * @path: (nullable): NVMe controller device to disconnect (e.g. `/dev/nvme0`). + * @owner: (nullable): The owner of the controllers to disconnect or %NULL for libblockdev. + * @flags: Flags to control the disconnect. + * @error: (out) (nullable): Place to store error (if any). + * + * Disconnects and removes existing NVMe over Fabrics controllers matching @subsysnqn + * or the controller represented by @path. Exactly one of @subsysnqn and @path has to + * be specified. + * + * Only controllers registered to @owner in the libnvme controller ownership registry + * (`/run/nvme/registry/`) are disconnected, see the `"owner"` argument of bd_nvme_connect(). + * With the #BD_NVME_DISCONNECT_FORCE flag all matching controllers are disconnected + * regardless of their owner, including controllers without any owner. Use with care, + * this may disconnect devices other components depend on, e.g. the root filesystem. + * + * libnvme-1.x has no ownership registry, @owner and @flags are ignored and all matching + * controllers are disconnected, same as with bd_nvme_disconnect(). + * + * Returns: %TRUE if all matching controllers were disconnected successfully, %FALSE with @error + * set in case of a disconnect error or when no matching controllers were found. + * The #BD_NVME_ERROR_OWNED error is returned when none of the matching controllers + * are owned by @owner. + * + * Tech category: %BD_NVME_TECH_FABRICS-%BD_NVME_TECH_MODE_INITIATOR + */ +gboolean bd_nvme_disconnect_full (const gchar *subsysnqn, const gchar *path, const gchar *owner, BDNVMEDisconnectFlags flags, GError **error) { + if ((subsysnqn == NULL) == (path == NULL)) { + g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, + "Exactly one of the subsystem NQN and the controller path has to be specified"); return FALSE; } - return TRUE; + + return _disconnect (subsysnqn, path, owner, flags, error); } diff --git a/src/plugins/nvme/nvme-libnvme1.c b/src/plugins/nvme/nvme-libnvme1.c index 8325db0f..4872e66a 100644 --- a/src/plugins/nvme/nvme-libnvme1.c +++ b/src/plugins/nvme/nvme-libnvme1.c @@ -386,7 +386,8 @@ gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { return TRUE; } -gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, gboolean *found, GError **error) { +gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, const gchar *owner G_GNUC_UNUSED, gboolean force G_GNUC_UNUSED, + gboolean *found, gboolean *disconnected, gchar **skipped_owner G_GNUC_UNUSED, GError **error) { nvme_root_t root; nvme_host_t host; nvme_subsystem_t subsys; @@ -422,6 +423,7 @@ gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, gboolea return FALSE; } *found = TRUE; + *disconnected = TRUE; } nvme_free_tree (root); return TRUE; diff --git a/src/plugins/nvme/nvme-libnvme3.c b/src/plugins/nvme/nvme-libnvme3.c index 66d10b7a..6438c880 100644 --- a/src/plugins/nvme/nvme-libnvme3.c +++ b/src/plugins/nvme/nvme-libnvme3.c @@ -408,6 +408,27 @@ static void _warn_legacy_config (void) { g_free (marker); } +struct connected_data { + struct libnvme_global_ctx *ctx; + const gchar *owner; +}; + +static void connected_hook (struct libnvmf_context *fctx G_GNUC_UNUSED, struct libnvme_ctrl *c, void *user_data) { + struct connected_data *data = user_data; + int ret; + + /* Only called for a controller that has just been created. Registering the owner + * here rather than via libnvme_set_owner() avoids claiming an already connected + * controller that might be owned by someone else. */ + if (*data->owner == '\0') + return; + + ret = libnvmf_registry_update (data->ctx, libnvme_ctrl_get_name (c), "owner", data->owner); + if (ret != 0) + bd_utils_log_format (BD_UTILS_LOG_WARNING, "Failed to register owner '%s' for %s: %s", + data->owner, libnvme_ctrl_get_name (c), libnvme_strerror (-ret)); +} + gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { int ret; const gchar *host_nqn; @@ -420,6 +441,7 @@ gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { struct libnvmf_context *fctx = NULL; struct libnvmf_config *config = NULL; struct config_match_data match = ZERO_INIT; + struct connected_data hook_data; gboolean success = FALSE; ctx = libnvme_create_global_ctx (); @@ -494,7 +516,9 @@ gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { goto out; } - ret = libnvmf_context_create (ctx, NULL, NULL, NULL, NULL, &fctx); + hook_data.ctx = ctx; + hook_data.owner = params->owner ? params->owner : NVME_DEFAULT_OWNER; + ret = libnvmf_context_create (ctx, NULL, connected_hook, NULL, &hook_data, &fctx); if (ret == 0) ret = libnvmf_context_set_connection (fctx, params->subsysnqn, params->transport, params->transport_addr, params->transport_svcid, @@ -539,11 +563,14 @@ gboolean _nvme_be_connect (const NVMeConnectParams *params, GError **error) { return success; } -gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, gboolean *found, GError **error) { +gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, const gchar *owner, gboolean force, + gboolean *found, gboolean *disconnected, gchar **skipped_owner, GError **error) { struct libnvme_global_ctx *ctx; struct libnvme_host *host; struct libnvme_subsystem *subsys; struct libnvme_ctrl *ctrl; + gchar *ctrl_name; + gchar *ctrl_owner; int ret; ctx = libnvme_create_global_ctx (); @@ -562,19 +589,40 @@ gboolean _nvme_be_disconnect (const gchar *subsysnqn, const gchar *name, gboolea } libnvme_for_each_host (ctx, host) libnvme_for_each_subsystem (host, subsys) - if (!subsysnqn || g_strcmp0 (libnvme_subsystem_get_subsysnqn (subsys), subsysnqn) == 0) - libnvme_subsystem_for_each_ctrl (subsys, ctrl) - if (!name || g_strcmp0 (libnvme_ctrl_get_name (ctrl), name) == 0) { - ret = libnvmf_disconnect_ctrl (ctrl); - if (ret != 0) { - g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, - "Error disconnecting the controller: %s", - strerror_l (-ret, _C_LOCALE)); - libnvme_free_global_ctx (ctx); - return FALSE; - } - *found = TRUE; + libnvme_subsystem_for_each_ctrl (subsys, ctrl) { + if (subsysnqn && g_strcmp0 (libnvme_subsystem_get_subsysnqn (subsys), subsysnqn) != 0) + continue; + if (name && g_strcmp0 (libnvme_ctrl_get_name (ctrl), name) != 0) + continue; + *found = TRUE; + + /* a registry read error does not count as a match, never disconnect on error */ + if (!force && libnvmf_registry_attr_equal (ctx, libnvme_ctrl_get_name (ctrl), "owner", owner) != 0) { + if (*skipped_owner == NULL) { + ctrl_owner = NULL; + if (libnvmf_registry_retrieve (ctx, libnvme_ctrl_get_name (ctrl), "owner", &ctrl_owner) == 0 && ctrl_owner) + *skipped_owner = g_strdup (ctrl_owner); + free (ctrl_owner); } + continue; + } + + /* the name is cleared by the disconnect */ + ctrl_name = g_strdup (libnvme_ctrl_get_name (ctrl)); + ret = libnvmf_disconnect_ctrl (ctrl); + if (ret != 0) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Error disconnecting the controller: %s", + strerror_l (-ret, _C_LOCALE)); + g_free (ctrl_name); + libnvme_free_global_ctx (ctx); + return FALSE; + } + /* the udev rule shipped with libnvme removes it as well, don't wait for it */ + libnvmf_registry_delete (ctx, ctrl_name); + g_free (ctrl_name); + *disconnected = TRUE; + } libnvme_free_global_ctx (ctx); return TRUE; } diff --git a/src/plugins/nvme/nvme.h b/src/plugins/nvme/nvme.h index ae3e4f8f..0e2e8f9f 100644 --- a/src/plugins/nvme/nvme.h +++ b/src/plugins/nvme/nvme.h @@ -28,6 +28,7 @@ GQuark bd_nvme_error_quark (void); * @BD_NVME_ERROR_CONNECT_NODEV: Invalid interface. * @BD_NVME_ERROR_CONNECT_OPNOTSUPP: Operation not supported. * @BD_NVME_ERROR_CONNECT_REFUSED: Connection refused. + * @BD_NVME_ERROR_OWNED: The controller is not owned by the caller (see bd_nvme_disconnect()). */ typedef enum { BD_NVME_ERROR_TECH_UNAVAIL, @@ -48,6 +49,7 @@ typedef enum { BD_NVME_ERROR_CONNECT_NODEV, BD_NVME_ERROR_CONNECT_OPNOTSUPP, BD_NVME_ERROR_CONNECT_REFUSED, + BD_NVME_ERROR_OWNED, } BDNVMEError; typedef enum { @@ -536,6 +538,16 @@ typedef enum { BD_NVME_SANITIZE_ACTION_CRYPTO_ERASE = 3, } BDNVMESanitizeAction; +/** + * BDNVMEDisconnectFlags: + * @BD_NVME_DISCONNECT_NONE: No flags. + * @BD_NVME_DISCONNECT_FORCE: Disconnect controllers regardless of their owner in the libnvme controller ownership registry. + */ +typedef enum { + BD_NVME_DISCONNECT_NONE = 0, + BD_NVME_DISCONNECT_FORCE = 1 << 0, +} BDNVMEDisconnectFlags; + void bd_nvme_controller_info_free (BDNVMEControllerInfo *info); BDNVMEControllerInfo * bd_nvme_controller_info_copy (BDNVMEControllerInfo *info); @@ -622,6 +634,11 @@ gboolean bd_nvme_disconnect (const gchar *subsysn GError **error); gboolean bd_nvme_disconnect_by_path (const gchar *path, GError **error); +gboolean bd_nvme_disconnect_full (const gchar *subsysnqn, + const gchar *path, + const gchar *owner, + BDNVMEDisconnectFlags flags, + GError **error); gchar ** bd_nvme_find_ctrls_for_ns (const gchar *ns_sysfs_path, const gchar *subsysnqn, diff --git a/tests/nvme_test.py b/tests/nvme_test.py index 25af8a10..e1a74fac 100644 --- a/tests/nvme_test.py +++ b/tests/nvme_test.py @@ -10,6 +10,7 @@ gi.require_version('GLib', '2.0') gi.require_version('BlockDev', '3.0') from gi.repository import GLib, BlockDev +from gi.module import get_introspection_module def plugin_uses_libnvme3(): @@ -416,6 +417,177 @@ def test_connect_single_ns(self): self.assertEqual(len(namespaces), 0) + def _get_owner(self, ctrl): + ret, out, _err = run_command("nvme registry retrieve %s -a owner" % os.path.basename(ctrl)) + return out.strip() if ret == 0 else None + + def _connect_new_ctrl(self, connect_fn): + before = set(find_nvme_ctrl_devs_for_subnqn(self.SUBNQN, wait_for_ready=False)) + connect_fn() + new = set(find_nvme_ctrl_devs_for_subnqn(self.SUBNQN)) - before + self.assertEqual(len(new), 1) + return new.pop() + + def _bd_connect(self, owner=None, duplicate=False): + extra = [] + if owner is not None: + extra.append(BlockDev.ExtraArg.new("owner", owner)) + if duplicate: + extra.append(BlockDev.ExtraArg.new("duplicate_connect", "True")) + return self._connect_new_ctrl(lambda: self.assertTrue( + BlockDev.nvme_connect(self.SUBNQN, 'loop', None, None, None, None, None, None, extra))) + + def _cli_connect(self, owner=None, duplicate=False): + cmd = "nvme connect --transport=loop --nqn=%s" % self.SUBNQN + if owner is not None: + cmd += " --owner=%s" % owner + if duplicate: + cmd += " --duplicate-connect" + + def _connect(): + ret, out, err = run_command(cmd) + if ret != 0: + raise RuntimeError("Error connecting '%s': '%s %s'" % (cmd, out, err)) + return self._connect_new_ctrl(_connect) + + def _assert_not_owned(self, fn, *args): + with self.assertRaisesRegex(GLib.GError, r"not disconnecting") as cm: + fn(*args) + # BlockDev.NVMEError is shadowed by the exception class from the overrides + self.assertEqual(cm.exception.code, get_introspection_module('BlockDev').NVMEError.OWNED) + + @tag_test(TestTags.CORE) + def test_connect_owner(self): + """Test that libblockdev only disconnects its own controllers""" + + if not plugin_uses_libnvme3(): + self.skipTest("the controller ownership registry is only supported with libnvme3") + + self._setup_target(1) + self.addCleanup(self._nvme_disconnect, self.SUBNQN, ignore_errors=True) + + # libblockdev is the default owner + own = self._bd_connect() + self.assertEqual(self._get_owner(own), "libblockdev") + + # a custom owner + udisks = self._bd_connect(owner="udisks", duplicate=True) + self.assertEqual(self._get_owner(udisks), "udisks") + + # an empty owner leaves the controller unowned + unowned = self._bd_connect(owner="", duplicate=True) + self.assertIsNone(self._get_owner(unowned)) + + # only the controller owned by libblockdev is disconnected, together with its registry entry + self.assertTrue(BlockDev.nvme_disconnect(self.SUBNQN)) + self.assertFalse(os.path.exists(own)) + self.assertIsNone(self._get_owner(own)) + self.assertTrue(os.path.exists(udisks)) + self.assertTrue(os.path.exists(unowned)) + + # nothing owned by libblockdev is left + self._assert_not_owned(BlockDev.nvme_disconnect, self.SUBNQN) + self._assert_not_owned(BlockDev.nvme_disconnect_by_path, udisks) + self._assert_not_owned(BlockDev.nvme_disconnect_by_path, unowned) + self._assert_not_owned(BlockDev.nvme_disconnect_full, self.SUBNQN, None, "someone-else", + BlockDev.NVMEDisconnectFlags.NONE) + self.assertTrue(os.path.exists(udisks)) + self.assertTrue(os.path.exists(unowned)) + + # the owner can disconnect its controllers + self.assertTrue(BlockDev.nvme_disconnect_full(None, udisks, "udisks", BlockDev.NVMEDisconnectFlags.NONE)) + self.assertFalse(os.path.exists(udisks)) + self.assertTrue(os.path.exists(unowned)) + + # force disconnects regardless of the owner + self.assertTrue(BlockDev.nvme_disconnect_full(self.SUBNQN, None, None, BlockDev.NVMEDisconnectFlags.FORCE)) + self.assertFalse(os.path.exists(unowned)) + self.assertEqual(len(find_nvme_ctrl_devs_for_subnqn(self.SUBNQN)), 0) + + # owned by libblockdev again after a reconnect + own = self._bd_connect() + self.assertEqual(self._get_owner(own), "libblockdev") + self.assertTrue(BlockDev.nvme_disconnect_by_path(own)) + self.assertFalse(os.path.exists(own)) + + @tag_test(TestTags.CORE) + def test_disconnect_foreign(self): + """Test that controllers connected by others are left alone""" + + if not plugin_uses_libnvme3(): + self.skipTest("the controller ownership registry is only supported with libnvme3") + + self._setup_target(1) + self.addCleanup(self._nvme_disconnect, self.SUBNQN, ignore_errors=True) + + # e.g. a boot volume connected from the NBFT and a manual 'nvme connect' + nbft = self._cli_connect(owner="nbft") + manual = self._cli_connect(duplicate=True) + self.assertEqual(self._get_owner(nbft), "nbft") + self.assertIsNone(self._get_owner(manual)) + + # an already connected controller must not be claimed by libblockdev + with self.assertRaisesRegex(GLib.GError, r"Error connecting the controller: already connected"): + BlockDev.nvme_connect(self.SUBNQN, 'loop', None, None, None, None, None, None) + self.assertEqual(self._get_owner(nbft), "nbft") + self.assertIsNone(self._get_owner(manual)) + + self._assert_not_owned(BlockDev.nvme_disconnect, self.SUBNQN) + self._assert_not_owned(BlockDev.nvme_disconnect_by_path, nbft) + self._assert_not_owned(BlockDev.nvme_disconnect_by_path, manual) + self.assertTrue(os.path.exists(nbft)) + self.assertTrue(os.path.exists(manual)) + + # a libblockdev controller next to them is disconnected without touching the others + own = self._bd_connect(duplicate=True) + self.assertTrue(BlockDev.nvme_disconnect(self.SUBNQN)) + self.assertFalse(os.path.exists(own)) + self.assertTrue(os.path.exists(nbft)) + self.assertTrue(os.path.exists(manual)) + + # explicitly forced + self.assertTrue(BlockDev.nvme_disconnect_full(None, manual, None, BlockDev.NVMEDisconnectFlags.FORCE)) + self.assertFalse(os.path.exists(manual)) + self.assertTrue(BlockDev.nvme_disconnect_full(None, nbft, "nbft", BlockDev.NVMEDisconnectFlags.NONE)) + self.assertFalse(os.path.exists(nbft)) + + @tag_test(TestTags.CORE) + def test_disconnect_full_args(self): + """Test argument validation of bd_nvme_disconnect_full()""" + + msg = r"Exactly one of the subsystem NQN and the controller path has to be specified" + with self.assertRaisesRegex(GLib.GError, msg): + BlockDev.nvme_disconnect_full(None, None, None, BlockDev.NVMEDisconnectFlags.FORCE) + with self.assertRaisesRegex(GLib.GError, msg): + BlockDev.nvme_disconnect_full(self.SUBNQN, "/dev/nvme0", None, BlockDev.NVMEDisconnectFlags.NONE) + + with self.assertRaisesRegex(GLib.GError, r"No subsystems matching '.*' NQN found."): + BlockDev.nvme_disconnect_full(self.SUBNQN + "xx", None, None, BlockDev.NVMEDisconnectFlags.FORCE) + with self.assertRaisesRegex(GLib.GError, r"No controllers matching the /dev/nvmexx device name found."): + BlockDev.nvme_disconnect_full(None, "/dev/nvmexx", None, BlockDev.NVMEDisconnectFlags.FORCE) + + + @tag_test(TestTags.CORE) + def test_disconnect_libnvme1(self): + """Test that libnvme-1.x keeps disconnecting all matching controllers""" + + if plugin_uses_libnvme3(): + self.skipTest("libnvme3 only disconnects controllers owned by libblockdev") + + self._setup_target(1) + self.addCleanup(self._nvme_disconnect, self.SUBNQN, ignore_errors=True) + + # there's no ownership registry, the owner is ignored + ctrls = [self._bd_connect(owner="udisks"), self._bd_connect(duplicate=True)] + self.assertTrue(BlockDev.nvme_disconnect(self.SUBNQN)) + for c in ctrls: + self.assertFalse(os.path.exists(c)) + + own = self._bd_connect() + self.assertTrue(BlockDev.nvme_disconnect_full(None, own, "someone-else", BlockDev.NVMEDisconnectFlags.NONE)) + self.assertFalse(os.path.exists(own)) + + @tag_test(TestTags.CORE) def test_connect_multiple_ns(self): """Test connect and disconnect multiple namespaces""" From 02deb9f618a45a08421b99e3af0c6015de36cbee Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Mon, 19 Oct 2026 15:11:51 +0000 Subject: [PATCH 7/7] nvme: Don't disconnect all controllers on a missing selector bd_nvme_disconnect() and bd_nvme_disconnect_by_path() called with NULL matched every NVMe-oF controller and disconnected all of them, including boot volumes or the root filesystem. This is how the code always behaved, but neither argument is nullable and disconnecting everything is the worst possible reaction to a programming error. Require exactly one of the subsystem NQN and the controller path for all disconnect functions, as bd_nvme_disconnect_full() already does. Bindings like PyGObject reject NULL for these arguments anyway, only C callers are affected. Co-Authored-By: Claude Opus 5.5 --- src/plugins/nvme/nvme-fabrics.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/plugins/nvme/nvme-fabrics.c b/src/plugins/nvme/nvme-fabrics.c index 29fd7e30..d97ed3c2 100644 --- a/src/plugins/nvme/nvme-fabrics.c +++ b/src/plugins/nvme/nvme-fabrics.c @@ -304,6 +304,13 @@ static gboolean _disconnect (const gchar *subsysnqn, const gchar *path, const gc gchar *skipped_owner = NULL; gchar *what; + /* the backends treat a NULL selector as "match all", never disconnect everything by accident */ + if ((subsysnqn == NULL) == (path == NULL)) { + g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, + "Exactly one of the subsystem NQN and the controller path has to be specified"); + return FALSE; + } + if (path) { name = path; if (g_str_has_prefix (name, "/dev/")) @@ -419,12 +426,6 @@ gboolean bd_nvme_disconnect_by_path (const gchar *path, GError **error) { * Tech category: %BD_NVME_TECH_FABRICS-%BD_NVME_TECH_MODE_INITIATOR */ gboolean bd_nvme_disconnect_full (const gchar *subsysnqn, const gchar *path, const gchar *owner, BDNVMEDisconnectFlags flags, GError **error) { - if ((subsysnqn == NULL) == (path == NULL)) { - g_set_error_literal (error, BD_NVME_ERROR, BD_NVME_ERROR_INVALID_ARGUMENT, - "Exactly one of the subsystem NQN and the controller path has to be specified"); - return FALSE; - } - return _disconnect (subsysnqn, path, owner, flags, error); }