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 <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #6933
Closes #6971

Author: behlendorf

module/zfs/spa.c

@@ -2341,7 +2341,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;
@@ -2358,7 +2359,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
@@ -2384,8 +2384,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);
 
@@ -2651,7 +2655,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) {

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
 

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
 }
 

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