From d7f8c21eed5ba232ae48863001b512c207427037 Mon Sep 17 00:00:00 2001 From: Frederick Dilger Date: Tue, 11 Jun 2024 17:35:35 -0400 Subject: [PATCH] LU-9544 utils: set -P if missing in 'set_param -d' The -P option to lctl set_param will now be added if the -d option (for delete) is specified by itself. As described in the ticket, if a value is erroneously supplied when using -P and -d then instead of being deleted, the parameter is set to the old value with a trailing '='. A non-regression test has been created to verify that this isn't happening. wait_update_cond and wait_update were modified to avoid unresolved errors from throwing an unwanted errnum during the wait. They still operate with logical equivalence to before. Signed-off-by: Frederick Dilger Change-Id: If35b2e9db51f7296da25b798205b9f9104830bca Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/55399 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin Reviewed-by: Andreas Dilger Reviewed-by: Sebastien Buisson --- lustre/tests/sanity.sh | 101 +++++++++++++++++++++++++++++------------ lustre/tests/test-framework.sh | 19 ++++---- lustre/utils/lustre_cfg.c | 2 + 3 files changed, 82 insertions(+), 40 deletions(-) diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index b30bd89..d82258e 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -29148,21 +29148,21 @@ test_401b() { # jobid_var may not allow arbitrary values, so use jobid_name # if available if $LCTL list_param jobid_name > /dev/null 2>&1; then - local testname=jobid_name tmp='testing%p' + local jobvarname=jobid_name tmp='testing%p' else - local testname=jobid_var tmp=testing + local jobvarname=jobid_var tmp=testing fi - local save=$($LCTL get_param -n $testname) + local save=$($LCTL get_param -n $jobvarname) - $LCTL set_param foo=bar $testname=$tmp bar=baz && + $LCTL set_param foo=bar $jobvarname=$tmp bar=baz && error "no error returned when setting bad parameters" - local jobid_new=$($LCTL get_param -n foe $testname baz) + local jobid_new=$($LCTL get_param -n foe $jobvarname baz) [[ "$jobid_new" == "$tmp" ]] || error "jobid tmp $jobid_new != $tmp" - $LCTL set_param -n fog=bam $testname=$save bat=fog - local jobid_old=$($LCTL get_param -n foe $testname bag) + $LCTL set_param -n fog=bam $jobvarname=$save bat=fog + local jobid_old=$($LCTL get_param -n foe $jobvarname bag) [[ "$jobid_old" == "$save" ]] || error "jobid new $jobid_old != $save" } run_test 401b "Verify 'lctl {get,set}_param' continue after error" @@ -29171,27 +29171,27 @@ test_401c() { # jobid_var may not allow arbitrary values, so use jobid_name # if available if $LCTL list_param jobid_name > /dev/null 2>&1; then - local testname=jobid_name + local jobvarname=jobid_name else - local testname=jobid_var + local jobvarname=jobid_var fi - local jobid_var_old=$($LCTL get_param -n $testname) + local jobid_var_old=$($LCTL get_param -n $jobvarname) local jobid_var_new - $LCTL set_param $testname= && + $LCTL set_param $jobvarname= && error "no error returned for 'set_param a='" - jobid_var_new=$($LCTL get_param -n $testname) + jobid_var_new=$($LCTL get_param -n $jobvarname) [[ "$jobid_var_old" == "$jobid_var_new" ]] || - error "$testname was changed by setting without value" + error "$jobvarname was changed by setting without value" - $LCTL set_param $testname && + $LCTL set_param $jobvarname && error "no error returned for 'set_param a'" - jobid_var_new=$($LCTL get_param -n $testname) + jobid_var_new=$($LCTL get_param -n $jobvarname) [[ "$jobid_var_old" == "$jobid_var_new" ]] || - error "$testname was changed by setting without value" + error "$jobvarname was changed by setting without value" } run_test 401c "Verify 'lctl set_param' without value fails in either format." @@ -29199,41 +29199,82 @@ test_401d() { # jobid_var may not allow arbitrary values, so use jobid_name # if available if $LCTL list_param jobid_name > /dev/null 2>&1; then - local testname=jobid_name new_value='foo=bar%p' + local jobvarname=jobid_name new_value='foo=bar%p' else - local testname=jobid_var new_value=foo=bar + local jobvarname=jobid_var new_value=foo=bar fi - local jobid_var_old=$($LCTL get_param -n $testname) + local jobid_var_old=$($LCTL get_param -n $jobvarname) local jobid_var_new - $LCTL set_param $testname=$new_value || + $LCTL set_param $jobvarname=$new_value || error "'set_param a=b' did not accept a value containing '='" - jobid_var_new=$($LCTL get_param -n $testname) + jobid_var_new=$($LCTL get_param -n $jobvarname) [[ "$jobid_var_new" == "$new_value" ]] || error "'set_param a=b' failed on a value containing '='" - # Reset the $testname to test the other format - $LCTL set_param $testname=$jobid_var_old - jobid_var_new=$($LCTL get_param -n $testname) + # Reset the $jobvarname to test the other format + $LCTL set_param $jobvarname=$jobid_var_old + jobid_var_new=$($LCTL get_param -n $jobvarname) [[ "$jobid_var_new" == "$jobid_var_old" ]] || - error "failed to reset $testname" + error "failed to reset $jobvarname" - $LCTL set_param $testname $new_value || + $LCTL set_param $jobvarname $new_value || error "'set_param a b' did not accept a value containing '='" - jobid_var_new=$($LCTL get_param -n $testname) + jobid_var_new=$($LCTL get_param -n $jobvarname) [[ "$jobid_var_new" == "$new_value" ]] || error "'set_param a b' failed on a value containing '='" - $LCTL set_param $testname $jobid_var_old - jobid_var_new=$($LCTL get_param -n $testname) + $LCTL set_param $jobvarname $jobid_var_old + jobid_var_new=$($LCTL get_param -n $jobvarname) [[ "$jobid_var_new" == "$jobid_var_old" ]] || - error "failed to reset $testname" + error "failed to reset $jobvarname" } run_test 401d "Verify 'lctl set_param' accepts values containing '='" +cleanup_401db() { + local saved_val=$1 + + echo "start cleanup... " + do_facet mgs $LCTL set_param -P at_min="$saved_val" + wait_update $HOSTNAME "$LCTL get_param -n at_min" "$saved_val" + echo "done" +} + +test_401db() { #LU-9544 + local new_val=6 + + local saved_val=$($LCTL get_param -n at_min) + + stack_trap "cleanup_401db $saved_val" + + do_facet mgs $LCTL set_param -P at_min=$new_val || + error "failed to set at_min=$new_val" + + wait_update $HOSTNAME "$LCTL get_param -n at_min" $new_val + local expected=$($LCTL get_param -n at_min) + + do_facet mgs $LCTL set_param -P -d at_min=$new_val || + error "failed to delete at_min" + + echo "Wait for erroneous changes" + wait_update_cond $HOSTNAME "$LCTL get_param -n at_min" != $new_val + local result=$($LCTL get_param -n at_min) + + ! [[ "$result" =~ "=" ]] || { + echo "result:$result" + error "'lctl set_param -P -d a=b' added trailing '='" + } + + [[ $result == $expected ]] || { + echo -e "result:$result\nexpected:$expected" + error "'lctl set_param -P -d a=b' changed value of a" + } +} +run_test 401db "Verify 'lctl set_param' does not add trailing '='" + test_401e() { # LU-14779 $LCTL list_param -R "ldlm.namespaces.MGC*" || error "lctl list_param MGC* failed" diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh index f7b4490..56e6c06 100755 --- a/lustre/tests/test-framework.sh +++ b/lustre/tests/test-framework.sh @@ -3460,8 +3460,8 @@ wait_update_cond() { local verbose local quiet - [[ "$1" == "--verbose" ]] && verbose="$1" && shift - [[ "$1" == "--quiet" || "$1" == "-q" ]] && quiet="$1" && shift + [[ "$1" == "--verbose" ]] && verbose="$1" && shift || true + [[ "$1" == "--quiet" || "$1" == "-q" ]] && quiet="$1" && shift || true local node=$1 local check="$2" @@ -3478,19 +3478,18 @@ wait_update_cond() { while (( $waited <= $max_wait )); do result=$(do_node $quiet $node "$check") - eval [[ "'$result'" $cond "'$expect'" ]] - if [[ $? == 0 ]]; then - [[ -n "$quiet" ]] && return 0 + if eval [[ "'$result'" $cond "'$expect'" ]]; then + [[ -z "$quiet" ]] || return 0 [[ -z "$result" || $waited -le $sleep ]] || echo "Updated after ${waited}s: want '$expect' got '$result'" return 0 fi if [[ -n "$verbose" && "$result" != "$prev_result" ]]; then - [[ -z "$quiet" && -n "$prev_result" ]] && + [[ -n "$quiet" || -z "$prev_result" ]] || echo "Changed after ${waited}s: from '$prev_result' to '$result'" prev_result="$result" fi - (( $waited % $print == 0 )) && { + (( $waited % $print != 0 )) || { [[ -z "$quiet" ]] && echo "Waiting $((max_wait - waited))s for '$expect'" } @@ -3499,7 +3498,7 @@ wait_update_cond() { waited=$((SECONDS - begin)) done - [[ -z "$quiet" ]] && + [[ -n "$quiet" ]] || echo "Update not seen after ${max_wait}s: want '$expect' got '$result'" return 3 @@ -3510,8 +3509,8 @@ wait_update() { local verbose local quiet - [[ "$1" == "--verbose" ]] && verbose="$1" && shift - [[ "$1" == "--quiet" || "$1" == "-q" ]] && quiet="$1" && shift + [[ "$1" == "--verbose" ]] && verbose="$1" && shift || true + [[ "$1" == "--quiet" || "$1" == "-q" ]] && quiet="$1" && shift || true local node="$1" local check="$2" diff --git a/lustre/utils/lustre_cfg.c b/lustre/utils/lustre_cfg.c index cd5dce8..a2fda2d 100644 --- a/lustre/utils/lustre_cfg.c +++ b/lustre/utils/lustre_cfg.c @@ -1785,6 +1785,8 @@ static int setparam_cmdline(int argc, char **argv, struct param_opts *popt) fprintf(stderr, "warning: ignoring -P option\n"); popt->po_perm = 0; } + if (popt->po_delete && !popt->po_perm) + popt->po_perm = 1; return optind; } -- 1.8.3.1