* [PATCH v2 0/4] Fix some tests that fail for exfat FS
@ 2021-03-30 22:00 Pavel Reichl
2021-03-30 22:00 ` [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}() Pavel Reichl
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Pavel Reichl @ 2021-03-30 22:00 UTC (permalink / raw)
To: fstests; +Cc: zlang, guan
Mostly just do not run test that require posix permissions and ownership.
Changes in V2 (Thank you for comments on version 1)
* Merged patch #1 and #2 (the one introducing the helper and the other actually
using them)
* Fixed generic/597 by introducing _require_symlink
* Fixed generic/598 by adding _require_chmod
** changes are part of patch #1
* Introduced _format_swapon_file()
Pavel Reichl (4):
common/rc: Add _require_{chown,chmod,symlink}()
common: hide permision warning from mkswap for exfat
generic/554: hide permision warning on exfat
generic/003: Amend the test for exfat
common/rc | 48 ++++++++++++++++++++++++++++++++++++++++++++++-
tests/generic/003 | 29 +++++++++++++++++++---------
tests/generic/087 | 1 +
tests/generic/088 | 1 +
tests/generic/125 | 1 +
tests/generic/126 | 1 +
tests/generic/128 | 1 +
tests/generic/193 | 1 +
tests/generic/314 | 1 +
tests/generic/317 | 1 +
tests/generic/355 | 1 +
tests/generic/554 | 3 ++-
tests/generic/597 | 1 +
tests/generic/598 | 1 +
14 files changed, 80 insertions(+), 11 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}()
2021-03-30 22:00 [PATCH v2 0/4] Fix some tests that fail for exfat FS Pavel Reichl
@ 2021-03-30 22:00 ` Pavel Reichl
2021-03-31 0:25 ` Zorro Lang
2021-04-01 3:38 ` Eryu Guan
2021-03-30 22:00 ` [PATCH v2 2/4] common: hide permision warning from mkswap for exfat Pavel Reichl
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Pavel Reichl @ 2021-03-30 22:00 UTC (permalink / raw)
To: fstests; +Cc: zlang, guan
Add helper functions that ensure that test is only executed on file
systems that implement chown, chmod and symbolic links.
Fixed test are: generic/{87,88,125,126,128,193,314,317,355,597,598}
Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
common/rc | 27 +++++++++++++++++++++++++++
tests/generic/087 | 1 +
tests/generic/088 | 1 +
tests/generic/125 | 1 +
tests/generic/126 | 1 +
tests/generic/128 | 1 +
tests/generic/193 | 1 +
tests/generic/314 | 1 +
tests/generic/317 | 1 +
tests/generic/355 | 1 +
tests/generic/597 | 1 +
tests/generic/598 | 1 +
12 files changed, 38 insertions(+)
diff --git a/common/rc b/common/rc
index 0ce3cb0d..9cdfe21c 100644
--- a/common/rc
+++ b/common/rc
@@ -2129,6 +2129,33 @@ _require_user()
[ "$?" == "0" ] || _notrun "$qa_user cannot execute commands."
}
+# check for a chown support
+#
+_require_chown()
+{
+ if [ "$FSTYP" = "exfat" ]; then
+ _notrun "chown is not supported on $FSTYP"
+ fi
+}
+
+# check for a chmod support
+#
+_require_chmod()
+{
+ if [ "$FSTYP" = "exfat" ]; then
+ _notrun "chmod is not supported on $FSTYP"
+ fi
+}
+
+# check for a symbolic links support
+#
+_require_symlink()
+{
+ if [ "$FSTYP" = "exfat" ]; then
+ _notrun "symbolic links are not supported on $FSTYP"
+ fi
+}
+
# check for a group on the machine, fsgqa as default
#
_require_group()
diff --git a/tests/generic/087 b/tests/generic/087
index 1f30dbf4..c3576117 100755
--- a/tests/generic/087
+++ b/tests/generic/087
@@ -37,6 +37,7 @@ _cleanup()
# real QA test starts here
_supported_fs generic
_require_test
+_require_chown
QA_FS_PERMS=$here/src/fs_perms
diff --git a/tests/generic/088 b/tests/generic/088
index 9388a083..ad99bd7e 100755
--- a/tests/generic/088
+++ b/tests/generic/088
@@ -29,6 +29,7 @@ _filter()
# real QA test starts here
_supported_fs generic
_require_test
+_require_chown
path=$TEST_DIR/t_access
$here/src/t_access_root $path | tee $seqres.full | _filter
diff --git a/tests/generic/125 b/tests/generic/125
index e84248d3..8c8f5cd7 100755
--- a/tests/generic/125
+++ b/tests/generic/125
@@ -25,6 +25,7 @@ _supported_fs generic
_require_test
_require_user
_require_odirect
+_require_chmod
TESTDIR=$TEST_DIR/ftrunc
TESTFILE=$TESTDIR/ftrunc.tmp
diff --git a/tests/generic/126 b/tests/generic/126
index ac25d294..636ca00d 100755
--- a/tests/generic/126
+++ b/tests/generic/126
@@ -27,6 +27,7 @@ _cleanup()
# real QA test starts here
_supported_fs generic
_require_test
+_require_chown
QA_FS_PERMS=$here/src/fs_perms
diff --git a/tests/generic/128 b/tests/generic/128
index b3e49eff..c1eae77a 100755
--- a/tests/generic/128
+++ b/tests/generic/128
@@ -24,6 +24,7 @@ _supported_fs generic
_require_scratch
_require_user
+_require_chmod
_scratch_mkfs >/dev/null 2>&1
_scratch_mount "-o nosuid"
diff --git a/tests/generic/193 b/tests/generic/193
index 3125efdd..fd0ebbf6 100755
--- a/tests/generic/193
+++ b/tests/generic/193
@@ -56,6 +56,7 @@ _supported_fs generic
_require_test
_require_user
+_require_chown
test_root=$TEST_DIR/$seq.$$.root
test_user=$TEST_DIR/$seq.$$.user
diff --git a/tests/generic/314 b/tests/generic/314
index 03df81ce..540f0feb 100755
--- a/tests/generic/314
+++ b/tests/generic/314
@@ -29,6 +29,7 @@ _cleanup()
_supported_fs generic
_require_test
_require_user
+_require_chown
rm -rf $TEST_DIR/$seq-dir
diff --git a/tests/generic/317 b/tests/generic/317
index 29c37a57..289dfabe 100755
--- a/tests/generic/317
+++ b/tests/generic/317
@@ -45,6 +45,7 @@ _require_scratch
_require_user
_require_ugid_map
_require_userns
+_require_chown
qa_user_id=`id -u $qa_user`
_filter_output()
diff --git a/tests/generic/355 b/tests/generic/355
index 161dd042..74fba0f9 100755
--- a/tests/generic/355
+++ b/tests/generic/355
@@ -32,6 +32,7 @@ _supported_fs generic
_require_test
_require_user
_require_odirect
+_require_chown
testfile=$TEST_DIR/$seq.test
rm -f $testfile
diff --git a/tests/generic/597 b/tests/generic/597
index ba769d73..f596406c 100755
--- a/tests/generic/597
+++ b/tests/generic/597
@@ -43,6 +43,7 @@ _require_sysctl_variable fs.protected_hardlinks
_require_user fsgqa2
# Do this SECOND so that qa_user is fsgqa, and _user_do uses that account
_require_user fsgqa
+_require_symlink
OWNER=fsgqa2
OTHER=fsgqa
diff --git a/tests/generic/598 b/tests/generic/598
index 6b765275..230c3ac7 100755
--- a/tests/generic/598
+++ b/tests/generic/598
@@ -43,6 +43,7 @@ _require_sysctl_variable fs.protected_fifos
_require_user fsgqa2
# Do this SECOND so that qa_user is fsgqa, and _user_do uses that account
_require_user fsgqa
+_require_chmod
USER1=fsgqa2
USER2=fsgqa
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/4] common: hide permision warning from mkswap for exfat
2021-03-30 22:00 [PATCH v2 0/4] Fix some tests that fail for exfat FS Pavel Reichl
2021-03-30 22:00 ` [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}() Pavel Reichl
@ 2021-03-30 22:00 ` Pavel Reichl
2021-04-07 17:03 ` Eric Sandeen
2021-03-30 22:00 ` [PATCH v2 3/4] generic/554: hide permision warning on exfat Pavel Reichl
2021-03-30 22:00 ` [PATCH v2 4/4] generic/003: Amend the test for exfat Pavel Reichl
3 siblings, 1 reply; 15+ messages in thread
From: Pavel Reichl @ 2021-03-30 22:00 UTC (permalink / raw)
To: fstests; +Cc: zlang, guan
exfat does not support posix file permisions, so warning from mkswap is
inavitable. This patch hides the warning message so the test won't fail.
Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
common/rc | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/common/rc b/common/rc
index 9cdfe21c..2d658711 100644
--- a/common/rc
+++ b/common/rc
@@ -2392,7 +2392,13 @@ _format_swapfile() {
# Swap files must be nocow on Btrfs.
$CHATTR_PROG +C "$fname" > /dev/null 2>&1
_pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full
- $MKSWAP_PROG "$fname" >> $seqres.full
+ if [ "$FSTYP" = "exfat" ]; then
+ # exfat does not support posix file permisions, so warning is
+ # to be expected
+ $MKSWAP_PROG "$fname" 2>&1 | grep -v 'insecure permission' >> $seqres.full
+ else
+ $MKSWAP_PROG "$fname" >> $seqres.full
+ fi
}
# Check that the filesystem supports swapfiles
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/4] generic/554: hide permision warning on exfat
2021-03-30 22:00 [PATCH v2 0/4] Fix some tests that fail for exfat FS Pavel Reichl
2021-03-30 22:00 ` [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}() Pavel Reichl
2021-03-30 22:00 ` [PATCH v2 2/4] common: hide permision warning from mkswap for exfat Pavel Reichl
@ 2021-03-30 22:00 ` Pavel Reichl
2021-04-01 3:40 ` Eryu Guan
2021-03-30 22:00 ` [PATCH v2 4/4] generic/003: Amend the test for exfat Pavel Reichl
3 siblings, 1 reply; 15+ messages in thread
From: Pavel Reichl @ 2021-03-30 22:00 UTC (permalink / raw)
To: fstests; +Cc: zlang, guan
Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
common/rc | 13 +++++++++++++
tests/generic/554 | 3 ++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/common/rc b/common/rc
index 2d658711..379140ea 100644
--- a/common/rc
+++ b/common/rc
@@ -2401,6 +2401,19 @@ _format_swapfile() {
fi
}
+_format_swapon_file() {
+ local fname="$1"
+
+ if [ "$FSTYP" = "exfat" ]; then
+ # exfat does not support posix file permisions, so warning is
+ # to be expected
+ swapon "$fname" |& grep -v "insecure permissions"
+ else
+ swapon "$fname"
+ fi
+
+}
+
# Check that the filesystem supports swapfiles
_require_scratch_swapfile()
{
diff --git a/tests/generic/554 b/tests/generic/554
index fa4f97d2..c2f9eee8 100755
--- a/tests/generic/554
+++ b/tests/generic/554
@@ -46,7 +46,8 @@ $XFS_IO_PROG -f -c "pwrite -S 0x61 0 128k" $SCRATCH_MNT/file >> $seqres.full 2>&
echo swap files return ETXTBUSY
_format_swapfile $SCRATCH_MNT/swapfile 16m
-swapon $SCRATCH_MNT/swapfile
+_format_swapon_file $SCRATCH_MNT/swapfile
+
$XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/file" $SCRATCH_MNT/swapfile
swapoff $SCRATCH_MNT/swapfile
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 4/4] generic/003: Amend the test for exfat
2021-03-30 22:00 [PATCH v2 0/4] Fix some tests that fail for exfat FS Pavel Reichl
` (2 preceding siblings ...)
2021-03-30 22:00 ` [PATCH v2 3/4] generic/554: hide permision warning on exfat Pavel Reichl
@ 2021-03-30 22:00 ` Pavel Reichl
2021-04-07 18:03 ` Eric Sandeen
3 siblings, 1 reply; 15+ messages in thread
From: Pavel Reichl @ 2021-03-30 22:00 UTC (permalink / raw)
To: fstests; +Cc: zlang, guan
Update the test so it can be run even for exfat which has 2 seconds
granularity for access_time and does not have a timestamp for
metadata change.
Signed-off-by: Pavel Reichl <preichl@redhat.com>
---
tests/generic/003 | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/tests/generic/003 b/tests/generic/003
index ec4fdfc1..39596665 100755
--- a/tests/generic/003
+++ b/tests/generic/003
@@ -37,6 +37,13 @@ _require_relatime
rm -f $seqres.full
+if [ "$FSTYP" = "exfat" ]; then
+ # exfat's timestamp for access_time has double seconds granularity
+ access_delay=2
+else
+ access_delay=1
+fi
+
_stat() {
stat -c "%x;%y;%z" $1
}
@@ -79,14 +86,14 @@ echo "aaa" > $TPATH/dir1/file1
file1_stat_before_first_access=`_stat $TPATH/dir1/file1`
# Accessing file1 the first time
-sleep 1
+sleep $access_delay
cat $TPATH/dir1/file1 > /dev/null
file1_stat_after_first_access=`_stat $TPATH/dir1/file1`
_compare_stat_times YNN "$file1_stat_before_first_access" \
"$file1_stat_after_first_access" "after accessing file1 first time"
# Accessing file1 a second time
-sleep 1
+sleep $access_delay
cat $TPATH/dir1/file1 > /dev/null
file1_stat_after_second_access=`_stat $TPATH/dir1/file1`
_compare_stat_times NNN "$file1_stat_after_first_access" \
@@ -109,7 +116,7 @@ _compare_stat_times NYY "$dir2_stat_before_file_creation" \
# Accessing file2
file2_stat_before_first_access=`_stat $TPATH/dir2/file2`
-sleep 1
+sleep $access_delay
cat $TPATH/dir2/file2 > /dev/null
file2_stat_after_first_access=`_stat $TPATH/dir2/file2`
_compare_stat_times YNN "$file2_stat_before_first_access" \
@@ -135,11 +142,15 @@ echo "xyz" > $TPATH/dir1/file1
file1_stat_after_modify=`_stat $TPATH/dir1/file1`
_compare_stat_times NYY "$file1_stat_before_modify" \
"$file1_stat_after_modify" "after modifying file1"
-sleep 1
-mv $TPATH/dir1/file1 $TPATH/dir1/file1_renamed
-file1_stat_after_change=`_stat $TPATH/dir1/file1_renamed`
-_compare_stat_times NNY "$file1_stat_after_modify" \
- "$file1_stat_after_change" "after changing file1"
+
+# exfat does not support last metadata change timestamp
+if [ "$FSTYP" != "exfat" ]; then
+ sleep 1
+ mv $TPATH/dir1/file1 $TPATH/dir1/file1_renamed
+ file1_stat_after_change=`_stat $TPATH/dir1/file1_renamed`
+ _compare_stat_times NNY "$file1_stat_after_modify" \
+ "$file1_stat_after_change" "after changing file1"
+fi
# Mounting with strictatime option and
# accessing a previously created file twice
@@ -148,7 +159,7 @@ cat $TPATH/dir2/file3 > /dev/null
file3_stat_after_second_access=`_stat $TPATH/dir2/file3`
_compare_stat_times YNN "$file3_stat_after_first_access" \
"$file3_stat_after_second_access" "after accessing file3 second time"
-sleep 1
+sleep $access_delay
cat $TPATH/dir2/file3 > /dev/null
file3_stat_after_third_access=`_stat $TPATH/dir2/file3`
_compare_stat_times YNN "$file3_stat_after_second_access" \
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}()
2021-03-30 22:00 ` [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}() Pavel Reichl
@ 2021-03-31 0:25 ` Zorro Lang
2021-03-31 7:17 ` Pavel Reichl
2021-04-01 3:38 ` Eryu Guan
1 sibling, 1 reply; 15+ messages in thread
From: Zorro Lang @ 2021-03-31 0:25 UTC (permalink / raw)
To: Pavel Reichl; +Cc: fstests, guan
On Wed, Mar 31, 2021 at 12:00:02AM +0200, Pavel Reichl wrote:
> Add helper functions that ensure that test is only executed on file
> systems that implement chown, chmod and symbolic links.
>
> Fixed test are: generic/{87,88,125,126,128,193,314,317,355,597,598}
>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
> common/rc | 27 +++++++++++++++++++++++++++
> tests/generic/087 | 1 +
> tests/generic/088 | 1 +
> tests/generic/125 | 1 +
> tests/generic/126 | 1 +
> tests/generic/128 | 1 +
> tests/generic/193 | 1 +
> tests/generic/314 | 1 +
> tests/generic/317 | 1 +
> tests/generic/355 | 1 +
> tests/generic/597 | 1 +
> tests/generic/598 | 1 +
> 12 files changed, 38 insertions(+)
>
> diff --git a/common/rc b/common/rc
> index 0ce3cb0d..9cdfe21c 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2129,6 +2129,33 @@ _require_user()
> [ "$?" == "0" ] || _notrun "$qa_user cannot execute commands."
> }
>
> +# check for a chown support
> +#
> +_require_chown()
> +{
> + if [ "$FSTYP" = "exfat" ]; then
> + _notrun "chown is not supported on $FSTYP"
> + fi
> +}
> +
> +# check for a chmod support
> +#
> +_require_chmod()
> +{
> + if [ "$FSTYP" = "exfat" ]; then
> + _notrun "chmod is not supported on $FSTYP"
> + fi
> +}
> +
> +# check for a symbolic links support
> +#
> +_require_symlink()
> +{
> + if [ "$FSTYP" = "exfat" ]; then
> + _notrun "symbolic links are not supported on $FSTYP"
> + fi
> +}
There's _require_symlinks() in common/rc, I remember Eric used it for some
exfat errors before. Does it work for you?
Thanks,
Zorro
> +
> # check for a group on the machine, fsgqa as default
> #
> _require_group()
> diff --git a/tests/generic/087 b/tests/generic/087
> index 1f30dbf4..c3576117 100755
> --- a/tests/generic/087
> +++ b/tests/generic/087
> @@ -37,6 +37,7 @@ _cleanup()
> # real QA test starts here
> _supported_fs generic
> _require_test
> +_require_chown
>
> QA_FS_PERMS=$here/src/fs_perms
>
> diff --git a/tests/generic/088 b/tests/generic/088
> index 9388a083..ad99bd7e 100755
> --- a/tests/generic/088
> +++ b/tests/generic/088
> @@ -29,6 +29,7 @@ _filter()
> # real QA test starts here
> _supported_fs generic
> _require_test
> +_require_chown
>
> path=$TEST_DIR/t_access
> $here/src/t_access_root $path | tee $seqres.full | _filter
> diff --git a/tests/generic/125 b/tests/generic/125
> index e84248d3..8c8f5cd7 100755
> --- a/tests/generic/125
> +++ b/tests/generic/125
> @@ -25,6 +25,7 @@ _supported_fs generic
> _require_test
> _require_user
> _require_odirect
> +_require_chmod
>
> TESTDIR=$TEST_DIR/ftrunc
> TESTFILE=$TESTDIR/ftrunc.tmp
> diff --git a/tests/generic/126 b/tests/generic/126
> index ac25d294..636ca00d 100755
> --- a/tests/generic/126
> +++ b/tests/generic/126
> @@ -27,6 +27,7 @@ _cleanup()
> # real QA test starts here
> _supported_fs generic
> _require_test
> +_require_chown
>
> QA_FS_PERMS=$here/src/fs_perms
>
> diff --git a/tests/generic/128 b/tests/generic/128
> index b3e49eff..c1eae77a 100755
> --- a/tests/generic/128
> +++ b/tests/generic/128
> @@ -24,6 +24,7 @@ _supported_fs generic
>
> _require_scratch
> _require_user
> +_require_chmod
>
> _scratch_mkfs >/dev/null 2>&1
> _scratch_mount "-o nosuid"
> diff --git a/tests/generic/193 b/tests/generic/193
> index 3125efdd..fd0ebbf6 100755
> --- a/tests/generic/193
> +++ b/tests/generic/193
> @@ -56,6 +56,7 @@ _supported_fs generic
>
> _require_test
> _require_user
> +_require_chown
>
> test_root=$TEST_DIR/$seq.$$.root
> test_user=$TEST_DIR/$seq.$$.user
> diff --git a/tests/generic/314 b/tests/generic/314
> index 03df81ce..540f0feb 100755
> --- a/tests/generic/314
> +++ b/tests/generic/314
> @@ -29,6 +29,7 @@ _cleanup()
> _supported_fs generic
> _require_test
> _require_user
> +_require_chown
>
> rm -rf $TEST_DIR/$seq-dir
>
> diff --git a/tests/generic/317 b/tests/generic/317
> index 29c37a57..289dfabe 100755
> --- a/tests/generic/317
> +++ b/tests/generic/317
> @@ -45,6 +45,7 @@ _require_scratch
> _require_user
> _require_ugid_map
> _require_userns
> +_require_chown
> qa_user_id=`id -u $qa_user`
>
> _filter_output()
> diff --git a/tests/generic/355 b/tests/generic/355
> index 161dd042..74fba0f9 100755
> --- a/tests/generic/355
> +++ b/tests/generic/355
> @@ -32,6 +32,7 @@ _supported_fs generic
> _require_test
> _require_user
> _require_odirect
> +_require_chown
>
> testfile=$TEST_DIR/$seq.test
> rm -f $testfile
> diff --git a/tests/generic/597 b/tests/generic/597
> index ba769d73..f596406c 100755
> --- a/tests/generic/597
> +++ b/tests/generic/597
> @@ -43,6 +43,7 @@ _require_sysctl_variable fs.protected_hardlinks
> _require_user fsgqa2
> # Do this SECOND so that qa_user is fsgqa, and _user_do uses that account
> _require_user fsgqa
> +_require_symlink
>
> OWNER=fsgqa2
> OTHER=fsgqa
> diff --git a/tests/generic/598 b/tests/generic/598
> index 6b765275..230c3ac7 100755
> --- a/tests/generic/598
> +++ b/tests/generic/598
> @@ -43,6 +43,7 @@ _require_sysctl_variable fs.protected_fifos
> _require_user fsgqa2
> # Do this SECOND so that qa_user is fsgqa, and _user_do uses that account
> _require_user fsgqa
> +_require_chmod
>
> USER1=fsgqa2
> USER2=fsgqa
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}()
2021-03-31 0:25 ` Zorro Lang
@ 2021-03-31 7:17 ` Pavel Reichl
0 siblings, 0 replies; 15+ messages in thread
From: Pavel Reichl @ 2021-03-31 7:17 UTC (permalink / raw)
To: Zorro Lang; +Cc: Eryu Guan, fstests
On 3/31/21 2:25 AM, Zorro Lang wrote:
> On Wed, Mar 31, 2021 at 12:00:02AM +0200, Pavel Reichl wrote:
>> Add helper functions that ensure that test is only executed on file
>> systems that implement chown, chmod and symbolic links.
>>
>> +
>> +# check for a symbolic links support
>> +#
>> +_require_symlink()
>> +{
>> + if [ "$FSTYP" = "exfat" ]; then
>> + _notrun "symbolic links are not supported on $FSTYP"
>> + fi
>> +}
>
> There's _require_symlinks() in common/rc, I remember Eric used it for some
> exfat errors before. Does it work for you?
>
Hi,
Thanks for the info! Yes, it works. I will fix it in the next version.
I'll just wait for more comments a few days...I guess.
Thanks!
Bye.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}()
2021-03-30 22:00 ` [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}() Pavel Reichl
2021-03-31 0:25 ` Zorro Lang
@ 2021-04-01 3:38 ` Eryu Guan
2021-04-01 9:47 ` Pavel Reichl
1 sibling, 1 reply; 15+ messages in thread
From: Eryu Guan @ 2021-04-01 3:38 UTC (permalink / raw)
To: Pavel Reichl; +Cc: fstests, zlang, guan
On Wed, Mar 31, 2021 at 12:00:02AM +0200, Pavel Reichl wrote:
> Add helper functions that ensure that test is only executed on file
> systems that implement chown, chmod and symbolic links.
>
> Fixed test are: generic/{87,88,125,126,128,193,314,317,355,597,598}
>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
> common/rc | 27 +++++++++++++++++++++++++++
> tests/generic/087 | 1 +
> tests/generic/088 | 1 +
> tests/generic/125 | 1 +
> tests/generic/126 | 1 +
> tests/generic/128 | 1 +
> tests/generic/193 | 1 +
> tests/generic/314 | 1 +
> tests/generic/317 | 1 +
> tests/generic/355 | 1 +
> tests/generic/597 | 1 +
> tests/generic/598 | 1 +
> 12 files changed, 38 insertions(+)
>
> diff --git a/common/rc b/common/rc
> index 0ce3cb0d..9cdfe21c 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2129,6 +2129,33 @@ _require_user()
> [ "$?" == "0" ] || _notrun "$qa_user cannot execute commands."
> }
>
> +# check for a chown support
> +#
> +_require_chown()
> +{
> + if [ "$FSTYP" = "exfat" ]; then
> + _notrun "chown is not supported on $FSTYP"
> + fi
> +}
> +
> +# check for a chmod support
> +#
> +_require_chmod()
> +{
> + if [ "$FSTYP" = "exfat" ]; then
> + _notrun "chmod is not supported on $FSTYP"
> + fi
> +}
> +
Does chown/chmod fail on exfat? Like the existing _require_symlink()
implementation and many other _require rules, we try to actually do the
action on $TEST_DIR, and check if command succeeds to see if the action
is supported by current $FSTYP. Is it possible for exfat to do the same
check?
We only use whitelist if it's impossible to do such check.
Thanks,
Eryu
> +# check for a symbolic links support
> +#
> +_require_symlink()
> +{
> + if [ "$FSTYP" = "exfat" ]; then
> + _notrun "symbolic links are not supported on $FSTYP"
> + fi
> +}
> +
> # check for a group on the machine, fsgqa as default
> #
> _require_group()
> diff --git a/tests/generic/087 b/tests/generic/087
> index 1f30dbf4..c3576117 100755
> --- a/tests/generic/087
> +++ b/tests/generic/087
> @@ -37,6 +37,7 @@ _cleanup()
> # real QA test starts here
> _supported_fs generic
> _require_test
> +_require_chown
>
> QA_FS_PERMS=$here/src/fs_perms
>
> diff --git a/tests/generic/088 b/tests/generic/088
> index 9388a083..ad99bd7e 100755
> --- a/tests/generic/088
> +++ b/tests/generic/088
> @@ -29,6 +29,7 @@ _filter()
> # real QA test starts here
> _supported_fs generic
> _require_test
> +_require_chown
>
> path=$TEST_DIR/t_access
> $here/src/t_access_root $path | tee $seqres.full | _filter
> diff --git a/tests/generic/125 b/tests/generic/125
> index e84248d3..8c8f5cd7 100755
> --- a/tests/generic/125
> +++ b/tests/generic/125
> @@ -25,6 +25,7 @@ _supported_fs generic
> _require_test
> _require_user
> _require_odirect
> +_require_chmod
>
> TESTDIR=$TEST_DIR/ftrunc
> TESTFILE=$TESTDIR/ftrunc.tmp
> diff --git a/tests/generic/126 b/tests/generic/126
> index ac25d294..636ca00d 100755
> --- a/tests/generic/126
> +++ b/tests/generic/126
> @@ -27,6 +27,7 @@ _cleanup()
> # real QA test starts here
> _supported_fs generic
> _require_test
> +_require_chown
>
> QA_FS_PERMS=$here/src/fs_perms
>
> diff --git a/tests/generic/128 b/tests/generic/128
> index b3e49eff..c1eae77a 100755
> --- a/tests/generic/128
> +++ b/tests/generic/128
> @@ -24,6 +24,7 @@ _supported_fs generic
>
> _require_scratch
> _require_user
> +_require_chmod
>
> _scratch_mkfs >/dev/null 2>&1
> _scratch_mount "-o nosuid"
> diff --git a/tests/generic/193 b/tests/generic/193
> index 3125efdd..fd0ebbf6 100755
> --- a/tests/generic/193
> +++ b/tests/generic/193
> @@ -56,6 +56,7 @@ _supported_fs generic
>
> _require_test
> _require_user
> +_require_chown
>
> test_root=$TEST_DIR/$seq.$$.root
> test_user=$TEST_DIR/$seq.$$.user
> diff --git a/tests/generic/314 b/tests/generic/314
> index 03df81ce..540f0feb 100755
> --- a/tests/generic/314
> +++ b/tests/generic/314
> @@ -29,6 +29,7 @@ _cleanup()
> _supported_fs generic
> _require_test
> _require_user
> +_require_chown
>
> rm -rf $TEST_DIR/$seq-dir
>
> diff --git a/tests/generic/317 b/tests/generic/317
> index 29c37a57..289dfabe 100755
> --- a/tests/generic/317
> +++ b/tests/generic/317
> @@ -45,6 +45,7 @@ _require_scratch
> _require_user
> _require_ugid_map
> _require_userns
> +_require_chown
> qa_user_id=`id -u $qa_user`
>
> _filter_output()
> diff --git a/tests/generic/355 b/tests/generic/355
> index 161dd042..74fba0f9 100755
> --- a/tests/generic/355
> +++ b/tests/generic/355
> @@ -32,6 +32,7 @@ _supported_fs generic
> _require_test
> _require_user
> _require_odirect
> +_require_chown
>
> testfile=$TEST_DIR/$seq.test
> rm -f $testfile
> diff --git a/tests/generic/597 b/tests/generic/597
> index ba769d73..f596406c 100755
> --- a/tests/generic/597
> +++ b/tests/generic/597
> @@ -43,6 +43,7 @@ _require_sysctl_variable fs.protected_hardlinks
> _require_user fsgqa2
> # Do this SECOND so that qa_user is fsgqa, and _user_do uses that account
> _require_user fsgqa
> +_require_symlink
>
> OWNER=fsgqa2
> OTHER=fsgqa
> diff --git a/tests/generic/598 b/tests/generic/598
> index 6b765275..230c3ac7 100755
> --- a/tests/generic/598
> +++ b/tests/generic/598
> @@ -43,6 +43,7 @@ _require_sysctl_variable fs.protected_fifos
> _require_user fsgqa2
> # Do this SECOND so that qa_user is fsgqa, and _user_do uses that account
> _require_user fsgqa
> +_require_chmod
>
> USER1=fsgqa2
> USER2=fsgqa
> --
> 2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/4] generic/554: hide permision warning on exfat
2021-03-30 22:00 ` [PATCH v2 3/4] generic/554: hide permision warning on exfat Pavel Reichl
@ 2021-04-01 3:40 ` Eryu Guan
0 siblings, 0 replies; 15+ messages in thread
From: Eryu Guan @ 2021-04-01 3:40 UTC (permalink / raw)
To: Pavel Reichl; +Cc: fstests, zlang, guan
On Wed, Mar 31, 2021 at 12:00:04AM +0200, Pavel Reichl wrote:
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
> common/rc | 13 +++++++++++++
> tests/generic/554 | 3 ++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index 2d658711..379140ea 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2401,6 +2401,19 @@ _format_swapfile() {
> fi
> }
>
> +_format_swapon_file() {
It's not a "format" action anymore, the _format prefix seems misleading.
I think _swapon_file seems fine.
Thanks,
Eryu
> + local fname="$1"
> +
> + if [ "$FSTYP" = "exfat" ]; then
> + # exfat does not support posix file permisions, so warning is
> + # to be expected
> + swapon "$fname" |& grep -v "insecure permissions"
> + else
> + swapon "$fname"
> + fi
> +
> +}
> +
> # Check that the filesystem supports swapfiles
> _require_scratch_swapfile()
> {
> diff --git a/tests/generic/554 b/tests/generic/554
> index fa4f97d2..c2f9eee8 100755
> --- a/tests/generic/554
> +++ b/tests/generic/554
> @@ -46,7 +46,8 @@ $XFS_IO_PROG -f -c "pwrite -S 0x61 0 128k" $SCRATCH_MNT/file >> $seqres.full 2>&
>
> echo swap files return ETXTBUSY
> _format_swapfile $SCRATCH_MNT/swapfile 16m
> -swapon $SCRATCH_MNT/swapfile
> +_format_swapon_file $SCRATCH_MNT/swapfile
> +
> $XFS_IO_PROG -f -c "copy_range -l 32k $SCRATCH_MNT/file" $SCRATCH_MNT/swapfile
> swapoff $SCRATCH_MNT/swapfile
>
> --
> 2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}()
2021-04-01 3:38 ` Eryu Guan
@ 2021-04-01 9:47 ` Pavel Reichl
2021-04-07 16:47 ` Eric Sandeen
0 siblings, 1 reply; 15+ messages in thread
From: Pavel Reichl @ 2021-04-01 9:47 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests, zlang, guan, Eric Sandeen, Dave Chinner
On 4/1/21 5:38 AM, Eryu Guan wrote:
> On Wed, Mar 31, 2021 at 12:00:02AM +0200, Pavel Reichl wrote:
>> Add helper functions that ensure that test is only executed on file
>> systems that implement chown, chmod and symbolic links.
>>
>> Fixed test are: generic/{87,88,125,126,128,193,314,317,355,597,598}
>>
>> Signed-off-by: Pavel Reichl <preichl@redhat.com>
>> ---
>> common/rc | 27 +++++++++++++++++++++++++++
>> tests/generic/087 | 1 +
>> tests/generic/088 | 1 +
>> tests/generic/125 | 1 +
>> tests/generic/126 | 1 +
>> tests/generic/128 | 1 +
>> tests/generic/193 | 1 +
>> tests/generic/314 | 1 +
>> tests/generic/317 | 1 +
>> tests/generic/355 | 1 +
>> tests/generic/597 | 1 +
>> tests/generic/598 | 1 +
>> 12 files changed, 38 insertions(+)
>>
>> diff --git a/common/rc b/common/rc
>> index 0ce3cb0d..9cdfe21c 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2129,6 +2129,33 @@ _require_user()
>> [ "$?" == "0" ] || _notrun "$qa_user cannot execute commands."
>> }
>>
>> +# check for a chown support
>> +#
>> +_require_chown()
>> +{
>> + if [ "$FSTYP" = "exfat" ]; then
>> + _notrun "chown is not supported on $FSTYP"
>> + fi
>> +}
>> +
>> +# check for a chmod support
>> +#
>> +_require_chmod()
>> +{
>> + if [ "$FSTYP" = "exfat" ]; then
>> + _notrun "chmod is not supported on $FSTYP"
>> + fi
>> +}
>> +
>
> Does chown/chmod fail on exfat? Like the existing _require_symlink()
> implementation and many other _require rules, we try to actually do the
> action on $TEST_DIR, and check if command succeeds to see if the action
> is supported by current $FSTYP. Is it possible for exfat to do the same
> check?
>
> We only use whitelist if it's impossible to do such check.
>
> Thanks,
> Eryu
>
Hi,
it does fail. It was actually my original intention to write the _require*() so it would check if the command succeeds as you are suggesting.
However, Eric and Dave were worried that adding more _require*() through the tests would lead to further slowing test execution. This worry actually makes sense to me.
Is there a significant benefit of testing the support vs. maintaining check based on FSTYP variable? I guess doing the check saves us from the need to update the code when new file-system is added, however actually doing the check increases time of test execution (but I haven't done any measurements yet - it's just my guess).
I really don't mind doing it either way and I'm happy to change the code - I'm just trying to explain :-)
Thanks for the comment.
Have a nice day.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}()
2021-04-01 9:47 ` Pavel Reichl
@ 2021-04-07 16:47 ` Eric Sandeen
0 siblings, 0 replies; 15+ messages in thread
From: Eric Sandeen @ 2021-04-07 16:47 UTC (permalink / raw)
To: Pavel Reichl, Eryu Guan; +Cc: fstests, zlang, guan, Eric Sandeen, Dave Chinner
On 4/1/21 4:47 AM, Pavel Reichl wrote:
>> We only use whitelist if it's impossible to do such check.
>>
>> Thanks,
>> Eryu
>>
> Hi,
>
> it does fail. It was actually my original intention to write the _require*() so it would check if the command succeeds as you are suggesting.
>
> However, Eric and Dave were worried that adding more _require*() through the tests would lead to further slowing test execution. This worry actually makes sense to me.
>
> Is there a significant benefit of testing the support vs. maintaining check based on FSTYP variable? I guess doing the check saves us from the need to update the code when new file-system is added, however actually doing the check increases time of test execution (but I haven't done any measurements yet - it's just my guess).
>
> I really don't mind doing it either way and I'm happy to change the code - I'm just trying to explain :-)
that's my fault, sorry. Dave had expressed some concern about exfat changes slowing down testing for every other fs, and we talked about a whitelist. But now that I think about it, I'm not sure this functiona test takes any real extra time.
Pavel, maybe you can just evaluate whether there really is any significant time difference, and if there is not, go back to the functional test.
Sorry for the hassle.
-Eric
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] common: hide permision warning from mkswap for exfat
2021-03-30 22:00 ` [PATCH v2 2/4] common: hide permision warning from mkswap for exfat Pavel Reichl
@ 2021-04-07 17:03 ` Eric Sandeen
2021-04-15 9:15 ` Pavel Reichl
0 siblings, 1 reply; 15+ messages in thread
From: Eric Sandeen @ 2021-04-07 17:03 UTC (permalink / raw)
To: Pavel Reichl, fstests; +Cc: zlang, guan
On 3/30/21 5:00 PM, Pavel Reichl wrote:
> exfat does not support posix file permisions, so warning from mkswap is
> inavitable. This patch hides the warning message so the test won't fail.
>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
hah, I never even thought about swap on exfat ;)
Assuming it actually works other than this warning, maybe you don't need to
special case it for exfat? If so just filter that warning for everyone (in the
same way that the CHATTR isn't special cased for btrfs; we just ignore
failures there, because they don't matter).
If there's a real need to special case exfat that's fine, but it might
be better to not litter special cases around if they aren't material
to the test.
also, maybe there is some risk to 2>&1 >> seqres "eating" any other
potential errors? Or maybe my bash fu is weak. ;)
I /think/ this will do the trick but give it a shot. It should filter only
the one string from stderr, and send all of stdout to $seqres.full
# Ignore permission complaints on filesystems that don't support perms
$MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full
-Eric
> ---
> common/rc | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index 9cdfe21c..2d658711 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2392,7 +2392,13 @@ _format_swapfile() {
> # Swap files must be nocow on Btrfs.
> $CHATTR_PROG +C "$fname" > /dev/null 2>&1
> _pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full
> - $MKSWAP_PROG "$fname" >> $seqres.full
> + if [ "$FSTYP" = "exfat" ]; then
> + # exfat does not support posix file permisions, so warning is
> + # to be expected
> + $MKSWAP_PROG "$fname" 2>&1 | grep -v 'insecure permission' >> $seqres.full
> + else
> + $MKSWAP_PROG "$fname" >> $seqres.full
> + fi
> }
>
> # Check that the filesystem supports swapfiles
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/4] generic/003: Amend the test for exfat
2021-03-30 22:00 ` [PATCH v2 4/4] generic/003: Amend the test for exfat Pavel Reichl
@ 2021-04-07 18:03 ` Eric Sandeen
2021-04-15 9:14 ` Pavel Reichl
0 siblings, 1 reply; 15+ messages in thread
From: Eric Sandeen @ 2021-04-07 18:03 UTC (permalink / raw)
To: Pavel Reichl, fstests; +Cc: zlang, guan
On 3/30/21 5:00 PM, Pavel Reichl wrote:
> Update the test so it can be run even for exfat which has 2 seconds
> granularity for access_time and does not have a timestamp for
> metadata change.
>
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
This looks a whole lot like what I was playing with earlier. :)
Can you run it say 20 times in a row, and make sure that you don't get
spurious failures by sleeping for /exactly/ the atime granularity?
Thanks,
-Eric
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/4] generic/003: Amend the test for exfat
2021-04-07 18:03 ` Eric Sandeen
@ 2021-04-15 9:14 ` Pavel Reichl
0 siblings, 0 replies; 15+ messages in thread
From: Pavel Reichl @ 2021-04-15 9:14 UTC (permalink / raw)
To: Eric Sandeen, fstests; +Cc: zlang, guan
On 4/7/21 8:03 PM, Eric Sandeen wrote:
> On 3/30/21 5:00 PM, Pavel Reichl wrote:
>> Update the test so it can be run even for exfat which has 2 seconds
>> granularity for access_time and does not have a timestamp for
>> metadata change.
>>
>> Signed-off-by: Pavel Reichl <preichl@redhat.com>
>
> This looks a whole lot like what I was playing with earlier. :)
good :-)
>
> Can you run it say 20 times in a row, and make sure that you don't get
> spurious failures by sleeping for /exactly/ the atime granularity?
I tried that for tens of times and no failure. However, since sleep() does not require integer as the parameter, in the next revision I'll propose something as a 2.1 instead of the 2 to be on the safe side, OK?
>
> Thanks,
> -Eric
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] common: hide permision warning from mkswap for exfat
2021-04-07 17:03 ` Eric Sandeen
@ 2021-04-15 9:15 ` Pavel Reichl
0 siblings, 0 replies; 15+ messages in thread
From: Pavel Reichl @ 2021-04-15 9:15 UTC (permalink / raw)
To: Eric Sandeen, fstests; +Cc: zlang, guan
On 4/7/21 7:03 PM, Eric Sandeen wrote:
> On 3/30/21 5:00 PM, Pavel Reichl wrote:
>> exfat does not support posix file permisions, so warning from mkswap is
>> inavitable. This patch hides the warning message so the test won't fail.
>>
>> Signed-off-by: Pavel Reichl <preichl@redhat.com>
>
> hah, I never even thought about swap on exfat ;)
>
> Assuming it actually works other than this warning, maybe you don't need to
> special case it for exfat? If so just filter that warning for everyone (in the
> same way that the CHATTR isn't special cased for btrfs; we just ignore
> failures there, because they don't matter).
>
> If there's a real need to special case exfat that's fine, but it might
> be better to not litter special cases around if they aren't material
> to the test.
>
> also, maybe there is some risk to 2>&1 >> seqres "eating" any other
> potential errors? Or maybe my bash fu is weak. ;)
>
> I /think/ this will do the trick but give it a shot. It should filter only
> the one string from stderr, and send all of stdout to $seqres.full
>
> # Ignore permission complaints on filesystems that don't support perms
> $MKSWAP_PROG "$fname" 2> >(grep -v 'insecure permission' >&2) >> $seqres.full
Works nice, thanks for the hint.
I also applied these comments on patch #3.
>
> -Eric
>
>> ---
>> common/rc | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/rc b/common/rc
>> index 9cdfe21c..2d658711 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2392,7 +2392,13 @@ _format_swapfile() {
>> # Swap files must be nocow on Btrfs.
>> $CHATTR_PROG +C "$fname" > /dev/null 2>&1
>> _pwrite_byte 0x61 0 "$sz" "$fname" >> $seqres.full
>> - $MKSWAP_PROG "$fname" >> $seqres.full
>> + if [ "$FSTYP" = "exfat" ]; then
>> + # exfat does not support posix file permisions, so warning is
>> + # to be expected
>> + $MKSWAP_PROG "$fname" 2>&1 | grep -v 'insecure permission' >> $seqres.full
>> + else
>> + $MKSWAP_PROG "$fname" >> $seqres.full
>> + fi
>> }
>>
>> # Check that the filesystem supports swapfiles
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-04-15 9:15 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-30 22:00 [PATCH v2 0/4] Fix some tests that fail for exfat FS Pavel Reichl
2021-03-30 22:00 ` [PATCH v2 1/4] common/rc: Add _require_{chown,chmod,symlink}() Pavel Reichl
2021-03-31 0:25 ` Zorro Lang
2021-03-31 7:17 ` Pavel Reichl
2021-04-01 3:38 ` Eryu Guan
2021-04-01 9:47 ` Pavel Reichl
2021-04-07 16:47 ` Eric Sandeen
2021-03-30 22:00 ` [PATCH v2 2/4] common: hide permision warning from mkswap for exfat Pavel Reichl
2021-04-07 17:03 ` Eric Sandeen
2021-04-15 9:15 ` Pavel Reichl
2021-03-30 22:00 ` [PATCH v2 3/4] generic/554: hide permision warning on exfat Pavel Reichl
2021-04-01 3:40 ` Eryu Guan
2021-03-30 22:00 ` [PATCH v2 4/4] generic/003: Amend the test for exfat Pavel Reichl
2021-04-07 18:03 ` Eric Sandeen
2021-04-15 9:14 ` Pavel Reichl
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.