Skip to content

Commit

Permalink
zpool: Dryrun fails to list some devices
Browse files Browse the repository at this point in the history 
`zpool create -n` fails to list cache and spare vdevs.
`zpool add -n` fails to list spare devices.
`zpool split -n` fails to list `special` and `dedup` labels.
`zpool add -n` and `zpool split -n` shouldn't list hole devices.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Attila Fülöp <[email protected]>
Closes openzfs#11122
Closes openzfs#11167
  • Loading branch information
@AttilaFueloep
AttilaFueloep committed Jan 8, 2021
1 parent 32a78e5 commit 2f60c8b
Show file tree
Hide file tree
Showing 10 changed files with 593 additions and 14 deletions.
94 changes: 89 additions & 5 deletions cmd/zpool/zpool_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -669,9 +669,16 @@ print_vdev_tree(zpool_handle_t *zhp, const char *name, nvlist_t *nv, int indent,
}

for (c = 0; c < children; c++) {
uint64_t is_log = B_FALSE;
uint64_t is_log = B_FALSE, is_hole = B_FALSE;
char *class = "";

(void) nvlist_lookup_uint64(child[c], ZPOOL_CONFIG_IS_HOLE,
&is_hole);

if (is_hole == B_TRUE) {
continue;
}

(void) nvlist_lookup_uint64(child[c], ZPOOL_CONFIG_IS_LOG,
&is_log);
if (is_log)
Expand All @@ -692,6 +699,54 @@ print_vdev_tree(zpool_handle_t *zhp, const char *name, nvlist_t *nv, int indent,
}
}

/*
* Print the list of l2cache devices for dry runs.
*/
static void
print_cache_list(nvlist_t *nv, int indent)
{
nvlist_t **child;
uint_t c, children;

if (nvlist_lookup_nvlist_array(nv, ZPOOL_CONFIG_L2CACHE,
&child, &children) == 0 && children > 0) {
(void) printf("\t%*s%s\n", indent, "", "cache");
} else {
return;
}
for (c = 0; c < children; c++) {
char *vname;

vname = zpool_vdev_name(g_zfs, NULL, child[c], 0);
(void) printf("\t%*s%s\n", indent + 2, "", vname);
free(vname);
}
}

/*
* Print the list of spares for dry runs.
*/
static void
print_spare_list(nvlist_t *nv, int indent)
{
nvlist_t **child;
uint_t c, children;

if (nvlist_lookup_nvlist_array(nv, ZPOOL_CONFIG_SPARES,
&child, &children) == 0 && children > 0) {
(void) printf("\t%*s%s\n", indent, "", "spares");
} else {
return;
}
for (c = 0; c < children; c++) {
char *vname;

vname = zpool_vdev_name(g_zfs, NULL, child[c], 0);
(void) printf("\t%*s%s\n", indent + 2, "", vname);
free(vname);
}
}

static boolean_t
prop_list_contains_feature(nvlist_t *proplist)
{
Expand Down Expand Up @@ -921,16 +976,16 @@ zpool_do_add(int argc, char **argv)

if (dryrun) {
nvlist_t *poolnvroot;
nvlist_t **l2child;
uint_t l2children, c;
nvlist_t **l2child, **sparechild;
uint_t l2children, sparechildren, c;
char *vname;
boolean_t hadcache = B_FALSE;
boolean_t hadcache = B_FALSE, hadspare = B_FALSE;

verify(nvlist_lookup_nvlist(config, ZPOOL_CONFIG_VDEV_TREE,
&poolnvroot) == 0);

(void) printf(gettext("would update '%s' to the following "
"configuration:\n"), zpool_get_name(zhp));
"configuration:\n\n"), zpool_get_name(zhp));

/* print original main pool and new tree */
print_vdev_tree(zhp, poolname, poolnvroot, 0, "",
Expand Down Expand Up @@ -991,6 +1046,29 @@ zpool_do_add(int argc, char **argv)
free(vname);
}
}
/* And finaly the spares */
if (nvlist_lookup_nvlist_array(poolnvroot, ZPOOL_CONFIG_SPARES,
&sparechild, &sparechildren) == 0 && sparechildren > 0) {
hadspare = B_TRUE;
(void) printf(gettext("\tspares\n"));
for (c = 0; c < sparechildren; c++) {
vname = zpool_vdev_name(g_zfs, NULL,
sparechild[c], name_flags);
(void) printf("\t %s\n", vname);
free(vname);
}
}
if (nvlist_lookup_nvlist_array(nvroot, ZPOOL_CONFIG_SPARES,
&sparechild, &sparechildren) == 0 && sparechildren > 0) {
if (!hadspare)
(void) printf(gettext("\tspares\n"));
for (c = 0; c < sparechildren; c++) {
vname = zpool_vdev_name(g_zfs, NULL,
sparechild[c], name_flags);
(void) printf("\t %s\n", vname);
free(vname);
}
}

ret = 0;
} else {
Expand Down Expand Up @@ -1548,6 +1626,8 @@ zpool_do_create(int argc, char **argv)
VDEV_ALLOC_BIAS_SPECIAL, 0);
print_vdev_tree(NULL, "logs", nvroot, 0,
VDEV_ALLOC_BIAS_LOG, 0);
print_cache_list(nvroot, 0);
print_spare_list(nvroot, 0);

ret = 0;
} else {
Expand Down Expand Up @@ -6515,6 +6595,10 @@ zpool_do_split(int argc, char **argv)
"following layout:\n\n"), newpool);
print_vdev_tree(NULL, newpool, config, 0, "",
flags.name_flags);
print_vdev_tree(NULL, "dedup", config, 0,
VDEV_ALLOC_BIAS_DEDUP, 0);
print_vdev_tree(NULL, "special", config, 0,
VDEV_ALLOC_BIAS_SPECIAL, 0);
}
}

Expand Down
24 changes: 23 additions & 1 deletion lib/libzfs/libzfs_pool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3388,7 +3388,7 @@ zpool_vdev_split(zpool_handle_t *zhp, char *newname, nvlist_t **newroot,
nvlist_t *props, splitflags_t flags)
{
zfs_cmd_t zc = {"\0"};
char msg[1024];
char msg[1024], *bias;
nvlist_t *tree, *config, **child, **newchild, *newconfig = NULL;
nvlist_t **varray = NULL, *zc_props = NULL;
uint_t c, children, newchildren, lastlog = 0, vcount, found = 0;
Expand Down Expand Up @@ -3446,6 +3446,7 @@ zpool_vdev_split(zpool_handle_t *zhp, char *newname, nvlist_t **newroot,

for (c = 0; c < children; c++) {
uint64_t is_log = B_FALSE, is_hole = B_FALSE;
boolean_t is_special = B_FALSE, is_dedup = B_FALSE;
char *type;
nvlist_t **mchild, *vdev;
uint_t mchildren;
Expand Down Expand Up @@ -3492,6 +3493,13 @@ zpool_vdev_split(zpool_handle_t *zhp, char *newname, nvlist_t **newroot,
goto out;
}

if (nvlist_lookup_string(child[c],
ZPOOL_CONFIG_ALLOCATION_BIAS, &bias) == 0) {
if (strcmp(bias, VDEV_ALLOC_BIAS_SPECIAL) == 0)
is_special = B_TRUE;
else if (strcmp(bias, VDEV_ALLOC_BIAS_DEDUP) == 0)
is_dedup = B_TRUE;
}
verify(nvlist_lookup_nvlist_array(child[c],
ZPOOL_CONFIG_CHILDREN, &mchild, &mchildren) == 0);

Expand All @@ -3509,6 +3517,20 @@ zpool_vdev_split(zpool_handle_t *zhp, char *newname, nvlist_t **newroot,

if (nvlist_dup(vdev, &varray[vcount++], 0) != 0)
goto out;

if (flags.dryrun != 0) {
if (is_dedup == B_TRUE) {
if (nvlist_add_string(varray[vcount - 1],
ZPOOL_CONFIG_ALLOCATION_BIAS,
VDEV_ALLOC_BIAS_DEDUP) != 0)
goto out;
} else if (is_special == B_TRUE) {
if (nvlist_add_string(varray[vcount - 1],
ZPOOL_CONFIG_ALLOCATION_BIAS,
VDEV_ALLOC_BIAS_SPECIAL) != 0)
goto out;
}
}
}

/* did we find every disk the user specified? */
Expand Down
7 changes: 4 additions & 3 deletions tests/runfiles/common.run
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ tags = ['functional', 'cli_root', 'zpool']
tests = ['zpool_add_001_pos', 'zpool_add_002_pos', 'zpool_add_003_pos',
'zpool_add_004_pos', 'zpool_add_006_pos', 'zpool_add_007_neg',
'zpool_add_008_neg', 'zpool_add_009_neg', 'zpool_add_010_pos',
'add-o_ashift', 'add_prop_ashift']
'add-o_ashift', 'add_prop_ashift', 'zpool_add_dryrun_output']
tags = ['functional', 'cli_root', 'zpool_add']

[tests/functional/cli_root/zpool_attach]
Expand All @@ -342,7 +342,7 @@ tests = ['zpool_create_001_pos', 'zpool_create_002_pos',
'zpool_create_features_001_pos', 'zpool_create_features_002_pos',
'zpool_create_features_003_pos', 'zpool_create_features_004_neg',
'zpool_create_features_005_pos',
'create-o_ashift', 'zpool_create_tempname']
'create-o_ashift', 'zpool_create_tempname', 'zpool_create_dryrun_output']
tags = ['functional', 'cli_root', 'zpool_create']

[tests/functional/cli_root/zpool_destroy]
Expand Down Expand Up @@ -460,7 +460,8 @@ tags = ['functional', 'cli_root', 'zpool_set']
[tests/functional/cli_root/zpool_split]
tests = ['zpool_split_cliargs', 'zpool_split_devices',
'zpool_split_encryption', 'zpool_split_props', 'zpool_split_vdevs',
'zpool_split_resilver', 'zpool_split_indirect']
'zpool_split_resilver', 'zpool_split_indirect',
'zpool_split_dryrun_output']
tags = ['functional', 'cli_root', 'zpool_split']

[tests/functional/cli_root/zpool_status]
Expand Down
3 changes: 2 additions & 1 deletion tests/zfs-tests/tests/functional/cli_root/zpool_add/Makefile.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ dist_pkgdata_SCRIPTS = \
zpool_add_010_pos.ksh \
add-o_ashift.ksh \
add_prop_ashift.ksh \
add_nested_replacing_spare.ksh
add_nested_replacing_spare.ksh \
zpool_add_dryrun_output.ksh

dist_pkgdata_DATA = \
zpool_add.cfg \
Expand Down
4 changes: 2 additions & 2 deletions tests/zfs-tests/tests/functional/cli_root/zpool_add/zpool_add_003_pos.ksh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ log_onexit cleanup
typeset TMPFILE_PREFIX="$TEST_BASE_DIR/zpool_add_003"
typeset STR_DRYRUN="would update '$TESTPOOL' to the following configuration:"
typeset VDEV_PREFIX="$TEST_BASE_DIR/filedev"
typeset -a VDEV_TYPES=("" "dedup" "special" "log" "cache")
typeset -a VDEV_TYPES=("" "dedup" "special" "log" "cache" "spare")

vdevs=""
config=""
Expand Down Expand Up @@ -91,7 +91,7 @@ log_must zpool add -f $TESTPOOL $config
zpool status $TESTPOOL | awk 'NR == 1, /NAME/ { next } /^$/ {exit}
{print $1}' > "$TMPFILE_PREFIX-vdevtree"
cat "$TMPFILE_PREFIX-dryrun" | awk 'NR == 1, /would/ {next}
{print $1}' > "$TMPFILE_PREFIX-vdevtree-n"
/^$/ {next} {print $1}' > "$TMPFILE_PREFIX-vdevtree-n"
log_must eval "diff $TMPFILE_PREFIX-vdevtree-n $TMPFILE_PREFIX-vdevtree"

log_pass "'zpool add -n <pool> <vdev> ...' executes successfully."
Loading

0 comments on commit 2f60c8b

Please sign in to comment.