autofs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.com>
To: Ian Kent <raven@themaw.net>
Cc: autofs mailing list <autofs@vger.kernel.org>,
	linux-nfs@vger.kernel.org, util-linux@vger.kernel.org
Subject: [autofs PATCH] Add -c option when calling /bin/umount - if supported.
Date: Wed, 07 Jun 2017 10:32:49 +1000	[thread overview]
Message-ID: <87d1agd4cu.fsf@notabene.neil.brown.name> (raw)

[-- Attachment #1: Type: text/plain, Size: 6518 bytes --]


The "-c" option has been supported by umount since util-linux 2.17.
It tells umount that the path name is "canonical", meaning no
symlinks or ".." etc.
This is appropriate for autofs to use and it always uses canonical
path names.
The advantage of "-c" is that it means umount doesn't need to
'lstat()' the path so much.
From util-linux 2.30, umount doesn't lstat the path at all when
"-c" is passed.  This is particularly beneficial when unmounting
an NFS filesystem for which the server is no reachable. In that
case an 'lstat()' will hang, but the umount(2) can succeed.

So check if "-c" is supported, and if so, use it.
Prior to 2.30 this doesn't help much, but doesn't hurt.
After 2.30 (and a similar small change to nfs-utils),
"-c" will mean that unmounting NFS filesystems will not hang.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 aclocal.m4          | 18 ++++++++++++++++++
 configure           | 33 +++++++++++++++++++++++++++++++++
 configure.in        | 14 ++++++++++++++
 daemon/spawn.c      | 23 +++++++++++++++--------
 include/config.h.in |  3 +++
 5 files changed, 83 insertions(+), 8 deletions(-)

diff --git a/aclocal.m4 b/aclocal.m4
index 00811e08dabe..40e1ee97e905 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -73,6 +73,24 @@ AC_DEFUN(AF_SLOPPY_MOUNT,
   fi
 fi])
 
+dnl --------------------------------------------------------------------------
+dnl AF_NO_CANON_UMOUNT
+dnl
+dnl Check to see if umount(8) supports the no-canonicalize (-c) option, and define
+dnl the cpp variable HAVE_NO_CANON_UMOUNT if so.  This requires that UMOUNT is
+dnl already defined by a call to AF_PATH_INCLUDE or AC_PATH_PROGS.
+dnl --------------------------------------------------------------------------
+AC_DEFUN(AF_NO_CANON_UMOUNT,
+[if test -n "$UMOUNT" ; then
+  AC_MSG_CHECKING([if umount accepts the -c option])
+  if "$UMOUNT" -h 2>&1 | grep -e '-c.*--no-canonicalize' > /dev/null 2>&1 ; then
+    enable_no_canon_umount=yes
+    AC_MSG_RESULT(yes)
+  else
+    AC_MSG_RESULT(no)
+  fi
+fi])
+
 
 dnl --------------------------------------------------------------------------
 dnl AF_LINUX_PROCFS
diff --git a/configure b/configure
index 10f907e36a9b..9ba267073e62 100755
--- a/configure
+++ b/configure
@@ -737,6 +737,7 @@ with_flagdir
 with_libtirpc
 with_dmalloc
 enable_sloppy_mount
+enable_no_canon_umount
 with_hesiod
 with_openldap
 with_sasl
@@ -1363,6 +1364,7 @@ Optional Features:
   --disable-FEATURE       do not include FEATURE (same as --enable-FEATURE=no)
   --enable-FEATURE[=ARG]  include FEATURE [ARG=yes]
   --enable-sloppy-mount         enable the use of the -s option to mount
+  --enable-no-canon-umount         enable the use of the -c option to umount
   --disable-ext-env	        disable search in environment for substitution variable
   --disable-mount-locking       disable use of locking when spawning mount command
   --enable-force-shutdown       enable USR1 signal to force unlink umount of any
@@ -3993,6 +3995,37 @@ $as_echo "#define HAVE_SLOPPY_MOUNT 1" >>confdefs.h
 
 fi
 
+#
+# Newer umounts have the -c (--no-canonicalize) option to avoid
+# stating the path and possible blocking.  Good for NFS.
+#
+# Check whether --enable-no-canon-umount was given.
+if test "${enable_no_canon_umount+set}" = set; then :
+  enableval=$enable_no_canon_umount;
+else
+  enable_no_canon_umount=auto
+fi
+
+if test x$enable_no_canon_umount = xauto; then
+	if test -n "$UMOUNT" ; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking if umount accepts the -c option" >&5
+$as_echo_n "checking if umount accepts the -c option... " >&6; }
+  if "$UMOUNT" -h 2>&1 | grep -e '-c.*--no-canonicalize' > /dev/null 2>&1 ; then
+    enable_no_canon_umount=yes
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+  else
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+  fi
+fi
+fi
+if test x$enable_no_canon_umount = xyes; then
+
+$as_echo "#define HAVE_NO_CANON_UMOUNT 1" >>confdefs.h
+
+fi
+
 # LDAP SASL auth needs libxml and Kerberos
 for ac_prog in xml2-config
 do
diff --git a/configure.in b/configure.in
index 05212521b2a9..d408b209cace 100644
--- a/configure.in
+++ b/configure.in
@@ -167,6 +167,20 @@ if test x$enable_sloppy_mount = xyes; then
 	AC_DEFINE(HAVE_SLOPPY_MOUNT, 1, [define if the mount command supports the -s option])
 fi
 
+#
+# Newer umounts have the -c (--no-canonicalize) option to avoid
+# stating the path and possible blocking.  Good for NFS.
+#
+AC_ARG_ENABLE(no-canon-umount,
+[  --enable-no-canon-umount         enable the use of the -c option to umount],,
+	enable_no_canon_umount=auto)
+if test x$enable_no_canon_umount = xauto; then
+	AF_NO_CANON_UMOUNT()
+fi
+if test x$enable_no_canon_umount = xyes; then
+	AC_DEFINE(HAVE_NO_CANON_UMOUNT, 1, [define if the umount command supports the -c option])
+fi
+
 # LDAP SASL auth needs libxml and Kerberos
 AF_CHECK_LIBXML()
 AF_CHECK_KRB5()
diff --git a/daemon/spawn.c b/daemon/spawn.c
index c640d97601da..4515607ba022 100644
--- a/daemon/spawn.c
+++ b/daemon/spawn.c
@@ -592,6 +592,11 @@ int spawn_umount(unsigned logopt, ...)
 	char **argv, **p;
 	char prog[] = PATH_UMOUNT;
 	char arg0[] = PATH_UMOUNT;
+#ifdef HAVE_NO_CANON_UMOUNT
+	char * const arg_c = "-c";
+#else
+	char * const arg_c = NULL;
+#endif
 	char argn[] = "-n";
 	unsigned int options;
 	unsigned int retries = MTAB_LOCK_RETRIES;
@@ -620,19 +625,21 @@ int spawn_umount(unsigned logopt, ...)
 			update_mtab = 0;
 		}
 	}
+	if (arg_c)
+		argc++;;
 
-	if (!(argv = alloca(sizeof(char *) * argc + 1)))
+	if (!(argv = alloca(sizeof(char *) * (argc + 1))))
 		return -1;
 
-	argv[0] = arg0;
+	p = argv;
+	*p++ = arg0;
+	if (arg_c)
+		*p++ = arg_c;
+
+	if (!update_mtab)
+		*p++ = argn;
 
 	va_start(arg, logopt);
-	if (update_mtab)
-		p = argv + 1;
-	else {
-		argv[1] = argn;
-		p = argv + 2;
-	}
 	while ((*p++ = va_arg(arg, char *)));
 	va_end(arg);
 
diff --git a/include/config.h.in b/include/config.h.in
index e8885092f858..6ed0d832bb60 100644
--- a/include/config.h.in
+++ b/include/config.h.in
@@ -57,6 +57,9 @@
 /* define if you have MOUNT_NFS */
 #undef HAVE_MOUNT_NFS
 
+/* define if the umount command supports the -c option */
+#undef HAVE_NO_CANON_UMOUNT
+
 /* define if the mount command supports the -s option */
 #undef HAVE_SLOPPY_MOUNT
 
-- 
2.12.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

             reply	other threads:[~2017-06-07  0:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-07  0:32 NeilBrown [this message]
2017-06-07 15:52 ` [autofs PATCH] Add -c option when calling /bin/umount - if supported Karel Zak
     [not found]   ` <20170607155202.5zyzuvy6l3fsawkl-xkT7n84Rsxv/9pzu0YdTqQ@public.gmane.org>
2017-06-07 15:55     ` Karel Zak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d1agd4cu.fsf@notabene.neil.brown.name \
    --to=neilb@suse.com \
    --cc=autofs@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=raven@themaw.net \
    --cc=util-linux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).