Fix zdb/ztest -o and refactor libzpool::set_global_var

cmd/ztest/ztest.c

@@ -158,6 +158,9 @@ enum ztest_class_state {
 	ZTEST_VDEV_CLASS_RND
 };
 
+#define	ZO_GVARS_MAX_ARGLEN	((size_t)64)
+#define	ZO_GVARS_MAX_COUNT	((size_t)10)
+
 typedef struct ztest_shared_opts {
 	char zo_pool[ZFS_MAX_DATASET_NAME_LEN];
 	char zo_dir[ZFS_MAX_DATASET_NAME_LEN];
@@ -185,6 +188,8 @@ typedef struct ztest_shared_opts {
 	int zo_mmp_test;
 	int zo_special_vdevs;
 	int zo_dump_dbgmsg;
+	int zo_gvars_count;
+	char zo_gvars[ZO_GVARS_MAX_COUNT][ZO_GVARS_MAX_ARGLEN];
 } ztest_shared_opts_t;
 
 static const ztest_shared_opts_t ztest_opts_defaults = {
@@ -212,6 +217,7 @@ static const ztest_shared_opts_t ztest_opts_defaults = {
 	.zo_maxloops = 50,		/* max loops during spa_freeze() */
 	.zo_metaslab_force_ganging = 64 << 10,
 	.zo_special_vdevs = ZTEST_VDEV_CLASS_RND,
+	.zo_gvars_count = 0,
 };
 
 extern uint64_t metaslab_force_ganging;
@@ -918,8 +924,21 @@ process_options(int argc, char **argv)
 			ztest_parse_name_value(optarg, zo);
 			break;
 		case 'o':
-			if (set_global_var(optarg) != 0)
+			if (zo->zo_gvars_count >= ZO_GVARS_MAX_COUNT) {
+				(void) fprintf(stderr,
+				    "max global var count (%zu) exceeded\n",
+				    ZO_GVARS_MAX_COUNT);
+				usage(B_FALSE);
+			}
+			char *v = zo->zo_gvars[zo->zo_gvars_count];
+			if (strlcpy(v, optarg, ZO_GVARS_MAX_ARGLEN) >=
+			    ZO_GVARS_MAX_ARGLEN) {
+				(void) fprintf(stderr,
+				    "global var option '%s' is too long\n",
+				    optarg);
 				usage(B_FALSE);
+			}
+			zo->zo_gvars_count++;
 			break;
 		case 'G':
 			zo->zo_dump_dbgmsg = 1;
@@ -6373,6 +6392,75 @@ ztest_fletcher_incr(ztest_ds_t *zd, uint64_t id)
 	}
 }
 
+static int
+ztest_set_global_vars(void)
+{
+	for (size_t i = 0; i < ztest_opts.zo_gvars_count; i++) {
+		char *kv = ztest_opts.zo_gvars[i];
+		VERIFY3U(strlen(kv), <=, ZO_GVARS_MAX_ARGLEN);
+		VERIFY3U(strlen(kv), >, 0);
+		int err = set_global_var(kv);
+		if (ztest_opts.zo_verbose > 0) {
+			(void) printf("setting global var %s ... %s\n", kv,
+			    err ? "failed" : "ok");
+		}
+		if (err != 0) {
+			(void) fprintf(stderr,
+			    "failed to set global var '%s'\n", kv);
+			return (err);
+		}
+	}
+	return (0);
+}
+
+static char **
+ztest_global_vars_to_zdb_args(void)
+{
+	char **args = calloc(2*ztest_opts.zo_gvars_count + 1, sizeof (char *));
+	char **cur = args;
+	for (size_t i = 0; i < ztest_opts.zo_gvars_count; i++) {
+		char *kv = ztest_opts.zo_gvars[i];
+		*cur = "-o";
+		cur++;
+		*cur = strdup(kv);
+		cur++;
+	}
+	ASSERT3P(cur, ==, &args[2*ztest_opts.zo_gvars_count]);
+	*cur = NULL;
+	return (args);
+}
+
+/* The end of strings is indicated by a NULL element */
+static char *
+join_strings(char **strings, const char *sep)
+{
+	size_t totallen = 0;
+	for (char **sp = strings; *sp != NULL; sp++) {
+		totallen += strlen(*sp);
+		totallen += strlen(sep);
+	}
+	if (totallen > 0) {
+		ASSERT(totallen >= strlen(sep));
+		totallen -= strlen(sep);
+	}
+
+	size_t buflen = totallen + 1;
+	char *o = malloc(buflen); /* trailing 0 byte */
+	o[0] = '\0';
+	for (char **sp = strings; *sp != NULL; sp++) {
+		size_t would;
+		would = strlcat(o, *sp, buflen);
+		VERIFY3U(would, <, buflen);
+		if (*(sp+1) == NULL) {
+			break;
+		}
+		would = strlcat(o, sep, buflen);
+		VERIFY3U(would, <, buflen);
+	}
+	ASSERT3S(strlen(o), ==, totallen);
+	return (o);
+}
+
 static int
 ztest_check_path(char *path)
 {
@@ -6601,13 +6689,21 @@ ztest_run_zdb(char *pool)
 
 	ztest_get_zdb_bin(bin, len);
 
-	(void) sprintf(zdb,
-	    "%s -bcc%s%s -G -d -Y -e -y -p %s %s",
+	char **set_gvars_args = ztest_global_vars_to_zdb_args();
+	char *set_gvars_args_joined = join_strings(set_gvars_args, " ");
+	free(set_gvars_args);
+
+	size_t would = snprintf(zdb, len,
+	    "%s -bcc%s%s -G -d -Y -e -y %s -p %s %s",
 	    bin,
 	    ztest_opts.zo_verbose >= 3 ? "s" : "",
 	    ztest_opts.zo_verbose >= 4 ? "v" : "",
+	    set_gvars_args_joined,
 	    ztest_opts.zo_dir,
 	    pool);
+	ASSERT3U(would, <, len);
+
+	free(set_gvars_args_joined);
 
 	if (ztest_opts.zo_verbose >= 5)
 		(void) printf("Executing %s\n", strstr(zdb, "zdb "));
@@ -7727,7 +7823,7 @@ main(int argc, char **argv)
 	char numbuf[NN_NUMBUF_SZ];
 	char *cmd;
 	boolean_t hasalt;
-	int f;
+	int f, err;
 	char *fd_data_str = getenv("ZTEST_FD_DATA");
 	struct sigaction action;
 
@@ -7794,6 +7890,15 @@ main(int argc, char **argv)
 	}
 	ASSERT3U(ztest_opts.zo_datasets, ==, ztest_shared_hdr->zh_ds_count);
 
+	err = ztest_set_global_vars();
+	if (err != 0 && !fd_data_str) {
+		/* error message done by ztest_set_global_vars */
+		exit(EXIT_FAILURE);
+	} else {
+		/* children should not be spawned if setting gvars fails */
+		VERIFY3S(err, ==, 0);
+	}
+
 	/* Override location of zpool.cache */
 	VERIFY3S(asprintf((char **)&spa_config_path, "%s/zpool.cache",
 	    ztest_opts.zo_dir), !=, -1);

include/sys/zfs_context.h

@@ -652,7 +652,7 @@ extern void random_fini(void);
 
 struct spa;
 extern void show_pool_stats(struct spa *);
-extern int set_global_var(char *arg);
+extern int set_global_var(char const *arg);
 
 typedef struct callb_cpr {
 	kmutex_t	*cc_lockp;

lib/libzpool/util.c

@@ -148,38 +148,67 @@ show_pool_stats(spa_t *spa)
 	nvlist_free(config);
 }
 
+/* *k_out must be freed by the caller */
+static int
+set_global_var_parse_kv(const char *arg, char **k_out, u_longlong_t *v_out)
+{
+	int err;
+	VERIFY(arg);
+	char *d = strdup(arg);
+
+	char *save = NULL;
+	char *k = strtok_r(d, "=", &save);
+	char *v_str = strtok_r(NULL, "=", &save);
+	char *follow = strtok_r(NULL, "=", &save);
+	if (k == NULL || v_str == NULL || follow != NULL) {
+		err = EINVAL;
+		goto err_free;
+	}
+
+	u_longlong_t val = strtoull(v_str, NULL, 0);
+	if (val > UINT32_MAX) {
+		fprintf(stderr, "Value for global variable '%s' must "
+		    "be a 32-bit unsigned integer, got '%s'\n", k, v_str);
+		err = EOVERFLOW;
+		goto err_free;
+	}
+
+	*k_out = k;
+	*v_out = val;
+	return (0);
+
+err_free:
+	free(k);
+
+	return (err);
+}
+
 /*
  * Sets given global variable in libzpool to given unsigned 32-bit value.
  * arg: "<variable>=<value>"
  */
 int
-set_global_var(char *arg)
+set_global_var(char const *arg)
 {
 	void *zpoolhdl;
-	char *varname = arg, *varval;
+	char *varname;
 	u_longlong_t val;
+	int ret;
 
-#ifndef _LITTLE_ENDIAN
+#ifndef _ZFS_LITTLE_ENDIAN
 	/*
 	 * On big endian systems changing a 64-bit variable would set the high
 	 * 32 bits instead of the low 32 bits, which could cause unexpected
 	 * results.
 	 */
 	fprintf(stderr, "Setting global variables is only supported on "
 	    "little-endian systems\n");
-	return (ENOTSUP);
+	ret = ENOTSUP;
+	goto out_ret;
 #endif
-	if (arg != NULL && (varval = strchr(arg, '=')) != NULL) {
-		*varval = '\0';
-		varval++;
-		val = strtoull(varval, NULL, 0);
-		if (val > UINT32_MAX) {
-			fprintf(stderr, "Value for global variable '%s' must "
-			    "be a 32-bit unsigned integer\n", varname);
-			return (EOVERFLOW);
-		}
-	} else {
-		return (EINVAL);
+
+	if ((ret = set_global_var_parse_kv(arg, &varname, &val)) != 0) {
+		goto out_ret;
 	}
 
 	zpoolhdl = dlopen("libzpool.so", RTLD_LAZY);
@@ -189,18 +218,25 @@ set_global_var(char *arg)
 		if (var == NULL) {
 			fprintf(stderr, "Global variable '%s' does not exist "
 			    "in libzpool.so\n", varname);
-			return (EINVAL);
+			ret = EINVAL;
+			goto out_dlclose;
 		}
 		*var = (uint32_t)val;
 
-		dlclose(zpoolhdl);
 	} else {
 		fprintf(stderr, "Failed to open libzpool.so to set global "
 		    "variable\n");
-		return (EIO);
+		ret = EIO;
+		goto out_dlclose;
 	}
 
-	return (0);
+	ret = 0;
+
+out_dlclose:
+	dlclose(zpoolhdl);
+	free(varname);
+out_ret:
+	return (ret);
 }
 
 static nvlist_t *