[PATCH nvme-cli v2 0/2] auto generate hostnqn file on installation

[PATCH nvme-cli v2 0/2] auto generate hostnqn file on installation

[Sagi Grimberg]

This set is designed to help the user by auto-generating a hostnqn and
placing it in /etc/nvme/hostnqn file upon installation of nvme-cli (in
case such a file doesn't exist).

The user can re-generate different hostnqns on demand by running:
$ nvme gen-hostnqn

Only the deb package is tested.

Changes from v1:
- don't keep local hostnqn var for sprintf
- don't delete /etc/nvme if it contains other files other
  than what we installed (hostnqn)

Changes from v0 (rfc):
- Moved hostnqn generation to nvme with the command gen-hostnqn
- Cleanup hostnqn file only on purge in debian

Sagi Grimberg (2):
  nvme-cli: Add nvme hostnqn generation option
  nvme.spec/debian: Auto generate host nqn as part of install

 Makefile        |  1 +
 debian/postinst | 20 ++++++++++++++++++++
 debian/postrm   | 10 ++++++++++
 nvme-builtin.h  |  1 +
 nvme.c          | 12 ++++++++++++
 nvme.spec.in    | 17 +++++++++++++++++
 6 files changed, 61 insertions(+)
 create mode 100644 debian/postinst
 create mode 100644 debian/postrm

-- 
2.7.4

[PATCH nvme-cli v2 1/2] nvme-cli: Add nvme hostnqn generation option

[Sagi Grimberg]

Add option to generate a NVMe qualified name of a given host
(in the form of: nqn.2014-08.org.nvmexpress:NVMf:uuid:<some_uuid>).
This hostnqn will be used for fabrics discovery and connect functions.

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 Makefile       |  1 +
 nvme-builtin.h |  1 +
 nvme.c         | 12 ++++++++++++
 3 files changed, 14 insertions(+)

diff --git a/Makefile b/Makefile
index 33c71902479a..2a946ce8cb61 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,6 @@
 CFLAGS += -std=gnu99 -O2 -g -Wall -Werror
 CPPFLAGS += -D_GNU_SOURCE -D__CHECK_ENDIAN__
+LDFLAGS += -luuid
 NVME = nvme
 INSTALL ?= install
 DESTDIR =
diff --git a/nvme-builtin.h b/nvme-builtin.h
index dc314cbbb371..b182b1e01f5b 100644
--- a/nvme-builtin.h
+++ b/nvme-builtin.h
@@ -49,6 +49,7 @@ COMMAND_LIST(
 	ENTRY("connect-all", "Discover and Connect to NVMeoF subsystems", connect_all_cmd)
 	ENTRY("connect", "Connect to NVMeoF subsystem", connect_cmd)
 	ENTRY("disconnect", "Disconnect from NVMeoF subsystem", disconnect_cmd)
+	ENTRY("gen-hostnqn", "Generate NVMeoF host NQN", gen_hostnqn_cmd)
 );
 
 #endif
diff --git a/nvme.c b/nvme.c
index 170fbec08d4c..82a958ca3f4c 100644
--- a/nvme.c
+++ b/nvme.c
@@ -44,6 +44,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/time.h>
+#include <uuid/uuid.h>
 
 #include "nvme-print.h"
 #include "nvme-ioctl.h"
@@ -2699,6 +2700,17 @@ static int admin_passthru(int argc, char **argv, struct command *cmd, struct plu
 	return passthru(argc, argv, NVME_IOCTL_ADMIN_CMD, desc, cmd);
 }
 
+static int gen_hostnqn_cmd(int argc, char **argv, struct command *command, struct plugin *plugin)
+{
+	uuid_t uuid;
+	char uuid_str[37]; /* e.g. 1b4e28ba-2fa1-11d2-883f-0016d3cca427 + \0 */
+
+	uuid_generate_random(uuid);
+	uuid_unparse_lower(uuid, uuid_str);
+	printf("nqn.2014-08.org.nvmexpress:NVMf:uuid:%s\n", uuid_str);
+	return 0;
+}
+
 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.7.4

[PATCH nvme-cli v2 1/2] nvme-cli: Add nvme hostnqn generation option

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch at lst.de>

[PATCH nvme-cli v2 1/2] nvme-cli: Add nvme hostnqn generation option

[J Freyensee]

On Thu, 2016-11-17@05:34 -0800, Christoph Hellwig wrote:
> Looks good,
> 
> Reviewed-by: Christoph Hellwig <hch at lst.de>

Dido,

Reviewed-by: Jay Freyensee <james_p_freyensee at linux.intel.com>

> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH nvme-cli v2 2/2] nvme.spec/debian: Auto generate host nqn as part of install

[Sagi Grimberg]

The installation will generate a hostnqn and store it in
/etc/nvme/hostnqn file (in case it doesn't exist).
This file will be removed upon uninstallation (purge on for debian).

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 debian/postinst | 20 ++++++++++++++++++++
 debian/postrm   | 10 ++++++++++
 nvme.spec.in    | 17 +++++++++++++++++
 3 files changed, 47 insertions(+)
 create mode 100644 debian/postinst
 create mode 100644 debian/postrm

diff --git a/debian/postinst b/debian/postinst
new file mode 100644
index 000000000000..b258cf569c3e
--- /dev/null
+++ b/debian/postinst
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+set -e
+
+case "$1" in
+    configure|install)
+	if [ ! -f /etc/nvme/hostnqn ]; then
+		install -D /dev/null /etc/nvme/hostnqn
+		echo $(nvme gen-hostnqn) > /etc/nvme/hostnqn
+        fi
+        ;;
+
+    upgrade|abort-upgrade)
+        ;;
+    *)
+        echo "postinst called with unknown argument \`$1'" >&2
+        exit 0
+        ;;
+esac
+exit 0
diff --git a/debian/postrm b/debian/postrm
new file mode 100644
index 000000000000..a7a724352327
--- /dev/null
+++ b/debian/postrm
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+if [ "$1" = "purge" ]; then
+	if [ -d /etc/nvme ]; then
+		rm -f /etc/nvme/hostnqn
+		if [ ! -n "$(ls -A /etc/nvme)" ]; then
+			rm -rf /etc/nvme
+		fi
+	fi
+fi
diff --git a/nvme.spec.in b/nvme.spec.in
index a4718773a962..6efb28847005 100644
--- a/nvme.spec.in
+++ b/nvme.spec.in
@@ -33,6 +33,23 @@ make install DESTDIR=%{buildroot} PREFIX=/usr
 %clean
 rm -rf $RPM_BUILD_ROOT
 
+%post
+if [ $1 = 1 ]; then # 1 : This package is being installed for the first time
+	if [ ! -f /etc/nvme/hostnqn ]; then
+		install -D /dev/null /etc/nvme/hostnqn
+		echo $(nvme gen-hostnqn) > /etc/nvme/hostnqn
+        fi
+fi
+
+%preun
+if [ "$1" = "remove" ]; then
+	if [ -d /etc/nvme ]; then
+		rm -f /etc/nvme/hostnqn
+		if [ ! -n "$(ls -A /etc/nvme)" ]; then
+			rm -rf /etc/nvme
+		fi
+	fi
+fi
 %changelog
 * Thu Oct 15 2015 Keith Busch <keith.busch at intel.com>
 - Initial RPM spec
-- 
2.7.4

[PATCH nvme-cli v2 2/2] nvme.spec/debian: Auto generate host nqn as part of install

[Christoph Hellwig]

On Thu, Nov 17, 2016@02:54:48PM +0200, Sagi Grimberg wrote:
> The installation will generate a hostnqn and store it in
> /etc/nvme/hostnqn file (in case it doesn't exist).
> This file will be removed upon uninstallation (purge on for debian).
> 
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>  debian/postinst | 20 ++++++++++++++++++++
>  debian/postrm   | 10 ++++++++++
>  nvme.spec.in    | 17 +++++++++++++++++
>  3 files changed, 47 insertions(+)
>  create mode 100644 debian/postinst
>  create mode 100644 debian/postrm
> 
> diff --git a/debian/postinst b/debian/postinst
> new file mode 100644
> index 000000000000..b258cf569c3e
> --- /dev/null
> +++ b/debian/postinst
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +
> +set -e
> +
> +case "$1" in
> +    configure|install)
> +	if [ ! -f /etc/nvme/hostnqn ]; then
> +		install -D /dev/null /etc/nvme/hostnqn
> +		echo $(nvme gen-hostnqn) > /etc/nvme/hostnqn

The $() syntax is a bash-ish.  To work with a traditional bourne shell
it needs to be `nvme gen-hostnqn`

> +		if [ ! -n "$(ls -A /etc/nvme)" ]; then

Same here.

> +			rm -rf /etc/nvme
> +		fi

But maybe you should just do a

		rmdir /etc/nvme >/dev/null

instead?

[PATCH nvme-cli v2 2/2] nvme.spec/debian: Auto generate host nqn as part of install

[J Freyensee]

On Thu, 2016-11-17@05:36 -0800, Christoph Hellwig wrote:
> On Thu, Nov 17, 2016@02:54:48PM +0200, Sagi Grimberg wrote:
> > 
> > The installation will generate a hostnqn and store it in
> > /etc/nvme/hostnqn file (in case it doesn't exist).
> > This file will be removed upon uninstallation (purge on for
> > debian).
> > 
> > Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> > ---
> > ?debian/postinst | 20 ++++++++++++++++++++
> > ?debian/postrm???| 10 ++++++++++
> > ?nvme.spec.in????| 17 +++++++++++++++++
> > ?3 files changed, 47 insertions(+)
> > ?create mode 100644 debian/postinst
> > ?create mode 100644 debian/postrm
> > 
> > diff --git a/debian/postinst b/debian/postinst
> > new file mode 100644
> > index 000000000000..b258cf569c3e
> > --- /dev/null
> > +++ b/debian/postinst
> > @@ -0,0 +1,20 @@
> > +#!/bin/sh
> > +
> > +set -e
> > +
> > +case "$1" in
> > +????configure|install)
> > +	if [ ! -f /etc/nvme/hostnqn ]; then
> > +		install -D /dev/null /etc/nvme/hostnqn
> > +		echo $(nvme gen-hostnqn) > /etc/nvme/hostnqn
> 
> The $() syntax is a bash-ish.??To work with a traditional bourne
> shell
> it needs to be `nvme gen-hostnqn`
> 
> > 
> > +		if [ ! -n "$(ls -A /etc/nvme)" ]; then
> 
> Same here.
> 
> > 
> > +			rm -rf /etc/nvme
> > +		fi
> 
> But maybe you should just do a
> 
> 		rmdir /etc/nvme >/dev/null

Please don't do a remove of the directory, just remove the files
created from this patch (/etc/nvme/hostnqn). ?As I mentioned earlier,
someone for example could have independently created a discovery.conf
file in /etc/nvme and might have not wanted that file removed when this
is uninstalled. ?If the directory was created from this patch then
either leave the directory /etc/nvme behind or test to see if /etc/nvme
is empty after removing /etc/nvme/hostnqn before attempting to remove
the directory /etc/nvme.


> 
> instead?

[PATCH nvme-cli v2 2/2] nvme.spec/debian: Auto generate host nqn as part of install

[J Freyensee]

On Thu, 2016-11-17@09:51 -0800, J Freyensee wrote:
> On Thu, 2016-11-17@05:36 -0800, Christoph Hellwig wrote:
> > 
> > On Thu, Nov 17, 2016@02:54:48PM +0200, Sagi Grimberg wrote:
> > > 
> > > 
> > > The installation will generate a hostnqn and store it in
> > > /etc/nvme/hostnqn file (in case it doesn't exist).
> > > This file will be removed upon uninstallation (purge on for
> > > debian).
> > > 
> > > Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> > > ---
> > > ?debian/postinst | 20 ++++++++++++++++++++
> > > ?debian/postrm???| 10 ++++++++++
> > > ?nvme.spec.in????| 17 +++++++++++++++++
> > > ?3 files changed, 47 insertions(+)
> > > ?create mode 100644 debian/postinst
> > > ?create mode 100644 debian/postrm
> > > 
> > > diff --git a/debian/postinst b/debian/postinst
> > > new file mode 100644
> > > index 000000000000..b258cf569c3e
> > > --- /dev/null
> > > +++ b/debian/postinst
> > > @@ -0,0 +1,20 @@
> > > +#!/bin/sh
> > > +
> > > +set -e
> > > +
> > > +case "$1" in
> > > +????configure|install)
> > > +	if [ ! -f /etc/nvme/hostnqn ]; then
> > > +		install -D /dev/null /etc/nvme/hostnqn
> > > +		echo $(nvme gen-hostnqn) > /etc/nvme/hostnqn
> > 
> > The $() syntax is a bash-ish.??To work with a traditional bourne
> > shell
> > it needs to be `nvme gen-hostnqn`
> > 
> > > 
> > > 
> > > +		if [ ! -n "$(ls -A /etc/nvme)" ]; then
> > 
> > Same here.
> > 
> > > 
> > > 
> > > +			rm -rf /etc/nvme
> > > +		fi
> > 
> > But maybe you should just do a
> > 
> > 		rmdir /etc/nvme >/dev/null
> 


Nevermind, I'm dumb, Nack the comment, I didn't read that right from
the comments and code separated...more coffee :-/



> Please don't do a remove of the directory, just remove the files
> created from this patch (/etc/nvme/hostnqn). ?As I mentioned earlier,
> someone for example could have independently created a discovery.conf
> file in /etc/nvme and might have not wanted that file removed when
> this
> is uninstalled. ?If the directory was created from this patch then
> either leave the directory /etc/nvme behind or test to see if
> /etc/nvme
> is empty after removing /etc/nvme/hostnqn before attempting to remove
> the directory /etc/nvme.
> 
> 
> > 
> > 
> > instead?
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH nvme-cli v2 0/2] auto generate hostnqn file on installation

[Christoph Hellwig]

Btw, while we're at it - can you add a trivial man page for the
command?