nvme-cli documentation updates

nvme-cli documentation updates

[Christoph Hellwig]

[third try, I'm really got as using the wrong list address today.  Sorry Jay.. :)]

This series improves the nvme-discover manpage a bit, and adds a new
one for nvme-connect-all.  I would have preferred to share a page for
both commands as there is a lot of duplication, but I couldn't find a way
to do that.

[PATCH 1/3] consolidate reporting bugs information

[Christoph Hellwig]

Add the Linux-nvme list to the main reporting bugs section and remove
the separate ones in the connect and discovery pages.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 Documentation/nvme-connect.txt  | 6 ------
 Documentation/nvme-discover.txt | 6 ------
 Documentation/nvme.txt          | 3 ++-
 3 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/Documentation/nvme-connect.txt b/Documentation/nvme-connect.txt
index 18338cb..9ba969e 100644
--- a/Documentation/nvme-connect.txt
+++ b/Documentation/nvme-connect.txt
@@ -78,12 +78,6 @@ This was co-written by mailto:james.p.freyensee at intel.com[Jay Freyensee]
 and mailto:hch at lst.de[Christoph Hellwig]
 for mailto:keith.busch at intel.com[Keith Busch].
 
-REPORTING BUGS
---------------
-Patches and issues for 'connect' should be submitted to the
-Linux kernel email list mailto:linux-nvme at lists.infradead.org[linux-nvme]
-before merging to https://github.com/linux-nvme/nvme-cli.
-
 NVME
 ----
 Part of the nvme-user suite
diff --git a/Documentation/nvme-discover.txt b/Documentation/nvme-discover.txt
index bff4767..9140f84 100644
--- a/Documentation/nvme-discover.txt
+++ b/Documentation/nvme-discover.txt
@@ -119,12 +119,6 @@ AUTHORS
 This was written by mailto:james.p.freyensee at intel.com[Jay Freyensee]
 for mailto:keith.busch at intel.com[Keith Busch].
 
-REPORTING BUGS
---------------
-Patches and issues for 'discover' should be submitted to the Linux
-kernel email list mailto:linux-nvme at lists.infradead.org[linux-nvme]
-before merging to https://github.com/linux-nvme/nvme-cli.
-
 NVME
 ----
 Part of the nvme-user suite
diff --git a/Documentation/nvme.txt b/Documentation/nvme.txt
index dcbc660..cb64082 100644
--- a/Documentation/nvme.txt
+++ b/Documentation/nvme.txt
@@ -46,7 +46,8 @@ This is written and maintained by mailto:keith.busch at intel.com[Keith Busch].
 Reporting Bugs
 --------------
 Patches and issues may be submitted to the official repository at
-https://github.com/linux-nvme/nvme-cli.
+https://github.com/linux-nvme/nvme-cli or the Linux NVMe mailing list
+mailto:linux-nvme at lists.infradead.org[linux-nvme]
 
 NVME
 ----
-- 
2.10.1.382.ga23ca1b

[PATCH 2/3] fine tune the nvme-discover manpage

[Christoph Hellwig]

 - remove the device argument, which as far as I can tell does not
   exist.
 - mention the /etc/nvme/hostnqn file, the default host nqn and explain
   host nqns a bit more
 - mention the loopback transport
 - be a bit more specific about IP addressing as a concept separate from
   the RDMA transport
 - improve the introduction a bit.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 Documentation/nvme-discover.txt | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/Documentation/nvme-discover.txt b/Documentation/nvme-discover.txt
index 9140f84..96c21f6 100644
--- a/Documentation/nvme-discover.txt
+++ b/Documentation/nvme-discover.txt
@@ -3,12 +3,12 @@ nvme-discover(1)
 
 NAME
 ----
-nvme-discover - Send Get Log Page request to Discovery Controller.
+nvme-discover - Send Discovery requests to Fabrics Discovery Controllers.
 
 SYNOPSIS
 --------
 [verse]
-'nvme discover'	[device]
+'nvme discover'
 		[--transport=<trtype> | -t <trtype>]
 		[--traddr=<traddr>    | -a <traddr>]
 		[--trsvcid=<trsvcid>  | -s <trsvcid>]
@@ -17,17 +17,18 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-For a given NVMe Host, send a Discovery Get Log Page request
-to a Discovery Controller on the network.
-
-The [device] parameter is OPTIONAL and will default to
-/dev/nvme-fabrics if none is given.
+Send one or more Discovery requests to a NVMe over Fabrics Discovery
+Controller.
 
 If no parameters are given, then 'nvme discover' will attempt to 
-find a /etc/nvme/discovery.conf file to use to supply the
-the discover command options.  If no /etc/nvme/discovery.conf file
+find a /etc/nvme/discovery.conf file to use to supply a list of
+Discovery commands to run.  If no /etc/nvme/discovery.conf file
 exists, the command will quit with an error.
 
+Otherwise a specific Discovery Controller should be specified using the
+--transport, --traddr and if nessecary the --trsvcid and a Di??covery
+request will be sent to the specified Discovery Controller.
+
 BACKGROUND
 ----------
 The NVMe-over-Fabrics specification defines the concept of a 
@@ -61,23 +62,29 @@ OPTIONS
 |Value|Definition
 |rdma|The network fabric is an rdma network (RoCE, iWARP, Infiniband, basic rdma, etc)
 |fc  |*WIP* The network fabric is a Fibre Channel network.
+|loop|Connect to a NVMe over Fabrics target on the local host
 |=================
 
 -a <traddr>::
 --traddr=<traddr>::
 	This field specifies the network address of the Discovery Controller.
+	For transports using IP addressing (e.g. rdma) this should be an IPv4
+	address.
 
 -s <trsvcid>::
 --trsvcid=<trsvcid>::
-	This field specifies the transport service id.  For IP addresses,
-	this field is the port number. By default, the IP port number
-	is 4420.
+	This field specifies the transport service id.  For transports using IP
+	addressing (e.g. rdma) this field is the port number. By default, the IP
+	port number for the RDMA transport is 4420.
  
 -q <hostnqn>::
 --hostnqn=<hostnqn>::
-	This field is the unique NQN of a NVMe Host. This field
-	will be used by the Discovery Controller to check what NVMe
-	Target resources are allocated to the NVMe Host for a connection.
+	Overrides the default host NQN that identifies the NVMe Host.  If this
+	option is not specified the default is read from /etc/nvme/hostnqn or
+	autogenerated by the kernel (in that order).
+	The Host NQN uniquely identifies the NVMe Host, and may be used by the
+	the Discovery Controller to control what NVMe Target resources are
+	allocated to the NVMe Host for a connection.
 
 -r <filename>::
 --raw=<filename>::
-- 
2.10.1.382.ga23ca1b

[PATCH 3/3] add documentation for the connect-all command

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 Documentation/nvme-connect-all.txt | 103 +++++++++++++++++++++++++++++++++++++
 Documentation/nvme-discover.txt    |   1 +
 2 files changed, 104 insertions(+)
 create mode 100644 Documentation/nvme-connect-all.txt

diff --git a/Documentation/nvme-connect-all.txt b/Documentation/nvme-connect-all.txt
new file mode 100644
index 0000000..fa40b41
--- /dev/null
+++ b/Documentation/nvme-connect-all.txt
@@ -0,0 +1,103 @@
+nvme-connect-all(1)
+==============
+
+NAME
+----
+nvme-connect-all - Discover and Connect to Fabrics controllers.
+
+SYNOPSIS
+--------
+[verse]
+'nvme connect-all'
+		[--transport=<trtype> | -t <trtype>]
+		[--traddr=<traddr>    | -a <traddr>]
+		[--trsvcid=<trsvcid>  | -s <trsvcid>]
+		[--hostnqn=<hostnqn>  | -q <hostnqn>]
+		[--raw=<filename>     | -r <filename>]
+
+DESCRIPTION
+-----------
+Send one or more Discovery requests to a NVMe over Fabrics Discovery
+Controller, and create controllers for the returned discovery records.
+
+If no parameters are given, then 'nvme connect-all' will attempt to
+find a /etc/nvme/discovery.conf file to use to supply a list of
+Connect-all commands to run. If no /etc/nvme/discovery.conf file exists,
+the command will quit with an error.
+
+Otherwise a specific Discovery Controller should be specified using the
+--transport, --traddr and if nessecary the --trsvcid and a Di??covery
+request will be sent to the specified Discovery Controller.
+
+See the documentation for the nvme-discover(1) command for further
+background.
+
+OPTIONS
+-------
+-t <trtype>::
+--transport=<trtype>::
+	This field specifies the network fabric being used for
+	a NVMe-over-Fabrics network.  Current string values include:
++
+[]
+|=================
+|Value|Definition
+|rdma|The network fabric is an rdma network (RoCE, iWARP, Infiniband, basic rdma, etc)
+|fc  |*WIP* The network fabric is a Fibre Channel network.
+|loop|Connect to a NVMe over Fabrics target on the local host
+|=================
+
+-a <traddr>::
+--traddr=<traddr>::
+	This field specifies the network address of the Discovery Controller.
+	For transports using IP addressing (e.g. rdma) this should be an IPv4
+	address.
+
+-s <trsvcid>::
+--trsvcid=<trsvcid>::
+	This field specifies the transport service id.  For transports using IP
+	addressing (e.g. rdma) this field is the port number. By default, the IP
+	port number for the RDMA transport is 4420.
+ 
+-q <hostnqn>::
+--hostnqn=<hostnqn>::
+	Overrides the default host NQN that identifies the NVMe Host.  If this
+	option is not specified the default is read from /etc/nvme/hostnqn or
+	autogenerated by the kernel (in that order).
+	The Host NQN uniquely identifies the NVMe Host, and may be used by the
+	the Discovery Controller to control what NVMe Target resources are
+	allocated to the NVMe Host for a connection.
+
+EXAMPLES
+--------
+* Connect to all records returned by the Discover Controller with IP4 address
+192.168.1.3 for all resources allocated for NVMe Host name host1-rogue-nqn on
+the RDMA network. Port 4420 is used by default:
++
+------------
+# nvme connect-all --transport=rdma --traddr=192.168.1.3 \
+--hostnqn=host1-rogue-nqn
+------------
++
+* Issue a 'nvme connect-all' command using a /etc/nvme/discovery.conf file:
+-----------
+# Machine default 'nvme discover' commands.  Query the
+# Discovery Controller's two ports (some resources may only
+# be accessible on a single port).  Note an official
+# nqn (Host) name defined in the NVMe specification is being used
+# in this example.
+-t rdma -a 192.168.69.33 -s 4420 -q nqn.2014-08.com.example:nvme:nvm-subsystem-sn-d78432
+-t rdma -a 192.168.1.4   -s 4420 -q nqn.2014-08.com.example:nvme:nvm-subsystem-sn-d78432
+
+At the prompt type "nvme connect-all".
+
+------------
+
+SEE ALSO
+--------
+nvme-discover(1)
+nvme-connect(1)
+
+NVME
+----
+Part of the nvme-user suite
diff --git a/Documentation/nvme-discover.txt b/Documentation/nvme-discover.txt
index 96c21f6..d43f088 100644
--- a/Documentation/nvme-discover.txt
+++ b/Documentation/nvme-discover.txt
@@ -120,6 +120,7 @@ At the prompt type "nvme discover".
 SEE ALSO
 --------
 nvme-connect(1)
+nvme-connect-all(1)
 
 AUTHORS
 -------
-- 
2.10.1.382.ga23ca1b

[PATCH 1/3] consolidate reporting bugs information

[J Freyensee]

On Wed, 2016-10-12@10:45 +0200, Christoph Hellwig wrote:
> Add the Linux-nvme list to the main reporting bugs section and remove
> the separate ones in the connect and discovery pages.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>

(Email bounce, re-sending)

Makes sense,

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

[PATCH 2/3] fine tune the nvme-discover manpage

[J Freyensee]

On Wed, 2016-10-12@10:45 +0200, Christoph Hellwig wrote:

(Email bounce, re-sending)

> ?- remove the device argument, which as far as I can tell does not
> ???exist.
> ?- mention the /etc/nvme/hostnqn file, the default host nqn and
> explain
> ???host nqns a bit more
> ?- mention the loopback transport
> ?- be a bit more specific about IP addressing as a concept separate
> from
> ???the RDMA transport
> ?- improve the introduction a bit.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
> ?Documentation/nvme-discover.txt | 37 ++++++++++++++++++++++---------
> ------
> ?1 file changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/nvme-discover.txt b/Documentation/nvme-
> discover.txt
> index 9140f84..96c21f6 100644
> --- a/Documentation/nvme-discover.txt
> +++ b/Documentation/nvme-discover.txt
> @@ -3,12 +3,12 @@ nvme-discover(1)
> ?
> ?NAME
> ?----
> -nvme-discover - Send Get Log Page request to Discovery Controller.
> +nvme-discover - Send Discovery requests to Fabrics Discovery
> Controllers.

I preferred mentioning "Get Log Page" because that is really what is
happening and reflects the NVMe specification, which I was aiming to
push the envelope a bit and help educate and accelerate the learning
curve for people new to these features (map more accurately what is
being described here to the NVMe specification(s).

> ?
> ?SYNOPSIS
> ?--------
> ?[verse]
> -'nvme discover'	[device]
> +'nvme discover'

It was there because nvme-cli works regardless if a device parameter is
there or not (such as /dev/nvme-fabrics/).??Granted it seems a bit
pointless to include it if the same correct behavior works without it,
but it made it common with the rest of the nvme-cli command man pages.

> ?		[--transport=<trtype> | -t <trtype>]
> ?		[--traddr=<traddr>????| -a <traddr>]
> ?		[--trsvcid=<trsvcid>??| -s <trsvcid>]
> @@ -17,17 +17,18 @@ SYNOPSIS
> ?
> ?DESCRIPTION
> ?-----------
> -For a given NVMe Host, send a Discovery Get Log Page request
> -to a Discovery Controller on the network.
> -
> -The [device] parameter is OPTIONAL and will default to
> -/dev/nvme-fabrics if none is given.
> +Send one or more Discovery requests to a NVMe over Fabrics Discovery
> +Controller.

Same blurb on what I said earlier for mentioning "Get Log Page".

> ?
> ?If no parameters are given, then 'nvme discover' will attempt to?
> -find a /etc/nvme/discovery.conf file to use to supply the
> -the discover command options.??If no /etc/nvme/discovery.conf file
> +find a /etc/nvme/discovery.conf file to use to supply a list of
> +Discovery commands to run.??If no /etc/nvme/discovery.conf file
> ?exists, the command will quit with an error.
> ?
> +Otherwise a specific Discovery Controller should be specified using
> the
> +--transport, --traddr and if nessecary the --trsvcid and a Di?covery
> +request will be sent to the specified Discovery Controller.
> +

This is unnecessary to be mentioning the flags here when they have
already been mentioned and shown in "SYNOPSIS" as well as described in
detail below with examples provided after that.??This isn't adding much
additional value and making this slightly harder to maintain if flags
are added/adjusted.

(necessary is misspelled btw).

> ?BACKGROUND
> ?----------
> ?The NVMe-over-Fabrics specification defines the concept of a?
> @@ -61,23 +62,29 @@ OPTIONS
> ?|Value|Definition
> ?|rdma|The network fabric is an rdma network (RoCE, iWARP,
> Infiniband, basic rdma, etc)
> ?|fc??|*WIP* The network fabric is a Fibre Channel network.
> +|loop|Connect to a NVMe over Fabrics target on the local host

Flat out missed that, good catch.

> ?|=================
> ?
> ?-a <traddr>::
> ?--traddr=<traddr>::
> ?	This field specifies the network address of the Discovery
> Controller.
> +	For transports using IP addressing (e.g. rdma) this should
> be an IPv4
> +	address.

We shouldn't be mentioning that this can be specifically a IPv4 address
when there has already been work to support an IPv6 address (for
example) and then this will have to be tweaked again.??I think the
explanation was fine the way it is and should be explained generically
that leaves out specific mentions of IPv4 (plus the examples provided
afterword show using an IPv4 address).

> ?
> ?-s <trsvcid>::
> ?--trsvcid=<trsvcid>::
> -	This field specifies the transport service id.??For IP
> addresses,
> -	this field is the port number. By default, the IP port
> number
> -	is 4420.
> +	This field specifies the transport service id.??For
> transports using IP
> +	addressing (e.g. rdma) this field is the port number. By
> default, the IP
> +	port number for the RDMA transport is 4420.
> ??

This is a little more clear.

> ?-q <hostnqn>::
> ?--hostnqn=<hostnqn>::
> -	This field is the unique NQN of a NVMe Host. This field
> -	will be used by the Discovery Controller to check what NVMe
> -	Target resources are allocated to the NVMe Host for a
> connection.
> +	Overrides the default host NQN that identifies the NVMe
> Host.??If this
> +	option is not specified the default is read from
> /etc/nvme/hostnqn or
> +	autogenerated by the kernel (in that order).

Good start, but I can see an interpretation that this can make it sound
like the kernel will autogenerate hostnqn if this parameter is not
supplied, which would be incorrect because it's already generated upon
NVMe Host module load to be used as a 'catch-all default'.??I think a
further tweak to this could be:

"If this option is not specified, the default is read from
/etc/nvme/hostnqn first and if that does not exist, the autogenerated
NQN value from the NVMe Host kernel module is used next."

> +	The Host NQN uniquely identifies the NVMe Host, and may be
> used by the
> +	the Discovery Controller to control what NVMe Target
> resources are
> +	allocated to the NVMe Host for a connection.

This sounds like it could go in the BACKGROUND section I wrote, which I
admit I didn't not state this as clearly as here, but then again it's
probably ok to keep this blurb here to re-emphasize this important
feature of NVMe devices.

> ?
> ?-r <filename>::
> ?--raw=<filename>::
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH 3/3] add documentation for the connect-all command

[J Freyensee]

On Wed, 2016-10-12@10:45 +0200, Christoph Hellwig wrote:

(email bounce, resend)

> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
> ?Documentation/nvme-connect-all.txt | 103
> +++++++++++++++++++++++++++++++++++++
> ?Documentation/nvme-discover.txt????|???1 +
> ?2 files changed, 104 insertions(+)
> ?create mode 100644 Documentation/nvme-connect-all.txt
> 
> diff --git a/Documentation/nvme-connect-all.txt b/Documentation/nvme-
> connect-all.txt
> new file mode 100644
> index 0000000..fa40b41
> --- /dev/null
> +++ b/Documentation/nvme-connect-all.txt
> @@ -0,0 +1,103 @@
> +nvme-connect-all(1)
> +==============
> +
> +NAME
> +----
> +nvme-connect-all - Discover and Connect to Fabrics controllers.
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'nvme connect-all'
> +		[--transport=<trtype> | -t <trtype>]
> +		[--traddr=<traddr>????| -a <traddr>]
> +		[--trsvcid=<trsvcid>??| -s <trsvcid>]
> +		[--hostnqn=<hostnqn>??| -q <hostnqn>]
> +		[--raw=<filename>?????| -r <filename>]
> +
> +DESCRIPTION
> +-----------
> +Send one or more Discovery requests to a NVMe over Fabrics Discovery
> +Controller, and create controllers for the returned discovery
> records.
> +
> +If no parameters are given, then 'nvme connect-all' will attempt to
> +find a /etc/nvme/discovery.conf file to use to supply a list of
> +Connect-all commands to run. If no /etc/nvme/discovery.conf file 

"connect-all"

> exists,
> +the command will quit with an error.
> +
> +Otherwise a specific Discovery Controller should be specified using
> the
> +--transport, --traddr and if nessecary the --trsvcid and a Di?covery
> +request will be sent to the specified Discovery Controller.

Again I don't think this adds additional use/learning having the flags
again here when they are mentioned in "Synopsis", described further
below, and then with examples, just makes maintenance of the man pages
a bit more tedious.

(and "necessary" is misspelled)

> +
> +See the documentation for the nvme-discover(1) command for further
> +background.
> +
> +OPTIONS
> +-------
> +-t <trtype>::
> +--transport=<trtype>::
> +	This field specifies the network fabric being used for
> +	a NVMe-over-Fabrics network.??Current string values include:
> ++
> +[]
> +|=================
> +|Value|Definition
> +|rdma|The network fabric is an rdma network (RoCE, iWARP,
> Infiniband, basic rdma, etc)
> +|fc??|*WIP* The network fabric is a Fibre Channel network.
> +|loop|Connect to a NVMe over Fabrics target on the local host
> +|=================
> +
> +-a <traddr>::
> +--traddr=<traddr>::
> +	This field specifies the network address of the Discovery
> Controller.
> +	For transports using IP addressing (e.g. rdma) this should
> be an IPv4
> +	address.

Again I think we can remove the mention of "IPv4"; the examples show
this and IPv6 will be used soon enough.

> +
> +-s <trsvcid>::
> +--trsvcid=<trsvcid>::
> +	This field specifies the transport service id.??For
> transports using IP
> +	addressing (e.g. rdma) this field is the port number. By
> default, the IP
> +	port number for the RDMA transport is 4420.
> +?
> +-q <hostnqn>::
> +--hostnqn=<hostnqn>::
> +	Overrides the default host NQN that identifies the NVMe
> Host.??If this
> +	option is not specified the default is read from
> /etc/nvme/hostnqn or
> +	autogenerated by the kernel (in that order).

Same comment as what was said in 2/3 for Discovery man page tweaks.

> +	The Host NQN uniquely identifies the NVMe Host, and may be
> used by the
> +	the Discovery Controller to control what NVMe Target
> resources are
> +	allocated to the NVMe Host for a connection.
> +
> +EXAMPLES
> +--------
> +* Connect to all records returned by the Discover Controller with
> IP4 address
> +192.168.1.3 for all resources allocated for NVMe Host name host1-
> rogue-nqn?

I'm getting ready for "Rogue One- a Star Wars story" :-).

nvme-cli documentation updates

[Keith Busch]

On Wed, Oct 12, 2016@10:45:50AM +0200, Christoph Hellwig wrote:
> [third try, I'm really got as using the wrong list address today.  Sorry Jay.. :)]
> 
> This series improves the nvme-discover manpage a bit, and adds a new
> one for nvme-connect-all.  I would have preferred to share a page for
> both commands as there is a lot of duplication, but I couldn't find a way
> to do that.

I applied them all and regenerated the man and html pages. I had to make
one minor change to connect-all.txt to get asciidoc to understand the
header portion.

I had to fixup the multipart conent type in patches 2/3 and 3/3, and
missed J's comments while doing that. I'll apply updates on top of the
current master if we need to change any wording.

[PATCH 2/3] fine tune the nvme-discover manpage

[Christoph Hellwig]

On Wed, Oct 12, 2016@10:18:05AM -0700, J Freyensee wrote:
> > -nvme-discover - Send Get Log Page request to Discovery Controller.
> > +nvme-discover - Send Discovery requests to Fabrics Discovery
> > Controllers.
> 
> I preferred mentioning "Get Log Page" because that is really what is
> happening and reflects the NVMe specification, which I was aiming to
> push the envelope a bit and help educate and accelerate the learning
> curve for people new to these features (map more accurately what is
> being described here to the NVMe specification(s).

While Get Log Page is technicaly correct I don't really think it
serves the user much - it's the way the discovery is implemented,
the important part is that we perform discovery.

> > ?SYNOPSIS
> > ?--------
> > ?[verse]
> > -'nvme discover'	[device]
> > +'nvme discover'
> 
> It was there because nvme-cli works regardless if a device parameter is
> there or not (such as /dev/nvme-fabrics/).??Granted it seems a bit
> pointless to include it if the same correct behavior works without it,
> but it made it common with the rest of the nvme-cli command man pages.

Well, the point is that we hardcode the /dev/nvme-fabrics path in
fabrics.c.  So while nvme-cli accept as a device argument it is ignored.

We'll need to fix the option parse to properly reject it, I think.

> This is unnecessary to be mentioning the flags here when they have
> already been mentioned and shown in "SYNOPSIS" as well as described in
> detail below with examples provided after that.??This isn't adding much
> additional value and making this slightly harder to maintain if flags
> are added/adjusted.
> 
> (necessary is misspelled btw).

The important addition I wanted is something to balance the description
of what happens if no arguments are specified. 

That being said I really, really hate the having the read config file
behavior the default without option.  I already complained about it
when it went in, and having to read the manpage it's even more confusing.

I'd much prefer making it a --config option that points to a config
file, as the prime user of it is the system startup script anyway.

> > ?|=================
> > ?
> > ?-a <traddr>::
> > ?--traddr=<traddr>::
> > ?	This field specifies the network address of the Discovery
> > Controller.
> > +	For transports using IP addressing (e.g. rdma) this should
> > be an IPv4
> > +	address.
> 
> We shouldn't be mentioning that this can be specifically a IPv4 address
> when there has already been work to support an IPv6 address (for
> example) and then this will have to be tweaked again.??I think the
> explanation was fine the way it is and should be explained generically
> that leaves out specific mentions of IPv4 (plus the examples provided
> afterword show using an IPv4 address).

I was tempted to add IPv6, but given that it's not supported I skipped it.
We'll need some guidance on what the valid address format are.  I'm not
sure what the best way is, but I don't think example are sufficient.

Maybe a table in a separate section or even man page.

> Good start, but I can see an interpretation that this can make it sound
> like the kernel will autogenerate hostnqn if this parameter is not
> supplied, which would be incorrect because it's already generated upon
> NVMe Host module load to be used as a 'catch-all default'.??I think a
> further tweak to this could be:

True.

> 
> "If this option is not specified, the default is read from
> /etc/nvme/hostnqn first and if that does not exist, the autogenerated
> NQN value from the NVMe Host kernel module is used next."

Sounds fine.

> > +	The Host NQN uniquely identifies the NVMe Host, and may be
> > used by the
> > +	the Discovery Controller to control what NVMe Target
> > resources are
> > +	allocated to the NVMe Host for a connection.
> 
> This sounds like it could go in the BACKGROUND section I wrote, which I
> admit I didn't not state this as clearly as here, but then again it's
> probably ok to keep this blurb here to re-emphasize this important
> feature of NVMe devices.

I'm fine with moving this around.

[PATCH 3/3] add documentation for the connect-all command

[Christoph Hellwig]

On Wed, Oct 12, 2016@10:21:38AM -0700, J Freyensee wrote:
> > +If no parameters are given, then 'nvme connect-all' will attempt to
> > +find a /etc/nvme/discovery.conf file to use to supply a list of
> > +Connect-all commands to run. If no /etc/nvme/discovery.conf file 
> 
> "connect-all"

ok..

[PATCH 2/3] fine tune the nvme-discover manpage

[J Freyensee]

On Thu, 2016-10-13@12:26 +0200, Christoph Hellwig wrote:
> On Wed, Oct 12, 2016@10:18:05AM -0700, J Freyensee wrote:
> > 
> > > 
> > > -nvme-discover - Send Get Log Page request to Discovery
> > > Controller.
> > > +nvme-discover - Send Discovery requests to Fabrics Discovery
> > > Controllers.
> > 
> > I preferred mentioning "Get Log Page" because that is really what
> > is
> > happening and reflects the NVMe specification, which I was aiming
> > to
> > push the envelope a bit and help educate and accelerate the
> > learning
> > curve for people new to these features (map more accurately what is
> > being described here to the NVMe specification(s).
> 
> While Get Log Page is technicaly correct I don't really think it
> serves the user much - it's the way the discovery is implemented,
> the important part is that we perform discovery.

I think it does serve the user, it educates the user a bit faster on
the NVMe Specification and what is really happening without spending
the time sorting through the spec, important for things like debugging
and diagnosis if something wonky happens.