[PATCH 0/6] More fix-ups for nfs-utils-1.2.4

[PATCH 0/6] More fix-ups for nfs-utils-1.2.4

[Chuck Lever]

Hi Steve-

Here are rather more final versions of the patches I posted here last
week for review.  There are a couple of extra clean-ups here as well.
I think these are ready to be considered for nfs-utils 1.2.4.

---

Chuck Lever (6):
      nfs(5): Document remount behavior
      nfs(5): Grammar and style fixes
      mount.nfs: mnt_freq and mnt_pass are always zero
      mount.nfs: Fix memory leak in nfs_sys_mount()
      mount: Fix compiler warning in nfs_parse_retry_option()
      nfs-utils: Remove all uses of AI_ADDRCONFIG


 support/export/hostname.c     |    6 --
 tests/nsm_client/nsm_client.c |    2 -
 utils/mount/mount.c           |   34 +++++++------
 utils/mount/network.c         |    3 -
 utils/mount/nfs.man           |  107 ++++++++++++++++++++++++++++++++---------
 utils/mount/stropts.c         |   20 ++++----
 utils/statd/hostname.c        |    4 --
 utils/statd/sm-notify.c       |    5 --
 8 files changed, 114 insertions(+), 67 deletions(-)

-- 
Chuck Lever

[PATCH 1/6] nfs-utils: Remove all uses of AI_ADDRCONFIG

[Chuck Lever]

Steve Dickson reports that, if only "lo" is up,

  mount.nfs 127.0.0.1:/export /mount

fails with "Name or service not known".

"man 3 getaddrinfo" says this:

  If hints.ai_flags includes the AI_ADDRCONFIG flag, then IPv4
  addresses are returned in the list pointed to by res only if the
  local system has at least one IPv4 address configured, and IPv6
  addresses are only returned if the local system has at least
  one IPv6 address configured.

The man page oversimplifies here.  A review of glibc shows that
getaddrinfo(3) explicitly ignores loopback addresses when deciding
whether an IPv4 or IPv6 address is configured.

This behavior around loopback is a problem not just for mount.nfs,
but also for RPC daemons that have to start up before a system's
networking is fully configured and started.  Given the history of
other problems with AI_ADDRCONFIG and the unpredictable behavior it
introduces, let's just remove it everywhere in nfs-utils.

This fix addresses:

  https://bugzilla.linux-nfs.org/show_bug.cgi?id=191

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 support/export/hostname.c     |    6 +-----
 tests/nsm_client/nsm_client.c |    2 +-
 utils/mount/network.c         |    3 ---
 utils/mount/stropts.c         |    5 -----
 utils/statd/hostname.c        |    4 ----
 utils/statd/sm-notify.c       |    5 -----
 6 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/support/export/hostname.c b/support/export/hostname.c
index 3c55ce7..efcb75c 100644
--- a/support/export/hostname.c
+++ b/support/export/hostname.c
@@ -30,10 +30,6 @@
 #include "sockaddr.h"
 #include "exportfs.h"
 
-#ifndef HAVE_DECL_AI_ADDRCONFIG
-#define AI_ADDRCONFIG	0
-#endif
-
 /**
  * host_ntop - generate presentation address given a sockaddr
  * @sap: pointer to socket address
@@ -170,7 +166,7 @@ host_addrinfo(const char *hostname)
 #endif
 		/* don't return duplicates */
 		.ai_protocol	= (int)IPPROTO_UDP,
-		.ai_flags	= AI_ADDRCONFIG | AI_CANONNAME,
+		.ai_flags	= AI_CANONNAME,
 	};
 	int error;
 
diff --git a/tests/nsm_client/nsm_client.c b/tests/nsm_client/nsm_client.c
index 0d1159a..0fa3422 100644
--- a/tests/nsm_client/nsm_client.c
+++ b/tests/nsm_client/nsm_client.c
@@ -205,7 +205,7 @@ nsm_client_get_rpcclient(const char *node)
 {
 	unsigned short		port;
 	struct addrinfo		*ai;
-	struct addrinfo		hints = { .ai_flags	= AI_ADDRCONFIG };
+	struct addrinfo		hints = { };
 	int			err;
 	CLIENT			*client = NULL;
 
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 5b515c3..21a7a2c 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -210,9 +210,6 @@ int nfs_lookup(const char *hostname, const sa_family_t family,
 {
 	struct addrinfo *gai_results;
 	struct addrinfo gai_hint = {
-#ifdef HAVE_DECL_AI_ADDRCONFIG
-		.ai_flags	= AI_ADDRCONFIG,
-#endif	/* HAVE_DECL_AI_ADDRCONFIG */
 		.ai_family	= family,
 	};
 	socklen_t len = *salen;
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 50a1a2a..bcc36f3 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -49,10 +49,6 @@
 #include "parse_dev.h"
 #include "conffile.h"
 
-#ifndef HAVE_DECL_AI_ADDRCONFIG
-#define AI_ADDRCONFIG	0
-#endif
-
 #ifndef NFS_PROGRAM
 #define NFS_PROGRAM	(100003)
 #endif
@@ -343,7 +339,6 @@ static int nfs_validate_options(struct nfsmount_info *mi)
 {
 	struct addrinfo hint = {
 		.ai_protocol	= (int)IPPROTO_UDP,
-		.ai_flags	= AI_ADDRCONFIG,
 	};
 	sa_family_t family;
 	int error;
diff --git a/utils/statd/hostname.c b/utils/statd/hostname.c
index 38f2265..616a3cb 100644
--- a/utils/statd/hostname.c
+++ b/utils/statd/hostname.c
@@ -39,10 +39,6 @@
 #include "statd.h"
 #include "xlog.h"
 
-#ifndef HAVE_DECL_AI_ADDRCONFIG
-#define AI_ADDRCONFIG	0
-#endif
-
 /**
  * statd_present_address - convert sockaddr to presentation address
  * @sap: pointer to socket address to convert
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index 437e37a..b7f4371 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -34,10 +34,6 @@
 #include "nsm.h"
 #include "nfsrpc.h"
 
-#ifndef HAVE_DECL_AI_ADDRCONFIG
-#define AI_ADDRCONFIG	0
-#endif
-
 #define NSM_TIMEOUT	2
 #define NSM_MAX_TIMEOUT	120	/* don't make this too big */
 
@@ -78,7 +74,6 @@ smn_lookup(const char *name)
 {
 	struct addrinfo	*ai = NULL;
 	struct addrinfo hint = {
-		.ai_flags	= AI_ADDRCONFIG,
 		.ai_family	= (nsm_family == AF_INET ? AF_INET: AF_UNSPEC),
 		.ai_protocol	= (int)IPPROTO_UDP,
 	};

[PATCH 2/6] mount: Fix compiler warning in nfs_parse_retry_option()

[Chuck Lever]

stropts.c: In function ‘nfs_parse_retry_option’:
stropts.c:131: warning: conversion to ‘unsigned int’ from ‘long int’ may alter its value

Make it more clear what the second argument is for, and flag the
switch fallthrough case.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/stropts.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index bcc36f3..29b1aaa 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -119,10 +119,12 @@ inline void nfs_default_version(struct nfsmount_info *mi) {}
  * Returns a time_t timeout timestamp, in seconds.
  */
 static time_t nfs_parse_retry_option(struct mount_options *options,
-				     unsigned int timeout_minutes)
+				     const time_t default_timeout)
 {
+	time_t timeout_minutes;
 	long tmp;
 
+	timeout_minutes = default_timeout;
 	switch (po_get_numeric(options, "retry", &tmp)) {
 	case PO_NOT_FOUND:
 		break;
@@ -131,6 +133,7 @@ static time_t nfs_parse_retry_option(struct mount_options *options,
 			timeout_minutes = tmp;
 			break;
 		}
+		/*FALLTHROUGH*/
 	case PO_BAD_VALUE:
 		if (verbose)
 			nfs_error(_("%s: invalid retry timeout was specified; "
@@ -138,7 +141,7 @@ static time_t nfs_parse_retry_option(struct mount_options *options,
 		break;
 	}
 
-	return time(NULL) + (time_t)(timeout_minutes * 60);
+	return time(NULL) + (timeout_minutes * 60);
 }
 
 /*

[PATCH 3/6] mount.nfs: Fix memory leak in nfs_sys_mount()

[Chuck Lever]

This appears to have been left behind by last year's adjustments to
how the extra_opts string is constructed.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/stropts.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index 29b1aaa..ac81616 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -568,16 +568,18 @@ static int nfs_sys_mount(struct nfsmount_info *mi, struct mount_options *opts)
 	char *options = NULL;
 	int result;
 
+	if (mi->fake)
+		return 1;
+
 	if (po_join(opts, &options) == PO_FAILED) {
 		errno = EIO;
 		return 0;
 	}
 
-	if (mi->fake)
-		return 1;
-
 	result = mount(mi->spec, mi->node, mi->type,
 			mi->flags & ~(MS_USER|MS_USERS), options);
+	free(options);
+
 	if (verbose && result) {
 		int save = errno;
 		nfs_error(_("%s: mount(2): %s"), progname, strerror(save));

[PATCH 4/6] mount.nfs: mnt_freq and mnt_pass are always zero

[Chuck Lever]

Clean up.

No need to pass constant zeros to add_mtab() from its only call site.
Ensure that initialization of a struct mntent is consistent in both
places that it is done.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/mount.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index b4da21f..a19af53 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -224,6 +224,20 @@ static char *fix_opts_string(int flags, const char *extra_opts)
 	return new_opts;
 }
 
+static void
+init_mntent(struct mntent *mnt, char *fsname, char *dir, char *type,
+		int flags, char *opts)
+{
+	mnt->mnt_fsname	= fsname;
+	mnt->mnt_dir	= dir;
+	mnt->mnt_type	= type;
+	mnt->mnt_opts	= fix_opts_string(flags & ~MS_NOMTAB, opts);
+
+	/* these are always zero for NFS */
+	mnt->mnt_freq	= 0;
+	mnt->mnt_passno	= 0;
+}
+
 /* Create mtab with a root entry.  */
 static void
 create_mtab (void) {
@@ -245,11 +259,8 @@ create_mtab (void) {
 	if ((fstab = getfsfile ("/")) || (fstab = getfsfile ("root"))) {
 		char *extra_opts;
 		parse_opts (fstab->m.mnt_opts, &flags, &extra_opts);
-		mnt.mnt_dir = "/";
-		mnt.mnt_fsname = xstrdup(fstab->m.mnt_fsname);
-		mnt.mnt_type = fstab->m.mnt_type;
-		mnt.mnt_opts = fix_opts_string (flags, extra_opts);
-		mnt.mnt_freq = mnt.mnt_passno = 0;
+		init_mntent(&mnt, xstrdup(fstab->m.mnt_fsname), "/",
+				fstab->m.mnt_type, flags, extra_opts);
 		free(extra_opts);
 
 		if (nfs_addmntent (mfp, &mnt) == 1) {
@@ -273,17 +284,12 @@ create_mtab (void) {
 }
 
 static int add_mtab(char *spec, char *mount_point, char *fstype,
-			int flags, char *opts, int freq, int pass)
+			int flags, char *opts)
 {
 	struct mntent ment;
 	int result = EX_SUCCESS;
 
-	ment.mnt_fsname = spec;
-	ment.mnt_dir = mount_point;
-	ment.mnt_type = fstype;
-	ment.mnt_opts = fix_opts_string(flags & ~MS_NOMTAB, opts);
-	ment.mnt_freq = freq;
-	ment.mnt_passno = pass;
+	init_mntent(&ment, spec, mount_point, fstype, flags, opts);
 
 	if (!nomtab && mtab_does_not_exist()) {
 		if (verbose > 1)
@@ -441,9 +447,7 @@ static int try_mount(char *spec, char *mount_point, int flags,
 	if (!fake)
 		print_one(spec, mount_point, fs_type, mount_opts);
 
-	ret = add_mtab(spec, mount_point, fs_type, flags, *extra_opts,
-			0, 0 /* these are always zero for NFS */ );
-	return ret;
+	return add_mtab(spec, mount_point, fs_type, flags, *extra_opts);
 }
 
 int main(int argc, char *argv[])

[PATCH 5/6] nfs(5): Grammar and style fixes

[Chuck Lever]

Clean up grammar and style issues introduced by recent updates.  Also,
I'm not certain inappropriate options are always ignored.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/nfs.man |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
index 01bc712..1b86768 100644
--- a/utils/mount/nfs.man
+++ b/utils/mount/nfs.man
@@ -69,10 +69,9 @@ for details on specifying raw IPv6 addresses.
 .P
 The
 .I fstype
-field contains "nfs", for whatever version of the protocol.
-The
-.B nfs
-allow several mount options, which are described below.
+field contains "nfs".  Use of the "nfs4" fstype in
+.I /etc/fstab
+is deprecated.
 .SH "MOUNT OPTIONS"
 Refer to
 .BR mount (8)
@@ -464,9 +463,9 @@ by other clients, but can impact application and server performance.
 .IP
 The DATA AND METADATA COHERENCE section contains a
 detailed discussion of these trade-offs.
-.SS "Options for versions 2 and 3 only"
+.SS "Options for NFS versions 2 and 3 only"
 Use these options, along with the options in the above subsection,
-for NFSv2/v3 only. They will be ignored for newer versions.
+for NFS versions 2 and 3 only.
 .TP 1.5i
 .BI proto= netid
 The transport protocol name and protocol family the NFS client uses
@@ -761,9 +760,9 @@ conflicting locks.
 .IP
 NOTE: When used together, the 'local_lock' mount option will be overridden
 by 'nolock'/'lock' mount option.
-.SS "Options for version 4 only"
+.SS "Options for NFS version 4 only"
 Use these options, along with the options in the first subsection above,
-for NFSv4 only. They will be ignored with older versions.
+for NFS version 4 and newer.
 .TP 1.5i
 .BI proto= netid
 The transport protocol name and protocol family the NFS client uses

[PATCH 6/6] nfs(5): Document remount behavior

[Chuck Lever]

It appears that, for a long while, NFS "remount" mounts have
completely wiped the existing mount options in /etc/mtab for a given
mount point.  This is a problem for umount.nfs, since it reads its
options out of /etc/mtab to find out how to do the unmount.

The mount(8) command provides the NFS mount subcommand with the mount
options to perform the remount.  There are four cases to consider:

  1. Both the device and mount directory are specified on the
     command line, and the target mount point is in /etc/fstab

  2. Only one of the device and mount directory is specified on
     the command line, and the target mount point is in
     /etc/fstab

  3. Both the device and mount directory are specified on the
     command line, and the target mount point is not in /etc/fstab

  4. Only one of the device and mount directory is specified on
     the command line, and the target mount point is not in
     /etc/fstab

Currently only case 4 works correctly.  In that case, mount(8)
provides the correct set of mount options to the mount.nfs
subcommand and it can update /etc/mtab correctly.

Cases 1 and 3 replace all mount options in /etc/mtab with the options
provided on the command line during a remount.  Case 2 replaces the
mount options in /etc/mtab with a mix of options from /etc/fstab and
/etc/mtab.

Cases 1 and 3 are historical behavior.  Basically this is a formal
interface to allow administrators to replace the mount options in
/etc/mtab completely, instead of merging in new ones.  The present
patch documents that behavior in nfs(5), and provides best practice
for remounting NFS mount points.

There are near-term plans to address case 2 by fixing mount(8)
(provided by utils-linux-ng in most distributions).

This is a partial fix for:

  https://bugzilla.linux-nfs.org/show_bug.cgi?id=188

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 utils/mount/nfs.man |   92 ++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 76 insertions(+), 16 deletions(-)

diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
index 1b86768..a357000 100644
--- a/utils/mount/nfs.man
+++ b/utils/mount/nfs.man
@@ -1523,32 +1523,92 @@ of Access Control Lists that are semantically richer than POSIX ACLs.
 NFS version 4 ACLs are not fully compatible with POSIX ACLs; as such,
 some translation between the two is required
 in an environment that mixes POSIX ACLs and NFS version 4.
-.SH FILES
-.TP 1.5i
-.I /etc/fstab
-file system table
-.SH BUGS
-The generic
-.B remount
-option is not fully supported.
-Generic options, such as
-.BR rw " and " ro
-can be modified using the
-.B remount
-option,
-but NFS-specific options are not all supported.
+.SH "THE REMOUNT OPTION"
+Generic mount options such as
+.BR rw " and " sync
+can be modified on NFS mount points using the
+.BR remount
+option.
+See
+.BR mount (8)
+for more information on generic mount options.
+.P
+With few exceptions, NFS-specific options
+are not able to be modified during a remount.
 The underlying transport or NFS version
 cannot be changed by a remount, for example.
+.P
 Performing a remount on an NFS file system mounted with the
 .B noac
 option may have unintended consequences.
 The
 .B noac
-option is a mixture of a generic option,
+option is a combination of the generic option
 .BR sync ,
-and an NFS-specific option
+and the NFS-specific option
 .BR actimeo=0 .
+.SS "Unmounting after a remount"
+For NFS versions 2 and 3, the NFS umount subcommand
+depends on
+.I /etc/mtab
+to contain the mount options needed to send an UMNT request
+to the server.
+The NFS mount subcommand stores these mount options in
+.I /etc/mtab
+after a successful mount operation is complete.
+A remount can alter these stored mount options, however.
 .P
+When it comes to what is written back to
+.I /etc/mtab
+during a remount, the
+.BR mount (8)
+command has two different modes of operation.
+One mode
+.I merges
+the command line mount options with the mount
+options already in
+.IR /etc/mtab ,
+and the other
+.I replaces
+the mount options in
+.I /etc/mtab
+with the command line mount options.
+.P
+To
+.I replace
+the mount options that are in
+.IR /etc/mtab ,
+specify both the server hostname and export path, and the
+local mount directory during a remount.
+To
+.I merge
+the mount options on the command line with
+the options already in
+.IR /etc/mtab ,
+specify either the local mount directory, or the server
+hostname and export pathname, but not both.  For example,
+.P
+.NF
+.TA 2.5i
+	mount -o remount,ro /mnt
+.FI
+.P
+merges the mount option
+.B ro
+with the mount options already in
+.I /etc/mtab
+for the NFS server mounted at /mnt.
+Merging is almost always the desired outcome, as it preserves the
+ability of the client to contact the server when it comes time
+to send an UMNT request.
+.SH FILES
+.TP 1.5i
+.I /etc/fstab
+file system table
+.TP 1.5i
+.I /etc/mtab
+table of mounted file systems
+.SH BUGS
 Before 2.4.7, the Linux NFS client did not support NFS over TCP.
 .P
 Before 2.4.20, the Linux NFS client used a heuristic

Re: [PATCH 6/6] nfs(5): Document remount behavior

[Steve Dickson]

Chuck,

I'm a bit concern about this patch.... 

I'm asking myself who is going care how remounts update 
/etc/mtab and what mode is used. I just thinking that type of info 
adds a lot verbiage that nobody really care about. Plus why are
we documenting something (/etc/mtab) that will be going 
away as soon as humanly possible? 
 
Basically I'm saying the entire "Unmounting after a remount" 
section is not needed. Only the 3 paragraphs in "THE REMOUNT OPTION"
section are needed IMHO... 

I'm also thinking we need to add the/etc/nfsmount.conf FILES section,
but I guess I can do that later... 

 
steved.

On 10/26/2010 11:58 AM, Chuck Lever wrote:
> It appears that, for a long while, NFS "remount" mounts have
> completely wiped the existing mount options in /etc/mtab for a given
> mount point.  This is a problem for umount.nfs, since it reads its
> options out of /etc/mtab to find out how to do the unmount.
> 
> The mount(8) command provides the NFS mount subcommand with the mount
> options to perform the remount.  There are four cases to consider:
> 
>   1. Both the device and mount directory are specified on the
>      command line, and the target mount point is in /etc/fstab
> 
>   2. Only one of the device and mount directory is specified on
>      the command line, and the target mount point is in
>      /etc/fstab
> 
>   3. Both the device and mount directory are specified on the
>      command line, and the target mount point is not in /etc/fstab
> 
>   4. Only one of the device and mount directory is specified on
>      the command line, and the target mount point is not in
>      /etc/fstab
> 
> Currently only case 4 works correctly.  In that case, mount(8)
> provides the correct set of mount options to the mount.nfs
> subcommand and it can update /etc/mtab correctly.
> 
> Cases 1 and 3 replace all mount options in /etc/mtab with the options
> provided on the command line during a remount.  Case 2 replaces the
> mount options in /etc/mtab with a mix of options from /etc/fstab and
> /etc/mtab.
> 
> Cases 1 and 3 are historical behavior.  Basically this is a formal
> interface to allow administrators to replace the mount options in
> /etc/mtab completely, instead of merging in new ones.  The present
> patch documents that behavior in nfs(5), and provides best practice
> for remounting NFS mount points.
> 
> There are near-term plans to address case 2 by fixing mount(8)
> (provided by utils-linux-ng in most distributions).
> 
> This is a partial fix for:
> 
>   https://bugzilla.linux-nfs.org/show_bug.cgi?id=188
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  utils/mount/nfs.man |   92 ++++++++++++++++++++++++++++++++++++++++++---------
>  1 files changed, 76 insertions(+), 16 deletions(-)
> 
> diff --git a/utils/mount/nfs.man b/utils/mount/nfs.man
> index 1b86768..a357000 100644
> --- a/utils/mount/nfs.man
> +++ b/utils/mount/nfs.man
> @@ -1523,32 +1523,92 @@ of Access Control Lists that are semantically richer than POSIX ACLs.
>  NFS version 4 ACLs are not fully compatible with POSIX ACLs; as such,
>  some translation between the two is required
>  in an environment that mixes POSIX ACLs and NFS version 4.
> -.SH FILES
> -.TP 1.5i
> -.I /etc/fstab
> -file system table
> -.SH BUGS
> -The generic
> -.B remount
> -option is not fully supported.
> -Generic options, such as
> -.BR rw " and " ro
> -can be modified using the
> -.B remount
> -option,
> -but NFS-specific options are not all supported.
> +.SH "THE REMOUNT OPTION"
> +Generic mount options such as
> +.BR rw " and " sync
> +can be modified on NFS mount points using the
> +.BR remount
> +option.
> +See
> +.BR mount (8)
> +for more information on generic mount options.
> +.P
> +With few exceptions, NFS-specific options
> +are not able to be modified during a remount.
>  The underlying transport or NFS version
>  cannot be changed by a remount, for example.
> +.P
>  Performing a remount on an NFS file system mounted with the
>  .B noac
>  option may have unintended consequences.
>  The
>  .B noac
> -option is a mixture of a generic option,
> +option is a combination of the generic option
>  .BR sync ,
> -and an NFS-specific option
> +and the NFS-specific option
>  .BR actimeo=0 .
> +.SS "Unmounting after a remount"
> +For NFS versions 2 and 3, the NFS umount subcommand
> +depends on
> +.I /etc/mtab
> +to contain the mount options needed to send an UMNT request
> +to the server.
> +The NFS mount subcommand stores these mount options in
> +.I /etc/mtab
> +after a successful mount operation is complete.
> +A remount can alter these stored mount options, however.
>  .P
> +When it comes to what is written back to
> +.I /etc/mtab
> +during a remount, the
> +.BR mount (8)
> +command has two different modes of operation.
> +One mode
> +.I merges
> +the command line mount options with the mount
> +options already in
> +.IR /etc/mtab ,
> +and the other
> +.I replaces
> +the mount options in
> +.I /etc/mtab
> +with the command line mount options.
> +.P
> +To
> +.I replace
> +the mount options that are in
> +.IR /etc/mtab ,
> +specify both the server hostname and export path, and the
> +local mount directory during a remount.
> +To
> +.I merge
> +the mount options on the command line with
> +the options already in
> +.IR /etc/mtab ,
> +specify either the local mount directory, or the server
> +hostname and export pathname, but not both.  For example,
> +.P
> +.NF
> +.TA 2.5i
> +	mount -o remount,ro /mnt
> +.FI
> +.P
> +merges the mount option
> +.B ro
> +with the mount options already in
> +.I /etc/mtab
> +for the NFS server mounted at /mnt.
> +Merging is almost always the desired outcome, as it preserves the
> +ability of the client to contact the server when it comes time
> +to send an UMNT request.
> +.SH FILES
> +.TP 1.5i
> +.I /etc/fstab
> +file system table
> +.TP 1.5i
> +.I /etc/mtab
> +table of mounted file systems
> +.SH BUGS
>  Before 2.4.7, the Linux NFS client did not support NFS over TCP.
>  .P
>  Before 2.4.20, the Linux NFS client used a heuristic
> 

Re: [PATCH 6/6] nfs(5): Document remount behavior

[Chuck Lever]

On Oct 28, 2010, at 2:02 PM, Steve Dickson wrote:

> Chuck,
> 
> I'm a bit concern about this patch.... 
> 
> I'm asking myself who is going care how remounts update 
> /etc/mtab and what mode is used.

Someone who has done a remount and found that it doesn't behave as they expect.  I've handled bug reports like this.  When customers want to see existing documentation of strange behavior, it's pretty important to have something to point them to.  Documentation is the first thing I'm asked for by customers and system engineers who encounter weird NFS behavior.

> I just thinking that type of info 
> adds a lot verbiage that nobody really care about.

Don't you think that's a bit harsh and even disrespectful?  What criteria do you use to decide that "no-one cares"?

This patch documents very confusing behavior that is not documented anywhere else.  It's not documented elsewhere because other file systems don't have a dependency on /etc/mtab as we have.  This also documents why replacing /etc/mtab with /proc/mounts will cause some fuzzy NFS umount behavior (which is exactly the documentation you were looking for last week).

The fact that it took me so long to figure out is evidence enough that this is not obvious and needs to be written down somewhere.  Where else should we document behavior that is related to /etc/fstab and mount(8), and is NFS specific?

> Plus why are
> we documenting something (/etc/mtab) that will be going 
> away as soon as humanly possible? 

Do you have a schedule for this?  I've talked very recently with Karel, and libmount is not even ready to be published.  The work to integrate libmount into every mount subcommand sounds like it could be a ways in the future.  (Karel and I even discussed me doing this work for mount.nfs).

I expect we will have a dependence on /etc/mtab for a while yet.  And, it's pretty easy to change these docs once we've transitioned.

> Basically I'm saying the entire "Unmounting after a remount" 
> section is not needed. Only the 3 paragraphs in "THE REMOUNT OPTION"
> section are needed IMHO... 


I'll consider recrafting it, but this is important and confusing behavior that must be documented.  We can argue about how the information is presented, but just throwing it all out and ignoring it is not an option.

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 6/6] nfs(5): Document remount behavior

[Steve Dickson]

On 10/28/2010 02:58 PM, Chuck Lever wrote:
> 
> On Oct 28, 2010, at 2:02 PM, Steve Dickson wrote:
> 
>> Chuck,
>>
>> I'm a bit concern about this patch.... 
>>
>> I'm asking myself who is going care how remounts update 
>> /etc/mtab and what mode is used.
> 
> Someone who has done a remount and found that it doesn't behave as they expect. 
> I've handled bug reports like this.  When customers want to see existing 
> documentation of strange behavior, it's pretty important to have something to
> point them to.  Documentation is the first thing I'm asked for by customers 
> and system engineers who encounter weird NFS behavior.
Documentation is great! I too get numerous requests for documentation.
That's why I think are the first 3 paragraphs in "THE REMOUNT OPTION"
is perfect! Well written and to the point... Its the rest of 
the section I don't think we needed... that's all... 
 
> 
>> I just thinking that type of info 
>> adds a lot verbiage that nobody really care about.
> 
> Don't you think that's a bit harsh and even disrespectful? 
By no means... whatsoever... I was just being honest...
maybe a bit too brutally... ;-) My apologies if it came 
off in that manner...  

> What criteria do you use to decide that "no-one cares"?
I can't image anybody caring about how the inners of /etc/mtab
are being maintained on an NFS umount... As long as the unmount
works, that's all they care about... 
> 
> This patch documents very confusing behavior that is not documented anywhere else.
> It's not documented elsewhere because other file systems don't have a dependency 
> on /etc/mtab as we have.  This also documents why replacing /etc/mtab with 
> /proc/mounts will cause some fuzzy NFS umount behavior (which is exactly the documentation you were looking for last week).
> 
> The fact that it took me so long to figure out is evidence enough that this is 
> not obvious and needs to be written down somewhere.  Where else should we document
> behavior that is related to /etc/fstab and mount(8), and is NFS specific?
How about the NFS FAQ? 

All I'm saying is that type of details information (how mtab is managed) is
not useful to a system admin trying figure out how to use remount 
on an NFS file system... So I think its a waste to put in the man page... 

> 
>> Plus why are
>> we documenting something (/etc/mtab) that will be going 
>> away as soon as humanly possible? 
> 
> Do you have a schedule for this?  I've talked very recently with Karel, 
> and libmount is not even ready to be published.  The work to integrate 
> libmount into every mount subcommand sounds like it could be a ways in 
> the future.  (Karel and I even discussed me doing this work for mount.nfs).
No I don't... but it can come soon enough... IMHO... 
Cool... Karel is a good guy to work with... 

> 
> I expect we will have a dependence on /etc/mtab for a while yet.  And, 
> it's pretty easy to change these docs once we've transitioned.
> 
>> Basically I'm saying the entire "Unmounting after a remount" 
>> section is not needed. Only the 3 paragraphs in "THE REMOUNT OPTION"
>> section are needed IMHO... 
> 
> 
> I'll consider recrafting it, but this is important and confusing 
> behavior that must be documented.  We can argue about how the information 
> is presented, but just throwing it all out and ignoring it is not an option.
> 
Good...

All I'm saying make the it concise and to the point so its useful
to a system admin... Like the first 3 paragraphs are... 

And maybe I'm wrong...  but... I just don't think an system admin 
really cares what happens to the mtab after a umount of a 
remounted mount point... As long as the unmount works... they are
happy... again... IMHO... 

steved.

Re: [PATCH 0/6] More fix-ups for nfs-utils-1.2.4

[Steve Dickson]

Hey Chuck, 

I'm assuming this 6 patch series should replace (not merge with) 
the 4 patch series you sent out earlier... 

steved.

On 10/26/2010 11:57 AM, Chuck Lever wrote:
> Hi Steve-
> 
> Here are rather more final versions of the patches I posted here last
> week for review.  There are a couple of extra clean-ups here as well.
> I think these are ready to be considered for nfs-utils 1.2.4.
> 
> ---
> 
> Chuck Lever (6):
>       nfs(5): Document remount behavior
>       nfs(5): Grammar and style fixes
>       mount.nfs: mnt_freq and mnt_pass are always zero
>       mount.nfs: Fix memory leak in nfs_sys_mount()
>       mount: Fix compiler warning in nfs_parse_retry_option()
>       nfs-utils: Remove all uses of AI_ADDRCONFIG
> 
> 
>  support/export/hostname.c     |    6 --
>  tests/nsm_client/nsm_client.c |    2 -
>  utils/mount/mount.c           |   34 +++++++------
>  utils/mount/network.c         |    3 -
>  utils/mount/nfs.man           |  107 ++++++++++++++++++++++++++++++++---------
>  utils/mount/stropts.c         |   20 ++++----
>  utils/statd/hostname.c        |    4 --
>  utils/statd/sm-notify.c       |    5 --
>  8 files changed, 114 insertions(+), 67 deletions(-)
> 

Re: [PATCH 0/6] More fix-ups for nfs-utils-1.2.4

[Chuck Lever]

On Oct 28, 2010, at 11:32 AM, Steve Dickson wrote:

> Hey Chuck, 
> 
> I'm assuming this 6 patch series should replace (not merge with) 
> the 4 patch series you sent out earlier... 

Yes, it replaces them.  The 4 patch series was labelled "for review only," so I never intended that series to be merged.

> steved.
> 
> On 10/26/2010 11:57 AM, Chuck Lever wrote:
>> Hi Steve-
>> 
>> Here are rather more final versions of the patches I posted here last
>> week for review.  There are a couple of extra clean-ups here as well.
>> I think these are ready to be considered for nfs-utils 1.2.4.
>> 
>> ---
>> 
>> Chuck Lever (6):
>>      nfs(5): Document remount behavior
>>      nfs(5): Grammar and style fixes
>>      mount.nfs: mnt_freq and mnt_pass are always zero
>>      mount.nfs: Fix memory leak in nfs_sys_mount()
>>      mount: Fix compiler warning in nfs_parse_retry_option()
>>      nfs-utils: Remove all uses of AI_ADDRCONFIG
>> 
>> 
>> support/export/hostname.c     |    6 --
>> tests/nsm_client/nsm_client.c |    2 -
>> utils/mount/mount.c           |   34 +++++++------
>> utils/mount/network.c         |    3 -
>> utils/mount/nfs.man           |  107 ++++++++++++++++++++++++++++++++---------
>> utils/mount/stropts.c         |   20 ++++----
>> utils/statd/hostname.c        |    4 --
>> utils/statd/sm-notify.c       |    5 --
>> 8 files changed, 114 insertions(+), 67 deletions(-)
>> 

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

Re: [PATCH 0/6] More fix-ups for nfs-utils-1.2.4

[Steve Dickson]

On 10/26/2010 11:57 AM, Chuck Lever wrote:
> Hi Steve-
> 
> Here are rather more final versions of the patches I posted here last
> week for review.  There are a couple of extra clean-ups here as well.
> I think these are ready to be considered for nfs-utils 1.2.4.
> 
> ---
> 
> Chuck Lever (6):
>       nfs(5): Document remount behavior
>       nfs(5): Grammar and style fixes
>       mount.nfs: mnt_freq and mnt_pass are always zero
>       mount.nfs: Fix memory leak in nfs_sys_mount()
>       mount: Fix compiler warning in nfs_parse_retry_option()
>       nfs-utils: Remove all uses of AI_ADDRCONFIG
The last five were committed..

steved.
> 
> 
>  support/export/hostname.c     |    6 --
>  tests/nsm_client/nsm_client.c |    2 -
>  utils/mount/mount.c           |   34 +++++++------
>  utils/mount/network.c         |    3 -
>  utils/mount/nfs.man           |  107 ++++++++++++++++++++++++++++++++---------
>  utils/mount/stropts.c         |   20 ++++----
>  utils/statd/hostname.c        |    4 --
>  utils/statd/sm-notify.c       |    5 --
>  8 files changed, 114 insertions(+), 67 deletions(-)
>