Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 4 commits into from
Closed

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a928a03
libzpool: set_global_var: fix endianness handling (fixes zdb -o )
problame Feb 15, 2021
838bba1
libzpool: set_global_var: refactor to not modify 'arg'
problame Feb 16, 2021
ff2aa4a
ztest: fix -o by calling set_global_var in child processes
problame Feb 16, 2021
fbb58f5
ztest: propagate -o to the zdb child process
problame Feb 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 51 additions & 2 deletions cmd/ztest/ztest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -7611,6 +7630,27 @@ setup_data(void)
ztest_shared_ds = (void *)&buf[offset];
}

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 boolean_t
exec_child(char *cmd, char *libpath, boolean_t ignorekill, int *statusp)
{
Expand Down Expand Up @@ -7727,7 +7767,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;

Expand Down Expand Up @@ -7794,6 +7834,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);
Expand Down
2 changes: 1 addition & 1 deletion include/sys/zfs_context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
74 changes: 55 additions & 19 deletions lib/libzpool/util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like zdb_dump_block in zdb.c needs to be updated as well. The _ZFS prefix was added by commit 5678d3f to resolve a conflict, it looks like we missed a couple instances.

/*
* 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);
Expand All @@ -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 *
Expand Down