[PATCH 0/4] systemd ID improvements

[PATCH 0/4] systemd ID improvements

[Andy Lutomirski]

Hi all-

This was github PR #594, but Keith asked me to email it.

Please review this carefully. Also, if you merge this, please
consider changing the 'luto.us' bit to something a bit more
sensible. Maybe com.github.linux-nvme?

I'm still dubious about the way that newlines in the ids are
handled, but my patches don't change that per se.

Andy Lutomirski (4):
  hostnqn: Fix the systemd-based NQN namespace to comply with the spec
  fabrics: Rename nvmf_hostnqn_file() to nvmf_hostnqn_load()
  Use systemd-generated hostid if no hostid is configured
  Do not install /etc/nvme/hostid or /etc/nvme/hostnqn by default

 Documentation/nvme-show-hostid.txt | 29 +++++++++++++
 Makefile                           |  4 +-
 fabrics.c                          | 68 +++++++++++++++++++++++++-----
 fabrics.h                          |  1 +
 nvme-builtin.h                     |  1 +
 nvme.c                             | 15 +++++++
 6 files changed, 105 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/nvme-show-hostid.txt

-- 
2.23.0


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH 1/4] hostnqn: Fix the systemd-based NQN namespace to comply with the spec

[Andy Lutomirski]

The NVME Base spec, section 7.9 explicitly states that
nqn.2014-08.org.nvmexpress:uuid: uses an RFC 4122 UUID, but we
weren't generating one.  We could change it by actually generating a
compliant UUID, but we can also just invent a new naming authority.
Use "nqn.2019-10.us.luto:sd_id128_app:"

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 fabrics.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fabrics.c b/fabrics.c
index 655bd36c5639..12be2e73986e 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -691,7 +691,12 @@ static char *hostnqn_generate_systemd(void)
 	if (sd_id128_get_machine_app_specific(NVME_HOSTNQN_ID, &id) < 0)
 		return NULL;
 
-	if (asprintf(&ret, "nqn.2014-08.org.nvmexpress:uuid:" SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)) == -1)
+	/*
+	 * "nqn.2019-10.us.luto:sd_id128_app:" is hereby defined as an SD_ID128
+	 * app-specific machine ID as generated here.
+	 */
+
+	if (asprintf(&ret, "nqn.2019-10.us.luto:sd_id128_app:" SD_ID128_FORMAT_STR "\n", SD_ID128_FORMAT_VAL(id)) == -1)
 		ret = NULL;
 
 	return ret;
-- 
2.23.0


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH 2/4] fabrics: Rename nvmf_hostnqn_file() to nvmf_hostnqn_load()

[Andy Lutomirski]

The old name was confusing, since the value didn't necessarily come
from a file.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 fabrics.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fabrics.c b/fabrics.c
index 12be2e73986e..4448416f9855 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -721,7 +721,7 @@ char *hostnqn_read(void)
 	return NULL;
 }
 
-static int nvmf_hostnqn_file(void)
+static int nvmf_hostnqn_load(void)
 {
 	cfg.hostnqn = hostnqn_read();
 
@@ -827,7 +827,7 @@ static int build_options(char *argstr, int max_len, bool discover)
 	    add_argument(&argstr, &max_len, "traddr", cfg.traddr) ||
 	    add_argument(&argstr, &max_len, "host_traddr", cfg.host_traddr) ||
 	    add_argument(&argstr, &max_len, "trsvcid", cfg.trsvcid) ||
-	    ((cfg.hostnqn || nvmf_hostnqn_file()) &&
+	    ((cfg.hostnqn || nvmf_hostnqn_load()) &&
 		    add_argument(&argstr, &max_len, "hostnqn", cfg.hostnqn)) ||
 	    ((cfg.hostid || nvmf_hostid_file()) &&
 		    add_argument(&argstr, &max_len, "hostid", cfg.hostid)) ||
-- 
2.23.0


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH 3/4] Use systemd-generated hostid if no hostid is configured

[Andy Lutomirski]

This is just like the hostnqn support.  It adds a show-hostid command
for introspection.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 Documentation/nvme-show-hostid.txt | 29 +++++++++++++++
 fabrics.c                          | 57 ++++++++++++++++++++++++++----
 fabrics.h                          |  1 +
 nvme-builtin.h                     |  1 +
 nvme.c                             | 15 ++++++++
 5 files changed, 96 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/nvme-show-hostid.txt

diff --git a/Documentation/nvme-show-hostid.txt b/Documentation/nvme-show-hostid.txt
new file mode 100644
index 000000000000..52bdc8a1f480
--- /dev/null
+++ b/Documentation/nvme-show-hostid.txt
@@ -0,0 +1,29 @@
+nvme-show-hostid(1)
+===================
+
+NAME
+----
+nvme-show-hostid - Generate a host NVMe ID
+
+SYNOPSIS
+--------
+[verse]
+'nvme show-hostid'
+
+DESCRIPTION
+-----------
+Show the host ID configured for the system.  If /etc/nvme/hostid is
+not present and systemd application-specific machine IDs are available,
+this will show the systemd-generated host ID for the system.
+
+OPTIONS
+-------
+No options needed
+
+EXAMPLES
+--------
+nvme show-hostid
+
+NVME
+----
+Part of the nvme-user suite
diff --git a/fabrics.c b/fabrics.c
index 4448416f9855..7c5b7efd5270 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -46,6 +46,7 @@
 #ifdef HAVE_SYSTEMD
 #include <systemd/sd-id128.h>
 #define NVME_HOSTNQN_ID SD_ID128_MAKE(c7,f4,61,81,12,be,49,32,8c,83,10,6f,9d,dd,d8,6b)
+#define NVME_HOSTID_ID SD_ID128_MAKE(df,66,bf,ec,f7,e4,21,0e,46,27,ac,a8,f2,8f,3b,98)
 #endif
 
 #define NVMF_HOSTID_SIZE	36
@@ -728,11 +729,11 @@ static int nvmf_hostnqn_load(void)
 	return cfg.hostnqn != NULL;
 }
 
-static int nvmf_hostid_file(void)
+static char *hostid_read_file(void)
 {
 	FILE *f;
 	char hostid[NVMF_HOSTID_SIZE + 1];
-	int ret = false;
+	char *ret = NULL;
 
 	f = fopen(PATH_NVMF_HOSTID, "r");
 	if (f == NULL)
@@ -741,16 +742,58 @@ static int nvmf_hostid_file(void)
 	if (fgets(hostid, sizeof(hostid), f) == NULL)
 		goto out;
 
-	cfg.hostid = strdup(hostid);
-	if (!cfg.hostid)
-		goto out;
+	ret = strdup(hostid);
+
 
-	ret = true;
 out:
 	fclose(f);
 	return ret;
 }
 
+static char *hostid_generate_systemd(void)
+{
+#if defined(LIBUUID) && defined(HAVE_SYSTEMD)
+	sd_id128_t id;
+	char uuidstr[37];
+	char *ret;
+
+	if (sd_id128_get_machine_app_specific(NVME_HOSTID_ID, &id) < 0)
+		return NULL;
+
+	uuid_unparse_lower(id.bytes, uuidstr);
+
+	if (asprintf(&ret, "%s\n", uuidstr) == -1)
+		ret = NULL;
+
+	return ret;
+#else
+	return NULL;
+#endif
+}
+
+/* returns an allocated string or NULL */
+char *hostid_read(void)
+{
+	char *ret;
+
+	ret = hostid_read_file();
+	if (ret)
+		return ret;
+
+	ret = hostid_generate_systemd();
+	if (ret)
+		return ret;
+
+	return NULL;
+}
+
+static int nvmf_hostid_load(void)
+{
+	cfg.hostid = hostid_read();
+
+	return cfg.hostid != NULL;
+}
+
 static int
 add_bool_argument(char **argstr, int *max_len, char *arg_str, bool arg)
 {
@@ -829,7 +872,7 @@ static int build_options(char *argstr, int max_len, bool discover)
 	    add_argument(&argstr, &max_len, "trsvcid", cfg.trsvcid) ||
 	    ((cfg.hostnqn || nvmf_hostnqn_load()) &&
 		    add_argument(&argstr, &max_len, "hostnqn", cfg.hostnqn)) ||
-	    ((cfg.hostid || nvmf_hostid_file()) &&
+	    ((cfg.hostid || nvmf_hostid_load()) &&
 		    add_argument(&argstr, &max_len, "hostid", cfg.hostid)) ||
 	    (!discover &&
 	      add_int_argument(&argstr, &max_len, "nr_io_queues",
diff --git a/fabrics.h b/fabrics.h
index b8e53f492b53..64aede897535 100644
--- a/fabrics.h
+++ b/fabrics.h
@@ -4,6 +4,7 @@
 #define NVMF_DEF_DISC_TMO	30
 
 extern char *hostnqn_read(void);
+extern char *hostid_read(void);
 
 extern int discover(const char *desc, int argc, char **argv, bool connect);
 extern int connect(const char *desc, int argc, char **argv);
diff --git a/nvme-builtin.h b/nvme-builtin.h
index bfb907dff9ef..907d470cbeac 100644
--- a/nvme-builtin.h
+++ b/nvme-builtin.h
@@ -71,6 +71,7 @@ COMMAND_LIST(
 	ENTRY("disconnect-all", "Disconnect from all connected NVMeoF subsystems", disconnect_all_cmd)
 	ENTRY("gen-hostnqn", "Generate NVMeoF host NQN", gen_hostnqn_cmd)
 	ENTRY("show-hostnqn", "Show NVMeoF host NQN", show_hostnqn_cmd)
+	ENTRY("show-hostid", "Show NVMeoF host ID", show_hostid_cmd)
 	ENTRY("dir-receive", "Submit a Directive Receive command, return results", dir_receive)
 	ENTRY("dir-send", "Submit a Directive Send command, return results", dir_send)
 	ENTRY("virt-mgmt", "Manage Flexible Resources between Primary and Secondary Controller ", virtual_mgmt)
diff --git a/nvme.c b/nvme.c
index 0c23eee8a477..d81f8226b84d 100644
--- a/nvme.c
+++ b/nvme.c
@@ -4771,6 +4771,21 @@ static int show_hostnqn_cmd(int argc, char **argv, struct command *command, stru
 	}
 }
 
+static int show_hostid_cmd(int argc, char **argv, struct command *command, struct plugin *plugin)
+{
+	char *hostid;
+
+	hostid = hostid_read();
+	if (hostid) {
+		fputs(hostid, stdout);
+		free(hostid);
+		return 0;
+	} else {
+		fprintf(stderr, "hostid is not available -- generate /etc/nvme/hostid\n");
+		return -ENOENT;
+	}
+}
+
 static int discover_cmd(int argc, char **argv, struct command *command, struct plugin *plugin)
 {
 	const char *desc = "Send Get Log Page request to Discovery Controller.";
-- 
2.23.0


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 3/4] Use systemd-generated hostid if no hostid is configured

[David Milburn]

Hello Andy,

On 12/19/2019 09:31 PM, Andy Lutomirski wrote:
> This is just like the hostnqn support.  It adds a show-hostid command
> for introspection.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>   Documentation/nvme-show-hostid.txt | 29 +++++++++++++++
>   fabrics.c                          | 57 ++++++++++++++++++++++++++----
>   fabrics.h                          |  1 +
>   nvme-builtin.h                     |  1 +
>   nvme.c                             | 15 ++++++++
>   5 files changed, 96 insertions(+), 7 deletions(-)
>   create mode 100644 Documentation/nvme-show-hostid.txt
> 
> diff --git a/Documentation/nvme-show-hostid.txt b/Documentation/nvme-show-hostid.txt
> new file mode 100644
> index 000000000000..52bdc8a1f480
> --- /dev/null
> +++ b/Documentation/nvme-show-hostid.txt
> @@ -0,0 +1,29 @@
> +nvme-show-hostid(1)
> +===================
> +
> +NAME
> +----
> +nvme-show-hostid - Generate a host NVMe ID
> +

I pulled the latest nvme-cli and applied your patch set

https://github.com/linux-nvme/nvme-cli

Seems to work on Fedora 31

$ nvme show-hostnqn
nqn.2019-10.us.luto:sd_id128_app:a9e4b2c8953340988142e32ca6d67922

$ nvme show-hostid
f467779d-1940-4659-b200-74278899b2ca

Thanks,
David


> +SYNOPSIS
> +--------
> +[verse]
> +'nvme show-hostid'
> +
> +DESCRIPTION
> +-----------
> +Show the host ID configured for the system.  If /etc/nvme/hostid is
> +not present and systemd application-specific machine IDs are available,
> +this will show the systemd-generated host ID for the system.
> +
> +OPTIONS
> +-------
> +No options needed
> +
> +EXAMPLES
> +--------
> +nvme show-hostid
> +
> +NVME
> +----
> +Part of the nvme-user suite
> diff --git a/fabrics.c b/fabrics.c
> index 4448416f9855..7c5b7efd5270 100644
> --- a/fabrics.c
> +++ b/fabrics.c
> @@ -46,6 +46,7 @@
>   #ifdef HAVE_SYSTEMD
>   #include <systemd/sd-id128.h>
>   #define NVME_HOSTNQN_ID SD_ID128_MAKE(c7,f4,61,81,12,be,49,32,8c,83,10,6f,9d,dd,d8,6b)
> +#define NVME_HOSTID_ID SD_ID128_MAKE(df,66,bf,ec,f7,e4,21,0e,46,27,ac,a8,f2,8f,3b,98)
>   #endif
>   
>   #define NVMF_HOSTID_SIZE	36
> @@ -728,11 +729,11 @@ static int nvmf_hostnqn_load(void)
>   	return cfg.hostnqn != NULL;
>   }
>   
> -static int nvmf_hostid_file(void)
> +static char *hostid_read_file(void)
>   {
>   	FILE *f;
>   	char hostid[NVMF_HOSTID_SIZE + 1];
> -	int ret = false;
> +	char *ret = NULL;
>   
>   	f = fopen(PATH_NVMF_HOSTID, "r");
>   	if (f == NULL)
> @@ -741,16 +742,58 @@ static int nvmf_hostid_file(void)
>   	if (fgets(hostid, sizeof(hostid), f) == NULL)
>   		goto out;
>   
> -	cfg.hostid = strdup(hostid);
> -	if (!cfg.hostid)
> -		goto out;
> +	ret = strdup(hostid);
> +
>   
> -	ret = true;
>   out:
>   	fclose(f);
>   	return ret;
>   }
>   
> +static char *hostid_generate_systemd(void)
> +{
> +#if defined(LIBUUID) && defined(HAVE_SYSTEMD)
> +	sd_id128_t id;
> +	char uuidstr[37];
> +	char *ret;
> +
> +	if (sd_id128_get_machine_app_specific(NVME_HOSTID_ID, &id) < 0)
> +		return NULL;
> +
> +	uuid_unparse_lower(id.bytes, uuidstr);
> +
> +	if (asprintf(&ret, "%s\n", uuidstr) == -1)
> +		ret = NULL;
> +
> +	return ret;
> +#else
> +	return NULL;
> +#endif
> +}
> +
> +/* returns an allocated string or NULL */
> +char *hostid_read(void)
> +{
> +	char *ret;
> +
> +	ret = hostid_read_file();
> +	if (ret)
> +		return ret;
> +
> +	ret = hostid_generate_systemd();
> +	if (ret)
> +		return ret;
> +
> +	return NULL;
> +}
> +
> +static int nvmf_hostid_load(void)
> +{
> +	cfg.hostid = hostid_read();
> +
> +	return cfg.hostid != NULL;
> +}
> +
>   static int
>   add_bool_argument(char **argstr, int *max_len, char *arg_str, bool arg)
>   {
> @@ -829,7 +872,7 @@ static int build_options(char *argstr, int max_len, bool discover)
>   	    add_argument(&argstr, &max_len, "trsvcid", cfg.trsvcid) ||
>   	    ((cfg.hostnqn || nvmf_hostnqn_load()) &&
>   		    add_argument(&argstr, &max_len, "hostnqn", cfg.hostnqn)) ||
> -	    ((cfg.hostid || nvmf_hostid_file()) &&
> +	    ((cfg.hostid || nvmf_hostid_load()) &&
>   		    add_argument(&argstr, &max_len, "hostid", cfg.hostid)) ||
>   	    (!discover &&
>   	      add_int_argument(&argstr, &max_len, "nr_io_queues",
> diff --git a/fabrics.h b/fabrics.h
> index b8e53f492b53..64aede897535 100644
> --- a/fabrics.h
> +++ b/fabrics.h
> @@ -4,6 +4,7 @@
>   #define NVMF_DEF_DISC_TMO	30
>   
>   extern char *hostnqn_read(void);
> +extern char *hostid_read(void);
>   
>   extern int discover(const char *desc, int argc, char **argv, bool connect);
>   extern int connect(const char *desc, int argc, char **argv);
> diff --git a/nvme-builtin.h b/nvme-builtin.h
> index bfb907dff9ef..907d470cbeac 100644
> --- a/nvme-builtin.h
> +++ b/nvme-builtin.h
> @@ -71,6 +71,7 @@ COMMAND_LIST(
>   	ENTRY("disconnect-all", "Disconnect from all connected NVMeoF subsystems", disconnect_all_cmd)
>   	ENTRY("gen-hostnqn", "Generate NVMeoF host NQN", gen_hostnqn_cmd)
>   	ENTRY("show-hostnqn", "Show NVMeoF host NQN", show_hostnqn_cmd)
> +	ENTRY("show-hostid", "Show NVMeoF host ID", show_hostid_cmd)
>   	ENTRY("dir-receive", "Submit a Directive Receive command, return results", dir_receive)
>   	ENTRY("dir-send", "Submit a Directive Send command, return results", dir_send)
>   	ENTRY("virt-mgmt", "Manage Flexible Resources between Primary and Secondary Controller ", virtual_mgmt)
> diff --git a/nvme.c b/nvme.c
> index 0c23eee8a477..d81f8226b84d 100644
> --- a/nvme.c
> +++ b/nvme.c
> @@ -4771,6 +4771,21 @@ static int show_hostnqn_cmd(int argc, char **argv, struct command *command, stru
>   	}
>   }
>   
> +static int show_hostid_cmd(int argc, char **argv, struct command *command, struct plugin *plugin)
> +{
> +	char *hostid;
> +
> +	hostid = hostid_read();
> +	if (hostid) {
> +		fputs(hostid, stdout);
> +		free(hostid);
> +		return 0;
> +	} else {
> +		fprintf(stderr, "hostid is not available -- generate /etc/nvme/hostid\n");
> +		return -ENOENT;
> +	}
> +}
> +
>   static int discover_cmd(int argc, char **argv, struct command *command, struct plugin *plugin)
>   {
>   	const char *desc = "Send Get Log Page request to Discovery Controller.";
> 


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 3/4] Use systemd-generated hostid if no hostid is configured

[Andy Lutomirski]

On Fri, Jan 3, 2020 at 10:14 AM David Milburn <dmilburn@redhat.com> wrote:
>
> Hello Andy,
>
> On 12/19/2019 09:31 PM, Andy Lutomirski wrote:
> > This is just like the hostnqn support.  It adds a show-hostid command
> > for introspection.
> >
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> >   Documentation/nvme-show-hostid.txt | 29 +++++++++++++++
> >   fabrics.c                          | 57 ++++++++++++++++++++++++++----
> >   fabrics.h                          |  1 +
> >   nvme-builtin.h                     |  1 +
> >   nvme.c                             | 15 ++++++++
> >   5 files changed, 96 insertions(+), 7 deletions(-)
> >   create mode 100644 Documentation/nvme-show-hostid.txt
> >
> > diff --git a/Documentation/nvme-show-hostid.txt b/Documentation/nvme-show-hostid.txt
> > new file mode 100644
> > index 000000000000..52bdc8a1f480
> > --- /dev/null
> > +++ b/Documentation/nvme-show-hostid.txt
> > @@ -0,0 +1,29 @@
> > +nvme-show-hostid(1)
> > +===================
> > +
> > +NAME
> > +----
> > +nvme-show-hostid - Generate a host NVMe ID
> > +
>
> I pulled the latest nvme-cli and applied your patch set
>
> https://github.com/linux-nvme/nvme-cli
>
> Seems to work on Fedora 31
>
> $ nvme show-hostnqn
> nqn.2019-10.us.luto:sd_id128_app:a9e4b2c8953340988142e32ca6d67922
>
> $ nvme show-hostid
> f467779d-1940-4659-b200-74278899b2ca
>

Good news!  When I run it, I get different output!

$ ./nvme show-hostnqn
nqn.2019-10.us.luto:sd_id128_app:a3cee2b99e504da68d4b9e7de354bebd
$ ./nvme show-hostid
c701865f-88c9-43c3-83a0-ead417dea2de

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH 4/4] Do not install /etc/nvme/hostid or /etc/nvme/hostnqn by default

[Andy Lutomirski]

Pre-generating them is nonsensical for package managers.  Moreover,
with the new systemd-generated ID support, it's preferable for the
files to be absent.  Don't generate them unless explicitly requested.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 Makefile | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index d88404dd51f6..acb6983cb5b1 100644
--- a/Makefile
+++ b/Makefile
@@ -140,14 +140,12 @@ install-hostparams: install-etc
 
 install-etc:
 	$(INSTALL) -d $(DESTDIR)$(SYSCONFDIR)/nvme
-	touch $(DESTDIR)$(SYSCONFDIR)/nvme/hostnqn
-	touch $(DESTDIR)$(SYSCONFDIR)/nvme/hostid
 	if [ ! -f $(DESTDIR)$(SYSCONFDIR)/nvme/discovery.conf ]; then \
 		$(INSTALL) -m 644 -T ./etc/discovery.conf.in $(DESTDIR)$(SYSCONFDIR)/nvme/discovery.conf; \
 	fi
 
 install-spec: install-bin install-man install-bash-completion install-zsh-completion install-etc install-systemd install-udev install-dracut
-install: install-spec install-hostparams
+install: install-spec
 
 nvme.spec: nvme.spec.in NVME-VERSION-FILE
 	sed -e 's/@@VERSION@@/$(NVME_VERSION)/g' < $< > $@+
-- 
2.23.0


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme