Re: [PATCH] btrfs: Vertify dir permissions when creating a stub subvolume

[Josef Bacik <josef@toxicpanda.com>]

On Thu, Aug 10, 2023 at 06:15:19PM -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>
> ---
>  tests/btrfs/300     | 42 ++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/300.out | 18 ++++++++++++++++++
>  2 files changed, 60 insertions(+)
>  create mode 100755 tests/btrfs/300
>  create mode 100644 tests/btrfs/300.out
> 
> diff --git a/tests/btrfs/300 b/tests/btrfs/300
> new file mode 100755
> index 00000000..3a34f8c0
> --- /dev/null
> +++ b/tests/btrfs/300
> @@ -0,0 +1,42 @@
> +#!/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
> +_register_cleanup "cleanup"
> +
> +test_dir="${TEST_DIR}/$(basename $0)"
> +cleanup() {
> +    [ -d "$test_dir" ] && rm -rf $test_dir
> +}
> +
> +mkdir $test_dir
> +chown fsgqa:fsgqa $test_dir
> +
> +_user_do "
> +cd ${test_dir};
> +unshare --user --keep-caps --map-auto --map-root-user;

I'm not sure how well supported these operations are, generally we want to make
sure that the commands exist and the options work.  Can you extract out the
require_unshare() helper from tests/overlay/020 into a actual helper in
common/rc, and then add

_require_unshare --user --keep-caps --map-auto --map-root-user

to your test, that way we don't get false failures if the environment doesn't
support these capabilities.

> +$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;
> +"

I assme you have to do this all in one fell swoop because of the unshare part?
Normally I'd prefer if we did one line per command, ie

_user_do "cd"
_user_do "unshare"
_user_do "subvol create"

but if that doesn't work it would be good to have a comment explaining that it
has to be done in one fell swoop.  If it does work then I'd prefer the above
pattern.  Thanks,

Josef