Skip to content

Commit

Permalink
Use cached feature info in spa_add_feature_stats()
Browse files Browse the repository at this point in the history
Avoid issuing I/O to the pool when retrieving feature flags information.
Trying to read the ZAPs from disk means that zpool clear would hang if
the pool is suspended and recovery would require a reboot. We need to
ensure the list of features enabled on the pool stays resident in
memory.

There already exists an nvlist for this purpose called
ZPOOL_CONFIG_ENABLED_FEAT in spa->spa_load_info. The features_for_write
features aren't added to the list, however, in the final import phase.
This patch updates spa_load_impl() so the features_for_read and
features_for_write features are both added to ZPOOL_CONFIG_ENABLED_FEAT
in each import phase. The full list of pool features and their initial
reference counts are now available for use in spa_add_feature_stats().

In spa_add_feature_stats() we iterate over the spa_feature_table[]
rather than the ZAP objects read from disk. We operate on
ZPOOL_CONFIG_ENABLED_FEAT in-place and refresh the reference counts for
the supported features from the values cached in the spa_t. This is
sufficient for 'zpool get all' to display correct values for the
unsupported@ and feature@ properties. For the unsupported features, if
the reference count is 0 is shows 'inactive' otherwise it shows
'read-only'. In either case, the unsupported reference counts could not
have changed after import, so it is safe to rely on the initial values.

Fixes openzfs#3082

Signed-off-by: Ned Bass <[email protected]>
  • Loading branch information
nedbass committed Mar 3, 2015
1 parent d14cfd8 commit fc9f056
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 37 deletions.
54 changes: 18 additions & 36 deletions module/zfs/spa.c
Original file line number Diff line number Diff line change
Expand Up @@ -2395,11 +2395,10 @@ spa_load_impl(spa_t *spa, uint64_t pool_guid, nvlist_t *config,
unsup_feat, enabled_feat))
missing_feat_read = B_TRUE;

if (spa_writeable(spa) || state == SPA_LOAD_TRYIMPORT) {
if (!spa_features_check(spa, B_TRUE,
unsup_feat, enabled_feat)) {
missing_feat_write = B_TRUE;
}
if (!spa_features_check(spa, B_TRUE,
unsup_feat, enabled_feat) &&
(spa_writeable(spa) || state == SPA_LOAD_TRYIMPORT)) {
missing_feat_write = B_TRUE;
}

fnvlist_add_nvlist(spa->spa_load_info,
Expand Down Expand Up @@ -3205,41 +3204,24 @@ static void
spa_add_feature_stats(spa_t *spa, nvlist_t *config)
{
nvlist_t *features;
zap_cursor_t zc;
zap_attribute_t za;
int i;

ASSERT(spa_config_held(spa, SCL_CONFIG, RW_READER));
VERIFY(nvlist_alloc(&features, NV_UNIQUE_NAME, KM_SLEEP) == 0);

if (spa->spa_feat_for_read_obj != 0) {
for (zap_cursor_init(&zc, spa->spa_meta_objset,
spa->spa_feat_for_read_obj);
zap_cursor_retrieve(&zc, &za) == 0;
zap_cursor_advance(&zc)) {
ASSERT(za.za_integer_length == sizeof (uint64_t) &&
za.za_num_integers == 1);
VERIFY3U(0, ==, nvlist_add_uint64(features, za.za_name,
za.za_first_integer));
}
zap_cursor_fini(&zc);
}

if (spa->spa_feat_for_write_obj != 0) {
for (zap_cursor_init(&zc, spa->spa_meta_objset,
spa->spa_feat_for_write_obj);
zap_cursor_retrieve(&zc, &za) == 0;
zap_cursor_advance(&zc)) {
ASSERT(za.za_integer_length == sizeof (uint64_t) &&
za.za_num_integers == 1);
VERIFY3U(0, ==, nvlist_add_uint64(features, za.za_name,
za.za_first_integer));
}
zap_cursor_fini(&zc);

VERIFY0(nvlist_lookup_nvlist(spa->spa_load_info,
ZPOOL_CONFIG_ENABLED_FEAT, &features));

for (i = 0; i < SPA_FEATURES; i++) {
zfeature_info_t feature = spa_feature_table[i];
uint64_t refcount;

if (feature_get_refcount(spa, &feature, &refcount) == 0)
VERIFY0(nvlist_add_uint64(features, feature.fi_guid,
refcount));
}

VERIFY(nvlist_add_nvlist(config, ZPOOL_CONFIG_FEATURE_STATS,
features) == 0);
nvlist_free(features);
VERIFY0(nvlist_add_nvlist(config, ZPOOL_CONFIG_FEATURE_STATS,
features));
}

int
Expand Down
2 changes: 1 addition & 1 deletion module/zfs/zfeature.c
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ spa_features_check(spa_t *spa, boolean_t for_write,
*
* Note: well-designed features will not need to use this; they should
* use spa_feature_is_enabled() and spa_feature_is_active() instead.
* However, this is non-static for zdb and zhack.
* However, this is non-static for zdb, zhack, and spa_add_feature_stats().
*/
int
feature_get_refcount(spa_t *spa, zfeature_info_t *feature, uint64_t *res)
Expand Down

0 comments on commit fc9f056

Please sign in to comment.