Fix 'zfs userspace' for received datasets in encrypted root

For encrypted receives, where user accounting is initially disabled on
creation, both 'zfs userspace' and 'zfs groupspace' fails with
EOPNOTSUPP: this is because dmu_objset_id_quota_upgrade_cb() forgets to
set OBJSET_FLAG_USERACCOUNTING_COMPLETE on the objset flags after a
successful dmu_objset_space_upgrade().

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Co-authored-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #9501
Closes #9596

include/sys/dmu_objset.h

@@ -245,7 +245,7 @@ void dmu_objset_evict(objset_t *os);
 void dmu_objset_do_userquota_updates(objset_t *os, dmu_tx_t *tx);
 void dmu_objset_userquota_get_ids(dnode_t *dn, boolean_t before, dmu_tx_t *tx);
 boolean_t dmu_objset_userused_enabled(objset_t *os);
-int dmu_objset_userspace_upgrade(objset_t *os);
+void dmu_objset_userspace_upgrade(objset_t *os);
 boolean_t dmu_objset_userspace_present(objset_t *os);
 boolean_t dmu_objset_userobjused_enabled(objset_t *os);
 boolean_t dmu_objset_userobjspace_upgradable(objset_t *os);

module/zfs/dmu_objset.c

@@ -778,11 +778,15 @@ dmu_objset_own(const char *name, dmu_objset_type_t type,
 	 * speed up pool import times and to keep this txg reserved
 	 * completely for recovery work.
 	 */
-	if ((dmu_objset_userobjspace_upgradable(*osp) ||
-	    dmu_objset_projectquota_upgradable(*osp)) &&
-	    !readonly && !dp->dp_spa->spa_claiming &&
-	    (ds->ds_dir->dd_crypto_obj == 0 || decrypt))
+	if (!readonly && !dp->dp_spa->spa_claiming &&
+	    (ds->ds_dir->dd_crypto_obj == 0 || decrypt)) {
+		if (dmu_objset_userobjspace_upgradable(*osp) ||
+		    dmu_objset_projectquota_upgradable(*osp)) {
 			dmu_objset_id_quota_upgrade(*osp);
+		} else if (dmu_objset_userused_enabled(*osp)) {
+			dmu_objset_userspace_upgrade(*osp);
+		}
+	}
 
 	dsl_pool_rele(dp, FTAG);
 	return (0);
@@ -2285,8 +2289,8 @@ dmu_objset_space_upgrade(objset_t *os)
 	return (0);
 }
 
-int
-dmu_objset_userspace_upgrade(objset_t *os)
+static int
+dmu_objset_userspace_upgrade_cb(objset_t *os)
 {
 	int err = 0;
 
@@ -2306,6 +2310,12 @@ dmu_objset_userspace_upgrade(objset_t *os)
 	return (0);
 }
 
+void
+dmu_objset_userspace_upgrade(objset_t *os)
+{
+	dmu_objset_upgrade(os, dmu_objset_userspace_upgrade_cb);
+}
+
 static int
 dmu_objset_id_quota_upgrade_cb(objset_t *os)
 {
@@ -2316,12 +2326,13 @@ dmu_objset_id_quota_upgrade_cb(objset_t *os)
 		return (0);
 	if (dmu_objset_is_snapshot(os))
 		return (SET_ERROR(EINVAL));
-	if (!dmu_objset_userobjused_enabled(os))
+	if (!dmu_objset_userused_enabled(os))
 		return (SET_ERROR(ENOTSUP));
 	if (!dmu_objset_projectquota_enabled(os) &&
 	    dmu_objset_userobjspace_present(os))
 		return (SET_ERROR(ENOTSUP));
 
+	if (dmu_objset_userobjused_enabled(os))
 		dmu_objset_ds(os)->ds_feature_activation[
 		    SPA_FEATURE_USEROBJ_ACCOUNTING] = (void *)B_TRUE;
 	if (dmu_objset_projectquota_enabled(os))
@@ -2332,6 +2343,8 @@ dmu_objset_id_quota_upgrade_cb(objset_t *os)
 	if (err)
 		return (err);
 
+	os->os_flags |= OBJSET_FLAG_USERACCOUNTING_COMPLETE;
+	if (dmu_objset_userobjused_enabled(os))
 		os->os_flags |= OBJSET_FLAG_USEROBJACCOUNTING_COMPLETE;
 	if (dmu_objset_projectquota_enabled(os))
 		os->os_flags |= OBJSET_FLAG_PROJECTQUOTA_COMPLETE;

module/zfs/zfs_ioctl.c

@@ -5856,7 +5856,6 @@ zfs_ioc_userspace_many(zfs_cmd_t *zc)
 static int
 zfs_ioc_userspace_upgrade(zfs_cmd_t *zc)
 {
-	objset_t *os;
 	int error = 0;
 	zfsvfs_t *zfsvfs;
 
@@ -5877,19 +5876,54 @@ zfs_ioc_userspace_upgrade(zfs_cmd_t *zc)
 				error = zfs_resume_fs(zfsvfs, newds);
 			}
 		}
-		if (error == 0)
-			error = dmu_objset_userspace_upgrade(zfsvfs->z_os);
+		if (error == 0) {
+			mutex_enter(&zfsvfs->z_os->os_upgrade_lock);
+			if (zfsvfs->z_os->os_upgrade_id == 0) {
+				/* clear potential error code and retry */
+				zfsvfs->z_os->os_upgrade_status = 0;
+				mutex_exit(&zfsvfs->z_os->os_upgrade_lock);
+
+				dsl_pool_config_enter(
+				    dmu_objset_pool(zfsvfs->z_os), FTAG);
+				dmu_objset_userspace_upgrade(zfsvfs->z_os);
+				dsl_pool_config_exit(
+				    dmu_objset_pool(zfsvfs->z_os), FTAG);
+			} else {
+				mutex_exit(&zfsvfs->z_os->os_upgrade_lock);
+			}
+
+			taskq_wait_id(zfsvfs->z_os->os_spa->spa_upgrade_taskq,
+			    zfsvfs->z_os->os_upgrade_id);
+			error = zfsvfs->z_os->os_upgrade_status;
+		}
 		zfs_vfs_rele(zfsvfs);
 	} else {
+		objset_t *os;
+
 		/* XXX kind of reading contents without owning */
 		error = dmu_objset_hold_flags(zc->zc_name, B_TRUE, FTAG, &os);
 		if (error != 0)
 			return (error);
 
-		error = dmu_objset_userspace_upgrade(os);
-		dmu_objset_rele_flags(os, B_TRUE, FTAG);
+		mutex_enter(&os->os_upgrade_lock);
+		if (os->os_upgrade_id == 0) {
+			/* clear potential error code and retry */
+			os->os_upgrade_status = 0;
+			mutex_exit(&os->os_upgrade_lock);
+
+			dmu_objset_userspace_upgrade(os);
+		} else {
+			mutex_exit(&os->os_upgrade_lock);
 		}
 
+		dsl_pool_rele(dmu_objset_pool(os), FTAG);
+
+		taskq_wait_id(os->os_spa->spa_upgrade_taskq, os->os_upgrade_id);
+		error = os->os_upgrade_status;
+
+		dsl_dataset_rele_flags(dmu_objset_ds(os), DS_HOLD_FLAG_DECRYPT,
+		    FTAG);
+	}
 	return (error);
 }
 

tests/runfiles/common.run

@@ -859,7 +859,7 @@ tests = [
     'userquota_004_pos', 'userquota_005_neg', 'userquota_006_pos',
     'userquota_007_pos', 'userquota_008_pos', 'userquota_009_pos',
     'userquota_010_pos', 'userquota_011_pos', 'userquota_012_neg',
-    'userspace_001_pos', 'userspace_002_pos']
+    'userspace_001_pos', 'userspace_002_pos', 'userspace_encrypted']
 tags = ['functional', 'userquota']
 
 [tests/functional/vdev_zaps]

tests/zfs-tests/tests/functional/userquota/Makefile.am

@@ -20,7 +20,8 @@ dist_pkgdata_SCRIPTS = \
 	userquota_013_pos.ksh \
 	userspace_001_pos.ksh \
 	userspace_002_pos.ksh \
-	userspace_003_pos.ksh
+	userspace_003_pos.ksh \
+	userspace_encrypted.ksh
 
 dist_pkgdata_DATA = \
 	userquota.cfg \

tests/zfs-tests/tests/functional/userquota/userspace_encrypted.ksh

@@ -0,0 +1,85 @@
+#!/bin/ksh -p
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2019, loli10K <ezomori.nozomu@gmail.com>. All rights reserved.
+#
+
+. $STF_SUITE/include/libtest.shlib
+. $STF_SUITE/tests/functional/userquota/userquota_common.kshlib
+
+#
+# DESCRIPTION:
+# 'zfs userspace' and 'zfs groupspace' can be used on encrypted datasets
+#
+#
+# STRATEGY:
+# 1. Create both un-encrypted and encrypted datasets
+# 2. Receive un-encrypted dataset in encrypted hierarchy
+# 3. Verify encrypted datasets support 'zfs userspace' and 'zfs groupspace'
+#
+
+function cleanup
+{
+	destroy_pool $POOLNAME
+	rm -f $FILEDEV
+}
+
+function log_must_unsupported
+{
+	log_must_retry "unsupported" 3 "$@"
+	(( $? != 0 )) && log_fail
+}
+
+log_onexit cleanup
+
+FILEDEV="$TEST_BASE_DIR/userspace_encrypted"
+POOLNAME="testpool$$"
+typeset -a POOL_OPTS=('' # all pool features enabled
+    '-d' # all pool features disabled
+    '-d -o feature@userobj_accounting=enabled' # only userobj_accounting enabled
+    '-d -o feature@project_quota=enabled') # only project_quota enabled
+DATASET_ENCROOT="$POOLNAME/encroot"
+DATASET_SENDFS="$POOLNAME/sendfs"
+
+log_assert "'zfs user/groupspace' should work on encrypted datasets"
+
+for opts in "${POOL_OPTS[@]}"; do
+	# Setup
+	truncate -s $SPA_MINDEVSIZE $FILEDEV
+	log_must zpool create $opts -o feature@encryption=enabled $POOLNAME \
+		$FILEDEV
+
+	# 1. Create both un-encrypted and encrypted datasets
+	log_must zfs create $DATASET_SENDFS
+	log_must eval "echo 'password' | zfs create -o encryption=on" \
+		"-o keyformat=passphrase -o keylocation=prompt " \
+		"$DATASET_ENCROOT"
+	log_must zfs create $DATASET_ENCROOT/fs
+
+	# 2. Receive un-encrypted dataset in encrypted hierarchy
+	log_must zfs snap $DATASET_SENDFS@snap
+	log_must eval "zfs send $DATASET_SENDFS@snap | zfs recv " \
+		"$DATASET_ENCROOT/recvfs"
+
+	# 3. Verify encrypted datasets support 'zfs userspace' and
+	# 'zfs groupspace'
+	log_must zfs userspace $DATASET_ENCROOT/fs
+	log_must zfs groupspace $DATASET_ENCROOT/fs
+	log_must_unsupported zfs userspace $DATASET_ENCROOT/recvfs
+	log_must_unsupported zfs groupspace $DATASET_ENCROOT/recvfs
+
+	# Cleanup
+	cleanup
+done
+
+log_pass "'zfs user/groupspace' works on encrypted datasets"