From 89de9adbf2b7d446abe9a27c8e384d205a996176 Mon Sep 17 00:00:00 2001 From: Emmanuel Dreyfus Date: Fri, 26 Sep 2014 02:28:15 +0200 Subject: Do not hardcode umount(8) path, emulate lazy umount 1) Use a system-dependent macro for umount(8) location instead of relying on $PATH to find it, for security and portability sake. 2) Introduce gf_umount_lazy() to replace umount -l (-l for lazy) invocations, which is only supported on Linux; On Linux behavior in unchanged. On other systems, we fork an external process (umountd) that will take care of periodically attempt to unmount, and optionally rmdir. Backport of Ia91167c0652f8ddab85136324b08f87c5ac1edd51d BUG: 1138897 Change-Id: I9d82c87e85af0dee79f2de39bc697c486b7103c8 Signed-off-by: Emmanuel Dreyfus Reviewed-on: http://review.gluster.org/8863 Tested-by: Gluster Build System Reviewed-by: Csaba Henk Reviewed-by: Vijay Bellur --- Makefile.am | 4 +- cli/src/cli-xml-output.c | 18 +- configure.ac | 9 + contrib/umountd/Makefile.am | 10 + contrib/umountd/umountd.c | 247 +++++++++++++++++++++ libglusterfs/src/Makefile.am | 3 +- libglusterfs/src/compat.c | 39 ++++ libglusterfs/src/compat.h | 15 ++ xlators/mgmt/glusterd/src/glusterd-handler.c | 14 +- xlators/mgmt/glusterd/src/glusterd-quota.c | 6 +- xlators/mgmt/glusterd/src/glusterd-replace-brick.c | 47 +--- xlators/mgmt/glusterd/src/glusterd-utils.c | 14 +- xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 19 +- 13 files changed, 356 insertions(+), 89 deletions(-) create mode 100644 contrib/umountd/Makefile.am create mode 100644 contrib/umountd/umountd.c diff --git a/Makefile.am b/Makefile.am index 0331c86f394..83515d338f0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -9,7 +9,9 @@ EXTRA_DIST = autogen.sh \ $(shell find $(top_srcdir)/tests -type f -print) SUBDIRS = $(ARGP_STANDALONE_DIR) libglusterfs rpc api xlators glusterfsd \ - $(FUSERMOUNT_SUBDIR) doc extras cli @SYNCDAEMON_SUBDIR@ + $(FUSERMOUNT_SUBDIR) doc extras cli @SYNCDAEMON_SUBDIR@ \ + @UMOUNTD_SUBDIR@ + pkgconfigdir = @pkgconfigdir@ pkgconfig_DATA = glusterfs-api.pc libgfchangelog.pc diff --git a/cli/src/cli-xml-output.c b/cli/src/cli-xml-output.c index 609f7847018..d0a72772085 100644 --- a/cli/src/cli-xml-output.c +++ b/cli/src/cli-xml-output.c @@ -2931,7 +2931,8 @@ cli_xml_output_vol_quota_limit_list (char *volname, char *limit_list, gf_log ("cli", GF_LOG_ERROR, "failed to mount glusterfs client"); ret = -1; - goto rm_dir; + rmdir (mountdir); + goto cont; } while (i < len) { @@ -2988,19 +2989,10 @@ cli_xml_output_vol_quota_limit_list (char *volname, char *limit_list, } unmount: - runinit (&runner); - runner_add_args (&runner, "umount", -#if GF_LINUX_HOST_OS - "-l", -#endif - mountdir, NULL); - ret = runner_run_reuse (&runner); + ret = gf_umount_lazy ("cli", mountdir, 1); if (ret) - runner_log (&runner, "cli", GF_LOG_WARNING, "error executing"); - runner_end (&runner); - -rm_dir: - rmdir (mountdir); + gf_log ("cli", GF_LOG_WARNING, "error unmounting %s: %s", + mountdir, strerror (errno)); cont: /* */ diff --git a/configure.ac b/configure.ac index 583f500e961..ddc2cc5cc4e 100644 --- a/configure.ac +++ b/configure.ac @@ -200,6 +200,7 @@ AC_CONFIG_FILES([Makefile extras/hook-scripts/reset/post/Makefile extras/hook-scripts/reset/pre/Makefile contrib/fuse-util/Makefile + contrib/umountd/Makefile contrib/uuid/uuid_types.h glusterfs-api.pc libgfchangelog.pc @@ -896,6 +897,14 @@ case $host_os in ;; esac +# lazy umount emulation +UMOUNTD_SUBDIR="" +if test "x${GF_HOST_OS}" != "xGF_LINUX_HOST_OS" ; then + UMOUNTD_SUBDIR="contrib/umountd" +fi +AC_SUBST(UMOUNTD_SUBDIR) + + # enable/disable QEMU AM_CONDITIONAL([ENABLE_QEMU_BLOCK], [test x$BUILD_QEMU_BLOCK = xyes]) diff --git a/contrib/umountd/Makefile.am b/contrib/umountd/Makefile.am new file mode 100644 index 00000000000..c03b0cbcae6 --- /dev/null +++ b/contrib/umountd/Makefile.am @@ -0,0 +1,10 @@ +sbin_PROGRAMS = umountd +umountd_SOURCES = umountd.c +umountd_CFLAGS = $(GF_CFLAGS) -DDATADIR=\"$(localstatedir)\" +umountd_LDADD = $(top_builddir)/libglusterfs/src/libglusterfs.la ${GF_LDADD} +umountd_LDFLAGS = $(GF_LDFLAGS) + +AM_CPPFLAGS = $(GF_CPPFLAGS) -I$(top_srcdir)/libglusterfs/src +AM_CFLAGS = -Wall $(GF_CFLAGS) + +CLEANFILES = diff --git a/contrib/umountd/umountd.c b/contrib/umountd/umountd.c new file mode 100644 index 00000000000..42f867d1983 --- /dev/null +++ b/contrib/umountd/umountd.c @@ -0,0 +1,247 @@ +/* + Copyright (c) 2008-2012 Red Hat, Inc. + This file is part of GlusterFS. + + This file is licensed to you under your choice of the GNU Lesser + General Public License, version 3 or any later version (LGPLv3 or + later), or the GNU General Public License, version 2 (GPLv2), in all + cases as published by the Free Software Foundation. +*/ + +#ifndef _CONFIG_H +#define _CONFIG_H +#include "config.h" +#endif + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "glusterfs.h" +#include "globals.h" +#include "logging.h" +#include "syscall.h" +#include "mem-types.h" + +static void +usage (void) +{ + fprintf (stderr, "Usage: umountd [-d dev] [-t timeout] [-r] path\n"); + exit (EXIT_FAILURE); +} + + +static int +sanity_check (char *path, dev_t *devp) +{ + struct stat st; + struct stat parent_st; + int ret; + char pathtmp[PATH_MAX]; + char *parent; + + if (path == NULL) + usage (); + + if (stat (path, &st) != 0) { + gf_log ("umountd", GF_LOG_ERROR, + "Cannot access %s\n", path, strerror (errno)); + goto out; + } + + /* If dev was not specified, get it from path */ + if (*devp == -1) + *devp = st.st_dev; + + strncpy (pathtmp, path, PATH_MAX); + parent = dirname (pathtmp); + + if (stat (parent, &parent_st) != 0) { + gf_log ("umountd", GF_LOG_ERROR, + "Cannot access %s\n", parent, strerror (errno)); + goto out; + } + + if (st.st_dev == parent_st.st_dev) { + gf_log ("umountd", GF_LOG_ERROR, + "No filesystem mounted on %s\n", path); + goto out; + } + + ret = 0; + +out: + return ret; +} + +static void +log_rotate (int signum) +{ + gf_log_logrotate (1); + + if (signal (SIGHUP, *log_rotate) == SIG_ERR) { + gf_log ("umountd", GF_LOG_ERROR, "signal () failed"); + exit (EXIT_FAILURE); + } + + return; +} + +static int +logging_init (void) +{ + glusterfs_ctx_t *ctx; + char log_file[PATH_MAX]; + int ret = -1; + + ctx = glusterfs_ctx_new (); + if (!ctx) { + fprintf (stderr, "glusterfs_ctx_new failed\n"); + goto out; + } + + ret = glusterfs_globals_init (ctx); + if (ret) { + fprintf (stderr, "glusterfs_globals_init failed\n"); + goto out; + } + + THIS->ctx = ctx; + xlator_mem_acct_init (THIS, gf_common_mt_end); + + snprintf (log_file, PATH_MAX, + "%s/umountd.log", DEFAULT_LOG_FILE_DIRECTORY); + + ret = gf_log_init (ctx, log_file, "umountd"); + if (ret) { + fprintf (stderr, "gf_log_init failed\n"); + goto out; + } + + if (signal (SIGHUP, *log_rotate) == SIG_ERR) { + gf_log ("umountd", GF_LOG_ERROR, "signal () failed"); + goto out; + } + + ret = 0; +out: + return ret; +} + +static int +umountd_async (char *path, dev_t dev, int frmdir, int timeout) +{ + int ret = -1; + struct stat stbuf = {0, }; + int unmount_ret = 0; + + do { + unmount_ret = unmount (path, 0); + if (unmount_ret == 0) + gf_log ("umountd", GF_LOG_INFO, "Unmounted %s", path); + + if (unmount_ret != 0 && errno != EBUSY) { + gf_log ("umountd", GF_LOG_WARNING, + "umount %s failed: %s", + path, strerror (errno)); + } + + ret = sys_lstat (path, &stbuf); + if (ret != 0) { + gf_log ("umountd", GF_LOG_WARNING, + "Cannot stat device from %s", + path, strerror (errno)); + break; + } + + if (stbuf.st_dev != dev) { + if (unmount_ret != 0) + gf_log ("umountd", GF_LOG_INFO, + "device mismatch " + "(expect %lld, found %lld), " + "someone else unmounted %s", + dev, stbuf.st_dev, path); + ret = 0; + break; + } + + sleep (timeout); + } while (1/*CONSTCOND*/); + + if (ret) { + gf_log ("umountd", GF_LOG_ERROR, + "Asynchronous unmount of %s failed: %s", + path, strerror (errno)); + } else { + if (frmdir) { + ret = rmdir (path); + if (ret) + gf_log ("umountd", GF_LOG_WARNING, + "rmdir %s failed: %s", + path, strerror (errno)); + else + gf_log ("umountd", GF_LOG_INFO, + "Removed %s", path); + } + } + + return ret; +} + +int +main (int argc, char **argv) +{ + char *path = NULL; + dev_t dev = -1; + int frmdir = 0; + int timeout = 30; + int f; + + while ((f = getopt (argc, argv, "d:rt:")) != -1) { + switch (f) { + case 'p': + path = optarg; + break; + case 'd': + dev = strtoll (optarg, NULL, 10); + break; + case 't': + timeout = atoi (optarg); + break; + case 'r': + frmdir = 1; + break; + default: + usage (); + break; + } + } + + argc -= optind; + argv += optind; + + if (argc != 1) + usage (); + + path = argv[0]; + + if (logging_init () != 0) + exit (EXIT_FAILURE); + + if (sanity_check (path, &dev) != 0) + exit (EXIT_FAILURE); + + if (daemon (0, 0) != 0) + exit (EXIT_FAILURE); + + if (umountd_async (path, dev, frmdir, timeout) != 0) + exit (EXIT_FAILURE); + + return EXIT_SUCCESS; +} diff --git a/libglusterfs/src/Makefile.am b/libglusterfs/src/Makefile.am index 25ee4c27a8b..318058b3195 100644 --- a/libglusterfs/src/Makefile.am +++ b/libglusterfs/src/Makefile.am @@ -4,7 +4,8 @@ libglusterfs_la_CFLAGS = -Wall $(GF_CFLAGS) $(GF_DARWIN_LIBGLUSTERFS_CFLAGS) \ libglusterfs_la_CPPFLAGS = $(GF_CPPFLAGS) -D__USE_FILE_OFFSET64 \ -DXLATORDIR=\"$(libdir)/glusterfs/$(PACKAGE_VERSION)/xlator\" \ -I$(top_srcdir)/rpc/rpc-lib/src/ -I$(CONTRIBDIR)/rbtree \ - -I$(CONTRIBDIR)/libexecinfo + -I$(CONTRIBDIR)/libexecinfo \ + -DSBIN_DIR=\"$(sbindir)\" libglusterfs_la_LIBADD = @LEXLIB@ libglusterfs_la_LDFLAGS = -version-info $(LIBGLUSTERFS_LT_VERSION) diff --git a/libglusterfs/src/compat.c b/libglusterfs/src/compat.c index eb6d8d4b7f1..93e7b45a69e 100644 --- a/libglusterfs/src/compat.c +++ b/libglusterfs/src/compat.c @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -28,6 +29,7 @@ #include "common-utils.h" #include "iatt.h" #include "inode.h" +#include "run.h" #ifdef GF_SOLARIS_HOST_OS int @@ -543,3 +545,40 @@ strnlen(const char *string, size_t maxlen) return len; } #endif /* STRNLEN */ + +int +gf_umount_lazy (char *xlname, char *path, int rmdir_flag) +{ + int ret = -1; + runner_t runner = {0,}; + + runinit (&runner); +#ifdef GF_LINUX_HOST_OS + runner_add_args (&runner, _PATH_UMOUNT, "-l", path, NULL); +#else + if (rmdir_flag) + runner_add_args (&runner, SBIN_DIR "/umountd", + "-r", path, NULL); + else + runner_add_args (&runner, SBIN_DIR "/umountd", + path, NULL); +#endif + ret = runner_run (&runner); + if (ret) { + gf_log (xlname, GF_LOG_ERROR, + "Lazy unmount of %s failed: %s", + path, strerror (errno)); + } + +#ifdef GF_LINUX_HOST_OS + if (!ret && rmdir_flag) { + ret = rmdir (path); + if (ret) + gf_log (xlname, GF_LOG_WARNING, + "rmdir %s failed: %s", + path, strerror (errno)); + } +#endif + + return ret; +} diff --git a/libglusterfs/src/compat.h b/libglusterfs/src/compat.h index 5920fce992b..20d15c44e70 100644 --- a/libglusterfs/src/compat.h +++ b/libglusterfs/src/compat.h @@ -40,6 +40,10 @@ #ifdef HAVE_ENDIAN_H #include #endif + +#ifndef _PATH_UMOUNT +#define _PATH_UMOUNT "/bin/umount" +#endif #endif /* GF_LINUX_HOST_OS */ #ifdef HAVE_XATTR_H @@ -161,6 +165,9 @@ enum { #define FALLOC_FL_KEEP_SIZE 0x01 /* default is extend size */ #define FALLOC_FL_PUNCH_HOLE 0x02 /* de-allocates range */ +#ifndef _PATH_UMOUNT + #define _PATH_UMOUNT "/sbin/umount" +#endif #endif /* GF_BSD_HOST_OS */ #ifdef GF_DARWIN_HOST_OS @@ -239,6 +246,9 @@ int32_t gf_darwin_compat_listxattr (int len, dict_t *dict, int size); int32_t gf_darwin_compat_getxattr (const char *key, dict_t *dict); int32_t gf_darwin_compat_setxattr (dict_t *dict); +#ifndef _PATH_UMOUNT + #define _PATH_UMOUNT "/sbin/umount" +#endif #endif /* GF_DARWIN_HOST_OS */ #ifdef GF_SOLARIS_HOST_OS @@ -318,6 +328,9 @@ enum { #ifndef _PATH_MOUNTED #define _PATH_MOUNTED "/etc/mtab" #endif +#ifndef _PATH_UMOUNT + #define _PATH_UMOUNT "/sbin/umount" +#endif #ifndef O_ASYNC #ifdef FASYNC @@ -464,4 +477,6 @@ int gf_mkostemp (char *tmpl, int suffixlen, int flags); #pragma GCC poison system popen #endif +int gf_umount_lazy(char *xlname, char *path, int rmdir); + #endif /* __COMPAT_H__ */ diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c index 8e4071221de..cb10d4dcb3f 100644 --- a/xlators/mgmt/glusterd/src/glusterd-handler.c +++ b/xlators/mgmt/glusterd/src/glusterd-handler.c @@ -2825,12 +2825,16 @@ __glusterd_handle_umount (rpcsvc_request_t *req) goto out; } - runinit (&runner); - runner_add_args (&runner, "umount", umnt_req.path, NULL); - if (umnt_req.lazy) - runner_add_arg (&runner, "-l"); synclock_unlock (&priv->big_lock); - rsp.op_ret = runner_run (&runner); + + if (umnt_req.lazy) { + rsp.op_ret = gf_umount_lazy (this->name, umnt_req.path, 0); + } else { + runinit (&runner); + runner_add_args (&runner, _PATH_UMOUNT, umnt_req.path, NULL); + rsp.op_ret = runner_run (&runner); + } + synclock_lock (&priv->big_lock); if (rsp.op_ret == 0) { if (realpath (umnt_req.path, mntp)) diff --git a/xlators/mgmt/glusterd/src/glusterd-quota.c b/xlators/mgmt/glusterd/src/glusterd-quota.c index 7338e826ca9..f1e0417dbe5 100644 --- a/xlators/mgmt/glusterd/src/glusterd-quota.c +++ b/xlators/mgmt/glusterd/src/glusterd-quota.c @@ -256,11 +256,9 @@ glusterd_quota_initiate_fs_crawl (glusterd_conf_t *priv, char *volname, #ifndef GF_LINUX_HOST_OS runner_end (&runner); /* blocks in waitpid */ - runcmd ("umount", mountdir, NULL); -#else - runcmd ("umount", "-l", mountdir, NULL); #endif - rmdir (mountdir); + gf_umount_lazy ("glusterd", mountdir, 1); + _exit (EXIT_SUCCESS); } ret = (waitpid (pid, &status, 0) == pid && diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c index 7c2b545905d..8bfa2d4bdd6 100644 --- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c +++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c @@ -849,20 +849,6 @@ rb_spawn_glusterfs_client (glusterd_volinfo_t *volinfo, if (ret) goto out; - runinit (&runner); - runner_add_args (&runner, "/bin/umount", "-l", mntpt, NULL); - ret = runner_run_reuse (&runner); - if (ret) { - runner_log (&runner, this->name, GF_LOG_DEBUG, - "Lazy unmount failed on maintenance client"); - runner_end (&runner); - goto out; - } else { - runner_log (&runner, this->name, GF_LOG_DEBUG, - "Successfully unmounted maintenance client"); - runner_end (&runner); - } - out: @@ -1049,27 +1035,6 @@ out: return ret; } -static int -rb_mountpoint_rmdir (glusterd_volinfo_t *volinfo, - glusterd_brickinfo_t *src_brickinfo) -{ - char mntpt[PATH_MAX] = {0,}; - int ret = -1; - - GLUSTERD_GET_RB_MNTPT (mntpt, sizeof (mntpt), volinfo); - ret = rmdir (mntpt); - if (ret) { - gf_log ("", GF_LOG_DEBUG, "rmdir failed, reason: %s", - strerror (errno)); - goto out; - } - - ret = 0; - -out: - return ret; -} - static int rb_destroy_maintenance_client (glusterd_volinfo_t *volinfo, glusterd_brickinfo_t *src_brickinfo) @@ -1079,6 +1044,7 @@ rb_destroy_maintenance_client (glusterd_volinfo_t *volinfo, char volfile[PATH_MAX] = {0,}; int ret = -1; int mntfd = -1; + char mntpt[PATH_MAX] = {0,}; this = THIS; priv = this->private; @@ -1094,11 +1060,14 @@ rb_destroy_maintenance_client (glusterd_volinfo_t *volinfo, goto out; } - ret = rb_mountpoint_rmdir (volinfo, src_brickinfo); + GLUSTERD_GET_RB_MNTPT (mntpt, sizeof (mntpt), volinfo); + ret = gf_umount_lazy (this->name, mntpt, 1); if (ret) { - gf_log (this->name, GF_LOG_DEBUG, "rmdir of mountpoint " - "failed"); - goto out; + gf_log (this->name, GF_LOG_WARNING, + "Lazy unmount failed on maintenance client"); + } else { + gf_log (this->name, GF_LOG_DEBUG, + "Successfully unmounted maintenance client"); } snprintf (volfile, PATH_MAX, "%s/vols/%s/%s", priv->workdir, diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index bee17e50c51..0696725c717 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -11972,7 +11972,6 @@ int glusterd_remove_auxiliary_mount (char *volname) { int ret = -1; - runner_t runner = {0,}; char mountdir[PATH_MAX] = {0,}; char pidfile[PATH_MAX] = {0,}; xlator_t *this = NULL; @@ -11989,20 +11988,11 @@ glusterd_remove_auxiliary_mount (char *volname) } GLUSTERD_GET_QUOTA_AUX_MOUNT_PATH (mountdir, volname, "/"); - runinit (&runner); - runner_add_args (&runner, "umount", - -#if GF_LINUX_HOST_OS - "-l", -#endif - mountdir, NULL); - ret = runner_run_reuse (&runner); + ret = gf_umount_lazy (this->name, mountdir, 1); if (ret) gf_log (this->name, GF_LOG_ERROR, "umount on %s failed, " "reason : %s", mountdir, strerror (errno)); - runner_end (&runner); - rmdir (mountdir); return ret; } @@ -12127,7 +12117,7 @@ glusterd_umount (const char *path) runinit (&runner); snprintf (msg, sizeof (msg), "umount path %s", path); - runner_add_args (&runner, "umount", "-f", path, NULL); + runner_add_args (&runner, _PATH_UMOUNT, "-f", path, NULL); runner_log (&runner, this->name, GF_LOG_DEBUG, msg); ret = runner_run (&runner); if (ret) diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c index 334aef9f412..837e4ec763f 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c @@ -2003,7 +2003,6 @@ glusterd_stop_volume (glusterd_volinfo_t *volinfo) int ret = -1; glusterd_brickinfo_t *brickinfo = NULL; char mountdir[PATH_MAX] = {0,}; - runner_t runner = {0,}; char pidfile[PATH_MAX] = {0,}; xlator_t *this = NULL; @@ -2040,19 +2039,11 @@ glusterd_stop_volume (glusterd_volinfo_t *volinfo) GLUSTERD_GET_QUOTA_AUX_MOUNT_PATH (mountdir, volinfo->volname, "/"); - runinit (&runner); - runner_add_args (&runner, "umount", - - #if GF_LINUX_HOST_OS - "-l", - #endif - mountdir, NULL); - ret = runner_run_reuse (&runner); + ret = gf_umount_lazy (this->name, mountdir, 0); if (ret) - gf_log (this->name, GF_LOG_ERROR, "umount on %s failed, " - "reason : %s", mountdir, strerror (errno)); - - runner_end (&runner); + gf_log (this->name, GF_LOG_ERROR, + "umount on %s failed, reason : %s", + mountdir, strerror (errno)); } ret = glusterd_handle_snapd_option (volinfo); @@ -2258,7 +2249,7 @@ glusterd_clearlocks_unmount (glusterd_volinfo_t *volinfo, char *mntpt) * stat() on mount can be due to network failures.*/ runinit (&runner); - runner_add_args (&runner, "/bin/umount", "-f", NULL); + runner_add_args (&runner, _PATH_UMOUNT, "-f", NULL); runner_argprintf (&runner, "%s", mntpt); synclock_unlock (&priv->big_lock); -- cgit