| Message ID | 20230814203759.3555213-1-lee@trager.us (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] fstests: Vertify dir permissions when creating a stub subvolume | expand |
On Mon, Aug 14, 2023 at 01:37:59PM -0700, Lee Trager wrote: > btrfs supports creating nesting subvolumes however snapshots are not > recurive. When a snapshot is taken of a volume which contains a subvolume > the subvolume is replaced with a stub subvolume which has the same name and > uses inode number 2. This test validates that the stub volume copies > permissions of the original volume. > Signed-off-by: Lee Trager <lee@trager.us> > --- > v3 > - Fixed whitepsace in 300.out due to find command not using './' before %P > v2: > - Migrated _require_unshare from overlay/020 into common_rc. Updated the error > message as most Linux systems should have unshare from util-linux. > - Added note about why the test must be done in one subshell process. > - chown command now uses $qa_user:$qa_group instead of hard coded values. > common/rc | 6 ++++++ > tests/btrfs/300 | 46 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/300.out | 18 ++++++++++++++++++ > tests/overlay/020 | 7 +------ > 4 files changed, 71 insertions(+), 6 deletions(-) > create mode 100755 tests/btrfs/300 > create mode 100644 tests/btrfs/300.out > > diff --git a/common/rc b/common/rc > index 5c4429ed..ca7c5c14 100644 > --- a/common/rc > +++ b/common/rc > @@ -5224,6 +5224,12 @@ _soak_loop_running() { > return 0 > } > > + (One more blank line) > +_require_unshare() { > + unshare -f -r -m -p -U true &>/dev/null || \ Should we check if `unshare` command supports "-f -r -m -p -U" options? Or let specific testcase decides which option they need? > + _notrun "unshare $@: not found, should be in util-linux" > +} > + > init_rc > > ################################################################################ > diff --git a/tests/btrfs/300 b/tests/btrfs/300 > new file mode 100755 > index 00000000..fb9cd49f > --- /dev/null > +++ b/tests/btrfs/300 > @@ -0,0 +1,46 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2022 Meta Platforms, Inc. All Rights Reserved. > +# > +# FS QA Test 300 > +# > +# Validate that snapshots taken while in a remapped namespace preserve > +# the permissions of the user. > +# > +. ./common/preamble > +_begin_fstest auto quick subvol snapshot > + > +_supported_fs btrfs > + > +_require_test > +_require_user > +_require_group > +_require_unix_perm_checking > +_require_unshare > +_register_cleanup "cleanup" Better to register cleanup ealier, you can move it below the _begin_fstest. > + > +test_dir="${TEST_DIR}/$(basename $0)" If you need "$(basename $0)", you can use "$seq" directly. > +cleanup() { > + [ -d "$test_dir" ] && rm -rf $test_dir I think the "[ -d "$test_dir" ]" isn't needed, due to the "-f" of rm command "ignore nonexistent files and arguments". Besides that, you also need below two lines in cleanup: cd / rm -r -f $tmp.* > +} > + > +mkdir $test_dir $TEST_DIR doesn't like $SCRATCH_MNT, We don't always remove all dirs/files of $TEST_DIR after a subcase test done. So we can't be sure your "$test_dir" isn't existed. So you'd better to do "rm -rf $test_dir" before using it. > +chown $qa_user:$qa_group $test_dir > + > +# _user_do executes each command as $qa_user in its own subshell. unshare > +# sets the namespace for the running shell. The test must run in one user > +# subshell to preserve the namespace over multiple commands. > +_user_do " > +cd ${test_dir}; Is relative path necessary? If the relative path is just for golden image output, you can use "| _filter_test_dir" after `BTRFS_UTIL_PROG` and `find` command lines. Anyway, as you do these in _user_do, so relative path is fine for me too. Thanks, Zorro > +unshare --user --keep-caps --map-auto --map-root-user; > +$BTRFS_UTIL_PROG subvolume create subvol; > +touch subvol/{1,2,3}; > +$BTRFS_UTIL_PROG subvolume create subvol/subsubvol; > +touch subvol/subsubvol/{4,5,6}; > +$BTRFS_UTIL_PROG subvolume snapshot subvol snapshot; > +" > + > +find $test_dir/. -printf "%M %u %g ./%P\n" > + > +status=0 > +exit > diff --git a/tests/btrfs/300.out b/tests/btrfs/300.out > new file mode 100644 > index 00000000..6e94447e > --- /dev/null > +++ b/tests/btrfs/300.out > @@ -0,0 +1,18 @@ > +QA output created by 300 > +Create subvolume './subvol' > +Create subvolume 'subvol/subsubvol' > +Create a snapshot of 'subvol' in './snapshot' > +drwxr-xr-x fsgqa fsgqa ./ > +drwxr-xr-x fsgqa fsgqa ./subvol > +-rw-r--r-- fsgqa fsgqa ./subvol/1 > +-rw-r--r-- fsgqa fsgqa ./subvol/2 > +-rw-r--r-- fsgqa fsgqa ./subvol/3 > +drwxr-xr-x fsgqa fsgqa ./subvol/subsubvol > +-rw-r--r-- fsgqa fsgqa ./subvol/subsubvol/4 > +-rw-r--r-- fsgqa fsgqa ./subvol/subsubvol/5 > +-rw-r--r-- fsgqa fsgqa ./subvol/subsubvol/6 > +drwxr-xr-x fsgqa fsgqa ./snapshot > +-rw-r--r-- fsgqa fsgqa ./snapshot/1 > +-rw-r--r-- fsgqa fsgqa ./snapshot/2 > +-rw-r--r-- fsgqa fsgqa ./snapshot/3 > +drwxr-xr-x fsgqa fsgqa ./snapshot/subsubvol > diff --git a/tests/overlay/020 b/tests/overlay/020 > index 98a33aec..9f82da34 100755 > --- a/tests/overlay/020 > +++ b/tests/overlay/020 > @@ -16,18 +16,13 @@ _begin_fstest auto quick copyup perms > > # real QA test starts here > > -require_unshare() { > - unshare -f -r "$@" true &>/dev/null || \ > - _notrun "unshare $@: not supported" > -} > - > # Modify as appropriate. > _supported_fs overlay > _fixed_by_kernel_commit 3fe6e52f0626 \ > "ovl: override creds with the ones from the superblock mounter" > > _require_scratch > -require_unshare -m -p -U > +_require_unshare > > # Remove all files from previous tests > _scratch_mkfs > -- > 2.34.1 >
On Mon, Aug 14, 2023 at 10:04 PM Lee Trager <lee@trager.us> wrote: > > btrfs supports creating nesting subvolumes however snapshots are not > recurive. When a snapshot is taken of a volume which contains a subvolume > the subvolume is replaced with a stub subvolume which has the same name and > uses inode number 2. This test validates that the stub volume copies > permissions of the original volume. > Signed-off-by: Lee Trager <lee@trager.us> > --- > v3 > - Fixed whitepsace in 300.out due to find command not using './' before %P > v2: > - Migrated _require_unshare from overlay/020 into common_rc. Updated the error > message as most Linux systems should have unshare from util-linux. > - Added note about why the test must be done in one subshell process. > - chown command now uses $qa_user:$qa_group instead of hard coded values. > common/rc | 6 ++++++ > tests/btrfs/300 | 46 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/300.out | 18 ++++++++++++++++++ > tests/overlay/020 | 7 +------ > 4 files changed, 71 insertions(+), 6 deletions(-) > create mode 100755 tests/btrfs/300 > create mode 100644 tests/btrfs/300.out > > diff --git a/common/rc b/common/rc > index 5c4429ed..ca7c5c14 100644 > --- a/common/rc > +++ b/common/rc > @@ -5224,6 +5224,12 @@ _soak_loop_running() { > return 0 > } > > + > +_require_unshare() { > + unshare -f -r -m -p -U true &>/dev/null || \ > + _notrun "unshare $@: not found, should be in util-linux" > +} > + > init_rc > > ################################################################################ > diff --git a/tests/btrfs/300 b/tests/btrfs/300 > new file mode 100755 > index 00000000..fb9cd49f > --- /dev/null > +++ b/tests/btrfs/300 > @@ -0,0 +1,46 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2022 Meta Platforms, Inc. All Rights Reserved. 2023 > +# > +# FS QA Test 300 > +# > +# Validate that snapshots taken while in a remapped namespace preserve > +# the permissions of the user. > +# > +. ./common/preamble > +_begin_fstest auto quick subvol snapshot > + > +_supported_fs btrfs > + > +_require_test > +_require_user > +_require_group > +_require_unix_perm_checking > +_require_unshare > +_register_cleanup "cleanup" So this is a test for the bug fix recently submitted: https://lore.kernel.org/linux-btrfs/20230811004657.1661696-1-lee@trager.us/ Which is in misc-next now: https://github.com/kdave/btrfs-devel/commit/ea6aa58a92299294b83a75defa467437a00011fe Please add a: _fixed_by_kernel_commit xxxxxxxxxxxx \ "btrfs: copy dir permission and time when creating a stub subvolume" The xxxxxxxxx should be replaced with the git commit id once it lands in Linus' tree, in the meanwhile you can leave the xxxxxxxxxx. Also please CC the linux-btrfs mailing list when you send btrfs tests or changes, so that it isn't missed by developers and nowadays it's even documented to do so in the MAINTAINERS file. There's also a typo in the subject: Vertify -> Verify Thanks. > + > +test_dir="${TEST_DIR}/$(basename $0)" > +cleanup() { > + [ -d "$test_dir" ] && rm -rf $test_dir > +} > + > +mkdir $test_dir > +chown $qa_user:$qa_group $test_dir > + > +# _user_do executes each command as $qa_user in its own subshell. unshare > +# sets the namespace for the running shell. The test must run in one user > +# subshell to preserve the namespace over multiple commands. > +_user_do " > +cd ${test_dir}; > +unshare --user --keep-caps --map-auto --map-root-user; > +$BTRFS_UTIL_PROG subvolume create subvol; > +touch subvol/{1,2,3}; > +$BTRFS_UTIL_PROG subvolume create subvol/subsubvol; > +touch subvol/subsubvol/{4,5,6}; > +$BTRFS_UTIL_PROG subvolume snapshot subvol snapshot; > +" > + > +find $test_dir/. -printf "%M %u %g ./%P\n" > + > +status=0 > +exit > diff --git a/tests/btrfs/300.out b/tests/btrfs/300.out > new file mode 100644 > index 00000000..6e94447e > --- /dev/null > +++ b/tests/btrfs/300.out > @@ -0,0 +1,18 @@ > +QA output created by 300 > +Create subvolume './subvol' > +Create subvolume 'subvol/subsubvol' > +Create a snapshot of 'subvol' in './snapshot' > +drwxr-xr-x fsgqa fsgqa ./ > +drwxr-xr-x fsgqa fsgqa ./subvol > +-rw-r--r-- fsgqa fsgqa ./subvol/1 > +-rw-r--r-- fsgqa fsgqa ./subvol/2 > +-rw-r--r-- fsgqa fsgqa ./subvol/3 > +drwxr-xr-x fsgqa fsgqa ./subvol/subsubvol > +-rw-r--r-- fsgqa fsgqa ./subvol/subsubvol/4 > +-rw-r--r-- fsgqa fsgqa ./subvol/subsubvol/5 > +-rw-r--r-- fsgqa fsgqa ./subvol/subsubvol/6 > +drwxr-xr-x fsgqa fsgqa ./snapshot > +-rw-r--r-- fsgqa fsgqa ./snapshot/1 > +-rw-r--r-- fsgqa fsgqa ./snapshot/2 > +-rw-r--r-- fsgqa fsgqa ./snapshot/3 > +drwxr-xr-x fsgqa fsgqa ./snapshot/subsubvol > diff --git a/tests/overlay/020 b/tests/overlay/020 > index 98a33aec..9f82da34 100755 > --- a/tests/overlay/020 > +++ b/tests/overlay/020 > @@ -16,18 +16,13 @@ _begin_fstest auto quick copyup perms > > # real QA test starts here > > -require_unshare() { > - unshare -f -r "$@" true &>/dev/null || \ > - _notrun "unshare $@: not supported" > -} > - > # Modify as appropriate. > _supported_fs overlay > _fixed_by_kernel_commit 3fe6e52f0626 \ > "ovl: override creds with the ones from the superblock mounter" > > _require_scratch > -require_unshare -m -p -U > +_require_unshare > > # Remove all files from previous tests > _scratch_mkfs > -- > 2.34.1 >
diff --git a/common/rc b/common/rc index 5c4429ed..ca7c5c14 100644 --- a/common/rc +++ b/common/rc @@ -5224,6 +5224,12 @@ _soak_loop_running() { return 0 } + +_require_unshare() { + unshare -f -r -m -p -U true &>/dev/null || \ + _notrun "unshare $@: not found, should be in util-linux" +} + init_rc ################################################################################ diff --git a/tests/btrfs/300 b/tests/btrfs/300 new file mode 100755 index 00000000..fb9cd49f --- /dev/null +++ b/tests/btrfs/300 @@ -0,0 +1,46 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2022 Meta Platforms, Inc. All Rights Reserved. +# +# FS QA Test 300 +# +# Validate that snapshots taken while in a remapped namespace preserve +# the permissions of the user. +# +. ./common/preamble +_begin_fstest auto quick subvol snapshot + +_supported_fs btrfs + +_require_test +_require_user +_require_group +_require_unix_perm_checking +_require_unshare +_register_cleanup "cleanup" + +test_dir="${TEST_DIR}/$(basename $0)" +cleanup() { + [ -d "$test_dir" ] && rm -rf $test_dir +} + +mkdir $test_dir +chown $qa_user:$qa_group $test_dir + +# _user_do executes each command as $qa_user in its own subshell. unshare +# sets the namespace for the running shell. The test must run in one user +# subshell to preserve the namespace over multiple commands. +_user_do " +cd ${test_dir}; +unshare --user --keep-caps --map-auto --map-root-user; +$BTRFS_UTIL_PROG subvolume create subvol; +touch subvol/{1,2,3}; +$BTRFS_UTIL_PROG subvolume create subvol/subsubvol; +touch subvol/subsubvol/{4,5,6}; +$BTRFS_UTIL_PROG subvolume snapshot subvol snapshot; +" + +find $test_dir/. -printf "%M %u %g ./%P\n" + +status=0 +exit diff --git a/tests/btrfs/300.out b/tests/btrfs/300.out new file mode 100644 index 00000000..6e94447e --- /dev/null +++ b/tests/btrfs/300.out @@ -0,0 +1,18 @@ +QA output created by 300 +Create subvolume './subvol' +Create subvolume 'subvol/subsubvol' +Create a snapshot of 'subvol' in './snapshot' +drwxr-xr-x fsgqa fsgqa ./ +drwxr-xr-x fsgqa fsgqa ./subvol +-rw-r--r-- fsgqa fsgqa ./subvol/1 +-rw-r--r-- fsgqa fsgqa ./subvol/2 +-rw-r--r-- fsgqa fsgqa ./subvol/3 +drwxr-xr-x fsgqa fsgqa ./subvol/subsubvol +-rw-r--r-- fsgqa fsgqa ./subvol/subsubvol/4 +-rw-r--r-- fsgqa fsgqa ./subvol/subsubvol/5 +-rw-r--r-- fsgqa fsgqa ./subvol/subsubvol/6 +drwxr-xr-x fsgqa fsgqa ./snapshot +-rw-r--r-- fsgqa fsgqa ./snapshot/1 +-rw-r--r-- fsgqa fsgqa ./snapshot/2 +-rw-r--r-- fsgqa fsgqa ./snapshot/3 +drwxr-xr-x fsgqa fsgqa ./snapshot/subsubvol diff --git a/tests/overlay/020 b/tests/overlay/020 index 98a33aec..9f82da34 100755 --- a/tests/overlay/020 +++ b/tests/overlay/020 @@ -16,18 +16,13 @@ _begin_fstest auto quick copyup perms # real QA test starts here -require_unshare() { - unshare -f -r "$@" true &>/dev/null || \ - _notrun "unshare $@: not supported" -} - # Modify as appropriate. _supported_fs overlay _fixed_by_kernel_commit 3fe6e52f0626 \ "ovl: override creds with the ones from the superblock mounter" _require_scratch -require_unshare -m -p -U +_require_unshare # Remove all files from previous tests _scratch_mkfs
btrfs supports creating nesting subvolumes however snapshots are not recurive. When a snapshot is taken of a volume which contains a subvolume the subvolume is replaced with a stub subvolume which has the same name and uses inode number 2. This test validates that the stub volume copies permissions of the original volume. Signed-off-by: Lee Trager <lee@trager.us> --- v3 - Fixed whitepsace in 300.out due to find command not using './' before %P v2: - Migrated _require_unshare from overlay/020 into common_rc. Updated the error message as most Linux systems should have unshare from util-linux. - Added note about why the test must be done in one subshell process. - chown command now uses $qa_user:$qa_group instead of hard coded values. common/rc | 6 ++++++ tests/btrfs/300 | 46 +++++++++++++++++++++++++++++++++++++++++++++ tests/btrfs/300.out | 18 ++++++++++++++++++ tests/overlay/020 | 7 +------ 4 files changed, 71 insertions(+), 6 deletions(-) create mode 100755 tests/btrfs/300 create mode 100644 tests/btrfs/300.out