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 <fdilger@whamcloud.com>
Change-Id: If35b2e9db51f7296da25b798205b9f9104830bca
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/55399
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Sebastien Buisson <sbuisson@ddn.com>

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index b30bd89..d82258e 100755 (executable)
--- 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 (executable)
--- 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 (file)
--- 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;
 }