From 75da9d77116fcae1df4db0302c53dd4662fa6026 Mon Sep 17 00:00:00 2001 From: Tomas Bzatek Date: Wed, 24 Sep 2025 11:17:24 +0200 Subject: [PATCH 1/2] nvme: Default to well-known tr_svcid values when not specified In case the transport_svcid argument is NULL, supply sane default values. For the TCP and RDMA transport it means port 4420 for I/O controllers and 8009 for discovery controllers, distinguished by the subsystem NQN specified. This may change default behaviour slightly from previous libblockdev versions as the behaviour was mostly undefined (from libblockdev side). --- src/lib/plugin_apis/nvme.api | 2 +- src/plugins/nvme/nvme-fabrics.c | 14 +++++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lib/plugin_apis/nvme.api b/src/lib/plugin_apis/nvme.api index 604eaf9b1..9939f95fe 100644 --- a/src/lib/plugin_apis/nvme.api +++ b/src/lib/plugin_apis/nvme.api @@ -1270,7 +1270,7 @@ gboolean bd_nvme_set_host_id (const gchar *host_id, GError **error); * @subsysnqn: The name for the NVMe subsystem to connect to. * @transport: The network fabric used for a NVMe-over-Fabrics network. * @transport_addr: (nullable): The network address of the Controller. For transports using IP addressing (e.g. `rdma`) this should be an IP-based address. - * @transport_svcid: (nullable): The transport service id. For transports using IP addressing (e.g. `rdma`) this field is the port number. By default, the IP port number for the `RDMA` transport is `4420`. + * @transport_svcid: (nullable): The transport service ID. For transports using IP addressing (e.g. `tcp`, `rdma`) this field is the port number. The default port number for the `tcp` and `rdma` transports is `4420` and `8009` respectively when the well-known Discovery NQN is specified. * @host_traddr: (nullable): The network address used on the host to connect to the Controller. For TCP, this sets the source address on the socket. * @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. diff --git a/src/plugins/nvme/nvme-fabrics.c b/src/plugins/nvme/nvme-fabrics.c index 1877845f2..fe46eb622 100644 --- a/src/plugins/nvme/nvme-fabrics.c +++ b/src/plugins/nvme/nvme-fabrics.c @@ -156,7 +156,7 @@ static void parse_extra_args (const BDExtraArg **extra, struct nvme_fabrics_conf * @subsysnqn: The name for the NVMe subsystem to connect to. * @transport: The network fabric used for a NVMe-over-Fabrics network. * @transport_addr: (nullable): The network address of the Controller. For transports using IP addressing (e.g. `rdma`) this should be an IP-based address. - * @transport_svcid: (nullable): The transport service id. For transports using IP addressing (e.g. `rdma`) this field is the port number. By default, the IP port number for the `RDMA` transport is `4420`. + * @transport_svcid: (nullable): The transport service ID. For transports using IP addressing (e.g. `tcp`, `rdma`) this field is the port number. The default port number for the `tcp` and `rdma` transports is `4420` and `8009` respectively when the well-known Discovery NQN is specified. * @host_traddr: (nullable): The network address used on the host to connect to the Controller. For TCP, this sets the source address on the socket. * @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. @@ -300,6 +300,18 @@ gboolean bd_nvme_connect (const gchar *subsysnqn, const gchar *transport, const if (hostsymname) nvme_host_set_hostsymname (host, hostsymname); + /* tr_svcid defaults */ + if (!transport_svcid) { + if (g_strcmp0 (transport, "tcp") == 0) { + if (g_strcmp0 (subsysnqn, NVME_DISC_SUBSYS_NAME) == 0) + transport_svcid = G_STRINGIFY (NVME_DISC_IP_PORT); + else + transport_svcid = G_STRINGIFY (NVME_RDMA_IP_PORT); + } else + if (g_strcmp0(transport, "rdma") == 0) + 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); From cb9fc6c05af84756d035d31037c8f700a5342cc7 Mon Sep 17 00:00:00 2001 From: Tomas Bzatek Date: Wed, 24 Sep 2025 11:59:38 +0200 Subject: [PATCH 2/2] nvme: Handle memory allocation failures from _nvme_alloc() --- src/plugins/nvme/nvme-info.c | 82 ++++++++++++++++++++++----------- src/plugins/nvme/nvme-op.c | 12 +++-- src/plugins/nvme/nvme-private.h | 2 +- 3 files changed, 65 insertions(+), 31 deletions(-) diff --git a/src/plugins/nvme/nvme-info.c b/src/plugins/nvme/nvme-info.c index 947b1f005..3373d2013 100644 --- a/src/plugins/nvme/nvme-info.c +++ b/src/plugins/nvme/nvme-info.c @@ -405,13 +405,16 @@ gint _open_dev (const gchar *device, GError **error) { /* backported from nvme-cli: https://github.com/linux-nvme/nvme-cli/pull/2051 */ #define ROUND_UP(N, S) ((((N) + (S) - 1) / (S)) * (S)) -void *_nvme_alloc (size_t len) +void *_nvme_alloc (size_t len, GError **error) { size_t _len = ROUND_UP (len, 0x1000); void *p; - if (posix_memalign ((void *) &p, getpagesize (), _len)) + if (posix_memalign ((void *) &p, getpagesize (), _len)) { + g_set_error (error, BD_NVME_ERROR, BD_NVME_ERROR_FAILED, + "Memory allocation failed: %m"); return NULL; + } memset (p, 0, _len); return p; @@ -475,8 +478,9 @@ BDNVMEControllerInfo * bd_nvme_get_controller_info (const gchar *device, GError if (fd < 0) return NULL; - ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl)); - g_warn_if_fail (ctrl_id != NULL); + ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); + if (!ctrl_id) + return NULL; /* send the NVME_IDENTIFY_CNS_CTRL ioctl */ ret = nvme_identify_ctrl (fd, ctrl_id); if (ret != 0) { @@ -616,8 +620,11 @@ BDNVMENamespaceInfo *bd_nvme_get_namespace_info (const gchar *device, GError **e } /* send the NVME_IDENTIFY_CNS_NS ioctl */ - ns_info = _nvme_alloc (sizeof (struct nvme_id_ns)); - g_warn_if_fail (ns_info != NULL); + ns_info = _nvme_alloc (sizeof (struct nvme_id_ns), error); + if (!ns_info) { + close (fd); + return NULL; + } ret = nvme_identify_ns (fd, nsid, ns_info); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); @@ -628,22 +635,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)); - g_warn_if_fail (ctrl_id != NULL); + ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); + if (!ctrl_id) { + close (fd); + free (ns_info); + return NULL; + } ret_ctrl = nvme_identify_ctrl (fd, 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); - g_warn_if_fail (descs != NULL); - ret_desc = nvme_identify_ns_descs (fd, nsid, descs); + descs = _nvme_alloc (NVME_IDENTIFY_DATA_SIZE, NULL); + if (descs != NULL) + ret_desc = nvme_identify_ns_descs (fd, 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)); - g_warn_if_fail (ns_info_ind != NULL); - ret_ns_ind = nvme_identify_independent_identify_ns (fd, nsid, ns_info_ind); + 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); } close (fd); @@ -762,8 +773,11 @@ BDNVMESmartLog * bd_nvme_get_smart_log (const gchar *device, GError **error) { return NULL; /* send the NVME_IDENTIFY_CNS_CTRL ioctl */ - ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl)); - g_warn_if_fail (ctrl_id != NULL); + ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); + if (!ctrl_id) { + close (fd); + return NULL; + } ret_identify = nvme_identify_ctrl (fd, ctrl_id); if (ret_identify != 0) { _nvme_status_to_error (ret_identify, FALSE, error); @@ -774,8 +788,12 @@ 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)); - g_warn_if_fail (smart_log != NULL); + smart_log = _nvme_alloc (sizeof (struct nvme_smart_log), error); + if (!smart_log) { + close (fd); + free (ctrl_id); + return NULL; + } ret = nvme_get_log_smart (fd, NVME_NSID_ALL, FALSE /* rae */, smart_log); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); @@ -867,8 +885,11 @@ BDNVMEErrorLogEntry ** bd_nvme_get_error_log_entries (const gchar *device, GErro 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)); - g_warn_if_fail (ctrl_id != NULL); + ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); + if (!ctrl_id) { + close (fd); + return NULL; + } ret = nvme_identify_ctrl (fd, ctrl_id); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); @@ -882,8 +903,11 @@ BDNVMEErrorLogEntry ** bd_nvme_get_error_log_entries (const gchar *device, GErro free (ctrl_id); /* send the NVME_LOG_LID_ERROR ioctl */ - err_log = _nvme_alloc (sizeof (struct nvme_error_log_page) * elpe); - g_warn_if_fail (err_log != NULL); + err_log = _nvme_alloc (sizeof (struct nvme_error_log_page) * elpe, error); + if (!err_log) { + close (fd); + return NULL; + } ret = nvme_get_log_error (fd, elpe, FALSE /* rae */, err_log); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); @@ -948,8 +972,11 @@ BDNVMESelfTestLog * bd_nvme_get_self_test_log (const gchar *device, GError **err return NULL; /* send the NVME_LOG_LID_DEVICE_SELF_TEST ioctl */ - self_test_log = _nvme_alloc (sizeof (struct nvme_self_test_log)); - g_warn_if_fail (self_test_log != NULL); + self_test_log = _nvme_alloc (sizeof (struct nvme_self_test_log), error); + if (!self_test_log) { + close (fd); + return NULL; + } ret = nvme_get_log_device_self_test (fd, self_test_log); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); @@ -1092,8 +1119,11 @@ BDNVMESanitizeLog * bd_nvme_get_sanitize_log (const gchar *device, GError **erro return NULL; /* send the NVME_LOG_LID_SANITIZE ioctl */ - sanitize_log = _nvme_alloc (sizeof (struct nvme_sanitize_log_page)); - g_warn_if_fail (sanitize_log != NULL); + sanitize_log = _nvme_alloc (sizeof (struct nvme_sanitize_log_page), error); + if (!sanitize_log) { + close (fd); + return NULL; + } ret = nvme_get_log_sanitize (fd, FALSE /* rae */, sanitize_log); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); diff --git a/src/plugins/nvme/nvme-op.c b/src/plugins/nvme/nvme-op.c index dbef4f3a2..6360a1887 100644 --- a/src/plugins/nvme/nvme-op.c +++ b/src/plugins/nvme/nvme-op.c @@ -121,8 +121,9 @@ static __u8 find_lbaf_for_size (int fd, __u32 nsid, guint16 lba_data_size, guint guint i; /* TODO: find first attached namespace instead of hardcoding NSID = 1 */ - ns_info = _nvme_alloc (sizeof (struct nvme_id_ns)); - g_warn_if_fail (ns_info != NULL); + 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); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); @@ -214,8 +215,11 @@ gboolean bd_nvme_format (const gchar *device, guint16 lba_data_size, guint16 met /* check the FNA controller bit when formatting a single namespace */ if (! ctrl_device) { - ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl)); - g_warn_if_fail (ctrl_id != NULL); + ctrl_id = _nvme_alloc (sizeof (struct nvme_id_ctrl), error); + if (!ctrl_id) { + close (args.fd); + return FALSE; + } ret = nvme_identify_ctrl (args.fd, ctrl_id); if (ret != 0) { _nvme_status_to_error (ret, FALSE, error); diff --git a/src/plugins/nvme/nvme-private.h b/src/plugins/nvme/nvme-private.h index 0d15fbb64..01656d80c 100644 --- a/src/plugins/nvme/nvme-private.h +++ b/src/plugins/nvme/nvme-private.h @@ -25,6 +25,6 @@ void _nvme_fabrics_errno_to_gerror (int result, int _errno, GError **error); G_GNUC_INTERNAL gint _open_dev (const gchar *device, GError **error); G_GNUC_INTERNAL -void *_nvme_alloc (size_t len); +void *_nvme_alloc (size_t len, GError **error); #endif /* BD_NVME_PRIVATE */