Skip to content

Commit

Permalink
Fix sa_add_projid to lookup and update SA_ZPL_DXATTR (avoid DXATTR loss)
Browse files Browse the repository at this point in the history
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 <jitendra.patidar@nutanix.com>
Closes openzfs#16287

Change-Id: I5209a528a58ed91b40b83d873e6f9387e50f6aff
  • Loading branch information
jsai20 committed Jun 21, 2024
1 parent c98295e commit 29180f3
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 2 deletions.
1 change: 1 addition & 0 deletions include/sys/sa.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand Down
1 change: 0 additions & 1 deletion include/sys/sa_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
41 changes: 41 additions & 0 deletions module/zfs/sa.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/runfiles/linux.run
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
1 change: 1 addition & 0 deletions tests/test-runner/bin/zts-report.py.in
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
1 change: 1 addition & 0 deletions tests/zfs-tests/tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
Original file line number Diff line number Diff line change
@@ -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"

0 comments on commit 29180f3

Please sign in to comment.