LU-7796 utils: print error with valueless lctl set_param

If lctl set_param is called with space-separated arguments, and there
is a trailing parameter without a value, print an error instead of
returning silently.  Also, allow values which contain equal signs when
lctl set_param is given space-separated arguments.

Print a warning message for both formats whenever the value contains
an '=' character because it is likely that the user made an error.

Also fix the return values of jt_lcfg_setparam, so they are always
negative on error.

Add tests for "lctl set_param" to verify that it returns an error when
a value is omitted in either format and to check that "lctl set_param"
accepts values which contain the '=' character in either format.

Signed-off-by: Ryan Haasken <haasken@cray.com>
Change-Id: Ie9d709c5aa533f778cc83f4b58f40fe4b084c754
Reviewed-on: http://review.whamcloud.com/18703
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Emoly Liu <emoly.liu@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index f4cf582..9c1770b 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -14477,6 +14477,58 @@ test_401b() {
 }
 run_test 401b "Verify 'lctl {get,set}_param' continue after error"
 
+test_401c() {
+       local jobid_var_old=$($LCTL get_param -n jobid_var)
+       local jobid_var_new
+
+       $LCTL set_param jobid_var= &&
+               error "no error returned for 'set_param a='"
+
+       jobid_var_new=$($LCTL get_param -n jobid_var)
+       [[ "$jobid_var_old" == "$jobid_var_new" ]] ||
+               error "jobid_var was changed by setting without value"
+
+       $LCTL set_param jobid_var &&
+               error "no error returned for 'set_param a'"
+
+       jobid_var_new=$($LCTL get_param -n jobid_var)
+       [[ "$jobid_var_old" == "$jobid_var_new" ]] ||
+               error "jobid_var was changed by setting without value"
+}
+run_test 401c "Verify 'lctl set_param' without value fails in either format."
+
+test_401d() {
+       local jobid_var_old=$($LCTL get_param -n jobid_var)
+       local jobid_var_new
+       local new_value="foo=bar"
+
+       $LCTL set_param jobid_var=$new_value ||
+               error "'set_param a=b' did not accept a value containing '='"
+
+       jobid_var_new=$($LCTL get_param -n jobid_var)
+       [[ "$jobid_var_new" == "$new_value" ]] ||
+               error "'set_param a=b' failed on a value containing '='"
+
+       # Reset the jobid_var to test the other format
+       $LCTL set_param jobid_var=$jobid_var_old
+       jobid_var_new=$($LCTL get_param -n jobid_var)
+       [[ "$jobid_var_new" == "$jobid_var_old" ]] ||
+               error "failed to reset jobid_var"
+
+       $LCTL set_param jobid_var $new_value ||
+               error "'set_param a b' did not accept a value containing '='"
+
+       jobid_var_new=$($LCTL get_param -n jobid_var)
+       [[ "$jobid_var_new" == "$new_value" ]] ||
+               error "'set_param a b' failed on a value containing '='"
+
+       $LCTL set_param jobid_var $jobid_var_old
+       jobid_var_new=$($LCTL get_param -n jobid_var)
+       [[ "$jobid_var_new" == "$jobid_var_old" ]] ||
+               error "failed to reset jobid_var"
+}
+run_test 401d "Verify 'lctl set_param' accepts values containing '='"
+
 test_402() {
        $LFS setdirstripe -i 0 $DIR/$tdir || error "setdirstripe -i 0 failed"
 #define OBD_FAIL_MDS_FLD_LOOKUP 0x15c

diff --git a/lustre/utils/lustre_cfg.c b/lustre/utils/lustre_cfg.c
index 7e5f0c0..f0c5d5d 100644 (file)
--- a/lustre/utils/lustre_cfg.c
+++ b/lustre/utils/lustre_cfg.c
@@ -1405,20 +1405,11 @@ int jt_lcfg_setparam(int argc, char **argv)
 
        for (i = index; i < argc; i++) {
                int rc2;
+               path = NULL;
 
                value = strchr(argv[i], '=');
                if (value != NULL) {
                        /* format: set_param a=b */
-                       if (path != NULL) {
-                               /* broken value "set_param a b=c" */
-                               fprintf(stderr,
-                                       "error: %s: setting %s=%s: bad value\n",
-                                       jt_cmdname(argv[0]), path, argv[i]);
-                               if (rc == 0)
-                                       rc = EINVAL;
-                               path = NULL;
-                               break;
-                       }
                        *value = '\0';
                        value++;
                        path = argv[i];
@@ -1427,14 +1418,20 @@ int jt_lcfg_setparam(int argc, char **argv)
                                        "error: %s: setting %s: no value\n",
                                        jt_cmdname(argv[0]), path);
                                if (rc == 0)
-                                       rc = EINVAL;
+                                       rc = -EINVAL;
                                continue;
                        }
                } else {
                        /* format: set_param a b */
-                       if (path == NULL) {
-                               path = argv[i];
-                               continue;
+                       path = argv[i];
+                       i++;
+                       if (i >= argc) {
+                               fprintf(stderr,
+                                       "error: %s: setting %s: no value\n",
+                                       jt_cmdname(argv[0]), path);
+                               if (rc == 0)
+                                       rc = -EINVAL;
+                               break;
                        } else {
                                value = argv[i];
                        }
@@ -1449,11 +1446,18 @@ int jt_lcfg_setparam(int argc, char **argv)
                        continue;
                }
 
+               /* A value containing '=' is indicative of user error, e.g.:
+                *     lctl set_param param1 param2=value2
+                *     lctl set_param param1=param2=value2
+                */
+               if (strchr(value, '=') != NULL)
+                       fprintf(stderr,
+                               "warning: %s: value '%s' contains '='\n",
+                               jt_cmdname(argv[0]), value);
+
                rc2 = param_display(&popt, path, value, SET_PARAM);
                if (rc == 0)
                        rc = rc2;
-               path = NULL;
-               value = NULL;
        }
 
        return rc;