[PATCH nvme-cli] fabrics: Use corresponding hostid when hostnqn is generated

[Shinichiro Kawasaki]

On Jul 05, 2023 / 11:22, Daniel Wagner wrote:
> Hi Shin'ichiro,
> 
> sorry for my late response.
> 
> On Sun, Jul 02, 2023 at 11:23:46AM +0900, Shin'ichiro Kawasaki wrote:
> > After the kernel commit ae8bd606e09b ("nvme-fabrics: prevent overriding
> > of existing host"), kernel ensures hostid and hostnqn maintain 1:1
> > mapping and "non 1:1 mapping will be rejected". This makes 'nvme
> > discover' and 'nvme connect' commands fail when they generate hostnqn,
> > since it does not use corresponding hostid.
> > 
> > To avoid the failures, prepare and use corresponding hostid to the
> > generated hostnqn, taking the hostid from the hostnqn string. Also warn
> > when the hostid is different from that in hostid file.
> > 
> > Link: https://lore.kernel.org/linux-nvme/l7vk7fnzltpmvkwujsbf2btrzip6wh7ug62iwa3totqcda25l6@siqx7tj6lt3l/
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki at wdc.com>
> > ---
> >  fabrics.c | 28 ++++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> > 
> > 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");
> > +
> 
> As mentioned in my my other original mail, I'd say it makes sense to have the
> check after ...
> 
> > +	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();
> 
> ... here

Thanks for the comment. Will move the check. Also revise the patch to warn,
 - if it uses genereated hostid different from the hostid from file, or,
 - if hostnqn from file and hostid from file are inconsistent