mount.nfs libmount support (v2)

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

Signed-off-by Steve Dickson <[email protected]>

utils/mount/mount_config.h +3 -3

index e86b4ba..69ffd1e 100644
@@ -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;
 }

utils/mount/stropts.c +1 -2

index e314b82..f1aa503 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

utils/mount/mount_libmount.c new file +63 -0

+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!
+				 */

Quoted 16 lines

utils/mount/mount_libmount.c new file +16 -0

+	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;

utils/mount/mount_libmount.c line 235

Quoted 6 lines

utils/mount/mount_libmount.c new file +6 -0

+			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

Quoted 16 lines · Steve Dickson

Quoted 16 lines

utils/mount/mount_libmount.c new file +16 -0

+	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;

Quoted 8 lines

utils/mount/mount_libmount.c line 235

Quoted 6 lines

utils/mount/mount_libmount.c new file +6 -0

+			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.

Quoted 7 lines

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.

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.

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 <[email protected]>

---
utils/mount/network.c | 2 +-

1 files changed, 1 insertions(+), 1 deletions(-)

utils/mount/network.c +1 -2

index 8049c1a..88ab222 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Steve-

Quoted 5 lines · 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

Quoted 4 lines

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 <[email protected]>

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).

Quoted 22 lines

---
utils/mount/network.c | 2 +-

1 files changed, 1 insertions(+), 1 deletions(-)

utils/mount/network.c +1 -2

index 8049c1a..88ab222 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 10 lines

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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 9 lines · Chuck Lever

Hi Steve-

Quoted 5 lines · 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

Quoted 6 lines

Quoted 4 lines

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 <[email protected]>

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.

Quoted 36 lines

Quoted 24 lines

---
utils/mount/network.c | 2 +-

1 files changed, 1 insertions(+), 1 deletions(-)

utils/mount/network.c +1 -2

index 8049c1a..88ab222 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 10 lines

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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html