Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions cmd/ztest.c
Original file line number Diff line number Diff line change
Expand Up @@ -3038,15 +3038,15 @@ ztest_spa_create_destroy(ztest_ds_t *zd, uint64_t id)
*/
nvroot = make_vdev_root("/dev/bogus", NULL, NULL, 0, 0, NULL, 0, 0, 1);
VERIFY3U(ENOENT, ==,
spa_create("ztest_bad_file", nvroot, NULL, NULL, NULL));
spa_create("ztest_bad_file", nvroot, NULL, NULL, NULL, NULL));
fnvlist_free(nvroot);

/*
* Attempt to create using a bad mirror.
*/
nvroot = make_vdev_root("/dev/bogus", NULL, NULL, 0, 0, NULL, 0, 2, 1);
VERIFY3U(ENOENT, ==,
spa_create("ztest_bad_mirror", nvroot, NULL, NULL, NULL));
spa_create("ztest_bad_mirror", nvroot, NULL, NULL, NULL, NULL));
fnvlist_free(nvroot);

/*
Expand All @@ -3056,7 +3056,7 @@ ztest_spa_create_destroy(ztest_ds_t *zd, uint64_t id)
(void) pthread_rwlock_rdlock(&ztest_name_lock);
nvroot = make_vdev_root("/dev/bogus", NULL, NULL, 0, 0, NULL, 0, 0, 1);
VERIFY3U(EEXIST, ==,
spa_create(zo->zo_pool, nvroot, NULL, NULL, NULL));
spa_create(zo->zo_pool, nvroot, NULL, NULL, NULL, NULL));
fnvlist_free(nvroot);

/*
Expand Down Expand Up @@ -3208,7 +3208,7 @@ ztest_spa_upgrade(ztest_ds_t *zd, uint64_t id)
props = fnvlist_alloc();
fnvlist_add_uint64(props,
zpool_prop_to_name(ZPOOL_PROP_VERSION), version);
VERIFY0(spa_create(name, nvroot, props, NULL, NULL));
VERIFY0(spa_create(name, nvroot, props, NULL, NULL, NULL));
fnvlist_free(nvroot);
fnvlist_free(props);

Expand Down Expand Up @@ -8686,7 +8686,8 @@ ztest_init(ztest_shared_t *zs)
free(buf);
}

VERIFY0(spa_create(ztest_opts.zo_pool, nvroot, props, NULL, NULL));
VERIFY0(spa_create(ztest_opts.zo_pool, nvroot, props,
NULL, NULL, NULL));
fnvlist_free(nvroot);
fnvlist_free(props);

Expand Down
3 changes: 3 additions & 0 deletions include/sys/fs/zfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,9 @@ typedef struct zpool_load_policy {
#define ZPOOL_CONFIG_BOOTFS "bootfs" /* not stored on disk */
#define ZPOOL_CONFIG_MISSING_DEVICES "missing_vdevs" /* not stored on disk */
#define ZPOOL_CONFIG_LOAD_INFO "load_info" /* not stored on disk */
#define ZPOOL_CONFIG_CREATE_INFO "create_info" /* not stored on disk */
#define ZPOOL_CREATE_INFO_VDEV "create_err_vdev"
#define ZPOOL_CREATE_INFO_POOL "create_err_pool"
#define ZPOOL_CONFIG_REWIND_INFO "rewind_info" /* not stored on disk */
#define ZPOOL_CONFIG_UNSUP_FEAT "unsup_feat" /* not stored on disk */
#define ZPOOL_CONFIG_ENABLED_FEAT "enabled_feat" /* not stored on disk */
Expand Down
2 changes: 1 addition & 1 deletion include/sys/spa.h
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ extern int spa_open_rewind(const char *pool, spa_t **, const void *tag,
extern int spa_get_stats(const char *pool, nvlist_t **config, char *altroot,
size_t buflen);
extern int spa_create(const char *pool, nvlist_t *nvroot, nvlist_t *props,
nvlist_t *zplprops, struct dsl_crypto_params *dcp);
nvlist_t *zplprops, struct dsl_crypto_params *dcp, nvlist_t **errinfo);
extern int spa_import(char *pool, nvlist_t *config, nvlist_t *props,
uint64_t flags);
extern nvlist_t *spa_tryimport(nvlist_t *tryconfig);
Expand Down
1 change: 1 addition & 0 deletions include/sys/spa_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ struct spa {
nvlist_t *spa_config_syncing; /* currently syncing config */
nvlist_t *spa_config_splitting; /* config for splitting */
nvlist_t *spa_load_info; /* info and errors from load */
nvlist_t *spa_create_info; /* info from create */
uint64_t spa_config_txg; /* txg of last config change */
uint32_t spa_sync_pass; /* iterate-to-convergence */
pool_state_t spa_state; /* pool state */
Expand Down
61 changes: 61 additions & 0 deletions lib/libzfs/libzfs_pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -1477,6 +1477,55 @@ zpool_is_draid_spare(const char *name)
return (B_FALSE);
}


/*
* Extract device-specific error information from a failed pool creation.
* If the kernel returned ZPOOL_CONFIG_CREATE_INFO in the ioctl output,
* set an appropriate error aux message identifying the problematic device.
*
* Returns B_TRUE if device-specific info was found and the error aux
* message was set, B_FALSE otherwise.
*/
static boolean_t
zpool_create_info(libzfs_handle_t *hdl, zfs_cmd_t *zc)
{
nvlist_t *outnv = NULL;
nvlist_t *errinfo = NULL;
const char *vdev = NULL;
const char *pname = NULL;
boolean_t found = B_FALSE;

if (zc->zc_nvlist_dst_size == 0)
return (B_FALSE);

if (nvlist_unpack((void *)(uintptr_t)zc->zc_nvlist_dst,
zc->zc_nvlist_dst_size, &outnv, 0) != 0 || outnv == NULL)
return (B_FALSE);

if (nvlist_lookup_nvlist(outnv,
ZPOOL_CONFIG_CREATE_INFO, &errinfo) != 0)
goto out;

if (nvlist_lookup_string(errinfo,
ZPOOL_CREATE_INFO_VDEV, &vdev) != 0)
goto out;

if (nvlist_lookup_string(errinfo,
ZPOOL_CREATE_INFO_POOL, &pname) == 0) {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"device '%s' is part of active pool '%s'"),
vdev, pname);
} else {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"device '%s' is in use"), vdev);
}
found = B_TRUE;

out:
nvlist_free(outnv);
return (found);
}

/*
* Create the named pool, using the provided vdev list. It is assumed
* that the consumer has already validated the contents of the nvlist, so we
Expand Down Expand Up @@ -1556,9 +1605,21 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
zcmd_write_src_nvlist(hdl, &zc, zc_props);

(void) strlcpy(zc.zc_name, pool, sizeof (zc.zc_name));
zcmd_alloc_dst_nvlist(hdl, &zc, 4096);

if ((ret = zfs_ioctl(hdl, ZFS_IOC_POOL_CREATE, &zc)) != 0) {

if (zpool_create_info(hdl, &zc)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we're going to have infrastructure to report more detailed errors in zpool create I suspect we'll want to make use of it in some of the other error cases. I've thrown together a completely untested commit which restructures things so we can access the nvlists where we need them. Can you take a look at 2c5fa0d, refine as needed, and then squash something that's actually tested in to this PR!

zcmd_free_nvlists(&zc);
nvlist_free(zc_props);
nvlist_free(zc_fsprops);
nvlist_free(hidden_args);
if (wkeydata != NULL)
free(wkeydata);
return (zfs_error(hdl,
EZFS_BADDEV, errbuf));
}

zcmd_free_nvlists(&zc);
nvlist_free(zc_props);
nvlist_free(zc_fsprops);
Expand Down
10 changes: 9 additions & 1 deletion module/zfs/spa.c
Original file line number Diff line number Diff line change
Expand Up @@ -1900,6 +1900,10 @@ spa_activate(spa_t *spa, spa_mode_t mode)
static void
spa_deactivate(spa_t *spa)
{
if (spa->spa_create_info != NULL) {
nvlist_free(spa->spa_create_info);
spa->spa_create_info = NULL;
}
ASSERT(spa->spa_sync_on == B_FALSE);
ASSERT0P(spa->spa_dsl_pool);
ASSERT0P(spa->spa_root_vdev);
Expand Down Expand Up @@ -7013,7 +7017,7 @@ spa_create_check_encryption_params(dsl_crypto_params_t *dcp,
*/
int
spa_create(const char *pool, nvlist_t *nvroot, nvlist_t *props,
nvlist_t *zplprops, dsl_crypto_params_t *dcp)
nvlist_t *zplprops, dsl_crypto_params_t *dcp, nvlist_t **errinfo)
{
spa_t *spa;
const char *altroot = NULL;
Expand Down Expand Up @@ -7164,6 +7168,10 @@ spa_create(const char *pool, nvlist_t *nvroot, nvlist_t *props,
spa_config_exit(spa, SCL_ALL, FTAG);

if (error != 0) {
if (errinfo != NULL) {
*errinfo = spa->spa_create_info;
spa->spa_create_info = NULL;
}
spa_unload(spa);
spa_deactivate(spa);
spa_remove(spa);
Expand Down
23 changes: 22 additions & 1 deletion module/zfs/vdev_label.c
Original file line number Diff line number Diff line change
Expand Up @@ -1109,8 +1109,29 @@ vdev_label_init(vdev_t *vd, uint64_t crtxg, vdev_labeltype_t reason)
* Determine if the vdev is in use.
*/
if (reason != VDEV_LABEL_REMOVE && reason != VDEV_LABEL_SPLIT &&
vdev_inuse(vd, crtxg, reason, &spare_guid, &l2cache_guid))
vdev_inuse(vd, crtxg, reason, &spare_guid, &l2cache_guid)) {
if (spa->spa_create_info == NULL) {
nvlist_t *nv = fnvlist_alloc();
nvlist_t *cfg;

if (vd->vdev_path != NULL)
fnvlist_add_string(nv,
ZPOOL_CREATE_INFO_VDEV, vd->vdev_path);

cfg = vdev_label_read_config(vd, -1ULL);
if (cfg != NULL) {
const char *pname;
if (nvlist_lookup_string(cfg,
ZPOOL_CONFIG_POOL_NAME, &pname) == 0)
fnvlist_add_string(nv,
ZPOOL_CREATE_INFO_POOL, pname);
nvlist_free(cfg);
}

spa->spa_create_info = nv;
}
return (SET_ERROR(EBUSY));
}

/*
* If this is a request to add or replace a spare or l2cache device
Expand Down
12 changes: 11 additions & 1 deletion module/zfs/zfs_ioctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1468,6 +1468,7 @@ zfs_ioc_pool_create(zfs_cmd_t *zc)
dsl_crypto_params_t *dcp = NULL;
const char *spa_name = zc->zc_name;
boolean_t unload_wkey = B_TRUE;
nvlist_t *errinfo = NULL;

if ((error = get_nvlist(zc->zc_nvlist_conf, zc->zc_nvlist_conf_size,
zc->zc_iflags, &config)))
Expand Down Expand Up @@ -1519,7 +1520,16 @@ zfs_ioc_pool_create(zfs_cmd_t *zc)
spa_name = tname;
}

error = spa_create(zc->zc_name, config, props, zplprops, dcp);
error = spa_create(zc->zc_name, config, props, zplprops, dcp,
&errinfo);
if (errinfo != NULL) {
nvlist_t *outnv = fnvlist_alloc();
fnvlist_add_nvlist(outnv,
ZPOOL_CONFIG_CREATE_INFO, errinfo);
(void) put_nvlist(zc, outnv);
nvlist_free(outnv);
nvlist_free(errinfo);
}

/*
* Set the remaining root properties
Expand Down
2 changes: 1 addition & 1 deletion tests/runfiles/common.run
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ tests = ['zpool_create_001_pos', 'zpool_create_002_pos',
'zpool_create_features_005_pos', 'zpool_create_features_006_pos',
'zpool_create_features_007_pos', 'zpool_create_features_008_pos',
'zpool_create_features_009_pos', 'create-o_ashift',
'zpool_create_tempname', 'zpool_create_dryrun_output']
'zpool_create_tempname', 'zpool_create_errinfo_001_neg', 'zpool_create_dryrun_output']
tags = ['functional', 'cli_root', 'zpool_create']

[tests/functional/cli_root/zpool_destroy]
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -1092,6 +1092,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
functional/cli_root/zpool_create/zpool_create_features_008_pos.ksh \
functional/cli_root/zpool_create/zpool_create_features_009_pos.ksh \
functional/cli_root/zpool_create/zpool_create_tempname.ksh \
functional/cli_root/zpool_create/zpool_create_errinfo_001_neg.ksh \
functional/cli_root/zpool_destroy/zpool_destroy_001_pos.ksh \
functional/cli_root/zpool_destroy/zpool_destroy_002_pos.ksh \
functional/cli_root/zpool_destroy/zpool_destroy_003_neg.ksh \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
#!/bin/ksh -p
# SPDX-License-Identifier: CDDL-1.0
#
# CDDL HEADER START
#
# The contents of this file are subject to the terms of the
# Common Development and Distribution License (the "License").
# You may not use this file except in compliance with the License.
#
# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
# or https://opensource.org/licenses/CDDL-1.0.
# See the License for the specific language governing permissions
# and limitations under the License.
#
# When distributing Covered Code, include this CDDL HEADER in each
# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
# If applicable, add the following below this CDDL HEADER, with the
# fields enclosed by brackets "[]" replaced with your own identifying
# information: Portions Copyright [yyyy] [name of copyright owner]
#
# CDDL HEADER END
#

#
# Copyright (c) 2026, Christos Longros. All rights reserved.
#

. $STF_SUITE/include/libtest.shlib

#
# DESCRIPTION:
# 'zpool create' should report which device is in use when it fails
# because a vdev belongs to an active pool.
#
# STRATEGY:
# 1. Create a backing file for two block devices.
# 2. Attach two block devices to the same file.
# 3. Attempt to create a mirror pool using both devices.
# 4. Verify the error message identifies the specific device.
# 5. Verify the error message names the active pool.
#

verify_runnable "global"

TESTFILE="$TEST_BASE_DIR/vdev_errinfo"
TESTPOOL2="testpool_errinfo"
BLKDEV1=""
BLKDEV2=""

function cleanup
{
destroy_pool $TESTPOOL2
destroy_pool $TESTPOOL

if is_linux; then
[[ -n "$BLKDEV1" ]] && losetup -d "$BLKDEV1" 2>/dev/null
[[ -n "$BLKDEV2" ]] && losetup -d "$BLKDEV2" 2>/dev/null
elif is_freebsd; then
[[ -n "$BLKDEV1" ]] && mdconfig -d -u "$BLKDEV1" 2>/dev/null
[[ -n "$BLKDEV2" ]] && mdconfig -d -u "$BLKDEV2" 2>/dev/null
fi

rm -f "$TESTFILE"
}

log_assert "'zpool create' reports device-specific errors for in-use vdevs."
log_onexit cleanup

# Create a file to back the block devices
log_must truncate -s $MINVDEVSIZE "$TESTFILE"

# Attach two block devices to the same file (platform-specific)
if is_linux; then
BLKDEV1=$(losetup -f --show "$TESTFILE")
BLKDEV2=$(losetup -f --show "$TESTFILE")
elif is_freebsd; then
BLKDEV1=/dev/$(mdconfig -a -t vnode -f "$TESTFILE")
BLKDEV2=/dev/$(mdconfig -a -t vnode -f "$TESTFILE")
else
log_unsupported "Platform not supported for this test"
fi

log_note "Using devices: $BLKDEV1 $BLKDEV2"

# Attempt to create a mirror pool; this should fail because both
# devices refer to the same underlying file.
log_mustnot zpool create $TESTPOOL2 mirror $BLKDEV1 $BLKDEV2

# Re-run to capture the error message for content verification
errmsg=$(zpool create $TESTPOOL2 mirror $BLKDEV1 $BLKDEV2 2>&1)
log_note "zpool create output: $errmsg"

# Error message should name one of the devices
log_must eval "echo '$errmsg' | grep -qE '$BLKDEV1|$BLKDEV2'"

# Error message should name the active pool
if echo "$errmsg" | grep -q "active pool"; then
log_note "Error message correctly identifies the active pool"
else
log_fail "Error message does not mention the active pool: $errmsg"
fi

log_pass "'zpool create' reports device-specific errors for in-use vdevs."
Loading