diff mbox series

[2/2] btrfs-progs: tests/mkfs: make sure rootdir got its xattr copied

Message ID 3daf7ed97305f5482800e28c960e3b5c2ed222b3.1696822345.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: make sure "mkfs --rootdir" copies the xattr for the rootdir | expand

Commit Message

Qu Wenruo Oct. 9, 2023, 3:34 a.m. UTC
The new test case would:

- Create a dir as source dir for --rootdir
- Add xattr for that source dir
- Create a file inside that source dir
- Add xattr for that file
- Run "mkfs.btrfs --rootdir" with that source dir
- Mount the created fs
- Make sure the following xattr exists:
  * Xattr for the rootdir
  * Xattr for that file inside the mount point

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/mkfs-tests/027-rootdir-xattr/test.sh | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100755 tests/mkfs-tests/027-rootdir-xattr/test.sh

Comments

David Sterba Oct. 9, 2023, 1:58 p.m. UTC | #1
On Mon, Oct 09, 2023 at 02:04:09PM +1030, Qu Wenruo wrote:
> The new test case would:
> 
> - Create a dir as source dir for --rootdir
> - Add xattr for that source dir
> - Create a file inside that source dir
> - Add xattr for that file
> - Run "mkfs.btrfs --rootdir" with that source dir
> - Mount the created fs
> - Make sure the following xattr exists:
>   * Xattr for the rootdir
>   * Xattr for that file inside the mount point
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/mkfs-tests/027-rootdir-xattr/test.sh | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100755 tests/mkfs-tests/027-rootdir-xattr/test.sh
> 
> diff --git a/tests/mkfs-tests/027-rootdir-xattr/test.sh b/tests/mkfs-tests/027-rootdir-xattr/test.sh
> new file mode 100755
> index 000000000000..bff9dc71d8e0
> --- /dev/null
> +++ b/tests/mkfs-tests/027-rootdir-xattr/test.sh
> @@ -0,0 +1,40 @@
> +#!/bin/bash
> +# Test if the source dir has xattr, "mkfs.btrfs --rootdir" would properly copy
> +# that xattr to the rootdir inode.
> +
> +source "$TEST_TOP/common" || exit
> +
> +setup_root_helper
> +prepare_test_dev
> +
> +check_global_prereq setfattr
> +check_global_prereq getfattr
> +
> +# Here we don't want to use /tmp, as it's pretty common /tmp is tmpfs, which
> +# doesn't support xattr.
> +# Instead we go $TEST_TOP/btrfs-progs-mkfs-tests-027.XXXXXX/ instead.
> +src_dir=$(mktemp -d --tmpdir=$TEST_TOP btrfs-progs-mkfs-tests-027.XXXXXX)

In case we want to be sure that the underlying filesystem for temporary
files supports the feature we want it's better to create a temporary
btrfs filesystem, either inside the test directory o in /tmp.

Chosing TEST_TOP is IMO wrong because that's where the test related
scritps and logs are, this is not meant for temporary files at all.

Also please use one of the _mktemp helpers.

I'll apply the first patch with fix, please update the test, thanks.
Qu Wenruo Oct. 9, 2023, 8:40 p.m. UTC | #2
On 2023/10/10 00:28, David Sterba wrote:
> On Mon, Oct 09, 2023 at 02:04:09PM +1030, Qu Wenruo wrote:
>> The new test case would:
>>
>> - Create a dir as source dir for --rootdir
>> - Add xattr for that source dir
>> - Create a file inside that source dir
>> - Add xattr for that file
>> - Run "mkfs.btrfs --rootdir" with that source dir
>> - Mount the created fs
>> - Make sure the following xattr exists:
>>    * Xattr for the rootdir
>>    * Xattr for that file inside the mount point
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   tests/mkfs-tests/027-rootdir-xattr/test.sh | 40 ++++++++++++++++++++++
>>   1 file changed, 40 insertions(+)
>>   create mode 100755 tests/mkfs-tests/027-rootdir-xattr/test.sh
>>
>> diff --git a/tests/mkfs-tests/027-rootdir-xattr/test.sh b/tests/mkfs-tests/027-rootdir-xattr/test.sh
>> new file mode 100755
>> index 000000000000..bff9dc71d8e0
>> --- /dev/null
>> +++ b/tests/mkfs-tests/027-rootdir-xattr/test.sh
>> @@ -0,0 +1,40 @@
>> +#!/bin/bash
>> +# Test if the source dir has xattr, "mkfs.btrfs --rootdir" would properly copy
>> +# that xattr to the rootdir inode.
>> +
>> +source "$TEST_TOP/common" || exit
>> +
>> +setup_root_helper
>> +prepare_test_dev
>> +
>> +check_global_prereq setfattr
>> +check_global_prereq getfattr
>> +
>> +# Here we don't want to use /tmp, as it's pretty common /tmp is tmpfs, which
>> +# doesn't support xattr.
>> +# Instead we go $TEST_TOP/btrfs-progs-mkfs-tests-027.XXXXXX/ instead.
>> +src_dir=$(mktemp -d --tmpdir=$TEST_TOP btrfs-progs-mkfs-tests-027.XXXXXX)
> 
> In case we want to be sure that the underlying filesystem for temporary
> files supports the feature we want it's better to create a temporary
> btrfs filesystem, either inside the test directory o in /tmp.

Indeed, another btrfs looks much better.

Would update the patch soon.

Thanks,
Qu

> 
> Chosing TEST_TOP is IMO wrong because that's where the test related
> scritps and logs are, this is not meant for temporary files at all.
> 
> Also please use one of the _mktemp helpers.
> 
> I'll apply the first patch with fix, please update the test, thanks.
diff mbox series

Patch

diff --git a/tests/mkfs-tests/027-rootdir-xattr/test.sh b/tests/mkfs-tests/027-rootdir-xattr/test.sh
new file mode 100755
index 000000000000..bff9dc71d8e0
--- /dev/null
+++ b/tests/mkfs-tests/027-rootdir-xattr/test.sh
@@ -0,0 +1,40 @@ 
+#!/bin/bash
+# Test if the source dir has xattr, "mkfs.btrfs --rootdir" would properly copy
+# that xattr to the rootdir inode.
+
+source "$TEST_TOP/common" || exit
+
+setup_root_helper
+prepare_test_dev
+
+check_global_prereq setfattr
+check_global_prereq getfattr
+
+# Here we don't want to use /tmp, as it's pretty common /tmp is tmpfs, which
+# doesn't support xattr.
+# Instead we go $TEST_TOP/btrfs-progs-mkfs-tests-027.XXXXXX/ instead.
+src_dir=$(mktemp -d --tmpdir=$TEST_TOP btrfs-progs-mkfs-tests-027.XXXXXX)
+run_mayfail setfattr -n user.rootdir "$src_dir"
+if [ $? -ne 0 ]; then
+	rm -rf -- "$src_dir"
+	_not_run "unable to set xattr to temporaray directory"
+fi
+run_check touch "$src_dir/foobar"
+run_check setfattr -n user.foobar "$src_dir/foobar"
+
+run_check_mkfs_test_dev --rootdir "$src_dir"
+run_check_mount_test_dev
+
+attr=$(run_check_stdout getfattr -n user.rootdir --absolute-names "$src_dir")
+if ! echo $attr | grep -q "user.rootdir" ; then
+	rm -rf -- "$src_dir"
+	_fail "no rootdir xattr found"
+fi
+
+attr=$(run_check_stdout getfattr -n user.foobar --absolute-names "$src_dir/foobar")
+if ! echo $attr | grep -q "user.foobar" ; then
+	rm -rf -- "$src_dir"
+	_fail "no regular file xattr found"
+fi
+
+rm -rf -- "$src_dir"