From c5d781e05599e9e7ad736d42c9c1033992c76ded Mon Sep 17 00:00:00 2001 From: Csaba Henk Date: Sun, 15 May 2011 04:52:33 +0000 Subject: upon daemonizing, wait on mtab update to terminate in parent This fixes the race in between the mtab update attempts of mount and umount when we do a lazy umount right after mounting, in order to hide the given fs instance; yet this way we still avoid the deadlock of the fs and mount which we can hit if we wait unconditionally for the mtab update to terminate (cf. bz #511). Signed-off-by: Csaba Henk Signed-off-by: Anand Avati BUG: 2690 (race between mtab updates of mount and umount) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=2690 --- contrib/apple/daemon.c | 26 +++++++++++------- contrib/apple/daemon.h | 1 + contrib/fuse-include/fuse-mount.h | 3 ++- contrib/fuse-lib/mount.c | 52 +++++++++++++++++++++--------------- contrib/macfuse/mount_darwin.c | 3 ++- glusterfsd/src/Makefile.am | 10 ++----- glusterfsd/src/glusterfsd.c | 33 ++++++++++++++++++----- libglusterfs/src/glusterfs.h | 1 + xlators/mount/fuse/src/fuse-bridge.c | 20 +++++++++++++- 9 files changed, 101 insertions(+), 48 deletions(-) diff --git a/contrib/apple/daemon.c b/contrib/apple/daemon.c index 9389201a1..07dbbc400 100644 --- a/contrib/apple/daemon.c +++ b/contrib/apple/daemon.c @@ -44,7 +44,7 @@ #include int -os_daemon(nochdir, noclose) +os_daemon_return(nochdir, noclose) int nochdir, noclose; { struct sigaction osa, sa; @@ -52,6 +52,7 @@ os_daemon(nochdir, noclose) pid_t newgrp; int oerrno; int osa_ok; + int ret; /* A SIGHUP may be thrown when the parent exits below. */ sigemptyset(&sa.sa_mask); @@ -59,14 +60,9 @@ os_daemon(nochdir, noclose) sa.sa_flags = 0; osa_ok = sigaction(SIGHUP, &sa, &osa); - switch (fork()) { - case -1: - return (-1); - case 0: - break; - default: - _exit(0); - } + ret = fork(); + if (ret) + return ret; newgrp = setsid(); oerrno = errno; @@ -90,3 +86,15 @@ os_daemon(nochdir, noclose) } return (0); } + +int +os_daemon(int nochdir, int noclose) +{ + int ret; + + ret = os_daemon_return(nochdir, noclose); + if (ret <= 0) + return ret; + + _exit(0); +} diff --git a/contrib/apple/daemon.h b/contrib/apple/daemon.h index 7a2824b6a..aa88d9baa 100644 --- a/contrib/apple/daemon.h +++ b/contrib/apple/daemon.h @@ -17,4 +17,5 @@ . */ +int os_daemon_return(int nochdir, int noclose); int os_daemon(int nochdir, int noclose); diff --git a/contrib/fuse-include/fuse-mount.h b/contrib/fuse-include/fuse-mount.h index ca571ce5e..9f83faf02 100644 --- a/contrib/fuse-include/fuse-mount.h +++ b/contrib/fuse-include/fuse-mount.h @@ -8,4 +8,5 @@ */ void gf_fuse_unmount (const char *mountpoint, int fd); -int gf_fuse_mount (const char *mountpoint, char *fsname, char *mnt_param); +int gf_fuse_mount (const char *mountpoint, char *fsname, char *mnt_param, + pid_t *mtab_pid); diff --git a/contrib/fuse-lib/mount.c b/contrib/fuse-lib/mount.c index 47592a62d..f922b07d4 100644 --- a/contrib/fuse-lib/mount.c +++ b/contrib/fuse-lib/mount.c @@ -97,7 +97,8 @@ static #endif int fuse_mnt_add_mount (const char *progname, const char *fsname, - const char *mnt, const char *type, const char *opts) + const char *mnt, const char *type, const char *opts, + pid_t *mtab_pid) { int res; int status; @@ -125,19 +126,22 @@ fuse_mnt_add_mount (const char *progname, const char *fsname, char templ[] = "/tmp/fusermountXXXXXX"; char *tmp; - /* mtab update done async, just log if fails */ - res = fork (); - if (res) - exit (res == -1 ? 1 : 0); - res = fork (); - if (res) { - if (res != -1) - res = waitpid (res, &status, 0); - if (res == -1) - GFFUSE_LOGERR ("%s: /etc/mtab update failed", - progname); - - exit (0); + if (!mtab_pid) { + /* mtab update done async, just log if fails */ + res = fork (); + if (res) + exit (res == -1 ? 1 : 0); + res = fork (); + if (res) { + if (res != -1) { + if (!(res == waitpid (res, &status, 0) + && status == 0)) + GFFUSE_LOGERR ("%s: /etc/mtab " + "update failed", + progname); + } + exit (0); + } } sigprocmask (SIG_SETMASK, &oldmask, NULL); @@ -165,13 +169,16 @@ fuse_mnt_add_mount (const char *progname, const char *fsname, progname, strerror (errno)); exit (1); } - res = waitpid (res, &status, 0); + if (mtab_pid) { + *mtab_pid = res; + res = 0; + } else { + if (!(res == waitpid (res, &status, 0) && status == 0)) + res = -1; + } if (res == -1) GFFUSE_LOGERR ("%s: waitpid: %s", progname, strerror (errno)); - if (status != 0) - res = -1; - out_restore: sigprocmask (SIG_SETMASK, &oldmask, NULL); return res; @@ -519,7 +526,7 @@ gf_fuse_unmount (const char *mountpoint, int fd) #ifndef FUSE_UTIL static int -fuse_mount_sys (const char *mountpoint, char *fsname, char *mnt_param) +fuse_mount_sys (const char *mountpoint, char *fsname, char *mnt_param, pid_t *mtab_pid) { int fd = -1, ret = -1; unsigned mounted = 0; @@ -573,7 +580,7 @@ fuse_mount_sys (const char *mountpoint, char *fsname, char *mnt_param) } ret = fuse_mnt_add_mount ("fuse", source, newmnt, fstype, - mnt_param); + mnt_param, mtab_pid); FREE (newmnt); if (ret == -1) { GFFUSE_LOGERR ("failed to add mtab entry"); @@ -625,13 +632,14 @@ escape (char *s) } int -gf_fuse_mount (const char *mountpoint, char *fsname, char *mnt_param) +gf_fuse_mount (const char *mountpoint, char *fsname, char *mnt_param, + pid_t *mtab_pid) { int fd = -1, rv = -1; char *fm_mnt_params = NULL, *p = NULL; char *efsname = NULL; - fd = fuse_mount_sys (mountpoint, fsname, mnt_param); + fd = fuse_mount_sys (mountpoint, fsname, mnt_param, mtab_pid); if (fd == -1) { gf_log ("glusterfs-fuse", GF_LOG_INFO, "direct mount failed (%s), " diff --git a/contrib/macfuse/mount_darwin.c b/contrib/macfuse/mount_darwin.c index 9d87fca35..c485583e9 100644 --- a/contrib/macfuse/mount_darwin.c +++ b/contrib/macfuse/mount_darwin.c @@ -133,7 +133,8 @@ Return: } int -gf_fuse_mount (const char *mountpoint, char *fsname, char *mnt_param) +gf_fuse_mount (const char *mountpoint, char *fsname, char *mnt_param, + pid_t *mtab_pid /* not used on OS X */) { int fd, pid; int result; diff --git a/glusterfsd/src/Makefile.am b/glusterfsd/src/Makefile.am index 537eda0bb..2056ca898 100644 --- a/glusterfsd/src/Makefile.am +++ b/glusterfsd/src/Makefile.am @@ -1,9 +1,6 @@ sbin_PROGRAMS = glusterfsd -glusterfsd_SOURCES = glusterfsd.c glusterfsd-mgmt.c -if GF_DARWIN_HOST_OS -glusterfsd_SOURCES += $(CONTRIBDIR)/apple/daemon.c -endif +glusterfsd_SOURCES = glusterfsd.c glusterfsd-mgmt.c $(CONTRIBDIR)/apple/daemon.c glusterfsd_LDADD = $(top_builddir)/libglusterfs/src/libglusterfs.la \ $(top_builddir)/rpc/rpc-lib/src/libgfrpc.la \ $(top_builddir)/rpc/xdr/src/libgfxdr.la \ @@ -14,10 +11,7 @@ noinst_HEADERS = glusterfsd.h glusterfsd-mem-types.h AM_CFLAGS = -fPIC -Wall -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -D$(GF_HOST_OS)\ -I$(top_srcdir)/libglusterfs/src -DDATADIR=\"$(localstatedir)\" \ -DCONFDIR=\"$(sysconfdir)/glusterfs\" $(GF_GLUSTERFS_CFLAGS) \ - -I$(top_srcdir)/rpc/rpc-lib/src -I$(top_srcdir)/rpc/xdr/src -if GF_DARWIN_HOST_OS -AM_CFLAGS += -I$(CONTRIBDIR)/apple -endif + -I$(top_srcdir)/rpc/rpc-lib/src -I$(top_srcdir)/rpc/xdr/src -I$(CONTRIBDIR)/apple CLEANFILES = diff --git a/glusterfsd/src/glusterfsd.c b/glusterfsd/src/glusterfsd.c index 9406d74ac..2d2024409 100644 --- a/glusterfsd/src/glusterfsd.c +++ b/glusterfsd/src/glusterfsd.c @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -75,11 +76,7 @@ #include #include "rpc-clnt.h" -#ifdef GF_DARWIN_HOST_OS #include "daemon.h" -#else -#define os_daemon(u, v) daemon (u, v) -#endif /* using argp for command line parsing */ @@ -315,6 +312,16 @@ create_fuse_mount (glusterfs_ctx_t *ctx) break; } + if (!cmd_args->no_daemon_mode) { + ret = dict_set_static_ptr (master->options, "sync-mtab", + "enable"); + if (ret < 0) { + gf_log ("glusterfsd", GF_LOG_ERROR, + "failed to set dict value for key sync-mtab"); + goto err; + } + } + ret = xlator_init (master); if (ret) { gf_log ("", GF_LOG_DEBUG, "failed to initialize fuse translator"); @@ -1310,6 +1317,7 @@ daemonize (glusterfs_ctx_t *ctx) { int ret = -1; cmd_args_t *cmd_args = NULL; + int cstatus = 0; cmd_args = &ctx->cmd_args; @@ -1323,11 +1331,24 @@ daemonize (glusterfs_ctx_t *ctx) if (cmd_args->debug_mode) goto postfork; - ret = os_daemon (0, 0); - if (ret == -1) { + ret = os_daemon_return (0, 0); + switch (ret) { + case -1: gf_log ("daemonize", GF_LOG_ERROR, "Daemonization failed: %s", strerror(errno)); goto out; + case 0: + break; + default: + if (ctx->mtab_pid > 0) { + ret = waitpid (ctx->mtab_pid, &cstatus, 0); + if (!(ret == ctx->mtab_pid && cstatus == 0)) { + gf_log ("daemonize", GF_LOG_ERROR, + "/etc/mtab update failed"); + exit (1); + } + } + _exit (0); } postfork: diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h index 56b5e2b9c..5553009b1 100644 --- a/libglusterfs/src/glusterfs.h +++ b/libglusterfs/src/glusterfs.h @@ -340,6 +340,7 @@ struct _glusterfs_ctx { int graph_id; /* Incremented per graph, value should indicate how many times the graph has got changed */ + pid_t mtab_pid; /* pid of the process which updates the mtab */ }; typedef struct _glusterfs_ctx glusterfs_ctx_t; diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c index d34a747f4..dafc0a93c 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -3485,6 +3485,8 @@ init (xlator_t *this_xl) int i = 0; int xl_name_allocated = 0; int fsname_allocated = 0; + glusterfs_ctx_t *ctx = NULL; + gf_boolean_t sync_mtab = _gf_false; if (this_xl == NULL) return -1; @@ -3492,6 +3494,10 @@ init (xlator_t *this_xl) if (this_xl->options == NULL) return -1; + ctx = glusterfs_ctx_get (); + if (!ctx) + return -1; + options = this_xl->options; if (this_xl->name == NULL) { @@ -3604,6 +3610,14 @@ init (xlator_t *this_xl) priv->fuse_dump_fd = ret; } + sync_mtab = _gf_false; + ret = dict_get_str (options, "sync-mtab", &value_string); + if (ret == 0) { + ret = gf_string2boolean (value_string, + &sync_mtab); + GF_ASSERT (ret == 0); + } + cmd_args = &this_xl->ctx->cmd_args; fsname = cmd_args->volfile; if (!fsname && cmd_args->volfile_server) { @@ -3630,7 +3644,8 @@ init (xlator_t *this_xl) priv->fd = gf_fuse_mount (priv->mount_point, fsname, "allow_other,default_permissions," - "max_read=131072"); + "max_read=131072", + sync_mtab ? &ctx->mtab_pid : NULL); if (priv->fd == -1) goto cleanup_exit; @@ -3727,5 +3742,8 @@ struct volume_options options[] = { { .key = {"client-pid"}, .type = GF_OPTION_TYPE_INT }, + { .key = {"sync-mtab"}, + .type = GF_OPTION_TYPE_BOOL + }, { .key = {NULL} }, }; -- cgit