From: Ryan Haasken <haasken@cray.com>
Date: Fri, 26 Feb 2016 19:52:39 +0000 (-0600)
Subject: LU-7796 utils: print error with valueless lctl set_param
X-Git-Tag: 2.8.52~27
X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=e2973925ca550ad8ef7ad9af72d7d8766f9c1f41

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
--- 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
--- 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;