Skip to content

Commit 33ed68f

Browse files
chrislongroschrislongros
authored
zpool create: report which device caused failure
When zpool create fails because a vdev is already in use, the error message now identifies the problematic device and the pool it belongs to, e.g.: cannot create 'tank': device '/dev/sdb1' is part of active pool 'rpool' Implementation follows the ZPOOL_CONFIG_LOAD_INFO pattern used by zpool import: - Add spa_create_info to spa_t to capture error info during vdev_label_init(), before vdev_close() resets vdev state - When vdev_inuse() detects a conflict, read the on-disk label to extract the pool name and store it with the device path - Return the info wrapped under ZPOOL_CONFIG_CREATE_INFO through the ioctl zc_nvlist_dst to userspace - In libzfs, zpool_create_info() unwraps the nvlist and formats the device-specific error message Restructure zpool_create() error handling so all switch cases use break instead of return, eliminating duplicated cleanup code and using the single create_failed exit path. Reviewed-by: Tony Hutter <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Christos Longros <[email protected]> Closes #18213
1 parent cfae167 commit 33ed68f

File tree

11 files changed

+233
-34
lines changed

11 files changed

+233
-34
lines changed

‎cmd/ztest.c‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3038,15 +3038,15 @@ ztest_spa_create_destroy(ztest_ds_t *zd, uint64_t id)
30383038
*/
30393039
nvroot = make_vdev_root("/dev/bogus", NULL, NULL, 0, 0, NULL, 0, 0, 1);
30403040
VERIFY3U(ENOENT, ==,
3041-
spa_create("ztest_bad_file", nvroot, NULL, NULL, NULL));
3041+
spa_create("ztest_bad_file", nvroot, NULL, NULL, NULL, NULL));
30423042
fnvlist_free(nvroot);
30433043

30443044
/*
30453045
* Attempt to create using a bad mirror.
30463046
*/
30473047
nvroot = make_vdev_root("/dev/bogus", NULL, NULL, 0, 0, NULL, 0, 2, 1);
30483048
VERIFY3U(ENOENT, ==,
3049-
spa_create("ztest_bad_mirror", nvroot, NULL, NULL, NULL));
3049+
spa_create("ztest_bad_mirror", nvroot, NULL, NULL, NULL, NULL));
30503050
fnvlist_free(nvroot);
30513051

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

30623062
/*
@@ -3208,7 +3208,7 @@ ztest_spa_upgrade(ztest_ds_t *zd, uint64_t id)
32083208
props = fnvlist_alloc();
32093209
fnvlist_add_uint64(props,
32103210
zpool_prop_to_name(ZPOOL_PROP_VERSION), version);
3211-
VERIFY0(spa_create(name, nvroot, props, NULL, NULL));
3211+
VERIFY0(spa_create(name, nvroot, props, NULL, NULL, NULL));
32123212
fnvlist_free(nvroot);
32133213
fnvlist_free(props);
32143214

@@ -8686,7 +8686,8 @@ ztest_init(ztest_shared_t *zs)
86868686
free(buf);
86878687
}
86888688

8689-
VERIFY0(spa_create(ztest_opts.zo_pool, nvroot, props, NULL, NULL));
8689+
VERIFY0(spa_create(ztest_opts.zo_pool, nvroot, props,
8690+
NULL, NULL, NULL));
86908691
fnvlist_free(nvroot);
86918692
fnvlist_free(props);
86928693

‎include/sys/fs/zfs.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,9 @@ typedef struct zpool_load_policy {
952952
#define ZPOOL_CONFIG_BOOTFS "bootfs" /* not stored on disk */
953953
#define ZPOOL_CONFIG_MISSING_DEVICES "missing_vdevs" /* not stored on disk */
954954
#define ZPOOL_CONFIG_LOAD_INFO "load_info" /* not stored on disk */
955+
#define ZPOOL_CONFIG_CREATE_INFO "create_info" /* not stored on disk */
956+
#define ZPOOL_CREATE_INFO_VDEV "create_err_vdev"
957+
#define ZPOOL_CREATE_INFO_POOL "create_err_pool"
955958
#define ZPOOL_CONFIG_REWIND_INFO "rewind_info" /* not stored on disk */
956959
#define ZPOOL_CONFIG_UNSUP_FEAT "unsup_feat" /* not stored on disk */
957960
#define ZPOOL_CONFIG_ENABLED_FEAT "enabled_feat" /* not stored on disk */

‎include/sys/spa.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -777,7 +777,7 @@ extern int spa_open_rewind(const char *pool, spa_t **, const void *tag,
777777
extern int spa_get_stats(const char *pool, nvlist_t **config, char *altroot,
778778
size_t buflen);
779779
extern int spa_create(const char *pool, nvlist_t *nvroot, nvlist_t *props,
780-
nvlist_t *zplprops, struct dsl_crypto_params *dcp);
780+
nvlist_t *zplprops, struct dsl_crypto_params *dcp, nvlist_t **errinfo);
781781
extern int spa_import(char *pool, nvlist_t *config, nvlist_t *props,
782782
uint64_t flags);
783783
extern nvlist_t *spa_tryimport(nvlist_t *tryconfig);

‎include/sys/spa_impl.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ struct spa {
229229
nvlist_t *spa_config_syncing; /* currently syncing config */
230230
nvlist_t *spa_config_splitting; /* config for splitting */
231231
nvlist_t *spa_load_info; /* info and errors from load */
232+
nvlist_t *spa_create_info; /* info from create */
232233
uint64_t spa_config_txg; /* txg of last config change */
233234
uint32_t spa_sync_pass; /* iterate-to-convergence */
234235
pool_state_t spa_state; /* pool state */

‎lib/libzfs/libzfs_pool.c‎

Lines changed: 75 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,6 +1536,50 @@ zpool_is_draid_spare(const char *name)
15361536
return (B_FALSE);
15371537
}
15381538

1539+
1540+
/*
1541+
* Extract device-specific error information from a failed pool creation.
1542+
* If the kernel returned ZPOOL_CONFIG_CREATE_INFO in the ioctl output,
1543+
* set an appropriate error aux message identifying the problematic device.
1544+
*/
1545+
static int
1546+
zpool_create_info(libzfs_handle_t *hdl, zfs_cmd_t *zc)
1547+
{
1548+
nvlist_t *outnv = NULL;
1549+
nvlist_t *info = NULL;
1550+
const char *vdev = NULL;
1551+
const char *pname = NULL;
1552+
1553+
if (zc->zc_nvlist_dst_size == 0)
1554+
return (ENOENT);
1555+
1556+
if (nvlist_unpack((void *)(uintptr_t)zc->zc_nvlist_dst,
1557+
zc->zc_nvlist_dst_size, &outnv, 0) != 0 || outnv == NULL)
1558+
return (EINVAL);
1559+
1560+
if (nvlist_lookup_nvlist(outnv, ZPOOL_CONFIG_CREATE_INFO, &info) != 0) {
1561+
nvlist_free(outnv);
1562+
return (EINVAL);
1563+
}
1564+
1565+
if (nvlist_lookup_string(info, ZPOOL_CREATE_INFO_VDEV, &vdev) != 0) {
1566+
nvlist_free(outnv);
1567+
return (EINVAL);
1568+
}
1569+
1570+
if (nvlist_lookup_string(info, ZPOOL_CREATE_INFO_POOL, &pname) == 0) {
1571+
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
1572+
"device '%s' is part of active pool '%s'"),
1573+
vdev, pname);
1574+
} else {
1575+
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
1576+
"device '%s' is in use"), vdev);
1577+
}
1578+
1579+
nvlist_free(outnv);
1580+
return (0);
1581+
}
1582+
15391583
/*
15401584
* Create the named pool, using the provided vdev list. It is assumed
15411585
* that the consumer has already validated the contents of the nvlist, so we
@@ -1615,16 +1659,9 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
16151659
zcmd_write_src_nvlist(hdl, &zc, zc_props);
16161660

16171661
(void) strlcpy(zc.zc_name, pool, sizeof (zc.zc_name));
1662+
zcmd_alloc_dst_nvlist(hdl, &zc, 4096);
16181663

16191664
if ((ret = zfs_ioctl(hdl, ZFS_IOC_POOL_CREATE, &zc)) != 0) {
1620-
1621-
zcmd_free_nvlists(&zc);
1622-
nvlist_free(zc_props);
1623-
nvlist_free(zc_fsprops);
1624-
nvlist_free(hidden_args);
1625-
if (wkeydata != NULL)
1626-
free(wkeydata);
1627-
16281665
switch (errno) {
16291666
case EBUSY:
16301667
/*
@@ -1634,11 +1671,14 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
16341671
* label. This can also happen under if the device is
16351672
* part of an active md or lvm device.
16361673
*/
1637-
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
1638-
"one or more vdevs refer to the same device, or "
1639-
"one of\nthe devices is part of an active md or "
1640-
"lvm device"));
1641-
return (zfs_error(hdl, EZFS_BADDEV, errbuf));
1674+
if (zpool_create_info(hdl, &zc) != 0) {
1675+
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
1676+
"one or more vdevs refer to the same "
1677+
"device, or one of\nthe devices is "
1678+
"part of an active md or lvm device"));
1679+
}
1680+
ret = zfs_error(hdl, EZFS_BADDEV, errbuf);
1681+
break;
16421682

16431683
case ERANGE:
16441684
/*
@@ -1653,7 +1693,8 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
16531693
*/
16541694
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
16551695
"record size invalid"));
1656-
return (zfs_error(hdl, EZFS_BADPROP, errbuf));
1696+
ret = zfs_error(hdl, EZFS_BADPROP, errbuf);
1697+
break;
16571698

16581699
case EOVERFLOW:
16591700
/*
@@ -1672,37 +1713,47 @@ zpool_create(libzfs_handle_t *hdl, const char *pool, nvlist_t *nvroot,
16721713
"one or more devices is less than the "
16731714
"minimum size (%s)"), buf);
16741715
}
1675-
return (zfs_error(hdl, EZFS_BADDEV, errbuf));
1716+
ret = zfs_error(hdl, EZFS_BADDEV, errbuf);
1717+
break;
16761718

16771719
case ENOSPC:
16781720
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
16791721
"one or more devices is out of space"));
1680-
return (zfs_error(hdl, EZFS_BADDEV, errbuf));
1722+
ret = zfs_error(hdl, EZFS_BADDEV, errbuf);
1723+
break;
16811724

16821725
case EINVAL:
16831726
if (zpool_has_draid_vdev(nvroot) &&
16841727
zfeature_lookup_name("draid", NULL) != 0) {
16851728
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
16861729
"dRAID vdevs are unsupported by the "
16871730
"kernel"));
1688-
return (zfs_error(hdl, EZFS_BADDEV, errbuf));
1731+
ret = zfs_error(hdl, EZFS_BADDEV, errbuf);
16891732
} else {
1690-
return (zpool_standard_error(hdl, errno,
1691-
errbuf));
1733+
ret = zpool_standard_error(hdl, errno, errbuf);
16921734
}
1735+
break;
16931736

16941737
case ENXIO:
1695-
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
1696-
"one or more devices could not be opened"));
1697-
return (zfs_error(hdl, EZFS_BADDEV, errbuf));
1738+
if (zpool_create_info(hdl, &zc) == 0) {
1739+
ret = zfs_error(hdl, EZFS_BADDEV, errbuf);
1740+
} else {
1741+
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
1742+
"one or more devices could not be "
1743+
"opened"));
1744+
ret = zfs_error(hdl, EZFS_BADDEV, errbuf);
1745+
}
1746+
break;
16981747

16991748
case EDOM:
17001749
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
17011750
"block size out of range or does not match"));
1702-
return (zfs_error(hdl, EZFS_BADDEV, errbuf));
1751+
ret = zfs_error(hdl, EZFS_BADDEV, errbuf);
1752+
break;
17031753

17041754
default:
1705-
return (zpool_standard_error(hdl, errno, errbuf));
1755+
ret = zpool_standard_error(hdl, errno, errbuf);
1756+
break;
17061757
}
17071758
}
17081759

‎module/zfs/spa.c‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1947,6 +1947,10 @@ spa_activate(spa_t *spa, spa_mode_t mode)
19471947
static void
19481948
spa_deactivate(spa_t *spa)
19491949
{
1950+
if (spa->spa_create_info != NULL) {
1951+
nvlist_free(spa->spa_create_info);
1952+
spa->spa_create_info = NULL;
1953+
}
19501954
ASSERT(spa->spa_sync_on == B_FALSE);
19511955
ASSERT0P(spa->spa_dsl_pool);
19521956
ASSERT0P(spa->spa_root_vdev);
@@ -7060,7 +7064,7 @@ spa_create_check_encryption_params(dsl_crypto_params_t *dcp,
70607064
*/
70617065
int
70627066
spa_create(const char *pool, nvlist_t *nvroot, nvlist_t *props,
7063-
nvlist_t *zplprops, dsl_crypto_params_t *dcp)
7067+
nvlist_t *zplprops, dsl_crypto_params_t *dcp, nvlist_t **errinfo)
70647068
{
70657069
spa_t *spa;
70667070
const char *altroot = NULL;
@@ -7212,6 +7216,10 @@ spa_create(const char *pool, nvlist_t *nvroot, nvlist_t *props,
72127216
spa_config_exit(spa, SCL_ALL, FTAG);
72137217

72147218
if (error != 0) {
7219+
if (errinfo != NULL) {
7220+
*errinfo = spa->spa_create_info;
7221+
spa->spa_create_info = NULL;
7222+
}
72157223
spa_unload(spa);
72167224
spa_deactivate(spa);
72177225
spa_remove(spa);

‎module/zfs/vdev_label.c‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1109,8 +1109,29 @@ vdev_label_init(vdev_t *vd, uint64_t crtxg, vdev_labeltype_t reason)
11091109
* Determine if the vdev is in use.
11101110
*/
11111111
if (reason != VDEV_LABEL_REMOVE && reason != VDEV_LABEL_SPLIT &&
1112-
vdev_inuse(vd, crtxg, reason, &spare_guid, &l2cache_guid))
1112+
vdev_inuse(vd, crtxg, reason, &spare_guid, &l2cache_guid)) {
1113+
if (spa->spa_create_info == NULL) {
1114+
nvlist_t *nv = fnvlist_alloc();
1115+
nvlist_t *cfg;
1116+
1117+
if (vd->vdev_path != NULL)
1118+
fnvlist_add_string(nv,
1119+
ZPOOL_CREATE_INFO_VDEV, vd->vdev_path);
1120+
1121+
cfg = vdev_label_read_config(vd, -1ULL);
1122+
if (cfg != NULL) {
1123+
const char *pname;
1124+
if (nvlist_lookup_string(cfg,
1125+
ZPOOL_CONFIG_POOL_NAME, &pname) == 0)
1126+
fnvlist_add_string(nv,
1127+
ZPOOL_CREATE_INFO_POOL, pname);
1128+
nvlist_free(cfg);
1129+
}
1130+
1131+
spa->spa_create_info = nv;
1132+
}
11131133
return (SET_ERROR(EBUSY));
1134+
}
11141135

11151136
/*
11161137
* If this is a request to add or replace a spare or l2cache device

‎module/zfs/zfs_ioctl.c‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1660,6 +1660,7 @@ zfs_ioc_pool_create(zfs_cmd_t *zc)
16601660
dsl_crypto_params_t *dcp = NULL;
16611661
const char *spa_name = zc->zc_name;
16621662
boolean_t unload_wkey = B_TRUE;
1663+
nvlist_t *errinfo = NULL;
16631664

16641665
if ((error = get_nvlist(zc->zc_nvlist_conf, zc->zc_nvlist_conf_size,
16651666
zc->zc_iflags, &config)))
@@ -1711,7 +1712,16 @@ zfs_ioc_pool_create(zfs_cmd_t *zc)
17111712
spa_name = tname;
17121713
}
17131714

1714-
error = spa_create(zc->zc_name, config, props, zplprops, dcp);
1715+
error = spa_create(zc->zc_name, config, props, zplprops, dcp,
1716+
&errinfo);
1717+
if (errinfo != NULL) {
1718+
nvlist_t *outnv = fnvlist_alloc();
1719+
fnvlist_add_nvlist(outnv,
1720+
ZPOOL_CONFIG_CREATE_INFO, errinfo);
1721+
(void) put_nvlist(zc, outnv);
1722+
nvlist_free(outnv);
1723+
nvlist_free(errinfo);
1724+
}
17151725

17161726
/*
17171727
* Set the remaining root properties

‎tests/runfiles/common.run‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,7 @@ tests = ['zpool_create_001_pos', 'zpool_create_002_pos',
430430
'zpool_create_features_005_pos', 'zpool_create_features_006_pos',
431431
'zpool_create_features_007_pos', 'zpool_create_features_008_pos',
432432
'zpool_create_features_009_pos', 'create-o_ashift',
433-
'zpool_create_tempname', 'zpool_create_dryrun_output']
433+
'zpool_create_tempname', 'zpool_create_errinfo_001_neg', 'zpool_create_dryrun_output']
434434
tags = ['functional', 'cli_root', 'zpool_create']
435435

436436
[tests/functional/cli_root/zpool_destroy]

‎tests/zfs-tests/tests/Makefile.am‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,6 +1098,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
10981098
functional/cli_root/zpool_create/zpool_create_features_008_pos.ksh \
10991099
functional/cli_root/zpool_create/zpool_create_features_009_pos.ksh \
11001100
functional/cli_root/zpool_create/zpool_create_tempname.ksh \
1101+
functional/cli_root/zpool_create/zpool_create_errinfo_001_neg.ksh \
11011102
functional/cli_root/zpool_destroy/zpool_destroy_001_pos.ksh \
11021103
functional/cli_root/zpool_destroy/zpool_destroy_002_pos.ksh \
11031104
functional/cli_root/zpool_destroy/zpool_destroy_003_neg.ksh \

0 commit comments

Comments
 (0)