From f6f3089cf6455e6b23faecd8987815c6a3c4f14c Mon Sep 17 00:00:00 2001 From: loli10K Date: Mon, 16 Nov 2020 18:10:29 +0100 Subject: [PATCH] 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 Co-authored-by: Brian Behlendorf Signed-off-by: loli10K Closes #9501 Closes #9596 --- include/sys/dmu_objset.h | 2 +- module/zfs/dmu_objset.c | 35 +++++--- module/zfs/zfs_ioctl.c | 46 ++++++++-- tests/runfiles/common.run | 2 +- .../tests/functional/userquota/Makefile.am | 3 +- .../userquota/userspace_encrypted.ksh | 85 +++++++++++++++++++ 6 files changed, 153 insertions(+), 20 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/userquota/userspace_encrypted.ksh diff --git a/include/sys/dmu_objset.h b/include/sys/dmu_objset.h index 1af69832c..f27417c1f 100644 --- a/include/sys/dmu_objset.h +++ b/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); diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c index af5935e23..6c1d23f2b 100644 --- a/module/zfs/dmu_objset.c +++ b/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)) - dmu_objset_id_quota_upgrade(*osp); + 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,14 +2326,15 @@ 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)); - dmu_objset_ds(os)->ds_feature_activation[ - SPA_FEATURE_USEROBJ_ACCOUNTING] = (void *)B_TRUE; + 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)) dmu_objset_ds(os)->ds_feature_activation[ SPA_FEATURE_PROJECT_QUOTA] = (void *)B_TRUE; @@ -2332,7 +2343,9 @@ dmu_objset_id_quota_upgrade_cb(objset_t *os) if (err) return (err); - os->os_flags |= OBJSET_FLAG_USEROBJACCOUNTING_COMPLETE; + 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; diff --git a/module/zfs/zfs_ioctl.c b/module/zfs/zfs_ioctl.c index 7f2cf2a75..33bd39aa2 100644 --- a/module/zfs/zfs_ioctl.c +++ b/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); } diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 7d3e3b557..8bb7cd468 100644 --- a/tests/runfiles/common.run +++ b/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] diff --git a/tests/zfs-tests/tests/functional/userquota/Makefile.am b/tests/zfs-tests/tests/functional/userquota/Makefile.am index 8f0287bc1..9100e4ada 100644 --- a/tests/zfs-tests/tests/functional/userquota/Makefile.am +++ b/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 \ diff --git a/tests/zfs-tests/tests/functional/userquota/userspace_encrypted.ksh b/tests/zfs-tests/tests/functional/userquota/userspace_encrypted.ksh new file mode 100755 index 000000000..429b16e04 --- /dev/null +++ b/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 . 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" -- 2.39.5