On Jun 29, 2023 / 09:40, Daniel Wagner wrote:
> On Wed, Jun 28, 2023 at 08:25:24AM +0000, Shinichiro Kawasaki wrote:
> > I looked in nvme-cli code. When it generates hostnqn, it does not set hostid.
> > A quick dirty patch below for nvme-cli avoided the failures for nvme_trtype=loop
> > condition. If this is the fix approach, the kernel commit will need
> > corresponding change in the nvme-cli side.
> >
> >
> > diff --git a/fabrics.c b/fabrics.c
> > index ac240cad..f1981206 100644
> > --- a/fabrics.c
> > +++ b/fabrics.c
> > @@ -753,8 +753,13 @@ int nvmf_discover(const char *desc, int argc, char **argv, bool connect)
> > hostid_arg = hostid;
> > if (!hostnqn)
> > hostnqn = hnqn = nvmf_hostnqn_from_file();
> > - if (!hostnqn)
> > + if (!hostnqn) {
> > + char *uuid;
> > hostnqn = hnqn = nvmf_hostnqn_generate();
> > + uuid = strstr(hostnqn, "uuid:");
> > + if (uuid)
> > + hostid = hid = strdup(uuid + strlen("uuid:"));
> > + }
> > if (!hostid)
> > hostid = hid = nvmf_hostid_from_file();
> > h = nvme_lookup_host(r, hostnqn, hostid);
> > @@ -966,8 +971,13 @@ int nvmf_connect(const char *desc, int argc, char **argv)
> >
> > if (!hostnqn)
> > hostnqn = hnqn = nvmf_hostnqn_from_file();
> > - if (!hostnqn)
> > + if (!hostnqn) {
> > + char *uuid;
> > hostnqn = hnqn = nvmf_hostnqn_generate();
> > + uuid = strstr(hostnqn, "uuid:");
> > + if (uuid)
> > + hostid = hid = strdup(uuid + strlen("uuid:"));
> > + }
> > if (!hostid)
> > hostid = hid = nvmf_hostid_from_file();
> > h = nvme_lookup_host(r, hostnqn, hostid);
>
> Looks reasonable. Though I would propably also add a warning iff
> nvmf_hostid_from_file() does return a not matching hostid.
Thanks for the comment. I revised the patch as below, and it looks working.
The blktests failures are already fixed, but I still think this fix in nvme-
cli will avoid confusions. Will post as a formal patch later.
diff --git a/fabrics.c b/fabrics.c
index ac240cad..2eeea48c 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -677,6 +677,26 @@ static int nvme_read_volatile_config(nvme_root_t r)
return ret;
}
+char *nvmf_hostid_from_hostnqn(const char *hostnqn)
+{
+ const char *uuid;
+ const char *hostid_from_file;
+
+ if (!hostnqn)
+ return NULL;
+
+ uuid = strstr(hostnqn, "uuid:");
+ if (!uuid)
+ return NULL;
+ uuid += strlen("uuid:");
+
+ hostid_from_file = nvmf_hostid_from_file();
+ if (hostid_from_file && strcmp(uuid, hostid_from_file))
+ fprintf(stderr, "warning: use generated hostid instead of hostid file\n");
+
+ return strdup(uuid);
+}
+
int nvmf_discover(const char *desc, int argc, char **argv, bool connect)
{
char *subsysnqn = NVME_DISC_SUBSYS_NAME;
@@ -753,8 +773,10 @@ int nvmf_discover(const char *desc, int argc, char **argv, bool connect)
hostid_arg = hostid;
if (!hostnqn)
hostnqn = hnqn = nvmf_hostnqn_from_file();
- if (!hostnqn)
+ if (!hostnqn) {
hostnqn = hnqn = nvmf_hostnqn_generate();
+ hostid = hid = nvmf_hostid_from_hostnqn(hostnqn);
+ }
if (!hostid)
hostid = hid = nvmf_hostid_from_file();
h = nvme_lookup_host(r, hostnqn, hostid);
@@ -966,8 +988,10 @@ int nvmf_connect(const char *desc, int argc, char **argv)
if (!hostnqn)
hostnqn = hnqn = nvmf_hostnqn_from_file();
- if (!hostnqn)
+ if (!hostnqn) {
hostnqn = hnqn = nvmf_hostnqn_generate();
+ hostid = hid = nvmf_hostid_from_hostnqn(hostnqn);
+ }
if (!hostid)
hostid = hid = nvmf_hostid_from_file();
h = nvme_lookup_host(r, hostnqn, hostid);
--
2.39.2
