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

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

[Sagi Grimberg]

Not sure if it's the best way to do this as my packeging understanding is
very limited, hence the RFC. the debian part is tested (don't have
access to rpm distros at the moment).

Sagi Grimberg (2):
  debain: generate hostnqn file on install and remove on uninstall
  nvme.spec: generate hostnqn file on install and remove on uninstall

 debian/postinst | 22 ++++++++++++++++++++++
 debian/prerm    |  5 +++++
 nvme.spec.in    | 14 ++++++++++++++
 3 files changed, 41 insertions(+)
 create mode 100644 debian/postinst
 create mode 100644 debian/prerm

-- 
2.7.4

[PATCH RFC nvme-cli 1/2] debain: generate hostnqn file on install and remove on uninstall

[Sagi Grimberg]

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 debian/postinst | 22 ++++++++++++++++++++++
 debian/prerm    |  5 +++++
 2 files changed, 27 insertions(+)
 create mode 100644 debian/postinst
 create mode 100644 debian/prerm

diff --git a/debian/postinst b/debian/postinst
new file mode 100644
index 000000000000..f0e3aaf8c4d0
--- /dev/null
+++ b/debian/postinst
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+set -e
+
+case "$1" in
+    configure|install)
+	if [ ! -f /etc/nvme/hostnqn ]; then
+		install -D /dev/null /etc/nvme/hostnqn
+		year_month=$(date | awk '{print $6"-"$3}')
+		uuid=$(cat /proc/sys/kernel/random/uuid)
+		echo "nqn.$year_month.org.nvmexpress:NVMf:uuid:$uuid" > /etc/nvme/hostnqn
+        fi
+        ;;
+
+    upgrade|abort-upgrade)
+        ;;
+    *)
+        echo "postinst called with unknown argument \`$1'" >&2
+        exit 0
+        ;;
+esac
+exit 0
diff --git a/debian/prerm b/debian/prerm
new file mode 100644
index 000000000000..40c6fea42284
--- /dev/null
+++ b/debian/prerm
@@ -0,0 +1,5 @@
+#!/bin/sh
+
+if [ "$1" = "remove" ]; then
+    rm -rf /etc/nvme
+fi
-- 
2.7.4

[PATCH RFC nvme-cli 1/2] debain: generate hostnqn file on install and remove on uninstall

[Christoph Hellwig]

I think the subject should read "debian" :)

> diff --git a/debian/prerm b/debian/prerm
> new file mode 100644
> index 000000000000..40c6fea42284
> --- /dev/null
> +++ b/debian/prerm
> @@ -0,0 +1,5 @@
> +#!/bin/sh
> +
> +if [ "$1" = "remove" ]; then
> +    rm -rf /etc/nvme
> +fi

Shouldn't we only do that for a purge?

[PATCH RFC nvme-cli 1/2] debain: generate hostnqn file on install and remove on uninstall

[Sagi Grimberg]

> I think the subject should read "debian" :)

Probably :)

>> diff --git a/debian/prerm b/debian/prerm
>> new file mode 100644
>> index 000000000000..40c6fea42284
>> --- /dev/null
>> +++ b/debian/prerm
>> @@ -0,0 +1,5 @@
>> +#!/bin/sh
>> +
>> +if [ "$1" = "remove" ]; then
>> +    rm -rf /etc/nvme
>> +fi
>
> Shouldn't we only do that for a purge?

I haven't given it much thought, it really was a 15 minute
thing that I thought I'd through out here so people can
tell suggest how to do it better...

I know rpms have .rpmsave for these sort of files but not
sure what are the rules for debian.

[PATCH RFC nvme-cli 1/2] debain: generate hostnqn file on install and remove on uninstall

[Gabriel Krisman Bertazi]

Sagi Grimberg <sagi at grimberg.me> writes:

>> I think the subject should read "debian" :)
>
> Probably :)
>
>>> diff --git a/debian/prerm b/debian/prerm
>>> new file mode 100644
>>> index 000000000000..40c6fea42284
>>> --- /dev/null
>>> +++ b/debian/prerm
>>> @@ -0,0 +1,5 @@
>>> +#!/bin/sh
>>> +
>>> +if [ "$1" = "remove" ]; then
>>> +    rm -rf /etc/nvme
>>> +fi
>>
>> Shouldn't we only do that for a purge?
>
> I haven't given it much thought, it really was a 15 minute
> thing that I thought I'd through out here so people can
> tell suggest how to do it better...
>

I think you need to use postrm and only do it for purge.

https://www.debian.org/doc/debian-policy/ch-maintainerscripts.html#s-mscriptsinstact
https://www.debian.org/doc/debian-policy/ap-pkg-conffiles.html#sE.2


-- 
Gabriel Krisman Bertazi

[PATCH RFC nvme-cli 2/2] nvme.spec: generate hostnqn file on install and remove on uninstall

[Sagi Grimberg]

Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
 nvme.spec.in | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/nvme.spec.in b/nvme.spec.in
index a4718773a962..e95810995395 100644
--- a/nvme.spec.in
+++ b/nvme.spec.in
@@ -33,6 +33,20 @@ 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
+		year_month=$(date | awk '{print $6"-"$3}')
+		uuid=$(cat /proc/sys/kernel/random/uuid)
+		echo "nqn.$year_month.org.nvmexpress:NVMf:uuid:$uuid" > /etc/nvme/hostnqn
+        fi
+fi
+
+%preun
+if [ "$1" = "remove" ]; then
+    rm -rf /etc/nvme
+fi
 %changelog
 * Thu Oct 15 2015 Keith Busch <keith.busch at intel.com>
 - Initial RPM spec
-- 
2.7.4

[PATCH RFC nvme-cli 2/2] nvme.spec: generate hostnqn file on install and remove on uninstall

[Johannes Thumshirn]

Hi Sagi,

On Sun, Nov 06, 2016@09:13:01PM +0200, Sagi Grimberg wrote:
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>  nvme.spec.in | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/nvme.spec.in b/nvme.spec.in
> index a4718773a962..e95810995395 100644
> --- a/nvme.spec.in
> +++ b/nvme.spec.in
> @@ -33,6 +33,20 @@ 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
> +		year_month=$(date | awk '{print $6"-"$3}')

Two things, a) are you sure this is correct? I interpret $year_month as year
and month, not year and day (date | awk '{print $6"-"$3}' evaluates to 2016-7
here) and b) 'date +"%Y-%m"' saves you a 2nd process.

Byte,
	Johannes

[PATCH RFC nvme-cli 2/2] nvme.spec: generate hostnqn file on install and remove on uninstall

[Sagi Grimberg]

> Two things, a) are you sure this is correct? I interpret $year_month as year
> and month, not year and day (date | awk '{print $6"-"$3}' evaluates to 2016-7
> here) and b) 'date +"%Y-%m"' saves you a 2nd process.

Thanks for the tip... I'm moving it to nvme utility though.

Cheers,
Sagi.

[PATCH RFC nvme-cli 2/2] nvme.spec: generate hostnqn file on install and remove on uninstall

[Johannes Thumshirn]

On Tue, Nov 08, 2016@12:31:07PM +0200, Sagi Grimberg wrote:
> 
> > Two things, a) are you sure this is correct? I interpret $year_month as year
> > and month, not year and day (date | awk '{print $6"-"$3}' evaluates to 2016-7
> > here) and b) 'date +"%Y-%m"' saves you a 2nd process.
> 
> Thanks for the tip... I'm moving it to nvme utility though.

Yes that's probably better.

Byte,
	Johannes
-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

[PATCH RFC nvme-cli 2/2] nvme.spec: generate hostnqn file on install and remove on uninstall

[J Freyensee]

On Sun, 2016-11-06@21:13 +0200, Sagi Grimberg wrote:
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
> ?nvme.spec.in | 14 ++++++++++++++
> ?1 file changed, 14 insertions(+)
> 
> diff --git a/nvme.spec.in b/nvme.spec.in
> index a4718773a962..e95810995395 100644
> --- a/nvme.spec.in
> +++ b/nvme.spec.in
> @@ -33,6 +33,20 @@ 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
> +		year_month=$(date | awk '{print $6"-"$3}')

Hey Sagi,

7.9 of the base NVMe spec makes it clear on unique names that the "2nd
format", which is what is being assembled here, uses a fixed date
"2014-08" in it's name, which is exactly what the NVMe Host driver does
as its default:

fabrics.c: ?"nqn.2014-08.org.nvmexpress:NVMf:uuid:%pUb", &host->id);

I would just do what the NVMe host is doing already and make it easier
on yourself ;-).

[PATCH RFC nvme-cli 2/2] nvme.spec: generate hostnqn file on install and remove on uninstall

[Sagi Grimberg]

> Hey Sagi,
>
> 7.9 of the base NVMe spec makes it clear on unique names that the "2nd
> format", which is what is being assembled here, uses a fixed date
> "2014-08" in it's name, which is exactly what the NVMe Host driver does
> as its default:
>
> fabrics.c:  "nqn.2014-08.org.nvmexpress:NVMf:uuid:%pUb", &host->id);
>
> I would just do what the NVMe host is doing already and make it easier
> on yourself ;-).

I'm fine with that...

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

[Christoph Hellwig]

On Sun, Nov 06, 2016@09:12:59PM +0200, Sagi Grimberg wrote:
> Not sure if it's the best way to do this as my packeging understanding is
> very limited, hence the RFC. the debian part is tested (don't have
> access to rpm distros at the moment).

My idea was to add a command to generate an automatic hostnqn to
the nvme tool instead, what would you think about that?  The install
scripts could then call that.

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

[Sagi Grimberg]

>> Not sure if it's the best way to do this as my packeging understanding is
>> very limited, hence the RFC. the debian part is tested (don't have
>> access to rpm distros at the moment).
>
> My idea was to add a command to generate an automatic hostnqn to
> the nvme tool instead, what would you think about that?

I'm fine with that too.

> The install scripts could then call that.

Sounds fine.

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

[J Freyensee]

On Sun, 2016-11-06@23:36 +0200, Sagi Grimberg wrote:
> > 
> > > 
> > > Not sure if it's the best way to do this as my packeging
> > > understanding is
> > > very limited, hence the RFC. the debian part is tested (don't
> > > have
> > > access to rpm distros at the moment).
> > 
> > My idea was to add a command to generate an automatic hostnqn to
> > the nvme tool instead, what would you think about that?
> 
> I'm fine with that too.

How about an extra tweak to that tool proposal? ?Have the tool have the
option to do both NQN supported formats:

A. the first format that uses a date code in which I think that is what
patch 2 in this series was trying to do

and

B. The second format the NVMe Host driver is already does in fabrics.c?

> 
> > 
> > The install scripts could then call that.
> 
> Sounds fine.

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

[Sagi Grimberg]

> How about an extra tweak to that tool proposal?  Have the tool have the
> option to do both NQN supported formats:
>
> A. the first format that uses a date code in which I think that is what
> patch 2 in this series was trying to do
>
> and
>
> B. The second format the NVMe Host driver is already does in fabrics.c

Lets not over-complicate this, perhaps just do (A)?

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

[J Freyensee]

On Tue, 2016-11-08@12:30 +0200, Sagi Grimberg wrote:
> > 
> > How about an extra tweak to that tool proposal???Have the tool have
> > the
> > option to do both NQN supported formats:
> > 
> > A. the first format that uses a date code in which I think that is
> > what
> > patch 2 in this series was trying to do
> > 
> > and
> > 
> > B. The second format the NVMe Host driver is already does in
> > fabrics.c
> 
> Lets not over-complicate this, perhaps just do (A)?

I'd do B. and be consistent with the NVMe Host driver.