From: Israel Rukshin <israelr@nvidia.com>
To: Max Gurtovoy <mgurtovoy@nvidia.com>,
Linux-nvme <linux-nvme@lists.infradead.org>,
Sagi Grimberg <sagi@grimberg.me>,
"Christoph Hellwig" <hch@lst.de>
Cc: Israel Rukshin <israelr@nvidia.com>
Subject: [PATCH] fabrics: Always pass hostid and hostnqn
Date: Tue, 16 Apr 2024 06:32:55 +0000 [thread overview]
Message-ID: <1713249176-7837-1-git-send-email-israelr@nvidia.com> (raw)
After the kernel commit ae8bd606e09b ("nvme-fabrics: prevent overriding
of existing host"), kernel ensures hostid and hostnqn maintain 1:1
mapping. This makes 'nvme discover' and 'nvme connect' commands fail
when providing only hostid or only hostnqn. This issue happens when
the user only enters NQN which doesn't contain UUID, so the generation
of the hostid fails.
There are few more issues that this commit is fixing:
- When the user provides hostid and NQN, the hostid is overridden
by generating it from the NQN.
- hostid is generated from the NQN file instead of the NQN that
the user enters at the command line.
- The warning "use generated hostid instead of hostid file" is
wrong when the user provides hostid via the command line.
The commit fixes those issues by doing the following logic:
1. If user provided both via command line - pass them as-is
2. If user doesn't enter them via command line - try to get
them from files.
3. If one of them is not provided - generate it from the other.
Use the new function nvmf_hostid_generate() when NQN doesn't
have UUID and use nvmf_hostnqn_generate(hostid) to generate
hostnqn from hostid.
4. If user provided none - generate them both. Before this commit,
nvme cli didn't do it.
Signed-off-by: Israel Rukshin <israelr@nvidia.com>
Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
fabrics.c | 74 ++++++++++++++++++++++++++++++-------------------------
nvme.c | 4 +--
2 files changed, 43 insertions(+), 35 deletions(-)
diff --git a/fabrics.c b/fabrics.c
index e081d963..ee15b8bc 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -643,20 +643,9 @@ char *nvmf_hostid_from_hostnqn(const char *hostnqn)
void nvmf_check_hostid_and_hostnqn(const char *hostid, const char *hostnqn, unsigned int verbose)
{
- char *hostid_from_file, *hostid_from_hostnqn;
+ char *hostid_from_hostnqn;
- if (!hostid)
- return;
-
- hostid_from_file = nvmf_hostid_from_file();
- if (hostid_from_file && strcmp(hostid_from_file, hostid)) {
- if (verbose)
- fprintf(stderr,
- "warning: use generated hostid instead of hostid file\n");
- free(hostid_from_file);
- }
-
- if (!hostnqn)
+ if (!hostnqn || !hostid)
return;
hostid_from_hostnqn = nvmf_hostid_from_hostnqn(hostnqn);
@@ -668,6 +657,28 @@ void nvmf_check_hostid_and_hostnqn(const char *hostid, const char *hostnqn, unsi
}
}
+void nvmf_set_hostid_and_hostnqn(char **hostid, char **hostnqn)
+{
+ if (!*hostid)
+ *hostid = nvmf_hostid_from_file();
+ if (!*hostnqn)
+ *hostnqn = nvmf_hostnqn_from_file();
+
+ if (!*hostid) {
+ if (*hostnqn) {
+ *hostid = nvmf_hostid_from_hostnqn(*hostnqn);
+ if (!*hostid)
+ *hostid = nvmf_hostid_generate();
+ } else {
+ *hostid = nvmf_hostid_generate();
+ *hostnqn = nvmf_hostnqn_generate(*hostid);
+ }
+ }
+
+ if (!*hostnqn)
+ *hostnqn = nvmf_hostnqn_generate(*hostid);
+}
+
int nvmf_discover(const char *desc, int argc, char **argv, bool connect)
{
char *subsysnqn = NVME_DISC_SUBSYS_NAME;
@@ -747,16 +758,13 @@ int nvmf_discover(const char *desc, int argc, char **argv, bool connect)
hostnqn_arg = hostnqn;
hostid_arg = hostid;
- if (!hostnqn)
- hostnqn = hnqn = nvmf_hostnqn_from_file();
- if (!hostnqn) {
- hostnqn = hnqn = nvmf_hostnqn_generate();
- hostid = hid = nvmf_hostid_from_hostnqn(hostnqn);
- }
- if (!hostid)
- hostid = hid = nvmf_hostid_from_file();
- if (!hostid && hostnqn)
- hostid = hid = nvmf_hostid_from_hostnqn(hostnqn);
+
+ nvmf_set_hostid_and_hostnqn(&hostid, &hostnqn);
+ if (!hostid_arg)
+ hid = hostid;
+ if (!hostnqn_arg)
+ hnqn = hostnqn;
+
nvmf_check_hostid_and_hostnqn(hostid, hostnqn, verbose);
h = nvme_lookup_host(r, hostnqn, hostid);
if (!h) {
@@ -906,6 +914,7 @@ int nvmf_connect(const char *desc, int argc, char **argv)
enum nvme_print_flags flags;
struct nvme_fabrics_config cfg = { 0 };
char *format = "normal";
+ char *hostnqn_arg, *hostid_arg;
NVMF_ARGS(opts, cfg,
@@ -973,16 +982,15 @@ int nvmf_connect(const char *desc, int argc, char **argv)
nvme_read_config(r, config_file);
nvme_read_volatile_config(r);
- if (!hostnqn)
- hostnqn = hnqn = nvmf_hostnqn_from_file();
- if (!hostnqn) {
- hostnqn = hnqn = nvmf_hostnqn_generate();
- hostid = hid = nvmf_hostid_from_hostnqn(hostnqn);
- }
- if (!hostid)
- hostid = hid = nvmf_hostid_from_file();
- if (!hostid && hostnqn)
- hostid = hid = nvmf_hostid_from_hostnqn(hostnqn);
+ hostnqn_arg = hostnqn;
+ hostid_arg = hostid;
+
+ nvmf_set_hostid_and_hostnqn(&hostid, &hostnqn);
+ if (!hostid_arg)
+ hid = hostid;
+ if (!hostnqn_arg)
+ hnqn = hostnqn;
+
nvmf_check_hostid_and_hostnqn(hostid, hostnqn, verbose);
h = nvme_lookup_host(r, hostnqn, hostid);
if (!h) {
diff --git a/nvme.c b/nvme.c
index 096d43c9..9f4a179d 100644
--- a/nvme.c
+++ b/nvme.c
@@ -8899,7 +8899,7 @@ static int gen_hostnqn_cmd(int argc, char **argv, struct command *command, struc
{
char *hostnqn;
- hostnqn = nvmf_hostnqn_generate();
+ hostnqn = nvmf_hostnqn_generate(NULL);
if (!hostnqn) {
nvme_show_error("\"%s\" not supported. Install lib uuid and rebuild.",
command->name);
@@ -8916,7 +8916,7 @@ static int show_hostnqn_cmd(int argc, char **argv, struct command *command, stru
hostnqn = nvmf_hostnqn_from_file();
if (!hostnqn)
- hostnqn = nvmf_hostnqn_generate();
+ hostnqn = nvmf_hostnqn_generate(NULL);
if (!hostnqn) {
nvme_show_error("hostnqn is not available -- use nvme gen-hostnqn");
--
2.18.2
next reply other threads:[~2024-04-16 6:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-16 6:32 Israel Rukshin [this message]
2024-04-16 6:32 ` [PATCH] libnvme: Introduce nvmf_hostid_generate function Israel Rukshin
2024-04-17 7:47 ` Daniel Wagner
2024-04-17 7:43 ` [PATCH] fabrics: Always pass hostid and hostnqn Daniel Wagner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1713249176-7837-1-git-send-email-israelr@nvidia.com \
--to=israelr@nvidia.com \
--cc=hch@lst.de \
--cc=linux-nvme@lists.infradead.org \
--cc=mgurtovoy@nvidia.com \
--cc=sagi@grimberg.me \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).