fuse: clean up mount flag processing

In general, when one invokes a mount helper program -- basically
anything that mounts something based on its command line, so thinking of
mount(8), mount.<fs-type> or fusermount, but also of FUSE servers in
general, including glusterfs -- the command line arguments that are to
affect mount(2) are mapped to a bitmask called the mount flags, which is
passed to mount(2), so that the kernel can interpret the flag bits and
adjusts properties of the mount accordingly.

There is a traditional syntax for this mechanism as implemented in
mount(8): one passes "-ocomma,separated,mount,options" and the
individual option name strings are mapped to flag bits in mount(8).

FUSE further explores this idea and typically the FUSE server command
lines allow further option names to be used in the "-ooption,name,list"
which are then separated from the kernel sanctioned option names (to
which we'll refer as "system mount options") and are passed to a
platform specific lower level fuse mount helper interface.

The separation of system mount option names and FUSE specific option
names is also platform specific, so the general mount interface
function, which in case of glusterfs is gf_fuse_mount(), should abstract
this away.

Therefore we change the signature of this function from

        int gf_fuse_mount (const char *mountpoint, char *fsname,
                           unsigned long mountflags, char *mnt_param,
                           pid_t *mtab_pid, int status_fd);

to

        int gf_fuse_mount (const char *mountpoint, char *fsname,
                           char *mnt_param, pid_t *mtab_pid,
                           int status_fd);

and deal with flag extraction in platform specific mount code. Note that
the sole purpose of the mountflags argument was to indicate read-only
mounting. The other system mount option names were expected to reside in
the comma-separated mnt_param string, but they were not properly
processed (see the referred BUG). With the new gf_fuse_mount signature
read-only mounting is to be indicated as a "ro" component in mnt_param.

- For Darwin, which has a dedicated, separate gf_fuse_mount
  implementation, gf_fuse_mount was ignoring mountflags, so only the
  signature had to to be adjusted. However, as bonus, we gain read-only
  support for Darwin, which was missing so far, given that it was
  indicated via the ignored mountflags. Darwin's low level mount helper
  relies on the "ro" component of the option string, which agrees with
  the new calling convention of gf_fuse_mount.

- On Linux, system mount option name handling (apart from the
  distinguished read-only option) used to have the inadvertent side
  effect of adding "nosuid,nodev" as indicated in BUG; since
  Ia89d975d1e27fcfa5ab2036ba546aa8fa0d2d1b0 this side effect is removed,
  but system mount option name handling was left broken (passing system
  mount options other than "ro" fails to mount).

- On other platforms, system mount option name handling is broken
  (expect for the distinguished read-only option).

As of this change, in the general (non-Darwin) implementation of
gf_fuse_mount we take care of proper separation of system mount names
and their conversion to mount flags. For Linux, we adopt the conversion
table from FUSE upstream. For other systems we just provide a best
effort to support those system mount options which are understood across
all Unices (nosuid,nodev,noatime,noexec,ro). (This can be improved later
to provide proper plaform support.)

BUG: 1297182
Change-Id: I5d10b5df46feba7a02bf5bf1018db69e6b52260a
Signed-off-by: Csaba Henk <csaba@redhat.com>
Reviewed-on: https://review.gluster.org/16313
Smoke: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Tested-by: Amar Tumballi <amarts@redhat.com>

1 files changed, 3 insertions, 6 deletions

diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c
index 6e5cd63a81..7056b6fd16 100644
--- a/xlators/mount/fuse/src/fuse-bridge.c
+++ b/xlators/mount/fuse/src/fuse-bridge.c
@@ -10,7 +10,6 @@
 
 #include <sys/wait.h>
 #include "fuse-bridge.h"
-#include "mount-gluster-compat.h"
 #include "glusterfs.h"
 #include "byte-order.h"
 #include "compat-errno.h"
@@ -5481,7 +5480,6 @@ init (xlator_t *this_xl)
         glusterfs_ctx_t   *ctx = NULL;
         gf_boolean_t       sync_to_mount = _gf_false;
         gf_boolean_t       fopen_keep_cache = _gf_false;
-        unsigned long      mntflags = 0;
         char              *mnt_args = NULL;
         eh_t              *event = NULL;
 
@@ -5717,10 +5715,9 @@ init (xlator_t *this_xl)
                 goto cleanup_exit;
         }
 
-        if (priv->read_only)
-                mntflags |= MS_RDONLY;
-        gf_asprintf (&mnt_args, "%s%s%sallow_other,max_read=131072",
+        gf_asprintf (&mnt_args, "%s%s%s%sallow_other,max_read=131072",
                      priv->acl ? "" : "default_permissions,",
+                     priv->read_only ? "ro," : "",
                      priv->fuse_mountopts ? priv->fuse_mountopts : "",
                      priv->fuse_mountopts ? "," : "");
         if (!mnt_args)
@@ -5732,7 +5729,7 @@ init (xlator_t *this_xl)
                 goto cleanup_exit;
         }
 
-        priv->fd = gf_fuse_mount (priv->mount_point, fsname, mntflags, mnt_args,
+        priv->fd = gf_fuse_mount (priv->mount_point, fsname, mnt_args,
                                   sync_to_mount ? &ctx->mnt_pid : NULL,
                                   priv->status_pipe[1]);
         if (priv->fd == -1)