LU-10906 checksum: enable/disable checksum correctly

There are three ways to set checksum support in Lustre. Their
order during client mount is:
- 1. configure --enable/disable-checksum, this(ENABLE_CHECKSUM)
  only affects the default mount option and is set in function
  client_obd_setup().
- 2. lctl set_param -P osc.*.checksums=0/1, when processing llog,
  this value will be set by osc_checksum_seq_write().
- 3. mount option checksum/nochecksum, this will be checked in
  ll_options() and be set in client_common_fill_super()->
  obd_set_info_async().

This patch fixes one issue in 3. That is if mount option
"-o checksum/nochecksum" is specified, checksum will be changed
accordingly, no matter what is set by "set_param -P" or the
default option; and if no mount option is specified, the value
set by "set_param -P" will be kept. Also, test_77k is added to
sanity.sh to verify this patch.

What's more, a minor initialization issue of cl_supp_cksum_types
is fixed. cl_supp_cksum_types should be always initialized no
matter checksum is enabled or not.

Change-Id: I95d73122d800f5cd44b5fabb0cf00b5be0a35443
Signed-off-by: Emoly Liu <emoly.liu@intel.com>
Reviewed-on: https://review.whamcloud.com/32095
Reviewed-by: Yingjin Qian <qian@ddn.com>
Tested-by: Jenkins
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Maloo <hpdd-maloo@intel.com>

diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c
index 278e5d5..deaa1bd 100644 (file)
--- a/lustre/ldlm/ldlm_lib.c
+++ b/lustre/ldlm/ldlm_lib.c
@@ -397,6 +397,8 @@ int client_obd_setup(struct obd_device *obddev, struct lustre_cfg *lcfg)
 
        init_waitqueue_head(&cli->cl_destroy_waitq);
        atomic_set(&cli->cl_destroy_in_flight, 0);
+
+       cli->cl_supp_cksum_types = OBD_CKSUM_CRC32;
 #ifdef ENABLE_CHECKSUM
        /* Turn on checksumming by default. */
        cli->cl_checksum = 1;
@@ -405,7 +407,7 @@ int client_obd_setup(struct obd_device *obddev, struct lustre_cfg *lcfg)
         * Set cl_chksum* to CRC32 for now to avoid returning screwed info
         * through procfs.
         */
-       cli->cl_cksum_type = cli->cl_supp_cksum_types = OBD_CKSUM_CRC32;
+       cli->cl_cksum_type = cli->cl_supp_cksum_types;
 #endif
        atomic_set(&cli->cl_resends, OSC_DEFAULT_RESENDS);
 

diff --git a/lustre/llite/llite_internal.h b/lustre/llite/llite_internal.h
index b885014..a83b131 100644 (file)
--- a/lustre/llite/llite_internal.h
+++ b/lustre/llite/llite_internal.h
@@ -491,7 +491,8 @@ struct ll_sb_info {
        unsigned int              ll_umounting:1,
                                  ll_xattr_cache_enabled:1,
                                  ll_xattr_cache_set:1, /* already set to 0/1 */
-                                 ll_client_common_fill_super_succeeded:1;
+                                 ll_client_common_fill_super_succeeded:1,
+                                 ll_checksum_set:1;
 
         struct lustre_client_ocd  ll_lco;
 

diff --git a/lustre/llite/llite_lib.c b/lustre/llite/llite_lib.c
index 0615e85..5d116f6 100644 (file)
--- a/lustre/llite/llite_lib.c
+++ b/lustre/llite/llite_lib.c
@@ -571,13 +571,15 @@ static int client_common_fill_super(struct super_block *sb, char *md, char *dt,
        }
 
        checksum = sbi->ll_flags & LL_SBI_CHECKSUM;
-       err = obd_set_info_async(NULL, sbi->ll_dt_exp, sizeof(KEY_CHECKSUM),
-                                KEY_CHECKSUM, sizeof(checksum), &checksum,
-                                NULL);
-       if (err) {
-               CERROR("%s: Set checksum failed: rc = %d\n",
-                      sbi->ll_dt_exp->exp_obd->obd_name, err);
-               GOTO(out_root, err);
+       if (sbi->ll_checksum_set) {
+               err = obd_set_info_async(NULL, sbi->ll_dt_exp,
+                                        sizeof(KEY_CHECKSUM), KEY_CHECKSUM,
+                                        sizeof(checksum), &checksum, NULL);
+               if (err) {
+                       CERROR("%s: Set checksum failed: rc = %d\n",
+                              sbi->ll_dt_exp->exp_obd->obd_name, err);
+                       GOTO(out_root, err);
+               }
        }
        cl_sb_init(sb);
 
@@ -774,23 +776,24 @@ void ll_kill_super(struct super_block *sb)
 
 static inline int ll_set_opt(const char *opt, char *data, int fl)
 {
-        if (strncmp(opt, data, strlen(opt)) != 0)
-                return(0);
-        else
-                return(fl);
+       if (strncmp(opt, data, strlen(opt)) != 0)
+               return 0;
+       else
+               return fl;
 }
 
 /* non-client-specific mount options are parsed in lmd_parse */
-static int ll_options(char *options, int *flags)
+static int ll_options(char *options, struct ll_sb_info *sbi)
 {
-        int tmp;
-        char *s1 = options, *s2;
-        ENTRY;
+       int tmp;
+       char *s1 = options, *s2;
+       int *flags = &sbi->ll_flags;
+       ENTRY;
 
-        if (!options)
-                RETURN(0);
+       if (!options)
+               RETURN(0);
 
-        CDEBUG(D_CONFIG, "Parsing opts %s\n", options);
+       CDEBUG(D_CONFIG, "Parsing opts %s\n", options);
 
         while (*s1) {
                 CDEBUG(D_SUPER, "next opt=%s\n", s1);
@@ -847,16 +850,18 @@ static int ll_options(char *options, int *flags)
                        goto next;
                }
 
-                tmp = ll_set_opt("checksum", s1, LL_SBI_CHECKSUM);
-                if (tmp) {
-                        *flags |= tmp;
-                        goto next;
-                }
-                tmp = ll_set_opt("nochecksum", s1, LL_SBI_CHECKSUM);
-                if (tmp) {
-                        *flags &= ~tmp;
-                        goto next;
-                }
+               tmp = ll_set_opt("checksum", s1, LL_SBI_CHECKSUM);
+               if (tmp) {
+                       *flags |= tmp;
+                       sbi->ll_checksum_set = 1;
+                       goto next;
+               }
+               tmp = ll_set_opt("nochecksum", s1, LL_SBI_CHECKSUM);
+               if (tmp) {
+                       *flags &= ~tmp;
+                       sbi->ll_checksum_set = 1;
+                       goto next;
+               }
                 tmp = ll_set_opt("lruresize", s1, LL_SBI_LRU_RESIZE);
                 if (tmp) {
                         *flags |= tmp;
@@ -1022,7 +1027,7 @@ int ll_fill_super(struct super_block *sb, struct vfsmount *mnt)
        if (!sbi)
                GOTO(out_free, err = -ENOMEM);
 
-       err = ll_options(lsi->lsi_lmd->lmd_opts, &sbi->ll_flags);
+       err = ll_options(lsi->lsi_lmd->lmd_opts, sbi);
        if (err)
                GOTO(out_free, err);
 

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index 28985d2..7135751 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -7186,6 +7186,46 @@ test_77j() { # bug 13805
 }
 run_test 77j "client only supporting ADLER32"
 
+test_77k() { # LU-10906
+       [ $PARALLEL == "yes" ] && skip "skip parallel run"
+       $GSS && skip_env "could not run with gss"
+
+       local cksum_param="osc.$FSNAME*.checksums"
+       local get_checksum="$LCTL get_param -n $cksum_param | head -n1"
+       local checksum
+       local i
+
+       [ "$ORIG_CSUM" ] || ORIG_CSUM=$(eval $get_checksum)
+       stack_trap "wait_update $HOSTNAME '$get_checksum' $ORIG_CSUM" EXIT
+       stack_trap "do_facet mgs $LCTL set_param -P $cksum_param=$ORIG_CSUM" \
+               EXIT
+
+       for i in 0 1; do
+               do_facet mgs $LCTL set_param -P $cksum_param=$i ||
+                       error "failed to set checksum=$i on MGS"
+               wait_update $HOSTNAME "$get_checksum" $i
+               #remount
+               echo "remount client, checksum should be $i"
+               remount_client $MOUNT || "failed to remount client"
+               checksum=$(eval $get_checksum)
+               [ $checksum -eq $i ] || error "checksum($checksum) != $i"
+       done
+
+       for opt in "checksum" "nochecksum"; do
+               #remount with mount option
+               echo "remount client with option $opt, checksum should be $i"
+               umount_client $MOUNT || "failed to umount client"
+               mount_client $MOUNT "$MOUNT_OPTS,$opt" ||
+                       "failed to mount client with option '$opt'"
+               checksum=$(eval $get_checksum)
+               [ $checksum -eq $i ] || error "checksum($checksum) != $i"
+               i=$((i - 1))
+       done
+
+       remount_client $MOUNT || "failed to remount client"
+}
+run_test 77k "enable/disable checksum correctly"
+
 [ "$ORIG_CSUM" ] && set_checksums $ORIG_CSUM || true
 rm -f $F77_TMP
 unset F77_TMP