LU-12859 llite: clear flock when using localflock

When mounting a client with "-o localflock" or equivalent option in
/etc/fstab, it does not clear out the "flock" mount option flag from
the superblock.  This results in "flock" still being the option used
and it displays both options in the /proc/mounts output:

  10.0.0.1@o2ib:/lfs on /mnt/lfs type lustre (rw,flock,localflock)

Mount a client with both "flock,localflock" as mount options and
verify that the "flock" option is cleared by "localflock", and
vice versa.  Verify that "noflock" clears both options.

Remove the "remount_client()" helper in conf-sanity.sh, since this
shadows a helper function of the same name in test-framework.sh and
is confusing.  Instead, use "mount_client()" now that it can accept
mount options, and just pass "remount" explicitly in a few places.

Fixes: 3613af3e15cb ("LU-10885 llite: enable flock mount option by default")
Test-Parameters: trivial testlist=conf-sanity
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
Change-Id: Ie31b0c4f6674c99d3ed5b73caa39cfc23d3ebbe5
Reviewed-on: https://review.whamcloud.com/36452
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Ben Evans <bevans@cray.com>
Reviewed-by: Hongchao Zhang <hongchao@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index 1edf572..e84f5e5 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -839,38 +839,38 @@ static int ll_options(char *options, struct ll_sb_info *sbi)
 
        CDEBUG(D_CONFIG, "Parsing opts %s\n", options);
 
-        while (*s1) {
-                CDEBUG(D_SUPER, "next opt=%s\n", s1);
-                tmp = ll_set_opt("nolock", s1, LL_SBI_NOLCK);
-                if (tmp) {
-                        *flags |= tmp;
-                        goto next;
-                }
-                tmp = ll_set_opt("flock", s1, LL_SBI_FLOCK);
-                if (tmp) {
-                        *flags |= tmp;
-                        goto next;
-                }
-                tmp = ll_set_opt("localflock", s1, LL_SBI_LOCALFLOCK);
-                if (tmp) {
-                        *flags |= tmp;
-                        goto next;
-                }
-                tmp = ll_set_opt("noflock", s1, LL_SBI_FLOCK|LL_SBI_LOCALFLOCK);
-                if (tmp) {
-                        *flags &= ~tmp;
-                        goto next;
-                }
-                tmp = ll_set_opt("user_xattr", s1, LL_SBI_USER_XATTR);
-                if (tmp) {
-                        *flags |= tmp;
-                        goto next;
-                }
-                tmp = ll_set_opt("nouser_xattr", s1, LL_SBI_USER_XATTR);
-                if (tmp) {
-                        *flags &= ~tmp;
-                        goto next;
-                }
+       while (*s1) {
+               CDEBUG(D_SUPER, "next opt=%s\n", s1);
+               tmp = ll_set_opt("nolock", s1, LL_SBI_NOLCK);
+               if (tmp) {
+                       *flags |= tmp;
+                       goto next;
+               }
+               tmp = ll_set_opt("flock", s1, LL_SBI_FLOCK);
+               if (tmp) {
+                       *flags = (*flags & ~LL_SBI_LOCALFLOCK) | tmp;
+                       goto next;
+               }
+               tmp = ll_set_opt("localflock", s1, LL_SBI_LOCALFLOCK);
+               if (tmp) {
+                       *flags = (*flags & ~LL_SBI_FLOCK) | tmp;
+                       goto next;
+               }
+               tmp = ll_set_opt("noflock", s1, LL_SBI_FLOCK|LL_SBI_LOCALFLOCK);
+               if (tmp) {
+                       *flags &= ~tmp;
+                       goto next;
+               }
+               tmp = ll_set_opt("user_xattr", s1, LL_SBI_USER_XATTR);
+               if (tmp) {
+                       *flags |= tmp;
+                       goto next;
+               }
+               tmp = ll_set_opt("nouser_xattr", s1, LL_SBI_USER_XATTR);
+               if (tmp) {
+                       *flags &= ~tmp;
+                       goto next;
+               }
                tmp = ll_set_opt("context", s1, 1);
                if (tmp)
                        goto next;
@@ -2762,7 +2762,7 @@ int ll_show_options(struct seq_file *seq, struct dentry *dentry)
 int ll_show_options(struct seq_file *seq, struct vfsmount *vfs)
 #endif
 {
-        struct ll_sb_info *sbi;
+       struct ll_sb_info *sbi;
 
 #ifdef HAVE_SUPEROPS_USE_DENTRY
        LASSERT((seq != NULL) && (dentry != NULL));
@@ -2772,20 +2772,25 @@ int ll_show_options(struct seq_file *seq, struct vfsmount *vfs)
        sbi = ll_s2sbi(vfs->mnt_sb);
 #endif
 
-        if (sbi->ll_flags & LL_SBI_NOLCK)
-                seq_puts(seq, ",nolock");
-
-        if (sbi->ll_flags & LL_SBI_FLOCK)
-                seq_puts(seq, ",flock");
+       if (sbi->ll_flags & LL_SBI_NOLCK)
+               seq_puts(seq, ",nolock");
 
-        if (sbi->ll_flags & LL_SBI_LOCALFLOCK)
-                seq_puts(seq, ",localflock");
+       /* "flock" is the default since 2.13, but it wasn't for many years,
+        * so it is still useful to print this to show it is enabled.
+        * Start to print "noflock" so it is now clear when flock is disabled.
+        */
+       if (sbi->ll_flags & LL_SBI_FLOCK)
+               seq_puts(seq, ",flock");
+       else if (sbi->ll_flags & LL_SBI_LOCALFLOCK)
+               seq_puts(seq, ",localflock");
+       else
+               seq_puts(seq, ",noflock");
 
-        if (sbi->ll_flags & LL_SBI_USER_XATTR)
-                seq_puts(seq, ",user_xattr");
+       if (sbi->ll_flags & LL_SBI_USER_XATTR)
+               seq_puts(seq, ",user_xattr");
 
-        if (sbi->ll_flags & LL_SBI_LAZYSTATFS)
-                seq_puts(seq, ",lazystatfs");
+       if (sbi->ll_flags & LL_SBI_LAZYSTATFS)
+               seq_puts(seq, ",lazystatfs");
 
        if (sbi->ll_flags & LL_SBI_USER_FID2PATH)
                seq_puts(seq, ",user_fid2path");
@@ -2793,7 +2798,7 @@ int ll_show_options(struct seq_file *seq, struct vfsmount *vfs)
        if (sbi->ll_flags & LL_SBI_ALWAYS_PING)
                seq_puts(seq, ",always_ping");
 
-        RETURN(0);
+       RETURN(0);
 }
 
 /**

diff --git a/lustre/tests/conf-sanity.sh b/lustre/tests/conf-sanity.sh
index 27fc2b1..b06db01 100644 (file)
--- a/lustre/tests/conf-sanity.sh
+++ b/lustre/tests/conf-sanity.sh
@@ -175,16 +175,11 @@ stop_ost2() {
 }
 
 mount_client() {
-       local MOUNTPATH=$1
-       echo "mount $FSNAME on ${MOUNTPATH}....."
-       zconf_mount $(hostname) $MOUNTPATH || return 96
-}
+       local mountpath=$1
+       local mountopt="$2"
 
-remount_client() {
-       local mountopt="remount,$1"
-       local MOUNTPATH=$2
-       echo "remount '$1' lustre on ${MOUNTPATH}....."
-       zconf_mount $(hostname) $MOUNTPATH "$mountopt" || return 96
+       echo "mount $FSNAME ${mountopt:+with opts $mountopt} on $mountpath....."
+       zconf_mount $HOSTNAME $mountpath $mountopt || return 96
 }
 
 umount_client() {
@@ -706,10 +701,10 @@ test_20() {
        mount_client $MOUNT || error "mount_client $MOUNT failed"
        check_mount || error "check_mount failed"
        rm -f $DIR/$tfile || error "remove $DIR/$tfile failed."
-       remount_client ro $MOUNT || error "remount_client with ro failed"
+       mount_client $MOUNT remount,ro || error "remount client with ro failed"
        touch $DIR/$tfile && error "$DIR/$tfile created incorrectly"
        [ -e $DIR/$tfile ] && error "$DIR/$tfile exists incorrectly"
-       remount_client rw $MOUNT || error "remount_client with rw failed"
+       mount_client $MOUNT remount,rw || error "remount client with rw failed"
        touch $DIR/$tfile || error "touch $DIR/$tfile failed"
        MCNT=$(grep -c $MOUNT' ' /etc/mtab)
        [ "$MCNT" -ne 1 ] && error "$MOUNT in /etc/mtab $MCNT times"
@@ -7265,7 +7260,7 @@ test_98()
        for ((x = 1; x <= 400; x++)); do
                mountopt="$mountopt,user_xattr"
        done
-       remount_client $mountopt $MOUNT  2>&1 | grep "too long" ||
+       mount_client $MOUNT remount,$mountopt 2>&1 | grep "too long" ||
                error "Buffer overflow check failed"
        cleanup || error "cleanup failed"
 }
@@ -7527,7 +7522,7 @@ test_103() {
 }
 run_test 103 "rename filesystem name"
 
-test_104() { # LU-6952
+test_104a() { # LU-6952
        local mds_mountopts=$MDS_MOUNT_OPTS
        local ost_mountopts=$OST_MOUNT_OPTS
        local mds_mountfsopts=$MDS_MOUNT_FS_OPTS
@@ -7577,7 +7572,22 @@ test_104() { # LU-6952
        OST_MOUNT_OPTS=$ost_mountopts
        MDS_MOUNT_FS_OPTS=$mds_mountfsopts
 }
-run_test 104 "Make sure user defined options are reflected in mount"
+run_test 104a "Make sure user defined options are reflected in mount"
+
+test_104b() { # LU-12859
+       mount_client $MOUNT3 flock,localflock
+       stack_trap "umount_client $MOUNT3" EXIT
+       mount | grep "$MOUNT3 .*,flock" && error "flock is still set"
+       mount | grep "$MOUNT3 .*,localflock" || error "localflock is not set"
+       umount_client $MOUNT3
+       mount_client $MOUNT3 localflock,flock
+       mount | grep "$MOUNT3 .*,localflock" && error "localflock is still set"
+       mount | grep "$MOUNT3 .*,flock" || error "flock is not set"
+       umount_client $MOUNT3
+       mount_client $MOUNT3 localflock,flock,noflock
+       flock_is_enabled $MOUNT3 && error "some flock is still enabled" || true
+}
+run_test 104b "Mount uses last flock argument"
 
 error_and_umount() {
        umount $TMP/$tdir

diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh
index 31e7ec3..896a983 100755 (executable)
--- a/lustre/tests/test-framework.sh
+++ b/lustre/tests/test-framework.sh
@@ -5948,8 +5948,10 @@ lru_resize_disable()
 
 flock_is_enabled()
 {
+       local mountpath=${1:-$MOUNT}
        local RC=0
-       [ -z "$(mount | grep "$MOUNT.*flock" | grep -v noflock)" ] && RC=1
+
+       [ -z "$(mount | grep "$mountpath .*flock" | grep -v noflock)" ] && RC=1
        return $RC
 }