Illumos 4168, 4169, 4170: ztest, zdb and zhack fixes

4168 ztest assertion failure in dbuf_undirty
4169 verbatim import causes zdb to segfault
4170 zhack leaves pool in ACTIVE state
Reviewed by: Adam Leventhal <ahl@delphix.com>
Reviewed by: Eric Schrock <eric.schrock@delphix.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Approved by: Dan McDonald <danmcd@nexenta.com>

References:
    https://www.illumos.org/issues/4168
    https://www.illumos.org/issues/4169
    https://www.illumos.org/issues/4170
    https://github.com/illumos/illumos-gate/commit/7fdd916

Porting notes:

Of particular interest when troubleshooting corrupted pools, the
commonly-used "zdb -e" operation may perform verbatim imports and
furthermore, it will soon have direct support for verbatim imports via
a new "-V" option.  The 4169 fix eliminates a common segfault case in
which spa_history_log_version() tries to access an un-opened dsl_pool_t.

Ported by: Tim Chase <tim@chase2k.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #2451
Closes #2283
Closes #2467
This commit is contained in:
George Wilson 2013-10-04 14:13:23 -08:00 committed by Brian Behlendorf
parent f4a4046bd6
commit 2fbc542ebd
3 changed files with 30 additions and 29 deletions

View File

@ -20,7 +20,7 @@
*/ */
/* /*
* Copyright (c) 2012 by Delphix. All rights reserved. * Copyright (c) 2013 by Delphix. All rights reserved.
* Copyright (c) 2013 Steven Hartland. All rights reserved. * Copyright (c) 2013 Steven Hartland. All rights reserved.
*/ */
@ -84,10 +84,15 @@ usage(void)
static void static void
fatal(const char *fmt, ...) fatal(spa_t *spa, void *tag, const char *fmt, ...)
{ {
va_list ap; va_list ap;
if (spa != NULL) {
spa_close(spa, tag);
(void) spa_export(g_pool, NULL, B_TRUE, B_FALSE);
}
va_start(ap, fmt); va_start(ap, fmt);
(void) fprintf(stderr, "%s: ", cmdname); (void) fprintf(stderr, "%s: ", cmdname);
(void) vfprintf(stderr, fmt, ap); (void) vfprintf(stderr, fmt, ap);
@ -158,13 +163,14 @@ import_pool(const char *target, boolean_t readonly)
g_importargs.can_be_active = B_TRUE; g_importargs.can_be_active = B_TRUE;
if (zpool_search_import(g_zfs, &g_importargs) != NULL || if (zpool_search_import(g_zfs, &g_importargs) != NULL ||
spa_open(target, &spa, FTAG) == 0) { spa_open(target, &spa, FTAG) == 0) {
fatal("cannot import '%s': pool is active; run " fatal(spa, FTAG, "cannot import '%s': pool is "
"\"zpool export %s\" first\n", "active; run " "\"zpool export %s\" "
g_pool, g_pool); "first\n", g_pool, g_pool);
} }
} }
fatal("cannot import '%s': no such pool available\n", g_pool); fatal(NULL, FTAG, "cannot import '%s': no such pool "
"available\n", g_pool);
} }
elem = nvlist_next_nvpair(pools, NULL); elem = nvlist_next_nvpair(pools, NULL);
@ -185,7 +191,8 @@ import_pool(const char *target, boolean_t readonly)
error = 0; error = 0;
if (error) if (error)
fatal("can't import '%s': %s", name, strerror(error)); fatal(NULL, FTAG, "can't import '%s': %s", name,
strerror(error));
} }
static void static void
@ -200,10 +207,11 @@ zhack_spa_open(const char *target, boolean_t readonly, void *tag, spa_t **spa)
zfeature_checks_disable = B_FALSE; zfeature_checks_disable = B_FALSE;
if (err != 0) if (err != 0)
fatal("cannot open '%s': %s", target, strerror(err)); fatal(*spa, FTAG, "cannot open '%s': %s", target,
strerror(err));
if (spa_version(*spa) < SPA_VERSION_FEATURES) { if (spa_version(*spa) < SPA_VERSION_FEATURES) {
fatal("'%s' has version %d, features not enabled", target, fatal(*spa, FTAG, "'%s' has version %d, features not enabled",
(int)spa_version(*spa)); target, (int)spa_version(*spa));
} }
} }
@ -336,15 +344,16 @@ zhack_do_feature_enable(int argc, char **argv)
feature.fi_guid = argv[1]; feature.fi_guid = argv[1];
if (!zfeature_is_valid_guid(feature.fi_guid)) if (!zfeature_is_valid_guid(feature.fi_guid))
fatal("invalid feature guid: %s", feature.fi_guid); fatal(NULL, FTAG, "invalid feature guid: %s", feature.fi_guid);
zhack_spa_open(target, B_FALSE, FTAG, &spa); zhack_spa_open(target, B_FALSE, FTAG, &spa);
mos = spa->spa_meta_objset; mos = spa->spa_meta_objset;
if (0 == zfeature_lookup_guid(feature.fi_guid, NULL)) if (0 == zfeature_lookup_guid(feature.fi_guid, NULL))
fatal("'%s' is a real feature, will not enable"); fatal(spa, FTAG, "'%s' is a real feature, will not enable");
if (0 == zap_contains(mos, spa->spa_feat_desc_obj, feature.fi_guid)) if (0 == zap_contains(mos, spa->spa_feat_desc_obj, feature.fi_guid))
fatal("feature already enabled: %s", feature.fi_guid); fatal(spa, FTAG, "feature already enabled: %s",
feature.fi_guid);
VERIFY0(dsl_sync_task(spa_name(spa), NULL, VERIFY0(dsl_sync_task(spa_name(spa), NULL,
feature_enable_sync, &feature, 5)); feature_enable_sync, &feature, 5));
@ -423,13 +432,14 @@ zhack_do_feature_ref(int argc, char **argv)
feature.fi_guid = argv[1]; feature.fi_guid = argv[1];
if (!zfeature_is_valid_guid(feature.fi_guid)) if (!zfeature_is_valid_guid(feature.fi_guid))
fatal("invalid feature guid: %s", feature.fi_guid); fatal(NULL, FTAG, "invalid feature guid: %s", feature.fi_guid);
zhack_spa_open(target, B_FALSE, FTAG, &spa); zhack_spa_open(target, B_FALSE, FTAG, &spa);
mos = spa->spa_meta_objset; mos = spa->spa_meta_objset;
if (0 == zfeature_lookup_guid(feature.fi_guid, NULL)) if (0 == zfeature_lookup_guid(feature.fi_guid, NULL))
fatal("'%s' is a real feature, will not change refcount"); fatal(spa, FTAG, "'%s' is a real feature, will not change "
"refcount");
if (0 == zap_contains(mos, spa->spa_feat_for_read_obj, if (0 == zap_contains(mos, spa->spa_feat_for_read_obj,
feature.fi_guid)) { feature.fi_guid)) {
@ -438,11 +448,12 @@ zhack_do_feature_ref(int argc, char **argv)
feature.fi_guid)) { feature.fi_guid)) {
feature.fi_can_readonly = B_TRUE; feature.fi_can_readonly = B_TRUE;
} else { } else {
fatal("feature is not enabled: %s", feature.fi_guid); fatal(spa, FTAG, "feature is not enabled: %s", feature.fi_guid);
} }
if (decr && !spa_feature_is_active(spa, &feature)) if (decr && !spa_feature_is_active(spa, &feature))
fatal("feature refcount already 0: %s", feature.fi_guid); fatal(spa, FTAG, "feature refcount already 0: %s",
feature.fi_guid);
VERIFY0(dsl_sync_task(spa_name(spa), NULL, VERIFY0(dsl_sync_task(spa_name(spa), NULL,
decr ? feature_decr_sync : feature_incr_sync, &feature, 5)); decr ? feature_decr_sync : feature_incr_sync, &feature, 5));
@ -530,8 +541,8 @@ main(int argc, char **argv)
usage(); usage();
} }
if (!g_readonly && spa_export(g_pool, NULL, B_TRUE, B_TRUE) != 0) { if (!g_readonly && spa_export(g_pool, NULL, B_TRUE, B_FALSE) != 0) {
fatal("pool export failed; " fatal(NULL, FTAG, "pool export failed; "
"changes may not be committed to disk\n"); "changes may not be committed to disk\n");
} }

View File

@ -1391,14 +1391,6 @@ dbuf_undirty(dmu_buf_impl_t *db, dmu_tx_t *tx)
DB_DNODE_ENTER(db); DB_DNODE_ENTER(db);
dn = DB_DNODE(db); dn = DB_DNODE(db);
/*
* Note: This code will probably work even if there are concurrent
* holders, but it is untested in that scenerio, as the ZPL and
* ztest have additional locking (the range locks) that prevents
* that type of concurrent access.
*/
ASSERT3U(refcount_count(&db->db_holds), ==, db->db_dirtycnt);
dprintf_dbuf(db, "size=%llx\n", (u_longlong_t)db->db.db_size); dprintf_dbuf(db, "size=%llx\n", (u_longlong_t)db->db.db_size);
ASSERT(db->db.db_size != 0); ASSERT(db->db.db_size != 0);

View File

@ -3901,8 +3901,6 @@ spa_import(char *pool, nvlist_t *config, nvlist_t *props, uint64_t flags)
spa_config_sync(spa, B_FALSE, B_TRUE); spa_config_sync(spa, B_FALSE, B_TRUE);
mutex_exit(&spa_namespace_lock); mutex_exit(&spa_namespace_lock);
spa_history_log_version(spa, "import");
return (0); return (0);
} }