From 419180ea1fb61f0f277e33403036a6dac2642768 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Thu, 8 May 2025 10:58:03 +0200 Subject: [PATCH 1/2] nvme: check errno not return value Unfortunately, libnvme follows the POSIX way to return errors. This means the return value is not containing the error code, it is in errno. While at it, also print the returned value only when debug mode is enabled. Signed-off-by: Daniel Wagner --- fabrics.c | 9 ++++++--- util/logging.c | 10 ++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/fabrics.c b/fabrics.c index 1e164228a9..caf5ea7384 100644 --- a/fabrics.c +++ b/fabrics.c @@ -179,12 +179,15 @@ static int nvme_add_ctrl(nvme_host_t h, nvme_ctrl_t c, retry: ret = nvmf_add_ctrl(h, c, cfg); - if (ret == EAGAIN || (ret == EINTR && !nvme_sigint_received)) { - printf("nvmf_add_ctrl returned '%s'\n", strerror(ret)); + if (!ret) + return 0; + + if (errno == EAGAIN || (errno == EINTR && !nvme_sigint_received)) { + print_debug("nvmf_add_ctrl returned '%s'\n", strerror(errno)); goto retry; } - return ret; + return -errno; } static nvme_ctrl_t __create_discover_ctrl(nvme_root_t r, nvme_host_t h, diff --git a/util/logging.c b/util/logging.c index e05483916a..8b40c14fce 100644 --- a/util/logging.c +++ b/util/logging.c @@ -116,8 +116,9 @@ int nvme_submit_passthru(int fd, unsigned long ioctl_cmd, if (!dry_run) { retry: err = ioctl(fd, ioctl_cmd, cmd); - if (err == EAGAIN || (err == EINTR && !nvme_sigint_received)) { - nvme_log_retry(err); + if (err && (errno == EAGAIN || + (errno == EINTR && !nvme_sigint_received))) { + nvme_log_retry(errno); goto retry; } } @@ -148,8 +149,9 @@ int nvme_submit_passthru64(int fd, unsigned long ioctl_cmd, if (!dry_run) { retry: err = ioctl(fd, ioctl_cmd, cmd); - if (err == EAGAIN || (err == EINTR && !nvme_sigint_received)) { - nvme_log_retry(err); + if (err && (errno == EAGAIN || + (errno == EINTR && !nvme_sigint_received))) { + nvme_log_retry(errno); goto retry; } } From 1f1027abf470407d0c30866625d027397899aabc Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Thu, 8 May 2025 11:23:47 +0200 Subject: [PATCH 2/2] fabrics: refactor error propagation in nvmf_connect With the introduction of nvme_add_ctrl we can use the return code directly and don't rely on the errno being set. Also we can directly leave the function on error because we don't have to cleanup things explicitly anymore. Signed-off-by: Daniel Wagner --- fabrics.c | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/fabrics.c b/fabrics.c index caf5ea7384..86b0e15a10 100644 --- a/fabrics.c +++ b/fabrics.c @@ -178,6 +178,11 @@ static int nvme_add_ctrl(nvme_host_t h, nvme_ctrl_t c, int ret; retry: + /* + * __create_discover_ctrl and callers depend on errno being set + * in the error case. + */ + errno = 0; ret = nvmf_add_ctrl(h, c, cfg); if (!ret) return 0; @@ -208,9 +213,7 @@ static nvme_ctrl_t __create_discover_ctrl(nvme_root_t r, nvme_host_t h, strcmp(trcfg->subsysnqn, NVME_DISC_SUBSYS_NAME)); tmo = set_discovery_kato(cfg); - errno = 0; ret = nvme_add_ctrl(h, c, cfg); - cfg->keep_alive_tmo = tmo; if (ret) { nvme_free_ctrl(c); @@ -1082,10 +1085,8 @@ int nvmf_connect(const char *desc, int argc, char **argv) return -errno; h = nvme_lookup_host(r, hnqn, hid); - if (!h) { - errno = ENOMEM; - goto out_free; - } + if (!h) + return -ENOMEM; if (hostkey) nvme_host_set_dhchap_key(h, hostkey); if (!trsvcid) @@ -1106,37 +1107,33 @@ int nvmf_connect(const char *desc, int argc, char **argv) c = lookup_ctrl(h, &trcfg); if (c && nvme_ctrl_get_name(c) && !cfg.duplicate_connect) { fprintf(stderr, "already connected\n"); - errno = EALREADY; - goto out_free; + return -EALREADY; } c = nvme_create_ctrl(r, subsysnqn, transport, traddr, cfg.host_traddr, cfg.host_iface, trsvcid); - if (!c) { - errno = ENOMEM; - goto out_free; - } + if (!c) + return -ENOMEM; if (ctrlkey) nvme_ctrl_set_dhchap_key(c, ctrlkey); nvme_parse_tls_args(keyring, tls_key, tls_key_identity, &cfg, c); - errno = 0; ret = nvme_add_ctrl(h, c, &cfg); - if (ret) + if (ret) { fprintf(stderr, "could not add new controller: %s\n", - nvme_strerror(errno)); - else { - errno = 0; - if (flags != -EINVAL) - nvme_show_connect_msg(c, flags); + nvme_strerror(-ret)); + return ret; } -out_free: + /* always print connected device */ + nvme_show_connect_msg(c, flags); + if (dump_config) nvme_dump_config(r); - return -errno; + + return 0; } static nvme_ctrl_t lookup_nvme_ctrl(nvme_root_t r, const char *name)