From 69323dd9a04acfdfe3c329d4b69d90950cf483fb Mon Sep 17 00:00:00 2001 From: Jitendra Patidar Date: Fri, 21 Jun 2024 14:19:08 +0000 Subject: [PATCH] Fix sa_add_projid to lookup and update SA_ZPL_DXATTR (avoid DXATTR loss) sa_add_projid() gets called via zfs_setattr() for setting project id on old file/dir, which were created before upgrading to project quota feature. This function does lookup for all possible SA and update them all together along with project ID at needed fixed offset. But its missing lookup and update of SA_ZPL_DXATTR, effectively it losses SA_ZPL_DXATTR. Signed-off-by: Jitendra Patidar Closes #16287 --- include/sys/sa.h | 1 + include/sys/sa_impl.h | 1 - module/zfs/sa.c | 41 ++++++++++ tests/runfiles/linux.run | 2 +- tests/test-runner/bin/zts-report.py.in | 1 + tests/zfs-tests/tests/Makefile.am | 1 + .../upgrade/upgrade_projectquota_002_pos.ksh | 80 +++++++++++++++++++ 7 files changed, 125 insertions(+), 2 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/upgrade/upgrade_projectquota_002_pos.ksh diff --git a/include/sys/sa.h b/include/sys/sa.h index c551acecab30..7d1448f43665 100644 --- a/include/sys/sa.h +++ b/include/sys/sa.h @@ -134,6 +134,7 @@ int sa_bulk_lookup(sa_handle_t *, sa_bulk_attr_t *, int count); int sa_bulk_lookup_locked(sa_handle_t *, sa_bulk_attr_t *, int count); int sa_bulk_update(sa_handle_t *, sa_bulk_attr_t *, int count, dmu_tx_t *); int sa_size(sa_handle_t *, sa_attr_type_t, int *); +int sa_size_locked(sa_handle_t *, sa_attr_type_t, int *); void sa_object_info(sa_handle_t *, dmu_object_info_t *); void sa_object_size(sa_handle_t *, uint32_t *, u_longlong_t *); void *sa_get_userdata(sa_handle_t *); diff --git a/include/sys/sa_impl.h b/include/sys/sa_impl.h index 744c8dcb7dfb..6eb0c96188fa 100644 --- a/include/sys/sa_impl.h +++ b/include/sys/sa_impl.h @@ -272,7 +272,6 @@ int sa_add_impl(sa_handle_t *, sa_attr_type_t, uint32_t, sa_data_locator_t, void *, dmu_tx_t *); void sa_register_update_callback_locked(objset_t *, sa_update_cb_t *); -int sa_size_locked(sa_handle_t *, sa_attr_type_t, int *); void sa_default_locator(void **, uint32_t *, uint32_t, boolean_t, void *); int sa_attr_size(sa_os_t *, sa_idx_tab_t *, sa_attr_type_t, diff --git a/module/zfs/sa.c b/module/zfs/sa.c index 0ae4c331dd36..4b59eaa42349 100644 --- a/module/zfs/sa.c +++ b/module/zfs/sa.c @@ -1623,6 +1623,19 @@ sa_add_projid(sa_handle_t *hdl, dmu_tx_t *tx, uint64_t projid) if (err != 0 && err != ENOENT) goto out; + char *dxattr_obj = NULL; + int dxattr_size = 0; + err = sa_size_locked(hdl, SA_ZPL_DXATTR(zfsvfs), &dxattr_size); + if (err != 0 && err != ENOENT) + goto out; + if (dxattr_size != 0) { + dxattr_obj = vmem_alloc(dxattr_size, KM_SLEEP); + err = sa_lookup_locked(hdl, SA_ZPL_DXATTR(zfsvfs), dxattr_obj, + dxattr_size); + if (err != 0 && err != ENOENT) + goto out; + } + zp->z_projid = projid; zp->z_pflags |= ZFS_PROJID; links = ZTONLNK(zp); @@ -1674,6 +1687,11 @@ sa_add_projid(sa_handle_t *hdl, dmu_tx_t *tx, uint64_t projid) zp->z_pflags &= ~ZFS_BONUS_SCANSTAMP; } + if (dxattr_obj) { + SA_ADD_BULK_ATTR(attrs, count, SA_ZPL_DXATTR(zfsvfs), + NULL, dxattr_obj, dxattr_size); + } + VERIFY(dmu_set_bonustype(db, DMU_OT_SA, tx) == 0); VERIFY(sa_replace_all_by_template_locked(hdl, attrs, count, tx) == 0); if (znode_acl.z_acl_extern_obj) { @@ -1688,6 +1706,8 @@ sa_add_projid(sa_handle_t *hdl, dmu_tx_t *tx, uint64_t projid) mutex_exit(&hdl->sa_lock); kmem_free(attrs, sizeof (sa_bulk_attr_t) * ZPL_END); kmem_free(bulk, sizeof (sa_bulk_attr_t) * ZPL_END); + if (dxattr_obj) + vmem_free(dxattr_obj, dxattr_size); return (err); } #endif @@ -2063,6 +2083,26 @@ sa_update(sa_handle_t *hdl, sa_attr_type_t type, * Return size of an attribute */ +int +sa_size_locked(sa_handle_t *hdl, sa_attr_type_t attr, int *size) +{ + sa_bulk_attr_t bulk; + int error; + + bulk.sa_data = NULL; + bulk.sa_attr = attr; + bulk.sa_data_func = NULL; + + ASSERT(hdl); + ASSERT(MUTEX_HELD(&hdl->sa_lock)); + if ((error = sa_attr_op(hdl, &bulk, 1, SA_LOOKUP, NULL)) != 0) { + return (error); + } + *size = bulk.sa_size; + + return (0); +} + int sa_size(sa_handle_t *hdl, sa_attr_type_t attr, int *size) { @@ -2237,6 +2277,7 @@ EXPORT_SYMBOL(sa_bulk_lookup); EXPORT_SYMBOL(sa_bulk_lookup_locked); EXPORT_SYMBOL(sa_bulk_update); EXPORT_SYMBOL(sa_size); +EXPORT_SYMBOL(sa_size_locked); EXPORT_SYMBOL(sa_object_info); EXPORT_SYMBOL(sa_object_size); EXPORT_SYMBOL(sa_get_userdata); diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index 92ce09ec6fcb..44201e6e4daf 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -201,7 +201,7 @@ tests = ['tmpfile_001_pos', 'tmpfile_002_pos', 'tmpfile_003_pos', tags = ['functional', 'tmpfile'] [tests/functional/upgrade:Linux] -tests = ['upgrade_projectquota_001_pos'] +tests = ['upgrade_projectquota_001_pos', 'upgrade_projectquota_002_pos'] tags = ['functional', 'upgrade'] [tests/functional/user_namespace:Linux] diff --git a/tests/test-runner/bin/zts-report.py.in b/tests/test-runner/bin/zts-report.py.in index 5ca130931364..15d49678c027 100755 --- a/tests/test-runner/bin/zts-report.py.in +++ b/tests/test-runner/bin/zts-report.py.in @@ -254,6 +254,7 @@ maybe = { 'tmpfile/setup': ['SKIP', tmpfile_reason], 'trim/setup': ['SKIP', trim_reason], 'upgrade/upgrade_projectquota_001_pos': ['SKIP', project_id_reason], + 'upgrade/upgrade_projectquota_002_pos': ['SKIP', project_id_reason], 'user_namespace/setup': ['SKIP', user_ns_reason], 'userquota/setup': ['SKIP', exec_reason], 'vdev_zaps/vdev_zaps_004_pos': ['FAIL', known_reason], diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 44eedcf6fae5..4285a98c4da3 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -2016,6 +2016,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/upgrade/cleanup.ksh \ functional/upgrade/setup.ksh \ functional/upgrade/upgrade_projectquota_001_pos.ksh \ + functional/upgrade/upgrade_projectquota_002_pos.ksh \ functional/upgrade/upgrade_readonly_pool.ksh \ functional/upgrade/upgrade_userobj_001_pos.ksh \ functional/user_namespace/cleanup.ksh \ diff --git a/tests/zfs-tests/tests/functional/upgrade/upgrade_projectquota_002_pos.ksh b/tests/zfs-tests/tests/functional/upgrade/upgrade_projectquota_002_pos.ksh new file mode 100755 index 000000000000..fe837435190c --- /dev/null +++ b/tests/zfs-tests/tests/functional/upgrade/upgrade_projectquota_002_pos.ksh @@ -0,0 +1,80 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or https://opensource.org/licenses/CDDL-1.0. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright (c) 2024 by Nutanix. All rights reserved. +# + +. $STF_SUITE/tests/functional/upgrade/upgrade_common.kshlib + +# +# DESCRIPTION: +# +# Check DXATTR is intact after sa re-layout by setting projid on old file/dir after upgrade +# +# STRATEGY: +# 1. Create a pool with all features disabled +# 2. Create a dataset for testing +# 3. Set DXATTR on file and directory +# 4. upgrade zpool to support all features +# 5. set project id on file and directory to trigger sa re-layout for projid +# 6. verify DXATTR on file and directory are intact +# + +TESTFS=$TESTPOOL/testfs +TESTFSDIR=$TESTDIR/testfs + +verify_runnable "global" + +log_assert "Check DXATTR is intact after sa re-layout by setting projid on old file/dir after upgrade" +log_onexit cleanup_upgrade + +log_must zpool create -d -m $TESTDIR $TESTPOOL $TMPDEV + +log_must zfs create -o xattr=sa $TESTFS +log_must mkdir $TESTFSDIR/dir +log_must touch $TESTFSDIR/file +log_must set_xattr test test $TESTFSDIR/dir +log_must set_xattr test test $TESTFSDIR/file + +dirino=$(stat -c '%i' $TESTFSDIR/dir) +fileino=$(stat -c '%i' $TESTFSDIR/file) +log_must zpool sync $TESTPOOL +log_must zdb -ddddd $TESTFS $dirino +log_must zdb -ddddd $TESTFS $fileino + +log_mustnot chattr -p 100 $TESTFSDIR/dir +log_mustnot chattr -p 100 $TESTFSDIR/file + +log_must zpool upgrade $TESTPOOL + +log_must chattr -p 100 $TESTFSDIR/dir +log_must chattr -p 100 $TESTFSDIR/file +log_must zpool sync $TESTPOOL +log_must zfs umount $TESTFS +log_must zfs mount $TESTFS +log_must zdb -ddddd $TESTFS $dirino +log_must zdb -ddddd $TESTFS $fileino +log_must get_xattr test $TESTFSDIR/dir +log_must get_xattr test $TESTFSDIR/file + +log_pass "Check DXATTR is intact after sa re-layout by setting projid on old file/dir after upgrade"