From cc900376085a521ab114570221d8d361209ff5dc Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Mon, 18 Dec 2017 10:28:27 -0800 Subject: [PATCH] Fix multihost stale cache file import When the multihost property is enabled it should be impossible to import an active pool even using the force (-f) option. This patch prevents a forced import from succeeding when importing with a stale cache file. The root cause of the problem is that the kernel modules trusted the hostid provided in configuration. This is always correct when the configuration is generated by scanning for the pool. However, when using an existing cache file the hostid could be stale which would result in the activity check being skipped. Resolve the issue by always using the hostid read from the label configuration where the best uberblock was found. Reviewed-by: Olaf Faaland Reviewed-by: Tony Hutter Signed-off-by: Brian Behlendorf Closes #6933 Closes #6971 --- module/zfs/spa.c | 12 ++++++++---- tests/zfs-tests/tests/functional/mmp/mmp.cfg | 2 ++ tests/zfs-tests/tests/functional/mmp/mmp.kshlib | 11 +++++++---- .../tests/functional/mmp/mmp_active_import.ksh | 10 +++++++++- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/module/zfs/spa.c b/module/zfs/spa.c index e06190f9db83..8844b9f7bc8a 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -2330,7 +2330,8 @@ vdev_count_verify_zaps(vdev_t *vd) * Determine whether the activity check is required. */ static boolean_t -spa_activity_check_required(spa_t *spa, uberblock_t *ub, nvlist_t *config) +spa_activity_check_required(spa_t *spa, uberblock_t *ub, nvlist_t *label, + nvlist_t *config) { uint64_t state = 0; uint64_t hostid = 0; @@ -2347,7 +2348,6 @@ spa_activity_check_required(spa_t *spa, uberblock_t *ub, nvlist_t *config) } (void) nvlist_lookup_uint64(config, ZPOOL_CONFIG_POOL_STATE, &state); - (void) nvlist_lookup_uint64(config, ZPOOL_CONFIG_HOSTID, &hostid); /* * Disable the MMP activity check - This is used by zdb which @@ -2373,8 +2373,12 @@ spa_activity_check_required(spa_t *spa, uberblock_t *ub, nvlist_t *config) /* * Allow the activity check to be skipped when importing the pool - * on the same host which last imported it. + * on the same host which last imported it. Since the hostid from + * configuration may be stale use the one read from the label. */ + if (nvlist_exists(label, ZPOOL_CONFIG_HOSTID)) + hostid = fnvlist_lookup_uint64(label, ZPOOL_CONFIG_HOSTID); + if (hostid == spa_get_hostid()) return (B_FALSE); @@ -2639,7 +2643,7 @@ spa_load_impl(spa_t *spa, uint64_t pool_guid, nvlist_t *config, * pool is truly inactive and can be safely imported. Prevent * hosts which don't have a hostid set from importing the pool. */ - activity_check = spa_activity_check_required(spa, ub, config); + activity_check = spa_activity_check_required(spa, ub, label, config); if (activity_check) { if (ub->ub_mmp_magic == MMP_MAGIC && ub->ub_mmp_delay && spa_get_hostid() == 0) { diff --git a/tests/zfs-tests/tests/functional/mmp/mmp.cfg b/tests/zfs-tests/tests/functional/mmp/mmp.cfg index 29e030a4301a..36f9954435d1 100644 --- a/tests/zfs-tests/tests/functional/mmp/mmp.cfg +++ b/tests/zfs-tests/tests/functional/mmp/mmp.cfg @@ -30,6 +30,8 @@ export TXG_TIMEOUT_DEFAULT=5 export MMP_POOL=mmppool export MMP_DIR=$TEST_BASE_DIR/mmp +export MMP_CACHE=$MMP_DIR/zpool.cache +export MMP_ZTEST_LOG=$MMP_DIR/ztest.log export MMP_HISTORY=100 export MMP_HISTORY_OFF=0 diff --git a/tests/zfs-tests/tests/functional/mmp/mmp.kshlib b/tests/zfs-tests/tests/functional/mmp/mmp.kshlib index 43e4f37119a3..4c46ae7a2add 100644 --- a/tests/zfs-tests/tests/functional/mmp/mmp.kshlib +++ b/tests/zfs-tests/tests/functional/mmp/mmp.kshlib @@ -97,21 +97,24 @@ function mmp_pool_create # pool dir { typeset pool=${1:-$MMP_POOL} typeset dir=${2:-$MMP_DIR} - typeset opts="-T120 -M -k0 -f $dir -E -p $pool" + typeset opts="-VVVVV -T120 -M -k0 -f $dir -E -p $pool" log_must mkdir -p $dir + log_must rm -f $dir/* log_must truncate -s $MINVDEVSIZE $dir/vdev1 $dir/vdev2 log_must mmp_clear_hostid log_must mmp_set_hostid $HOSTID1 - log_must zpool create -f $pool mirror $dir/vdev1 $dir/vdev2 + log_must zpool create -f -o cachefile=$MMP_CACHE $pool \ + mirror $dir/vdev1 $dir/vdev2 log_must zpool set multihost=on $pool + log_must mv $MMP_CACHE ${MMP_CACHE}.stale log_must zpool export $pool log_must mmp_clear_hostid log_must mmp_set_hostid $HOSTID2 log_note "Starting ztest in the background as hostid $HOSTID1" - log_must eval "ZFS_HOSTID=$HOSTID1 ztest $opts >/dev/null 2>&1 &" + log_must eval "ZFS_HOSTID=$HOSTID1 ztest $opts >$MMP_ZTEST_LOG 2>&1 &" while ! is_pool_imported "$pool" "-d $dir"; do log_must pgrep ztest @@ -134,7 +137,7 @@ function mmp_pool_destroy # pool dir destroy_pool $pool fi - rm -Rf $dir + log_must rm -f $dir/* mmp_clear_hostid } diff --git a/tests/zfs-tests/tests/functional/mmp/mmp_active_import.ksh b/tests/zfs-tests/tests/functional/mmp/mmp_active_import.ksh index 0a487d5b1499..035264fe0f4e 100755 --- a/tests/zfs-tests/tests/functional/mmp/mmp_active_import.ksh +++ b/tests/zfs-tests/tests/functional/mmp/mmp_active_import.ksh @@ -41,7 +41,7 @@ verify_runnable "both" function cleanup { - mmp_pool_destroy $MMP_DIR $MMP_POOL + mmp_pool_destroy $MMP_POOL $MMP_DIR log_must set_tunable64 zfs_multihost_interval $MMP_INTERVAL_DEFAULT log_must mmp_clear_hostid } @@ -60,11 +60,19 @@ log_must is_pool_imported $MMP_POOL "-d $MMP_DIR" # 3. Verify 'zpool import [-f] $MMP_POOL' cannot import the pool. MMP_IMPORTED_MSG="Cannot import '$MMP_POOL': pool is imported" + log_must try_pool_import $MMP_POOL "-d $MMP_DIR" "$MMP_IMPORTED_MSG" for i in {1..10}; do log_must try_pool_import $MMP_POOL "-f -d $MMP_DIR" "$MMP_IMPORTED_MSG" done +log_must try_pool_import $MMP_POOL "-c ${MMP_CACHE}.stale" "$MMP_IMPORTED_MSG" + +for i in {1..10}; do + log_must try_pool_import $MMP_POOL "-f -c ${MMP_CACHE}.stale" \ + "$MMP_IMPORTED_MSG" +done + # 4. Kill ztest to make pool eligible for import. Poll with 'zpool status'. ZTESTPID=$(pgrep ztest) if [ -n "$ZTESTPID" ]; then