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 <jitendra.patidar@nutanix.com>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Rob Norris <rob.norris@klarasystems.com>

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,

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 @@ 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
@@ -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)
 {

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]

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],

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 \

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"