Re: [bug report] most of blktests nvme/ failed on the latest linux tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux