mount.nfs libmount support (v2)

mount.nfs libmount support (v2)

[Karel Zak]

This is the second version of the mount.nfs with libmount support. The code
depends on util-linux >= v2.19 (e.g. Fedora 15).

Chnages:
  - move nfs_umount_do_umnt() to network.c
  - use union nfs_sockaddr
  - add missing GPL address paragraph
  - use abstract function the storing and retrieval mount options
  - use "static const" for struct option
  - use "strncmp() == 0" everywhere


The locale issue is not fixed. It seems that _() macro from nls.h is used on
many places in the package, but ENABLE_NLS macro is nowhere defined and 
setlocale() is not called at all.

I believe that this issue should be fixed independently on the libmount
patches.

[PATCH 1/2] mount: move generic functions to utils.c and network.c

[Karel Zak]

Move generic code that could be shared between standard mount.nfs and
libmount version to utils.c and network.c.

CC: Chuck Lever <chuck.lever@oracle.com>
CC: Steve Dickson <steved@redhat.com>
Signed-off-by: Karel Zak <kzak@redhat.com>
---
 utils/mount/Makefile.am |    2 +-
 utils/mount/mount.c     |   83 +----------------------
 utils/mount/network.c   |   68 ++++++++++++++++++
 utils/mount/network.h   |    3 +
 utils/mount/nfsumount.c |  119 +-------------------------------
 utils/mount/utils.c     |  175 +++++++++++++++++++++++++++++++++++++++++++++++
 utils/mount/utils.h     |   36 ++++++++++
 7 files changed, 286 insertions(+), 200 deletions(-)
 create mode 100644 utils/mount/utils.c
 create mode 100644 utils/mount/utils.h

diff --git a/utils/mount/Makefile.am b/utils/mount/Makefile.am
index 299384a..a9553dc 100644
--- a/utils/mount/Makefile.am
+++ b/utils/mount/Makefile.am
@@ -16,7 +16,7 @@ mount_nfs_SOURCES = mount.c error.c network.c fstab.c token.c \
 		    mount_constants.h error.h network.h fstab.h token.h \
 		    parse_opt.h parse_dev.h \
 		    nfs4_mount.h nfs_mount4.h stropts.h version.h \
-			mount_config.h
+		    mount_config.h utils.c utils.h
 
 if MOUNT_CONFIG
 mount_nfs_SOURCES += configfile.c
diff --git a/utils/mount/mount.c b/utils/mount/mount.c
index a19af53..f3f0a83 100644
--- a/utils/mount/mount.c
+++ b/utils/mount/mount.c
@@ -47,7 +47,7 @@
 #include "mount.h"
 #include "error.h"
 #include "stropts.h"
-#include "version.h"
+#include "utils.h"
 
 char *progname;
 int nfs_mount_data_version;
@@ -150,49 +150,6 @@ static const struct opt_map opt_map[] = {
 static void parse_opts(const char *options, int *flags, char **extra_opts);
 
 /*
- * Choose the version of the nfs_mount_data structure that is appropriate
- * for the kernel that is doing the mount.
- *
- * NFS_MOUNT_VERSION:		maximum version supported by these sources
- * nfs_mount_data_version:	maximum version supported by the running kernel
- */
-static void discover_nfs_mount_data_version(void)
-{
-	unsigned int kernel_version = linux_version_code();
-
-	if (kernel_version) {
-		if (kernel_version < MAKE_VERSION(2, 1, 32))
-			nfs_mount_data_version = 1;
-		else if (kernel_version < MAKE_VERSION(2, 2, 18))
-			nfs_mount_data_version = 3;
-		else if (kernel_version < MAKE_VERSION(2, 3, 0))
-			nfs_mount_data_version = 4;
-		else if (kernel_version < MAKE_VERSION(2, 3, 99))
-			nfs_mount_data_version = 3;
-		else if (kernel_version < MAKE_VERSION(2, 6, 3))
-			nfs_mount_data_version = 4;
-		else
-			nfs_mount_data_version = 6;
-	}
-	if (nfs_mount_data_version > NFS_MOUNT_VERSION)
-		nfs_mount_data_version = NFS_MOUNT_VERSION;
-	else
-		if (kernel_version > MAKE_VERSION(2, 6, 22))
-			string++;
-}
-
-static void print_one(char *spec, char *node, char *type, char *opts)
-{
-	if (!verbose)
-		return;
-
-	if (opts)
-		printf(_("%s on %s type %s (%s)\n"), spec, node, type, opts);
-	else
-		printf(_("%s on %s type %s\n"), spec, node, type);
-}
-
-/*
  * Build a canonical mount option string for /etc/mtab.
  */
 static char *fix_opts_string(int flags, const char *extra_opts)
@@ -327,22 +284,6 @@ static int add_mtab(char *spec, char *mount_point, char *fstype,
 	return result;
 }
 
-static void mount_usage(void)
-{
-	printf(_("usage: %s remotetarget dir [-rvVwfnsih] [-o nfsoptions]\n"),
-		progname);
-	printf(_("options:\n"));
-	printf(_("\t-r\t\tMount file system readonly\n"));
-	printf(_("\t-v\t\tVerbose\n"));
-	printf(_("\t-V\t\tPrint version\n"));
-	printf(_("\t-w\t\tMount file system read-write\n"));
-	printf(_("\t-f\t\tFake mount, do not actually mount\n"));
-	printf(_("\t-n\t\tDo not update /etc/mtab\n"));
-	printf(_("\t-s\t\tTolerate sloppy mount options rather than fail\n"));
-	printf(_("\t-h\t\tPrint this help\n"));
-	printf(_("\tnfsoptions\tRefer to mount.nfs(8) or nfs(5)\n\n"));
-}
-
 static void parse_opt(const char *opt, int *mask, char *extra_opts, size_t len)
 {
 	const struct opt_map *om;
@@ -403,26 +344,6 @@ static void parse_opts(const char *options, int *flags, char **extra_opts)
 	}
 }
 
-static int chk_mountpoint(char *mount_point)
-{
-	struct stat sb;
-
-	if (stat(mount_point, &sb) < 0){
-		mount_error(NULL, mount_point, errno);
-		return 1;
-	}
-	if (S_ISDIR(sb.st_mode) == 0){
-		mount_error(NULL, mount_point, ENOTDIR);
-		return 1;
-	}
-	if (access(mount_point, X_OK) < 0) {
-		mount_error(NULL, mount_point, errno);
-		return 1;
-	}
-
-	return 0;
-}
-
 static int try_mount(char *spec, char *mount_point, int flags,
 			char *fs_type, char **extra_opts, char *mount_opts,
 			int fake, int bg)
@@ -459,7 +380,7 @@ int main(int argc, char *argv[])
 
 	progname = basename(argv[0]);
 
-	discover_nfs_mount_data_version();
+	nfs_mount_data_version = discover_nfs_mount_data_version(&string);
 
 	if(!strncmp(progname, "umount", strlen("umount")))
 		exit(nfsumount(argc, argv));
diff --git a/utils/mount/network.c b/utils/mount/network.c
index 21a7a2c..8049c1a 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1626,3 +1626,71 @@ int nfs_options2pmap(struct mount_options *options,
 
 	return 1;
 }
+
+/*
+ * Discover mount server's hostname/address by examining mount options
+ *
+ * Returns a pointer to a string that the caller must free, on
+ * success; otherwise NULL is returned.
+ */
+static char *nfs_umount_hostname(struct mount_options *options,
+				 char *hostname)
+{
+	char *option;
+
+	option = po_get(options, "mountaddr");
+	if (option)
+		goto out;
+	option = po_get(options, "mounthost");
+	if (option)
+		goto out;
+	option = po_get(options, "addr");
+	if (option)
+		goto out;
+
+	return hostname;
+
+out:
+	free(hostname);
+	return strdup(option);
+}
+
+
+/*
+ * Returns EX_SUCCESS if mount options and device name have been
+ * parsed successfully; otherwise EX_FAIL.
+ */
+int nfs_umount_do_umnt(struct mount_options *options,
+		       char **hostname, char **dirname)
+{
+	union nfs_sockaddr address;
+	struct sockaddr *sap = &address.sa;
+	socklen_t salen = sizeof(address);
+	struct pmap nfs_pmap, mnt_pmap;
+	sa_family_t family;
+
+	if (!nfs_options2pmap(options, &nfs_pmap, &mnt_pmap))
+		return EX_FAIL;
+
+	/* Skip UMNT call for vers=4 mounts */
+	if (nfs_pmap.pm_vers == 4)
+		return EX_SUCCESS;
+
+	*hostname = nfs_umount_hostname(options, *hostname);
+	if (!*hostname) {
+		nfs_error(_("%s: out of memory"), progname);
+		return EX_FAIL;
+	}
+
+	if (!nfs_mount_proto_family(options, &family))
+		return 0;
+	if (!nfs_lookup(*hostname, family, sap, &salen))
+		/* nfs_lookup reports any errors */
+		return EX_FAIL;
+
+	if (nfs_advise_umount(sap, salen, &mnt_pmap, dirname) == 0)
+		/* nfs_advise_umount reports any errors */
+		return EX_FAIL;
+
+	return EX_SUCCESS;
+}
diff --git a/utils/mount/network.h b/utils/mount/network.h
index 2a3a110..81c6f22 100644
--- a/utils/mount/network.h
+++ b/utils/mount/network.h
@@ -75,4 +75,7 @@ int nfs_advise_umount(const struct sockaddr *, const socklen_t,
 CLIENT *mnt_openclnt(clnt_addr_t *, int *);
 void mnt_closeclnt(CLIENT *, int);
 
+int nfs_umount_do_umnt(struct mount_options *options,
+		       char **hostname, char **dirname);
+
 #endif	/* _NFS_UTILS_MOUNT_NETWORK_H */
diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
index 02d40ff..8cd2852 100644
--- a/utils/mount/nfsumount.c
+++ b/utils/mount/nfsumount.c
@@ -37,6 +37,7 @@
 #include "network.h"
 #include "parse_opt.h"
 #include "parse_dev.h"
+#include "utils.h"
 
 #define MOUNTSFILE	"/proc/mounts"
 #define LINELEN		(4096)
@@ -139,113 +140,6 @@ static int del_mtab(const char *spec, const char *node)
 }
 
 /*
- * Discover mount server's hostname/address by examining mount options
- *
- * Returns a pointer to a string that the caller must free, on
- * success; otherwise NULL is returned.
- */
-static char *nfs_umount_hostname(struct mount_options *options,
-				 char *hostname)
-{
-	char *option;
-
-	option = po_get(options, "mountaddr");
-	if (option)
-		goto out;
-	option = po_get(options, "mounthost");
-	if (option)
-		goto out;
-	option = po_get(options, "addr");
-	if (option)
-		goto out;
-
-	return hostname;
-
-out:
-	free(hostname);
-	return strdup(option);
-}
-
-/*
- * Returns EX_SUCCESS if mount options and device name have been
- * parsed successfully; otherwise EX_FAIL.
- */
-static int nfs_umount_do_umnt(struct mount_options *options,
-			      char **hostname, char **dirname)
-{
-	union {
-		struct sockaddr		sa;
-		struct sockaddr_in	s4;
-		struct sockaddr_in6	s6;
-	} address;
-	struct sockaddr *sap = &address.sa;
-	socklen_t salen = sizeof(address);
-	struct pmap nfs_pmap, mnt_pmap;
-	sa_family_t family;
-
-	if (!nfs_options2pmap(options, &nfs_pmap, &mnt_pmap))
-		return EX_FAIL;
-
-	/* Skip UMNT call for vers=4 mounts */
-	if (nfs_pmap.pm_vers == 4)
-		return EX_SUCCESS;
-
-	*hostname = nfs_umount_hostname(options, *hostname);
-	if (!*hostname) {
-		nfs_error(_("%s: out of memory"), progname);
-		return EX_FAIL;
-	}
-
-	if (!nfs_mount_proto_family(options, &family))
-		return 0;
-	if (!nfs_lookup(*hostname, family, sap, &salen))
-		/* nfs_lookup reports any errors */
-		return EX_FAIL;
-
-	if (nfs_advise_umount(sap, salen, &mnt_pmap, dirname) == 0)
-		/* nfs_advise_umount reports any errors */
-		return EX_FAIL;
-
-	return EX_SUCCESS;
-}
-
-/*
- * Pick up certain mount options used during the original mount
- * from /etc/mtab.  The basics include the server's IP address and
- * the server pathname of the share to unregister.
- *
- * These options might also describe the mount port, mount protocol
- * version, and transport protocol used to punch through a firewall.
- * We will need this information to get through the firewall again
- * to do the umount.
- *
- * Note that option parsing failures won't necessarily cause the
- * umount request to fail.  Those values will be left zero in the
- * pmap tuple.  If the GETPORT call later fails to disambiguate them,
- * then we fail.
- */
-static int nfs_umount23(const char *devname, char *string)
-{
-	char *hostname, *dirname;
-	struct mount_options *options;
-	int result = EX_FAIL;
-
-	if (!nfs_parse_devname(devname, &hostname, &dirname))
-		return EX_USAGE;
-
-	options = po_split(string);
-	if (options) {
-		result = nfs_umount_do_umnt(options, &hostname, &dirname);
-		po_destroy(options);
-	} else
-		nfs_error(_("%s: option parsing error"), progname);
-
-	free(hostname);
-	free(dirname);
-	return result;
-}
-
-/*
  * Detect NFSv4 mounts.
  *
  * Consult /proc/mounts to determine if the mount point
@@ -340,17 +234,6 @@ static struct option umount_longopts[] =
   { NULL, 0, 0, 0 }
 };
 
-static void umount_usage(void)
-{
-	printf(_("usage: %s dir [-fvnrlh]\n"), progname);
-	printf(_("options:\n\t-f\t\tforce unmount\n"));
-	printf(_("\t-v\tverbose\n"));
-	printf(_("\t-n\tDo not update /etc/mtab\n"));
-	printf(_("\t-r\tremount\n"));
-	printf(_("\t-l\tlazy unmount\n"));
-	printf(_("\t-h\tprint this help\n\n"));
-}
-
 int nfsumount(int argc, char *argv[])
 {
 	int c, ret;
diff --git a/utils/mount/utils.c b/utils/mount/utils.c
new file mode 100644
index 0000000..298db39
--- /dev/null
+++ b/utils/mount/utils.c
@@ -0,0 +1,175 @@
+/*
+ * Copyright (C) 2010 Karel Zak <kzak@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include "sockaddr.h"
+#include "nfs_mount.h"
+#include "nls.h"
+#include "xcommon.h"
+#include "version.h"
+#include "error.h"
+#include "utils.h"
+#include "mount.h"
+#include "network.h"
+#include "parse_dev.h"
+
+extern int verbose;
+extern char *progname;
+
+/*
+ * Choose the version of the nfs_mount_data structure that is appropriate
+ * for the kernel that is doing the mount.
+ *
+ * NFS_MOUNT_VERSION:		maximum version supported by these sources
+ * nfs_mount_data_version:	maximum version supported by the running kernel
+ */
+int discover_nfs_mount_data_version(int *string_ver)
+{
+	unsigned int kernel_version = linux_version_code();
+	int ver = 0;
+
+	*string_ver = 0;
+
+	if (kernel_version) {
+		if (kernel_version < MAKE_VERSION(2, 1, 32))
+			ver = 1;
+		else if (kernel_version < MAKE_VERSION(2, 2, 18))
+			ver = 3;
+		else if (kernel_version < MAKE_VERSION(2, 3, 0))
+			ver = 4;
+		else if (kernel_version < MAKE_VERSION(2, 3, 99))
+			ver = 3;
+		else if (kernel_version < MAKE_VERSION(2, 6, 3))
+			ver = 4;
+		else
+			ver = 6;
+	}
+	if (ver > NFS_MOUNT_VERSION)
+		ver = NFS_MOUNT_VERSION;
+	else
+		if (kernel_version > MAKE_VERSION(2, 6, 22))
+			(*string_ver)++;
+
+	return ver;
+}
+
+void print_one(char *spec, char *node, char *type, char *opts)
+{
+	if (!verbose)
+		return;
+
+	if (opts)
+		printf(_("%s on %s type %s (%s)\n"), spec, node, type, opts);
+	else
+		printf(_("%s on %s type %s\n"), spec, node, type);
+}
+
+void mount_usage(void)
+{
+	printf(_("usage: %s remotetarget dir [-rvVwfnsih] [-o nfsoptions]\n"),
+		progname);
+	printf(_("options:\n"));
+	printf(_("\t-r\t\tMount file system readonly\n"));
+	printf(_("\t-v\t\tVerbose\n"));
+	printf(_("\t-V\t\tPrint version\n"));
+	printf(_("\t-w\t\tMount file system read-write\n"));
+	printf(_("\t-f\t\tFake mount, do not actually mount\n"));
+	printf(_("\t-n\t\tDo not update /etc/mtab\n"));
+	printf(_("\t-s\t\tTolerate sloppy mount options rather than fail\n"));
+	printf(_("\t-h\t\tPrint this help\n"));
+	printf(_("\tnfsoptions\tRefer to mount.nfs(8) or nfs(5)\n\n"));
+}
+
+void umount_usage(void)
+{
+	printf(_("usage: %s dir [-fvnrlh]\n"), progname);
+	printf(_("options:\n\t-f\t\tforce unmount\n"));
+	printf(_("\t-v\tverbose\n"));
+	printf(_("\t-n\tDo not update /etc/mtab\n"));
+	printf(_("\t-r\tremount\n"));
+	printf(_("\t-l\tlazy unmount\n"));
+	printf(_("\t-h\tprint this help\n\n"));
+}
+
+int chk_mountpoint(const char *mount_point)
+{
+	struct stat sb;
+
+	if (stat(mount_point, &sb) < 0){
+		mount_error(NULL, mount_point, errno);
+		return 1;
+	}
+	if (S_ISDIR(sb.st_mode) == 0){
+		mount_error(NULL, mount_point, ENOTDIR);
+		return 1;
+	}
+	if (access(mount_point, X_OK) < 0) {
+		mount_error(NULL, mount_point, errno);
+		return 1;
+	}
+
+	return 0;
+}
+
+/*
+ * Pick up certain mount options used during the original mount
+ * from /etc/mtab.  The basics include the server's IP address and
+ * the server pathname of the share to unregister.
+ *
+ * These options might also describe the mount port, mount protocol
+ * version, and transport protocol used to punch through a firewall.
+ * We will need this information to get through the firewall again
+ * to do the umount.
+ *
+ * Note that option parsing failures won't necessarily cause the
+ * umount request to fail.  Those values will be left zero in the
+ * pmap tuple.  If the GETPORT call later fails to disambiguate them,
+ * then we fail.
+ */
+int nfs_umount23(const char *devname, char *string)
+{
+	char *hostname = NULL, *dirname = NULL;
+	struct mount_options *options;
+	int result = EX_FAIL;
+
+	if (!nfs_parse_devname(devname, &hostname, &dirname))
+		return EX_USAGE;
+
+	options = po_split(string);
+	if (options) {
+		result = nfs_umount_do_umnt(options, &hostname, &dirname);
+		po_destroy(options);
+	} else
+		nfs_error(_("%s: option parsing error"), progname);
+
+	free(hostname);
+	free(dirname);
+	return result;
+}
diff --git a/utils/mount/utils.h b/utils/mount/utils.h
new file mode 100644
index 0000000..3fcd504
--- /dev/null
+++ b/utils/mount/utils.h
@@ -0,0 +1,36 @@
+/*
+ * utils.h -- misc utils for mount and umount
+ *
+ * Copyright (C) 2010 Karel Zak <kzak@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ *
+ */
+
+#ifndef _NFS_UTILS_MOUNT_UTILS_H
+#define _NFS_UTILS_MOUNT_UTILS_H
+
+#include "parse_opt.h"
+
+int discover_nfs_mount_data_version(int *string_ver);
+void print_one(char *spec, char *node, char *type, char *opts);
+void mount_usage(void);
+void umount_usage(void);
+int chk_mountpoint(const char *mount_point);
+
+int nfs_umount23(const char *devname, char *string);
+
+#endif	/* !_NFS_UTILS_MOUNT_UTILS_H */
-- 
1.7.3.4

[PATCH 2/2] mount: add --enable-libmount-mount

[Karel Zak]

This patch allows to link mount.nfs with libmount from util-linux >=
v2.19. The new libmount based code is enabled by CONFIG_LIBMOUNT and
is stored in mount_libmount.c. The old code is not affected by this
change.

The libmount does not have officially stable API yet, so the
--enable-libmount-mount is marked as experimental in the configure
help output.

The ./configure option is the same as we use in util-linux to enable
support for libmount in mount(8).

The addr= (and some other options necessary for remount/umount) are
stored to /etc/mtab or to /dev/.mount/utab. The utab file is *private*
libmount file. It's possible that some mount options (for example
user=) will be moved to kernel, so the utab will not be necessary.

About libmount:

  * supports systems without and with regular /etc/mtab
  * does not store VFS and FS mount options in userspace
  * manages user= option and evaluate permissions
  * parses VFS mount options and generate MS_* flags
  * parses /etc/{fstab,mtab}, /proc/mounts or /proc/self/mountinfo
  * long-term goal is to use the same code in all mount.<type> helpers

Note, use

   LIBMOUNT_DEBUG=0xffff mount.nfs foo:/path /path

to debug the library.

On systems with util-linux v2.19 the findmnt(8) command uses libmount
to list all/selected mount points:

   $ findmnt /path
   $ findmnt --mtab /path

the --mtab appends userspace mount options (e.g. user=) to the output.

CC: Chuck Lever <chuck.lever@oracle.com>
CC: Steve Dickson <steved@redhat.com>
Signed-off-by: Karel Zak <kzak@redhat.com>
---
 configure.ac                 |   16 ++
 utils/mount/Makefile.am      |   13 ++-
 utils/mount/mount_libmount.c |  411 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 438 insertions(+), 2 deletions(-)
 create mode 100644 utils/mount/mount_libmount.c

diff --git a/configure.ac b/configure.ac
index 2e074d9..9c384d8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,6 +132,15 @@ AC_ARG_ENABLE(mount,
 	enable_mount=$enableval,
 	enable_mount=yes)
 	AM_CONDITIONAL(CONFIG_MOUNT, [test "$enable_mount" = "yes"])
+
+if test "$enable_mount" = yes; then
+	AC_ARG_ENABLE(libmount-mount,
+		[AC_HELP_STRING([--enable-libmount-mount],
+				[Link mount.nfs with libmount (EXPERIMENTAL)])],
+		enable_libmount=yes,
+		enable_libmount=no)
+fi
+
 AC_ARG_ENABLE(tirpc,
 	[AC_HELP_STRING([--enable-tirpc],
 			[enable use of TI-RPC @<:@default=yes@:>@])],
@@ -285,6 +294,13 @@ AC_SUBST(LIBCRYPT)
 AC_SUBST(LIBBSD)
 AC_SUBST(LIBBLKID)
 
+if test "$enable_libmount" != no; then
+   AC_CHECK_LIB(mount, mnt_context_do_mount, [LIBMOUNT="-lmount"], AC_MSG_ERROR([libmount needed]))
+   AC_CHECK_HEADER(libmount/libmount.h, , AC_MSG_ERROR([Cannot find libmount header file libmount/libmount.h]))
+fi
+AM_CONDITIONAL(CONFIG_LIBMOUNT, [test "$enable_libmount" = "yes"])
+AC_SUBST(LIBMOUNT)
+
 if test "$enable_gss" = yes; then
   dnl 'gss' requires getnameinfo - at least for gssd_proc.c
   AC_CHECK_FUNC([getnameinfo], , [AC_MSG_ERROR([GSSAPI support requires 'getnameinfo' function])])
diff --git a/utils/mount/Makefile.am b/utils/mount/Makefile.am
index a9553dc..056293c 100644
--- a/utils/mount/Makefile.am
+++ b/utils/mount/Makefile.am
@@ -9,7 +9,7 @@ man5_MANS	= nfs.man
 
 sbin_PROGRAMS	= mount.nfs
 EXTRA_DIST = nfsmount.x $(man8_MANS) $(man5_MANS)
-mount_nfs_SOURCES = mount.c error.c network.c fstab.c token.c \
+mount_common = error.c network.c fstab.c token.c \
 		    parse_opt.c parse_dev.c \
 		    nfsmount.c nfs4mount.c stropts.c\
 		    nfsumount.c \
@@ -19,7 +19,7 @@ mount_nfs_SOURCES = mount.c error.c network.c fstab.c token.c \
 		    mount_config.h utils.c utils.h
 
 if MOUNT_CONFIG
-mount_nfs_SOURCES += configfile.c
+mount_common += configfile.c
 man5_MANS += nfsmount.conf.man
 EXTRA_DIST += nfsmount.conf
 endif
@@ -27,6 +27,15 @@ endif
 mount_nfs_LDADD = ../../support/nfs/libnfs.a \
 		  ../../support/export/libexport.a
 
+mount_nfs_SOURCES = $(mount_common)
+
+if CONFIG_LIBMOUNT
+mount_nfs_SOURCES += mount_libmount.c
+mount_nfs_LDADD += $(LIBMOUNT)
+else
+mount_nfs_SOURCES += mount.c
+endif
+
 MAINTAINERCLEANFILES = Makefile.in
 
 install-exec-hook:
diff --git a/utils/mount/mount_libmount.c b/utils/mount/mount_libmount.c
new file mode 100644
index 0000000..f433c8d
--- /dev/null
+++ b/utils/mount/mount_libmount.c
@@ -0,0 +1,411 @@
+/*
+ * mount_libmount.c -- Linux NFS [u]mount based on libmount
+ *
+ * Copyright (C) 2011 Karel Zak <kzak@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <unistd.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <getopt.h>
+
+#include <libmount/libmount.h>
+
+#include "nls.h"
+#include "mount_config.h"
+
+#include "nfs_mount.h"
+#include "nfs4_mount.h"
+#include "stropts.h"
+#include "version.h"
+#include "xcommon.h"
+
+#include "error.h"
+#include "utils.h"
+
+char *progname;
+int nfs_mount_data_version;
+int verbose;
+int sloppy;
+int string;
+int nomtab;
+
+#define FOREGROUND	(0)
+#define BACKGROUND	(1)
+
+/*
+ * Store mount options to mtab (or /dev/.mount/utab), called from mount.nfs.
+ *
+ * Note that on systems without /etc/mtab the fs-specific options are not
+ * managed by libmount at all. We have to use "mount attributes" that are
+ * private for mount.<type> helpers.
+ */
+static void store_mount_options(struct libmnt_fs *fs, const char *opts)
+{
+	mnt_fs_set_fs_options(fs, opts);	/* for mtab */
+	mnt_fs_set_attributes(fs, opts);	/* for non-mtab systems */
+}
+
+/*
+ * Retrieve mount options from mtab (or /dev/.mount/utab) called from umount.nfs.
+ *
+ * The result can passed to free().
+ */
+char *retrieve_mount_options(struct libmnt_fs *fs)
+{
+	const char *opts;
+
+	if (!fs)
+		return NULL;
+
+	opts = mnt_fs_get_attributes(fs);	/* /dev/.mount/utab */
+	if (opts)
+		return strdup(opts);
+
+	return mnt_fs_strdup_options(fs);	/* /etc/mtab */
+}
+
+static int try_mount(struct libmnt_context *cxt, int bg)
+{
+	struct libmnt_fs *fs;
+	const char *p;
+	char *src = NULL, *tgt = NULL, *type = NULL, *opts = NULL;
+	unsigned long flags = 0;
+	int fake, ret = 0;
+
+	fs = mnt_context_get_fs(cxt);
+
+	/* libmount returns read-only pointers (const char)
+	 * so, reallocate for nfsmount() functions.
+	 */
+	if ((p = mnt_fs_get_source(fs)))	/* spec */
+		src = strdup(p);
+	if ((p = mnt_fs_get_target(fs)))	/* mountpoint */
+		tgt = strdup(p);
+	if ((p = mnt_fs_get_fstype(fs)))	/* FS type */
+		type = strdup(p);
+	if ((p = mnt_fs_get_fs_options(fs)))	/* mount options */
+		opts = strdup(p);
+
+	mnt_context_get_mflags(cxt, &flags);	/* mount(2) flags */
+	fake = mnt_context_is_fake(cxt);
+
+	if (string)
+		ret = nfsmount_string(src, tgt, type, flags, &opts, fake, bg);
+
+	else if (strcmp(type, "nfs4") == 0)
+		ret = nfs4mount(src, tgt, flags, &opts, fake, bg);
+	else
+		ret = nfsmount(src, tgt, flags, &opts, fake, bg);
+
+	/* Store mount options if not called with mount --no-mtab */
+	if (!ret && !mnt_context_is_nomtab(cxt))
+		store_mount_options(fs, opts);
+
+	free(src);
+	free(tgt);
+	free(type);
+	free(opts);
+
+	return ret;
+}
+
+/* returns: error = -1, success = 0 , unknown = 1 */
+static int is_vers4(struct libmnt_context *cxt)
+{
+	struct libmnt_fs *fs = mnt_context_get_fs(cxt);
+	struct libmnt_table *tb = NULL;
+	const char *src = mnt_context_get_source(cxt),
+		   *tgt = mnt_context_get_target(cxt);
+	int rc = 1;
+
+	if (!src || !tgt)
+		return -1;
+
+	if (!mnt_fs_is_kernel(fs)) {
+		struct libmnt_table *tb = mnt_new_table_from_file("/proc/mounts");
+
+		if (!tb)
+			return -1;
+		fs = mnt_table_find_pair(tb, src, tgt, MNT_ITER_BACKWARD);
+	}
+
+	if (fs) {
+		const char *type = mnt_fs_get_fstype(fs);
+		if (type && strcmp(type, "nfs4") == 0)
+			rc = 0;
+	}
+	mnt_free_table(tb);
+	return rc;
+}
+
+static int umount_main(struct libmnt_context *cxt, int argc, char **argv)
+{
+	int rc, c;
+	char *spec = NULL, *opts = NULL;
+
+	static const struct option longopts[] = {
+		{ "force", 0, 0, 'f' },
+		{ "help", 0, 0, 'h' },
+		{ "no-mtab", 0, 0, 'n' },
+		{ "verbose", 0, 0, 'v' },
+		{ "read-only", 0, 0, 'r' },
+		{ "lazy", 0, 0, 'l' },
+		{ "types", 1, 0, 't' },
+		{ NULL, 0, 0, 0 }
+	};
+
+	mnt_context_init_helper(cxt, MNT_ACT_UMOUNT, 0);
+
+	while ((c = getopt_long (argc, argv, "fvnrlh", longopts, NULL)) != -1) {
+
+		rc = mnt_context_helper_setopt(cxt, c, optarg);
+		if (rc == 0)		/* valid option */
+			continue;
+		if (rc < 0)		/* error (probably ENOMEM) */
+			goto err;
+					/* rc==1 means unknow option */
+		umount_usage();
+		return EX_USAGE;
+	}
+
+	if (optind < argc)
+		spec = argv[optind++];
+
+	if (!spec || (*spec != '/' && strchr(spec,':') == NULL)) {
+		nfs_error(_("%s: no mount point provided"), progname);
+		return EX_USAGE;
+	}
+
+	if (mnt_context_set_target(cxt, spec))
+		goto err;
+	if (mnt_context_set_fstype_pattern(cxt, "nfs,nfs4"))	/* restrict filesystems */
+		goto err;
+
+	/* read mtab/fstab, evaluate permissions, etc. */
+	rc = mnt_context_prepare_umount(cxt);
+	if (rc) {
+		nfs_error(_("%s: failed to prepare umount: %s\n"),
+					progname, strerror(-rc));
+		goto err;
+	}
+
+	opts = retrieve_mount_options(mnt_context_get_fs(cxt));
+
+	if (!mnt_context_is_lazy(cxt)) {
+		if (opts) {
+			/* we have full FS description (e.g. from mtab or /proc) */
+			switch (is_vers4(cxt)) {
+			case 0:
+				/* We ignore the error from nfs_umount23.
+				 * If the actual umount succeeds (in del_mtab),
+				 * we don't want to signal an error, as that
+				 * could cause /sbin/mount to retry!
+				 */
+				nfs_umount23(mnt_context_get_source(cxt), opts);
+				break;
+			case 1:			/* unknown */
+				break;
+			default:		/* error */
+				goto err;
+			}
+		} else
+			/* strange, no entry in mtab or /proc not mounted */
+			nfs_umount23(spec, "tcp,v3");
+	}
+
+	rc = mnt_context_do_umount(cxt);	/* call umount(2) syscall */
+	mnt_context_finalize_mount(cxt);	/* mtab update */
+
+	if (!rc)
+		goto err;
+
+	free(opts);
+	return EX_SUCCESS;
+err:
+	free(opts);
+	return EX_FAIL;
+}
+
+static int mount_main(struct libmnt_context *cxt, int argc, char **argv)
+{
+	int rc, c;
+	struct libmnt_fs *fs;
+	char *spec = NULL, *mount_point = NULL, *opts = NULL;
+
+	static const struct option longopts[] = {
+	  { "fake", 0, 0, 'f' },
+	  { "help", 0, 0, 'h' },
+	  { "no-mtab", 0, 0, 'n' },
+	  { "read-only", 0, 0, 'r' },
+	  { "ro", 0, 0, 'r' },
+	  { "verbose", 0, 0, 'v' },
+	  { "version", 0, 0, 'V' },
+	  { "read-write", 0, 0, 'w' },
+	  { "rw", 0, 0, 'w' },
+	  { "options", 1, 0, 'o' },
+	  { "sloppy", 0, 0, 's' },
+	  { NULL, 0, 0, 0 }
+	};
+
+	mount_config_init(progname);
+	mnt_context_init_helper(cxt, MNT_ACT_MOUNT, 0);
+
+	while ((c = getopt_long(argc, argv, "fhnrVvwo:s", longopts, NULL)) != -1) {
+
+		rc = mnt_context_helper_setopt(cxt, c, optarg);
+		if (rc == 0)		/* valid option */
+			continue;
+		if (rc < 0)		/* error (probably ENOMEM) */
+			goto err;
+					/* rc==1 means unknow option */
+		switch (c) {
+		case 'V':
+			printf("%s: ("PACKAGE_STRING")\n", progname);
+			return EX_SUCCESS;
+		case 'h':
+		default:
+			mount_usage();
+			return EX_USAGE;
+		}
+	}
+
+	if (optind < argc)
+		spec = argv[optind++];
+	if (optind < argc)
+		mount_point = argv[optind++];
+
+	if (!mount_point) {
+		nfs_error(_("%s: no mount point provided"), progname);
+		goto err;
+	}
+	if (!spec) {
+		nfs_error(_("%s: no mount spec provided"), progname);
+		goto err;
+	}
+
+	if (geteuid() != 0) {
+		nfs_error(_("%s: not installed setuid - "
+			    "\"user\" NFS mounts not supported."), progname);
+		goto err;
+	}
+
+	verbose = mnt_context_is_verbose(cxt);
+	sloppy = mnt_context_is_sloppy(cxt);
+	nomtab = mnt_context_is_nomtab(cxt);
+
+	if (strcmp(progname, "mount.nfs4") == 0)
+		mnt_context_set_fstype(cxt, "nfs4");
+	else
+		mnt_context_set_fstype(cxt, "nfs");	/* default */
+
+	rc = mnt_context_set_source(cxt, spec);
+	if (!rc)
+		mnt_context_set_target(cxt, mount_point);
+	if (rc) {
+		nfs_error(_("%s: failed to set spec or mountpoint: %s"),
+				progname, strerror(errno));
+		goto err;
+	}
+
+	mount_point = mnt_resolve_path(mount_point,
+				       mnt_context_get_cache(cxt));
+
+	if (chk_mountpoint(mount_point))
+		goto err;
+	/*
+	 * Concatenate mount options from the configuration file
+	 */
+	fs = mnt_context_get_fs(cxt);
+	if (fs) {
+		opts = mnt_fs_strdup_options(fs);
+
+		opts = mount_config_opts(spec, mount_point, opts);
+		mnt_fs_set_options(fs, opts);
+	}
+
+	rc = mnt_context_prepare_mount(cxt);
+	if (rc) {
+		nfs_error(_("%s: failed to prepare mount: %s\n"),
+					progname, strerror(-rc));
+		goto err;
+	}
+
+	rc = try_mount(cxt, FOREGROUND);
+
+	if (rc == EX_BG) {
+		printf(_("%s: backgrounding \"%s\"\n"),
+			progname, mnt_context_get_source(cxt));
+		printf(_("%s: mount options: \"%s\"\n"),
+			progname, opts);
+
+		fflush(stdout);
+
+		if (daemon(0, 0)) {
+			nfs_error(_("%s: failed to start "
+					"background process: %s\n"),
+					progname, strerror(errno));
+			exit(EX_FAIL);
+		}
+
+		rc = try_mount(cxt, BACKGROUND);
+
+		if (verbose && rc)
+			printf(_("%s: giving up \"%s\"\n"),
+				progname, mnt_context_get_source(cxt));
+	}
+
+	mnt_context_set_syscall_status(cxt, rc);
+	mnt_context_finalize_mount(cxt);	/* mtab update */
+
+	return rc;
+err:
+	return EX_FAIL;
+}
+
+int main(int argc, char *argv[])
+{
+	struct libmnt_context *cxt;
+	int rc;
+
+	mnt_init_debug(0);
+	cxt = mnt_new_context();
+	if (!cxt) {
+		nfs_error(_("Can't initilize libmount: %s"),
+					strerror(errno));
+		rc = EX_FAIL;
+		goto done;
+	}
+
+	progname = basename(argv[0]);
+	nfs_mount_data_version = discover_nfs_mount_data_version(&string);
+
+	if(strncmp(progname, "umount", 6) == 0)
+		rc = umount_main(cxt, argc, argv);
+	else
+		rc = mount_main(cxt, argc, argv);
+done:
+	mnt_free_context(cxt);
+	return rc;
+}
-- 
1.7.3.4

Re: mount.nfs libmount support (v2)

[Chuck Lever]

On Mar 3, 2011, at 8:43 AM, Karel Zak wrote:

> This is the second version of the mount.nfs with libmount support. The code
> depends on util-linux >= v2.19 (e.g. Fedora 15).
> 
> Chnages:
>  - move nfs_umount_do_umnt() to network.c
>  - use union nfs_sockaddr
>  - add missing GPL address paragraph
>  - use abstract function the storing and retrieval mount options
>  - use "static const" for struct option
>  - use "strncmp() == 0" everywhere
> 
> 
> The locale issue is not fixed. It seems that _() macro from nls.h is used on
> many places in the package, but ENABLE_NLS macro is nowhere defined and 
> setlocale() is not called at all.
> 
> I believe that this issue should be fixed independently on the libmount
> patches.

Thanks for the update.

The overall architecture looks sane to me.  I think we would accelerate adoption by having nfs-utils take this now as an experimental "technology preview" thing, so the new code is widely available in a shared repo.  That would make it easier for people to play with it as libmount becomes available in more distributions.  We can then quickly make any minor adjustments needed.  Steve, thoughts?

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

Re: [PATCH 2/2] mount: add --enable-libmount-mount

[Steve Dickson]

On 03/03/2011 08:43 AM, Karel Zak wrote:

> +static int umount_main(struct libmnt_context *cxt, int argc, char **argv)
> +{
> +	int rc, c;
> +	char *spec = NULL, *opts = NULL;
> +
> +	static const struct option longopts[] = {
> +		{ "force", 0, 0, 'f' },
> +		{ "help", 0, 0, 'h' },
> +		{ "no-mtab", 0, 0, 'n' },
> +		{ "verbose", 0, 0, 'v' },
> +		{ "read-only", 0, 0, 'r' },
> +		{ "lazy", 0, 0, 'l' },
> +		{ "types", 1, 0, 't' },
> +		{ NULL, 0, 0, 0 }
> +	};
> +
> +	mnt_context_init_helper(cxt, MNT_ACT_UMOUNT, 0);
> +
> +	while ((c = getopt_long (argc, argv, "fvnrlh", longopts, NULL)) != -1) {
> +
> +		rc = mnt_context_helper_setopt(cxt, c, optarg);
> +		if (rc == 0)		/* valid option */
> +			continue;
> +		if (rc < 0)		/* error (probably ENOMEM) */
> +			goto err;
> +					/* rc==1 means unknow option */
> +		umount_usage();
> +		return EX_USAGE;
> +	}
> +
> +	if (optind < argc)
> +		spec = argv[optind++];
> +
> +	if (!spec || (*spec != '/' && strchr(spec,':') == NULL)) {
> +		nfs_error(_("%s: no mount point provided"), progname);
> +		return EX_USAGE;
> +	}
> +
> +	if (mnt_context_set_target(cxt, spec))
> +		goto err;
> +	if (mnt_context_set_fstype_pattern(cxt, "nfs,nfs4"))	/* restrict filesystems */
> +		goto err;
> +
> +	/* read mtab/fstab, evaluate permissions, etc. */
> +	rc = mnt_context_prepare_umount(cxt);
> +	if (rc) {
> +		nfs_error(_("%s: failed to prepare umount: %s\n"),
> +					progname, strerror(-rc));
> +		goto err;
> +	}
> +
> +	opts = retrieve_mount_options(mnt_context_get_fs(cxt));
> +
> +	if (!mnt_context_is_lazy(cxt)) {
> +		if (opts) {
> +			/* we have full FS description (e.g. from mtab or /proc) */
> +			switch (is_vers4(cxt)) {
> +			case 0:
> +				/* We ignore the error from nfs_umount23.
> +				 * If the actual umount succeeds (in del_mtab),
> +				 * we don't want to signal an error, as that
> +				 * could cause /sbin/mount to retry!
> +				 */
> +				nfs_umount23(mnt_context_get_source(cxt), opts);
> +				break;
> +			case 1:			/* unknown */
> +				break;
> +			default:		/* error */
> +				goto err;
> +			}
> +		} else
> +			/* strange, no entry in mtab or /proc not mounted */
> +			nfs_umount23(spec, "tcp,v3");
Question: since retrieve_mount_options() returns NULL why do you assume 
the mount exists? What's happing is I'm doing an umount.nfs on an 
not existing mount point and the error is 
    umount.nfs: remote share not in 'host:dir' format

Instead of
    umount.nfs: /mnt/home: not mounted

steved.
  

[PATCH 1.5/2] mount: Remove MOUNT_CONFIG warning

[Steve Dickson]

The following changes are needed to remove compile warnings when
MOUNT_CONFIG is not define

Signed-off-by: Steve Dickson <steved@redhat.com>

diff --git a/utils/mount/mount_config.h b/utils/mount/mount_config.h
index e86b4ba..69ffd1e 100644
--- a/utils/mount/mount_config.h
+++ b/utils/mount/mount_config.h
@@ -39,10 +39,10 @@ static inline char *mount_config_opts(char *spec,
 
 #else /* MOUNT_CONFIG */
 
-static inline void mount_config_init(char *program) { }
+static inline void mount_config_init(__attribute__ ((unused)) char *program) { }
 
-static inline char *mount_config_opts(char *spec,
-		char *mount_point, char *mount_opts)
+static inline char *mount_config_opts(__attribute__ ((unused)) char *spec,
+		__attribute__ ((unused)) char *mount_point, char *mount_opts)
 {
 	return mount_opts;
 }
diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
index e314b82..f1aa503 100644
--- a/utils/mount/stropts.c
+++ b/utils/mount/stropts.c
@@ -110,7 +110,7 @@ static void nfs_default_version(struct nfsmount_info *mi)
 	}
 }
 #else
-inline void nfs_default_version(struct nfsmount_info *mi) {}
+inline void nfs_default_version(__attribute__ ((unused)) struct nfsmount_info *mi) {}
 #endif /* MOUNT_CONFIG */

[PATCH 3/2] mount: Allow 'port=0' to be a valid port value.

[Steve Dickson]

With the new util-linux, /etc/mtab is now is symbolically linked
to /proc/mounts. In /proc/mounts, the 'port=0' exits which causes
the umount.nfs to error out with the following error:
   umount.nfs: invalid value for 'port=' option

Setting port=0 is a valid value. It just means go out and query
remote rpcbind to get the correct port.

Signed-off-by: Steve Dickson <steved@redhat.com>
---
 utils/mount/network.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/utils/mount/network.c b/utils/mount/network.c
index 8049c1a..88ab222 100644
--- a/utils/mount/network.c
+++ b/utils/mount/network.c
@@ -1346,7 +1346,7 @@ nfs_nfs_port(struct mount_options *options, unsigned long *port)
 	case PO_NOT_FOUND:
 		break;
 	case PO_FOUND:
-		if (tmp >= 1 && tmp <= 65535) {
+		if (tmp >= 0 && tmp <= 65535) {
 			*port = tmp;
 			return 1;
 		}

Re: [PATCH 3/2] mount: Allow 'port=0' to be a valid port value.

[Chuck Lever]

Hi Steve-

On Mar 3, 2011, at 3:04 PM, Steve Dickson wrote:

> With the new util-linux, /etc/mtab is now is symbolically linked
> to /proc/mounts. In /proc/mounts, the 'port=0' exits which causes
> the umount.nfs to error out with the following error:
>   umount.nfs: invalid value for 'port=' option
> 
> Setting port=0 is a valid value. It just means go out and query
> remote rpcbind to get the correct port.
> 
> Signed-off-by: Steve Dickson <steved@redhat.com>

Why are you signing off on this?  I sent you this patch weeks ago.  (In fact my version of the patch also fixes this problem in nfs_mount_port() as well).

> ---
> utils/mount/network.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/utils/mount/network.c b/utils/mount/network.c
> index 8049c1a..88ab222 100644
> --- a/utils/mount/network.c
> +++ b/utils/mount/network.c
> @@ -1346,7 +1346,7 @@ nfs_nfs_port(struct mount_options *options, unsigned long *port)
> 	case PO_NOT_FOUND:
> 		break;
> 	case PO_FOUND:
> -		if (tmp >= 1 && tmp <= 65535) {
> +		if (tmp >= 0 && tmp <= 65535) {
> 			*port = tmp;
> 			return 1;
> 		}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH 3/2] mount: Allow 'port=0' to be a valid port value.

[Chuck Lever]

On Mar 3, 2011, at 3:12 PM, Chuck Lever wrote:

> Hi Steve-
> 
> On Mar 3, 2011, at 3:04 PM, Steve Dickson wrote:
> 
>> With the new util-linux, /etc/mtab is now is symbolically linked
>> to /proc/mounts. In /proc/mounts, the 'port=0' exits which causes
>> the umount.nfs to error out with the following error:
>>  umount.nfs: invalid value for 'port=' option
>> 
>> Setting port=0 is a valid value. It just means go out and query
>> remote rpcbind to get the correct port.
>> 
>> Signed-off-by: Steve Dickson <steved@redhat.com>
> 
> Why are you signing off on this?  I sent you this patch weeks ago.  (In fact my version of the patch also fixes this problem in nfs_mount_port() as well).

Could have been lost in the shuffle... we were all traveling.  Anyway, you need to fix this in nfs_mount_port() too.


> 
>> ---
>> utils/mount/network.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/utils/mount/network.c b/utils/mount/network.c
>> index 8049c1a..88ab222 100644
>> --- a/utils/mount/network.c
>> +++ b/utils/mount/network.c
>> @@ -1346,7 +1346,7 @@ nfs_nfs_port(struct mount_options *options, unsigned long *port)
>> 	case PO_NOT_FOUND:
>> 		break;
>> 	case PO_FOUND:
>> -		if (tmp >= 1 && tmp <= 65535) {
>> +		if (tmp >= 0 && tmp <= 65535) {
>> 			*port = tmp;
>> 			return 1;
>> 		}
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH 3/2] mount: Allow 'port=0' to be a valid port value.

[Steve Dickson]

On 03/03/2011 03:16 PM, Chuck Lever wrote:
> 
> On Mar 3, 2011, at 3:12 PM, Chuck Lever wrote:
> 
>> Hi Steve-
>>
>> On Mar 3, 2011, at 3:04 PM, Steve Dickson wrote:
>>
>>> With the new util-linux, /etc/mtab is now is symbolically linked
>>> to /proc/mounts. In /proc/mounts, the 'port=0' exits which causes
>>> the umount.nfs to error out with the following error:
>>>  umount.nfs: invalid value for 'port=' option
>>>
>>> Setting port=0 is a valid value. It just means go out and query
>>> remote rpcbind to get the correct port.
>>>
>>> Signed-off-by: Steve Dickson <steved@redhat.com>
>>
>> Why are you signing off on this?  I sent you this patch weeks ago.  (In fact my version of the patch also fixes this problem in nfs_mount_port() as well).
> 
> Could have been lost in the shuffle... we were all traveling.  Anyway, you need to fix this in  () too.
Yeah... I don't see it... but see your point WRT nfs_mount_port()

steved.
> 
> 
>>
>>> ---
>>> utils/mount/network.c |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/utils/mount/network.c b/utils/mount/network.c
>>> index 8049c1a..88ab222 100644
>>> --- a/utils/mount/network.c
>>> +++ b/utils/mount/network.c
>>> @@ -1346,7 +1346,7 @@ nfs_nfs_port(struct mount_options *options, unsigned long *port)
>>> 	case PO_NOT_FOUND:
>>> 		break;
>>> 	case PO_FOUND:
>>> -		if (tmp >= 1 && tmp <= 65535) {
>>> +		if (tmp >= 0 && tmp <= 65535) {
>>> 			*port = tmp;
>>> 			return 1;
>>> 		}
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> -- 
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>>
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/2] mount: add --enable-libmount-mount

[Karel Zak]

On Thu, Mar 03, 2011 at 02:53:06PM -0500, Steve Dickson wrote:
> > +	opts = retrieve_mount_options(mnt_context_get_fs(cxt));
> > +
> > +	if (!mnt_context_is_lazy(cxt)) {
> > +		if (opts) {
> > +			/* we have full FS description (e.g. from mtab or /proc) */
> > +			switch (is_vers4(cxt)) {
> > +			case 0:
> > +				/* We ignore the error from nfs_umount23.
> > +				 * If the actual umount succeeds (in del_mtab),
> > +				 * we don't want to signal an error, as that
> > +				 * could cause /sbin/mount to retry!
> > +				 */
> > +				nfs_umount23(mnt_context_get_source(cxt), opts);
> > +				break;
> > +			case 1:			/* unknown */
> > +				break;
> > +			default:		/* error */
> > +				goto err;
> > +			}
> > +		} else
> > +			/* strange, no entry in mtab or /proc not mounted */
> > +			nfs_umount23(spec, "tcp,v3");
> Question: since retrieve_mount_options() returns NULL why do you assume 
> the mount exists?

 because on systems with /etc/mtab is possible that the file is empty,
 so we have no clue about the mount point.
 
 This is normal umount(8) behavior, for root user it always tries to
 call umount(2) syscall. We should not assume that there is usable
 /proc or /etc/mtab. It's important be able to umount any device
 independently on /proc or /etc.

 See nfsumount.c, the current code uses the same logic.

> What's happing is I'm doing an umount.nfs on an 
> not existing mount point and the error is 
>     umount.nfs: remote share not in 'host:dir' format
> 
> Instead of
>     umount.nfs: /mnt/home: not mounted

 Good catch, this is bug. There is missing umount_error();

 I'll update the patch and add your change to mount_config_init() too.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 2/2] mount: add --enable-libmount-mount

[Steve Dickson]

On 03/04/2011 05:18 AM, Karel Zak wrote:
> On Thu, Mar 03, 2011 at 02:53:06PM -0500, Steve Dickson wrote:
>>> +	opts = retrieve_mount_options(mnt_context_get_fs(cxt));
>>> +
>>> +	if (!mnt_context_is_lazy(cxt)) {
>>> +		if (opts) {
>>> +			/* we have full FS description (e.g. from mtab or /proc) */
>>> +			switch (is_vers4(cxt)) {
>>> +			case 0:
>>> +				/* We ignore the error from nfs_umount23.
>>> +				 * If the actual umount succeeds (in del_mtab),
>>> +				 * we don't want to signal an error, as that
>>> +				 * could cause /sbin/mount to retry!
>>> +				 */
>>> +				nfs_umount23(mnt_context_get_source(cxt), opts);
>>> +				break;
>>> +			case 1:			/* unknown */
>>> +				break;
>>> +			default:		/* error */
>>> +				goto err;
>>> +			}
>>> +		} else
>>> +			/* strange, no entry in mtab or /proc not mounted */
>>> +			nfs_umount23(spec, "tcp,v3");
>> Question: since retrieve_mount_options() returns NULL why do you assume 
>> the mount exists?
> 
>  because on systems with /etc/mtab is possible that the file is empty,
>  so we have no clue about the mount point.
Ah... I am so glad mtab is going away! 

>  
>  This is normal umount(8) behavior, for root user it always tries to
>  call umount(2) syscall. We should not assume that there is usable
>  /proc or /etc/mtab. It's important be able to umount any device
>  independently on /proc or /etc.
> 
>  See nfsumount.c, the current code uses the same logic.
Got it..

> 
>> What's happing is I'm doing an umount.nfs on an 
>> not existing mount point and the error is 
>>     umount.nfs: remote share not in 'host:dir' format
>>
>> Instead of
>>     umount.nfs: /mnt/home: not mounted
> 
>  Good catch, this is bug. There is missing umount_error();
> 
>  I'll update the patch and add your change to mount_config_init() too.
Thank you...

steved.

Re: [PATCH 1.5/2] mount: Remove MOUNT_CONFIG warning

[Steve Dickson]

Hey Karel,

On 03/03/2011 03:03 PM, Steve Dickson wrote:
> The following changes are needed to remove compile warnings when
> MOUNT_CONFIG is not define
Don't worry about this one... I just committed it.. 

steved.

> 
> Signed-off-by: Steve Dickson <steved@redhat.com>
> 
> diff --git a/utils/mount/mount_config.h b/utils/mount/mount_config.h
> index e86b4ba..69ffd1e 100644
> --- a/utils/mount/mount_config.h
> +++ b/utils/mount/mount_config.h
> @@ -39,10 +39,10 @@ static inline char *mount_config_opts(char *spec,
>  
>  #else /* MOUNT_CONFIG */
>  
> -static inline void mount_config_init(char *program) { }
> +static inline void mount_config_init(__attribute__ ((unused)) char *program) { }
>  
> -static inline char *mount_config_opts(char *spec,
> -		char *mount_point, char *mount_opts)
> +static inline char *mount_config_opts(__attribute__ ((unused)) char *spec,
> +		__attribute__ ((unused)) char *mount_point, char *mount_opts)
>  {
>  	return mount_opts;
>  }
> diff --git a/utils/mount/stropts.c b/utils/mount/stropts.c
> index e314b82..f1aa503 100644
> --- a/utils/mount/stropts.c
> +++ b/utils/mount/stropts.c
> @@ -110,7 +110,7 @@ static void nfs_default_version(struct nfsmount_info *mi)
>  	}
>  }
>  #else
> -inline void nfs_default_version(struct nfsmount_info *mi) {}
> +inline void nfs_default_version(__attribute__ ((unused)) struct nfsmount_info *mi) {}
>  #endif /* MOUNT_CONFIG */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html