From d60debbf599b485dbd2e2c4c2abf3ec7943327f7 Mon Sep 17 00:00:00 2001 From: Jitendra Patidar Date: Thu, 1 Aug 2024 07:11:49 +0530 Subject: [PATCH] Fix sa_add_projid to lookup and update SA_ZPL_DXATTR (avoid DXATTR loss) (#16288) 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. Closes #16287 Signed-off-by: Jitendra Patidar Reviewed-by: Tony Hutter Reviewed-by: Rob Norris --- include/sys/sa_impl.h | 1 - module/zfs/sa.c | 82 +++++++++++++------ 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 ++++++++++++++++++ 6 files changed, 139 insertions(+), 28 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/upgrade/upgrade_projectquota_002_pos.ksh diff --git a/include/sys/sa_impl.h b/include/sys/sa_impl.h index 744c8dcb7..6eb0c9618 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 a1b2ffa87..676fc10a4 100644 --- a/module/zfs/sa.c +++ b/module/zfs/sa.c @@ -1501,6 +1501,42 @@ sa_lookup(sa_handle_t *hdl, sa_attr_type_t attr, void *buf, uint32_t buflen) return (error); } +/* + * Return size of an attribute + */ + +static 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) +{ + int error; + + mutex_enter(&hdl->sa_lock); + error = sa_size_locked(hdl, attr, size); + mutex_exit(&hdl->sa_lock); + + return (error); +} + #ifdef _KERNEL int sa_lookup_uio(sa_handle_t *hdl, sa_attr_type_t attr, zfs_uio_t *uio) @@ -1623,6 +1659,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 +1723,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 +1742,8 @@ out: 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 @@ -2057,32 +2113,6 @@ sa_update(sa_handle_t *hdl, sa_attr_type_t type, return (error); } -/* - * Return size of an attribute - */ - -int -sa_size(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); - mutex_enter(&hdl->sa_lock); - if ((error = sa_attr_op(hdl, &bulk, 1, SA_LOOKUP, NULL)) != 0) { - mutex_exit(&hdl->sa_lock); - return (error); - } - *size = bulk.sa_size; - - mutex_exit(&hdl->sa_lock); - return (0); -} - int sa_bulk_lookup_locked(sa_handle_t *hdl, sa_bulk_attr_t *attrs, int count) { diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index bd6cc56f3..5817e6490 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 de06c7c6e..8c0a89804 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 73a6f00b5..3a85e33b1 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -2023,6 +2023,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 000000000..fe8374351 --- /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"