From 702b2912970e7cc19416aff7d3696d15977efc2f Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Fri, 9 Nov 2012 14:43:38 +0100 Subject: fuse: handle mountflags properly The internal mount API had no access to the generic mountflags used by mount(2). Thus the "ro" mount option that needs to be passed down to mount(2) as as a mountflag was incorrectly mangled into the fuse-specific mount parameter string (cf. http://review.gluster.com/655). This commit fixes the internal API and the "ro" issue. It also adds a check for the "rw" and "ro" mount options in tests/basic/mount.t. Thanks to Csaba Henk (csaba@) for suggestions and proposing an updated patch. Change-Id: I7f7bf49ae44d148f5c16f10736a0e412fb8f5e67 BUG: 853895 Signed-off-by: Niels de Vos Reviewed-on: http://review.gluster.org/4163 Reviewed-by: Csaba Henk Tested-by: Gluster Build System --- contrib/fuse-include/fuse-mount.h | 3 ++- contrib/fuse-lib/mount.c | 32 +++++++++++++++++++++++--------- tests/basic/mount.t | 18 +++++++++++++++++- xlators/mount/fuse/src/fuse-bridge.c | 8 +++++--- 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/contrib/fuse-include/fuse-mount.h b/contrib/fuse-include/fuse-mount.h index 7a3756d92..9358ac810 100644 --- a/contrib/fuse-include/fuse-mount.h +++ b/contrib/fuse-include/fuse-mount.h @@ -8,5 +8,6 @@ */ 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, + unsigned long mountflags, char *mnt_param, pid_t *mtab_pid, int status_fd); diff --git a/contrib/fuse-lib/mount.c b/contrib/fuse-lib/mount.c index f02a835b3..fedde3fce 100644 --- a/contrib/fuse-lib/mount.c +++ b/contrib/fuse-lib/mount.c @@ -100,7 +100,8 @@ escape (char *s) } static int -fuse_mount_fusermount (const char *mountpoint, char *fsname, char *mnt_param, +fuse_mount_fusermount (const char *mountpoint, char *fsname, + unsigned long mountflags, char *mnt_param, int fd) { int pid = -1; @@ -124,7 +125,8 @@ fuse_mount_fusermount (const char *mountpoint, char *fsname, char *mnt_param, return -1; } ret = asprintf (&fm_mnt_params, - "%s,fsname=%s,nonempty,subtype=glusterfs", + "%s%s,fsname=%s,nonempty,subtype=glusterfs", + (mountflags & MS_RDONLY) ? "ro," : "", mnt_param, efsname); FREE (efsname); if (ret == -1) { @@ -169,7 +171,8 @@ fuse_mount_fusermount (const char *mountpoint, char *fsname, char *mnt_param, } static int -fuse_mount_sys (const char *mountpoint, char *fsname, char *mnt_param, int fd) +fuse_mount_sys (const char *mountpoint, char *fsname, + unsigned long mountflags, char *mnt_param, int fd) { int ret = -1; unsigned mounted = 0; @@ -185,7 +188,7 @@ fuse_mount_sys (const char *mountpoint, char *fsname, char *mnt_param, int fd) goto out; } - ret = mount (source, mountpoint, fstype, 0, + ret = mount (source, mountpoint, fstype, mountflags, mnt_param_mnt); if (ret == -1 && errno == ENODEV) { /* fs subtype support was added by 79c0b2df aka @@ -209,6 +212,7 @@ fuse_mount_sys (const char *mountpoint, char *fsname, char *mnt_param, int fd) #ifndef __NetBSD__ if (geteuid () == 0) { char *newmnt = fuse_mnt_resolve_path ("fuse", mountpoint); + char *mnt_param_mtab = NULL; if (!newmnt) { ret = -1; @@ -216,8 +220,17 @@ fuse_mount_sys (const char *mountpoint, char *fsname, char *mnt_param, int fd) goto out; } - ret = fuse_mnt_add_mount ("fuse", source, newmnt, fstype, - mnt_param); + ret = asprintf (&mnt_param_mtab, "%s%s", + mountflags & MS_RDONLY ? "ro," : "", + mnt_param); + if (ret == -1) + GFFUSE_LOGERR ("Out of memory"); + else { + ret = fuse_mnt_add_mount ("fuse", source, newmnt, + fstype, mnt_param_mtab); + FREE (mnt_param_mtab); + } + FREE (newmnt); if (ret == -1) { GFFUSE_LOGERR ("failed to add mtab entry"); @@ -240,7 +253,8 @@ out: } int -gf_fuse_mount (const char *mountpoint, char *fsname, char *mnt_param, +gf_fuse_mount (const char *mountpoint, char *fsname, + unsigned long mountflags, char *mnt_param, pid_t *mnt_pid, int status_fd) { int fd = -1; @@ -268,14 +282,14 @@ gf_fuse_mount (const char *mountpoint, char *fsname, char *mnt_param, exit (pid == -1 ? 1 : 0); } - ret = fuse_mount_sys (mountpoint, fsname, mnt_param, fd); + ret = fuse_mount_sys (mountpoint, fsname, mountflags, mnt_param, fd); if (ret == -1) { gf_log ("glusterfs-fuse", GF_LOG_INFO, "direct mount failed (%s), " "retry to mount via fusermount", strerror (errno)); - ret = fuse_mount_fusermount (mountpoint, fsname, + ret = fuse_mount_fusermount (mountpoint, fsname, mountflags, mnt_param, fd); } diff --git a/tests/basic/mount.t b/tests/basic/mount.t index 0fdef65de..7b2769643 100755 --- a/tests/basic/mount.t +++ b/tests/basic/mount.t @@ -34,9 +34,20 @@ EXPECT 'Started' volinfo_field $V0 'Status'; ## Make volume tightly consistent for metdata TEST $CLI volume set $V0 performance.stat-prefetch off; -## Mount FUSE with caching disabled +## Mount FUSE with caching disabled (read-write) TEST glusterfs --entry-timeout=0 --attribute-timeout=0 -s $H0 --volfile-id $V0 $M0; +## Check consistent "rw" option +TEST 'mount -t fuse.glusterfs | grep -E "^$H0:$V0 .+ \(rw,"'; +TEST 'grep -E "^$H0:$V0 .+ ,?rw," /proc/mounts'; + +## Mount FUSE with caching disabled (read-only) +TEST glusterfs --entry-timeout=0 --attribute-timeout=0 --read-only -s $H0 --volfile-id $V0 $M1; + +## Check consistent "ro" option +TEST 'mount -t fuse.glusterfs | grep -E "^$H0:$V0 .+ \(ro,"'; +TEST 'grep -E "^$H0:$V0 .+ ,?ro,.+" /proc/mounts'; + ## Wait for volume to register with rpc.mountd sleep 5; @@ -45,11 +56,16 @@ TEST mount -t nfs -o vers=3,nolock,soft,intr $H0:/$V0 $N0; ## Test for consistent views between NFS and FUSE mounts +## write access to $M1 should fail TEST ! stat $M0/newfile; +TEST ! touch $M1/newfile; TEST touch $M0/newfile; +TEST stat $M1/newfile; TEST stat $N0/newfile; +TEST ! rm $M1/newfile; TEST rm $N0/newfile; TEST ! stat $M0/newfile; +TEST ! stat $M1/newfile; ## Finish up diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c index 9be7d91cc..544dec0c3 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -4678,6 +4678,7 @@ init (xlator_t *this_xl) int fsname_allocated = 0; glusterfs_ctx_t *ctx = NULL; gf_boolean_t sync_to_mount = _gf_false; + unsigned long mntflags = 0; char *mnt_args = NULL; eh_t *event = NULL; @@ -4889,8 +4890,9 @@ init (xlator_t *this_xl) goto cleanup_exit; } - gf_asprintf (&mnt_args, "%s%s%s%sallow_other,max_read=131072", - priv->read_only ? "ro," : "", + if (priv->read_only) + mntflags |= MS_RDONLY; + gf_asprintf (&mnt_args, "%s%s%sallow_other,max_read=131072", priv->acl ? "" : "default_permissions,", priv->fuse_mountopts ? priv->fuse_mountopts : "", priv->fuse_mountopts ? "," : ""); @@ -4903,7 +4905,7 @@ init (xlator_t *this_xl) goto cleanup_exit; } - priv->fd = gf_fuse_mount (priv->mount_point, fsname, mnt_args, + priv->fd = gf_fuse_mount (priv->mount_point, fsname, mntflags, mnt_args, sync_to_mount ? &ctx->mnt_pid : NULL, priv->status_pipe[1]); if (priv->fd == -1) -- cgit