Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove feature@xattr_compat and xattr_fallback #24

Merged
3 commits merged into from
Oct 8, 2021
Merged

Remove feature@xattr_compat and xattr_fallback #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68e8cc7
Remove feature@xattr_compat activation
Oct 7, 2021
32e583e
Remove xattr_fallback property
Oct 7, 2021
7a545f8
Bump changelog for 2.1.1
Oct 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion contrib/truenas/changelog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
openzfs (2.1.1-0) unstable; urgency=medium

* Merged OpenZFS 2.1.1
* Removed feature@xattr_compat and xattr_fallback property

-- Ryan Moeller <ryan@ixsystems.com> Thu, 07 Oct 2021 14:09:30 -0400

openzfs (2.1.0-0) unstable; urgency=medium

* Rebased to OpenZFS 2.1.0

-- Ryan Moeller <ryan@ixsystems.com> Thu, 01 Apr 2021 13:00:00 -500
-- Ryan Moeller <ryan@ixsystems.com> Thu, 01 Apr 2021 13:00:00 -0500

openzfs (2.0.4-0) unstable; urgency=medium

Expand Down
1 change: 0 additions & 1 deletion include/os/freebsd/zfs/sys/zfs_vfsops_os.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ struct zfsvfs {

#define ZSB_XATTR 0x0001 /* Enable user xattrs */
#define ZSB_XATTR_COMPAT 0x0002 /* Enable cross-platform user xattrs */
#define ZSB_XATTR_FALLBACK 0x0004 /* Enable user xattr compat fallback */

/*
* Normal filesystems (those not under .zfs/snapshot) have a total
Expand Down
1 change: 0 additions & 1 deletion include/os/linux/zfs/sys/zfs_vfsops_os.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ struct zfsvfs {

#define ZSB_XATTR 0x0001 /* Enable user xattrs */
#define ZSB_XATTR_COMPAT 0x0002 /* Enable cross-platform user xattrs */
#define ZSB_XATTR_FALLBACK 0x0004 /* Enable user xattr compat fallback */

/*
* Allow a maximum number of links. While ZFS does not internally limit
Expand Down
1 change: 0 additions & 1 deletion include/sys/fs/zfs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ typedef enum {
ZFS_PROP_REDACTED,
ZFS_PROP_REDACT_SNAPS,
ZFS_PROP_XATTR_COMPAT,
ZFS_PROP_XATTR_FALLBACK,
ZFS_NUM_PROPS
} zfs_prop_t;

Expand Down
1 change: 0 additions & 1 deletion include/zfeature_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ typedef enum spa_feature {
SPA_FEATURE_DEVICE_REBUILD,
SPA_FEATURE_ZSTD_COMPRESS,
SPA_FEATURE_DRAID,
SPA_FEATURE_XATTR_COMPAT,
SPA_FEATURES
} spa_feature_t;

Expand Down
9 changes: 3 additions & 6 deletions lib/libzfs/libzfs.abi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,8 +822,7 @@
<enumerator name='SPA_FEATURE_DEVICE_REBUILD' value='31'/>
<enumerator name='SPA_FEATURE_ZSTD_COMPRESS' value='32'/>
<enumerator name='SPA_FEATURE_DRAID' value='33'/>
<enumerator name='SPA_FEATURE_XATTR_COMPAT' value='34'/>
<enumerator name='SPA_FEATURES' value='35'/>
<enumerator name='SPA_FEATURES' value='34'/>
</enum-decl>
<typedef-decl name='zfeature_flags_t' type-id='type-id-88' filepath='../../include/zfeature_common.h' line='96' column='1' id='type-id-84'/>
<enum-decl name='zfeature_flags' filepath='../../include/zfeature_common.h' line='84' column='1' id='type-id-88'>
Expand Down Expand Up @@ -1691,8 +1690,7 @@
<enumerator name='ZFS_PROP_REDACTED' value='93'/>
<enumerator name='ZFS_PROP_REDACT_SNAPS' value='94'/>
<enumerator name='ZFS_PROP_XATTR_COMPAT' value='95'/>
<enumerator name='ZFS_PROP_XATTR_FALLBACK' value='96'/>
<enumerator name='ZFS_NUM_PROPS' value='97'/>
<enumerator name='ZFS_NUM_PROPS' value='96'/>
</enum-decl>
<enum-decl name='__anonymous_enum__2' is-anonymous='yes' filepath='../../include/sys/fs/zfs.h' line='52' column='1' id='type-id-182'>
<underlying-type type-id='type-id-35'/>
Expand Down Expand Up @@ -2105,8 +2103,7 @@
<enumerator name='ZFS_PROP_REDACTED' value='93'/>
<enumerator name='ZFS_PROP_REDACT_SNAPS' value='94'/>
<enumerator name='ZFS_PROP_XATTR_COMPAT' value='95'/>
<enumerator name='ZFS_PROP_XATTR_FALLBACK' value='96'/>
<enumerator name='ZFS_NUM_PROPS' value='97'/>
<enumerator name='ZFS_NUM_PROPS' value='96'/>
</enum-decl>
<typedef-decl name='uu_avl_pool_t' type-id='type-id-206' filepath='../../include/libuutil.h' line='287' column='1' id='type-id-216'/>
<typedef-decl name='uu_avl_t' type-id='type-id-205' filepath='../../include/libuutil.h' line='288' column='1' id='type-id-217'/>
Expand Down
30 changes: 3 additions & 27 deletions man/man7/zfsprops.7
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1845,9 +1845,7 @@ mount options.
Controls the preferred encoding of xattrs in the user namespace.
When set to
.Sy all
(the default) with
.Sy feature@xattr_compat
enabled on the pool, xattrs written in the user namespace are stored in a
(the default), xattrs written in the user namespace are stored in a
format compatible across all supported platforms, and xattrs in the user
namespace from all platforms are accessible.
There is no notion of xattr namespaces on illumos, so all xattrs from
Expand All @@ -1858,36 +1856,14 @@ Existing xattrs in the
.Sy xattr_compat=linux
format are accessible and are replaced with the cross-platform compatible
format when written.
When
.Sy feature@xattr_compat
is disabled, xattrs behave as with
.Sy xattr_compat=linux
on Linux and as with
.Sy xattr_compat=all
elsewhere.
When set to
.Sy linux ,
xattrs written in the user namespace are stored in a format that is compatible
with ZFS on Linux prior to
.Sy feature@xattr_compat
but not compatible with ZFS on other platforms prior to this feature.
See
.Sy feature@xattr_compat
in
.Xr zpool-features 5
for more information.
with upstream OpenZFS on Linux but not compatible with ZFS on other platforms.
.Pp
The default value in TrueNAS SCALE is
.Sy xattr_compat=linux
until this feature is merged into upstream OpenZFS.
.It Sy xattr_fallback Ns = Ns Sy on Ns | Ns Sy off
Controls whether to fall back to the alternative encoding of xattrs in the
user namespace for lookups.
If accessing an xattr in the user namespace fails for the format configured by
.Sy xattr_compat ,
when set to
.Sy on
(the default), a second attempt will be made using the other xattr name format.
until this property is merged into upstream OpenZFS.
.It Sy jailed Ns = Ns Sy off Ns | Ns Sy on
Controls whether the dataset is managed from a jail.
See
Expand Down
11 changes: 0 additions & 11 deletions man/man7/zpool-features.7
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,17 +782,6 @@ by user and group.
\*[instant-never]
\*[remount-upgrade]
.
.feature com.ixsystems xattr_compat yes extensible_dataset
This feature enables the use of a cross-platform compatible encoding for xattr
names in the "user" namespace on Linux.
.Pp
This feature becomes
.Sy active
on a filesystem when an xattr is written to the "user" namespace on Linux after
the feature has been enabled, and returns to
.Sy enabled
after all filesystems with the feature active have been destroyed.
.
.feature com.delphix zpool_checkpoint yes
This feature enables the
.Nm zpool Cm checkpoint
Expand Down
29 changes: 0 additions & 29 deletions module/os/freebsd/zfs/zfs_vfsops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
#include <sys/sa_impl.h>
#include <sys/policy.h>
#include <sys/atomic.h>
#include <sys/zfeature.h>
#include <sys/zfs_ioctl.h>
#include <sys/zfs_ctldir.h>
#include <sys/zfs_fuid.h>
Expand Down Expand Up @@ -499,16 +498,6 @@ xattr_compat_changed_cb(void *arg, uint64_t newval)
{
zfsvfs_t *zfsvfs = arg;

/*
* Force the old cross-platform compatible behavior if
* feature@xattr_compat is disabled. This contrasts with
* Linux where the behavior prior to feature@xattr_compat
* was to use the incompatible Linux-only xattr format.
*/
if (!spa_feature_is_enabled(dmu_objset_spa(zfsvfs->z_os),
SPA_FEATURE_XATTR_COMPAT))
newval = ZFS_XATTR_COMPAT_ALL;

switch (newval) {
case ZFS_XATTR_COMPAT_ALL:
zfsvfs->z_flags |= ZSB_XATTR_COMPAT;
Expand All @@ -519,17 +508,6 @@ xattr_compat_changed_cb(void *arg, uint64_t newval)
}
}

static void
xattr_fallback_changed_cb(void *arg, uint64_t newval)
{
zfsvfs_t *zfsvfs = arg;

if (newval)
zfsvfs->z_flags |= ZSB_XATTR_FALLBACK;
else
zfsvfs->z_flags &= ~ZSB_XATTR_FALLBACK;
}

static void
blksz_changed_cb(void *arg, uint64_t newval)
{
Expand Down Expand Up @@ -776,9 +754,6 @@ zfs_register_callbacks(vfs_t *vfsp)
error = error ? error : dsl_prop_register(ds,
zfs_prop_to_name(ZFS_PROP_XATTR_COMPAT), xattr_compat_changed_cb,
zfsvfs);
error = error ? error : dsl_prop_register(ds,
zfs_prop_to_name(ZFS_PROP_XATTR_FALLBACK),
xattr_fallback_changed_cb, zfsvfs);
error = error ? error : dsl_prop_register(ds,
zfs_prop_to_name(ZFS_PROP_RECORDSIZE), blksz_changed_cb, zfsvfs);
error = error ? error : dsl_prop_register(ds,
Expand Down Expand Up @@ -1299,10 +1274,6 @@ zfs_domount(vfs_t *vfsp, char *osname)
"xattr_compat", &pval, NULL)))
goto out;
xattr_compat_changed_cb(zfsvfs, pval);
if ((error = dsl_prop_get_integer(osname,
"xattr_fallback", &pval, NULL)))
goto out;
xattr_fallback_changed_cb(zfsvfs, pval);
if ((error = dsl_prop_get_integer(osname,
"acltype", &pval, NULL)))
goto out;
Expand Down
18 changes: 5 additions & 13 deletions module/os/freebsd/zfs/zfs_vnops_os.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5288,7 +5288,7 @@ zfs_create_attrname(int attrnamespace, const char *name, char *attrname,
} else {
/*
* This is compatible with the user namespace encoding
* on Linux prior to feature@xattr_compat, but nothing
* on Linux prior to xattr_compat, but nothing
* else.
*/
prefix = "";
Expand Down Expand Up @@ -5463,10 +5463,8 @@ zfs_getextattr(struct vop_getextattr_args *ap)
rw_enter(&zp->z_xattr_lock, RW_READER);

boolean_t compat = (zfsvfs->z_flags & ZSB_XATTR_COMPAT) != 0;
boolean_t fallback = (zfsvfs->z_flags & ZSB_XATTR_FALLBACK) != 0;
error = zfs_getextattr_impl(ap, compat);
if (fallback && error == ENOENT &&
ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) {
if (error == ENOENT && ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) {
/*
* Fall back to the alternate namespace format if we failed to
* find a user xattr.
Expand Down Expand Up @@ -5605,10 +5603,8 @@ zfs_deleteextattr(struct vop_deleteextattr_args *ap)
rw_enter(&zp->z_xattr_lock, RW_WRITER);

boolean_t compat = (zfsvfs->z_flags & ZSB_XATTR_COMPAT) != 0;
boolean_t fallback = (zfsvfs->z_flags & ZSB_XATTR_FALLBACK) != 0;
error = zfs_deleteextattr_impl(ap, compat);
if (fallback && error == ENOENT &&
ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) {
if (error == ENOENT && ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) {
/*
* Fall back to the alternate namespace format if we failed to
* find a user xattr.
Expand Down Expand Up @@ -5752,9 +5748,7 @@ zfs_setextattr_impl(struct vop_setextattr_args *ap, boolean_t compat)
zfs_deleteextattr_sa(&vda, attrname);
}
}
boolean_t fallback = (zfsvfs->z_flags & ZSB_XATTR_FALLBACK) != 0;
if (fallback && error == 0 &&
ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) {
if (error == 0 && ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) {
/*
* Also clear all versions of the alternate compat name.
*/
Expand Down Expand Up @@ -5993,10 +5987,8 @@ zfs_listextattr(struct vop_listextattr_args *ap)
rw_enter(&zp->z_xattr_lock, RW_READER);

boolean_t compat = (zfsvfs->z_flags & ZSB_XATTR_COMPAT) != 0;
boolean_t fallback = (zfsvfs->z_flags & ZSB_XATTR_FALLBACK) != 0;
error = zfs_listextattr_impl(ap, compat);
if (fallback && error == 0 &&
ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) {
if (error == 0 && ap->a_attrnamespace == EXTATTR_NAMESPACE_USER) {
/* Also list user xattrs with the alternate format. */
error = zfs_listextattr_impl(ap, !compat);
}
Expand Down
27 changes: 0 additions & 27 deletions module/os/linux/zfs/zfs_vfsops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
#include <sys/dsl_dir.h>
#include <sys/spa_boot.h>
#include <sys/objlist.h>
#include <sys/zfeature.h>
#include <sys/zpl.h>
#include <linux/vfs_compat.h>
#include "zfs_comutil.h"
Expand Down Expand Up @@ -358,14 +357,6 @@ xattr_compat_changed_cb(void *arg, uint64_t newval)
{
zfsvfs_t *zfsvfs = arg;

/*
* Force the old incompatible Linux behavior
* if feature@xattr_compat is disabled.
*/
if (!spa_feature_is_enabled(dmu_objset_spa(zfsvfs->z_os),
SPA_FEATURE_XATTR_COMPAT))
newval = ZFS_XATTR_COMPAT_LINUX;

switch (newval) {
case ZFS_XATTR_COMPAT_ALL:
zfsvfs->z_flags |= ZSB_XATTR_COMPAT;
Expand All @@ -376,17 +367,6 @@ xattr_compat_changed_cb(void *arg, uint64_t newval)
}
}

static void
xattr_fallback_changed_cb(void *arg, uint64_t newval)
{
zfsvfs_t *zfsvfs = arg;

if (newval)
zfsvfs->z_flags |= ZSB_XATTR_FALLBACK;
else
zfsvfs->z_flags &= ~ZSB_XATTR_FALLBACK;
}

static void
acltype_changed_cb(void *arg, uint64_t newval)
{
Expand Down Expand Up @@ -548,9 +528,6 @@ zfs_register_callbacks(vfs_t *vfsp)
error = error ? error : dsl_prop_register(ds,
zfs_prop_to_name(ZFS_PROP_XATTR_COMPAT), xattr_compat_changed_cb,
zfsvfs);
error = error ? error : dsl_prop_register(ds,
zfs_prop_to_name(ZFS_PROP_XATTR_FALLBACK),
xattr_fallback_changed_cb, zfsvfs);
error = error ? error : dsl_prop_register(ds,
zfs_prop_to_name(ZFS_PROP_RECORDSIZE), blksz_changed_cb, zfsvfs);
error = error ? error : dsl_prop_register(ds,
Expand Down Expand Up @@ -1573,10 +1550,6 @@ zfs_domount(struct super_block *sb, zfs_mnt_t *zm, int silent)
"xattr_compat", &pval, NULL)))
goto out;
xattr_compat_changed_cb(zfsvfs, pval);
if ((error = dsl_prop_get_integer(osname,
"xattr_fallback", &pval, NULL)))
goto out;
xattr_fallback_changed_cb(zfsvfs, pval);
if ((error = dsl_prop_get_integer(osname,
"acltype", &pval, NULL)))
goto out;
Expand Down
17 changes: 5 additions & 12 deletions module/os/linux/zfs/zpl_xattr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,6 @@ __zpl_xattr_user_get(struct inode *ip, const char *name,
void *value, size_t size)
{
boolean_t compat = !!(ITOZSB(ip)->z_flags & ZSB_XATTR_COMPAT);
boolean_t fallback = !!(ITOZSB(ip)->z_flags & ZSB_XATTR_FALLBACK);
int error;
/* xattr_resolve_name will do this for us if this is defined */
#ifndef HAVE_XATTR_HANDLER_NAME
Expand All @@ -744,12 +743,10 @@ __zpl_xattr_user_get(struct inode *ip, const char *name,
/*
* Try to look up the name without the namespace prefix first for
* compatibility with xattrs from other platforms. If that fails,
* try again with the namespace prefix if fallback is allowed.
* try again with the namespace prefix.
*/
error = -ENODATA;
if (compat || fallback)
error = zpl_xattr_get(ip, name, value, size);
if (!compat || (fallback && error == -ENODATA)) {
error = zpl_xattr_get(ip, name, value, size);
if (!compat || error == -ENODATA) {
char *xattr_name;
xattr_name = kmem_asprintf("%s%s", XATTR_USER_PREFIX, name);
error = zpl_xattr_get(ip, xattr_name, value, size);
Expand All @@ -766,7 +763,6 @@ __zpl_xattr_user_set(struct inode *ip, const char *name,
{
char *xattr_name;
boolean_t compat = !!(ITOZSB(ip)->z_flags & ZSB_XATTR_COMPAT);
boolean_t fallback = !!(ITOZSB(ip)->z_flags & ZSB_XATTR_FALLBACK);
int error = 0;
/* xattr_resolve_name will do this for us if this is defined */
#ifndef HAVE_XATTR_HANDLER_NAME
Expand All @@ -788,21 +784,18 @@ __zpl_xattr_user_set(struct inode *ip, const char *name,
* XATTR_REPLACE: fail if xattr does not exist
*/
xattr_name = kmem_asprintf("%s%s", XATTR_USER_PREFIX, name);
if (compat && fallback)
if (compat)
error = zpl_xattr_set(ip, xattr_name, NULL, 0, flags);
if (!compat)
error = zpl_xattr_set(ip, xattr_name, value, size, flags);
kmem_strfree(xattr_name);

if (!compat || error == -EEXIST)
return (error);
if (fallback && error == 0 && (flags & XATTR_REPLACE))
if (error == 0 && (flags & XATTR_REPLACE))
flags &= ~XATTR_REPLACE;
error = zpl_xattr_set(ip, name, value, size, flags);

dsl_dataset_t *ds = dmu_objset_ds(ITOZSB(ip)->z_os);
ds->ds_feature_activation[SPA_FEATURE_XATTR_COMPAT] = (void *)B_TRUE;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking, who of users could already have this feature activated (like that one who had complained)? Only those who hit the original version of the patch before we made Scale default to Linux format? We should figure out what to write on release notes.

Copy link
Author

@ghost ghost Oct 8, 2021 •
edited by ghost
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users would have had to upgrade their existing pool from CORE in SCALE or created a new pool in SCALE to enable the feature, but then the default is xattr_compat=linux, so they would have had to change that property manually as well as far as I'm aware. I don't see anything in middleware that changes xattr_compat from the default for you.

xattr_compat was merged in #8 on June 29 and the default was changed to xattr_compat=linux in #16 on July 15, so only nightly users who created or upgraded a pool using a build from those two weeks would have xattr_compat=all on Linux unwittingly.

return (error);
}
ZPL_XATTR_SET_WRAPPER(zpl_xattr_user_set);
Expand Down
Loading