diff --git a/kmod/src/options.c b/kmod/src/options.c index 9a3d3471..2a19a9e5 100644 --- a/kmod/src/options.c +++ b/kmod/src/options.c @@ -131,10 +131,8 @@ static void init_default_options(struct scoutfs_mount_options *opts) opts->quorum_slot_nr = -1; } -static int set_quorum_heartbeat_timeout_ms(struct super_block *sb, int ret, u64 val) +static int verify_quorum_heartbeat_timeout_ms(struct super_block *sb, int ret, u64 val) { - DECLARE_OPTIONS_INFO(sb, optinf); - if (ret < 0) { scoutfs_err(sb, "failed to parse quorum_heartbeat_timeout_ms value"); return -EINVAL; @@ -145,10 +143,6 @@ static int set_quorum_heartbeat_timeout_ms(struct super_block *sb, int ret, u64 return -EINVAL; } - write_seqlock(&optinf->seqlock); - optinf->opts.quorum_heartbeat_timeout_ms = val; - write_sequnlock(&optinf->seqlock); - return 0; } @@ -232,9 +226,10 @@ static int parse_options(struct super_block *sb, char *options, struct scoutfs_m case Opt_quorum_heartbeat_timeout_ms: ret = match_u64(args, &nr64); - ret = set_quorum_heartbeat_timeout_ms(sb, ret, nr64); + ret = verify_quorum_heartbeat_timeout_ms(sb, ret, nr64); if (ret < 0) return ret; + opts->quorum_heartbeat_timeout_ms = nr64; break; case Opt_quorum_slot_nr: @@ -493,6 +488,7 @@ static ssize_t quorum_heartbeat_timeout_ms_store(struct kobject *kobj, struct ko const char *buf, size_t count) { struct super_block *sb = SCOUTFS_SYSFS_ATTRS_SB(kobj); + DECLARE_OPTIONS_INFO(sb, optinf); char nullterm[30]; /* more than enough for octal -U64_MAX */ u64 val; int len; @@ -503,9 +499,13 @@ static ssize_t quorum_heartbeat_timeout_ms_store(struct kobject *kobj, struct ko nullterm[len] = '\0'; ret = kstrtoll(nullterm, 0, &val); - ret = set_quorum_heartbeat_timeout_ms(sb, ret, val); - if (ret == 0) + ret = verify_quorum_heartbeat_timeout_ms(sb, ret, val); + if (ret == 0) { + write_seqlock(&optinf->seqlock); + optinf->opts.quorum_heartbeat_timeout_ms = val; + write_sequnlock(&optinf->seqlock); ret = count; + } return ret; } diff --git a/tests/funcs/fs.sh b/tests/funcs/fs.sh index 950e0e7e..b4442252 100644 --- a/tests/funcs/fs.sh +++ b/tests/funcs/fs.sh @@ -153,7 +153,27 @@ t_mount() test "$nr" -lt "$T_NR_MOUNTS" || \ t_fail "fs nr $nr invalid" - eval t_quiet mount -t scoutfs \$T_O$nr \$T_DB$nr \$T_M$nr + eval t_quiet mount -t scoutfs \$T_O$nr\$opt \$T_DB$nr \$T_M$nr +} + +# +# Mount with an optional mount option string. If the string is empty +# then the saved mount options are used. If the string has contents +# then it is appended to the end of the saved options with a separating +# comma. +# +# Unlike t_mount this won't inherently fail in t_quiet, errors are +# returned so bad options can be tested. +# +t_mount_opt() +{ + local nr="$1" + local opt="${2:+,$2}" + + test "$nr" -lt "$T_NR_MOUNTS" || \ + t_fail "fs nr $nr invalid" + + eval mount -t scoutfs \$T_O$nr\$opt \$T_DB$nr \$T_M$nr } t_umount() diff --git a/tests/golden/quorum-heartbeat-timeout b/tests/golden/quorum-heartbeat-timeout index ff389993..1638c30b 100644 --- a/tests/golden/quorum-heartbeat-timeout +++ b/tests/golden/quorum-heartbeat-timeout @@ -1,2 +1,5 @@ == bad timeout values fail -== test different timeouts +== bad mount option fails +== mount option +== sysfs +== reset all options diff --git a/tests/tests/quorum-heartbeat-timeout.sh b/tests/tests/quorum-heartbeat-timeout.sh index 0d0eb906..30563192 100644 --- a/tests/tests/quorum-heartbeat-timeout.sh +++ b/tests/tests/quorum-heartbeat-timeout.sh @@ -17,43 +17,52 @@ set_bad_timeout() { t_fail "set bad q hb to $to" } -set_quorum_timeouts() +set_timeout() { - local to="$1" - local was + local nr="$1" + local how="$2" + local to="$3" local is - for nr in $(t_quorum_nrs); do - local mnt="$(eval echo \$T_M$nr)" - - was=$(t_get_sysfs_mount_option $nr quorum_heartbeat_timeout_ms) + if [ $how == "sysfs" ]; then t_set_sysfs_mount_option $nr quorum_heartbeat_timeout_ms $to - is=$(t_get_sysfs_mount_option $nr quorum_heartbeat_timeout_ms) + fi + if [ $how == "mount" ]; then + t_umount $nr + t_mount_opt $nr "quorum_heartbeat_timeout_ms=$to" + fi - if [ "$is" != "$to" ]; then - t_fail "tried to set qhbto on $nr to $to but got $is" - fi - done + is=$(t_get_sysfs_mount_option $nr quorum_heartbeat_timeout_ms) + + if [ "$is" != "$to" ]; then + t_fail "tried to set qhbto on $nr via $how to $to but got $is" + fi } test_timeout() { - local to="$1" - local orig_to + local how="$1" + local to="$2" local start local nr + local sv local delay + local low + local high - # set new timeouts, saving original - orig_to=$(t_get_sysfs_mount_option 0 quorum_heartbeat_timeout_ms) - set_quorum_timeouts $to + # set timeout on non-server quorum mounts + sv=$(t_server_nr) + for nr in $(t_quorum_nrs); do + if [ $nr -ne $sv ]; then + set_timeout $nr $how $to + fi + done # give followers time to recv heartbeats and reset timeouts sleep 1 # tear down the current server/leader - nr=$(t_server_nr) - t_force_umount $nr + t_force_umount $sv # see how long it takes for the next leader to start start=$(time_ms) @@ -64,15 +73,15 @@ test_timeout() echo "to $to delay $delay" >> $T_TMP.delay # restore the mount that we tore down - t_mount $nr + t_mount $sv - # reset the original timeouts - set_quorum_timeouts $orig_to + # make sure the new leader delay was reasonable, allowing for some slack + low=$((to - 1000)) + high=$((to + 3000)) # make sure the new leader delay was reasonable - test "$delay" -gt "$to" || t_fail "delay $delay < to $to" - # allow 5 seconds of slop - test "$delay" -lt $(($to + 5000)) || t_fail "delay $delay > to $to + 5sec" + test "$delay" -lt "$low" && t_fail "delay $delay < low $low (to $to)" + test "$delay" -gt "$high" && t_fail "delay $delay > high $high (to $to)" } echo "== bad timeout values fail" @@ -80,10 +89,29 @@ set_bad_timeout 0 set_bad_timeout -1 set_bad_timeout 1000000 -echo "== test different timeouts" +echo "== bad mount option fails" +if [ "$(t_server_nr)" == 0 ]; then + nr=1 +else + nr=0 +fi +t_umount $nr +t_mount_opt $nr "quorum_heartbeat_timeout_ms=1000000" 2>/dev/null && \ + t_fail "bad mount option succeeded" +t_mount $nr + +echo "== mount option" def=$(t_get_sysfs_mount_option 0 quorum_heartbeat_timeout_ms) -test_timeout $def -test_timeout 3000 -test_timeout $((def + 19000)) +test_timeout mount $def +test_timeout mount 3000 +test_timeout mount $((def + 19000)) + +echo "== sysfs" +test_timeout sysfs $def +test_timeout sysfs 3000 +test_timeout sysfs $((def + 19000)) + +echo "== reset all options" +t_remount_all t_pass