From 073b34b3eefaa25db5320a4000ad1f33ec7da5a6 Mon Sep 17 00:00:00 2001 From: Paul Dagnelie Date: Wed, 1 Oct 2025 12:14:56 -0700 Subject: [PATCH] Fix display of default xattr to show 'sa' When the default value of the xattr property was changed from 'dir' to 'sa', the code that displays the property's value was not affected. The problem with this state of affairs is that 1) user tooling that specifically looked for 'sa' before will be confused now that the code displays 'on' instead. And 2) users may be confused when manually running the commands about which specific type of xattr is in use unless they are up to date on the latest zfs changes. The fix here is to show the actual type always, rather than 'on' if we happen to be using the default. This turns out to be easy to do, by simply reordering the list of xattr values in the properties code. When the property is displayed, we iterate down the table until we find a row with a matching value, and use that row's name as the display. Reordering the row fixes the display without affecting any other code. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Reviewed-by: Alexander Motin Reviewed-by: Brian Behlendorf Reviewed-by: Rob Norris Reviewed-by: George Melikov Signed-off-by: Paul Dagnelie Closes #17801 --- module/zcommon/zfs_prop.c | 2 +- tests/runfiles/common.run | 3 +- tests/runfiles/sanity.run | 2 +- tests/zfs-tests/tests/Makefile.am | 1 + .../delegate/delegate_common.kshlib | 6 +-- .../tests/functional/xattr/xattr_014_pos.ksh | 53 +++++++++++++++++++ 6 files changed, 61 insertions(+), 6 deletions(-) create mode 100755 tests/zfs-tests/tests/functional/xattr/xattr_014_pos.ksh diff --git a/module/zcommon/zfs_prop.c b/module/zcommon/zfs_prop.c index 864e3898b..9190ae036 100644 --- a/module/zcommon/zfs_prop.c +++ b/module/zcommon/zfs_prop.c @@ -364,8 +364,8 @@ zfs_prop_init(void) static const zprop_index_t xattr_table[] = { { "off", ZFS_XATTR_OFF }, - { "on", ZFS_XATTR_SA }, { "sa", ZFS_XATTR_SA }, + { "on", ZFS_XATTR_SA }, { "dir", ZFS_XATTR_DIR }, { NULL } }; diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index b0e8f8d43..9f531411f 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -1090,7 +1090,8 @@ tags = ['functional', 'write_dirs'] [tests/functional/xattr] tests = ['xattr_001_pos', 'xattr_002_neg', 'xattr_003_neg', 'xattr_004_pos', 'xattr_005_pos', 'xattr_006_pos', 'xattr_007_neg', - 'xattr_011_pos', 'xattr_012_pos', 'xattr_013_pos', 'xattr_compat'] + 'xattr_011_pos', 'xattr_012_pos', 'xattr_013_pos', 'xattr_014_pos', + 'xattr_compat'] tags = ['functional', 'xattr'] [tests/functional/zvol/zvol_ENOSPC] diff --git a/tests/runfiles/sanity.run b/tests/runfiles/sanity.run index b56ffc3a4..249b41502 100644 --- a/tests/runfiles/sanity.run +++ b/tests/runfiles/sanity.run @@ -622,7 +622,7 @@ tags = ['functional', 'vdev_zaps'] [tests/functional/xattr] tests = ['xattr_001_pos', 'xattr_002_neg', 'xattr_003_neg', 'xattr_004_pos', 'xattr_005_pos', 'xattr_006_pos', 'xattr_007_neg', - 'xattr_011_pos', 'xattr_013_pos', 'xattr_compat'] + 'xattr_011_pos', 'xattr_013_pos', 'xattr_014_pos', 'xattr_compat'] tags = ['functional', 'xattr'] [tests/functional/zvol/zvol_ENOSPC] diff --git a/tests/zfs-tests/tests/Makefile.am b/tests/zfs-tests/tests/Makefile.am index 43eaa4591..678c01b58 100644 --- a/tests/zfs-tests/tests/Makefile.am +++ b/tests/zfs-tests/tests/Makefile.am @@ -2234,6 +2234,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \ functional/xattr/xattr_011_pos.ksh \ functional/xattr/xattr_012_pos.ksh \ functional/xattr/xattr_013_pos.ksh \ + functional/xattr/xattr_014_pos.ksh \ functional/xattr/xattr_compat.ksh \ functional/zap_shrink/cleanup.ksh \ functional/zap_shrink/zap_shrink_001_pos.ksh \ diff --git a/tests/zfs-tests/tests/functional/delegate/delegate_common.kshlib b/tests/zfs-tests/tests/functional/delegate/delegate_common.kshlib index 0a402e71e..345239b88 100644 --- a/tests/zfs-tests/tests/functional/delegate/delegate_common.kshlib +++ b/tests/zfs-tests/tests/functional/delegate/delegate_common.kshlib @@ -1234,10 +1234,10 @@ function verify_fs_aedsx typeset oldval set -A modes "on" "off" oldval=$(get_prop $perm $fs) - if [[ $oldval == "on" ]]; then - n=1 - elif [[ $oldval == "off" ]]; then + if [[ $oldval == "off" ]]; then n=0 + else + n=1 fi log_note "$user zfs set $perm=${modes[$n]} $fs" user_run $user zfs set $perm=${modes[$n]} $fs diff --git a/tests/zfs-tests/tests/functional/xattr/xattr_014_pos.ksh b/tests/zfs-tests/tests/functional/xattr/xattr_014_pos.ksh new file mode 100755 index 000000000..d4c9a0a41 --- /dev/null +++ b/tests/zfs-tests/tests/functional/xattr/xattr_014_pos.ksh @@ -0,0 +1,53 @@ +#!/bin/ksh -p +# SPDX-License-Identifier: CDDL-1.0 +# +# 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) 2025 by Klara, Inc. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/xattr/xattr_common.kshlib + +# +# DESCRIPTION: +# The default xattr should be shown as 'sa', not 'on', for clarity. +# +# STRATEGY: +# 1. Create a filesystem. +# 2. Verify that the xattra is shown as 'sa'. +# 3. Manually set the value to 'dir', 'sa', 'on', and 'off'. +# 4. Verify that it is shown as 'dir', 'sa', 'sa', and 'off. +# + +log_assert "The default and specific xattr values are displayed correctly." + +set -A args "dir" "sa" "on" "off" +set -A display "dir" "sa" "sa" "off" + +log_must eval "[[ 'sa' == '$(zfs get -Hpo value xattr $TESTPOOL)' ]]" + +for i in `seq 0 3`; do + log_must zfs set xattr="${args[$i]}" $TESTPOOL + log_must eval "[[ '${display[$i]}' == '$(zfs get -Hpo value xattr $TESTPOOL)' ]]" +done +log_pass "The default and specific xattr values are displayed correctly."