[PATCH 0/2] Fix failing overlay nonsamefs fstests

[PATCH 0/2] Fix failing overlay nonsamefs fstests

[Amir Goldstein]

Eryu,

The modified 3 tests were introduced to track a non-standard
behavior of overlayfs w.r.t. inode numbers with nonsamefs
configuration.

This behavior cannot be fixed with all underlying filesystems
unless user opts-in for the xino feature.

xino feature was introduced in kernel v4.17-rc1. This patch
set converts the tests to notrun instead of failing on old kernels
and opts-in for xino with new kernels.

FYI, Miklos has a patch set in review to fix the last of the
non-standard behavior tracking test that is still failing (overlay/016).
After that patch set is merged, all overlay tests will be expected to
pass on master - new era ;-)

Thanks,
Amir.

Amir Goldstein (2):
  common/rc: factor out _require_scratch_fs_option
  overlay/04{1,3,4}: enable xino feature

 common/rc         | 29 ++++++++++++++++++++++-------
 tests/overlay/041 | 15 +++++++++++----
 tests/overlay/043 | 13 ++++++++++---
 tests/overlay/044 | 15 +++++++++++----
 4 files changed, 54 insertions(+), 18 deletions(-)

-- 
2.7.4

[PATCH 1/2] common/rc: factor out _require_scratch_fs_option

[Amir Goldstein]

Factor out _require_scratch_fs_option and _check_scratch_fs_option
from the helper _require_scratch_dax.
Those are generic helpers to test if mount option is supported and
if mount option was enabled.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 common/rc | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/common/rc b/common/rc
index 622afd6..23c3978 100644
--- a/common/rc
+++ b/common/rc
@@ -3036,20 +3036,35 @@ _require_scratch_shutdown()
 	_scratch_unmount
 }
 
-# Does dax mount option work on this dev/fs?
-_require_scratch_dax()
+_check_scratch_fs_option()
+{
+	local opt=$1
+
+	_fs_options $SCRATCH_DEV | grep -qw "$opt"
+}
+
+# Does mount option work on this dev/fs?
+_require_scratch_fs_option()
 {
+	local opt=$1
+
 	_require_scratch
 	_scratch_mkfs > /dev/null 2>&1
-	_try_scratch_mount -o dax || \
-		_notrun "mount $SCRATCH_DEV with dax failed"
-	# Check options to be sure. XFS ignores dax option
+	_try_scratch_mount -o "$opt" || \
+		_notrun "mount $SCRATCH_DEV with $opt failed"
+	# Check options to be sure. For example, XFS ignores dax option
 	# and goes on if dev underneath does not support dax.
-	_fs_options $SCRATCH_DEV | grep -qw "dax" || \
-		_notrun "$SCRATCH_DEV $FSTYP does not support -o dax"
+	_check_scratch_fs_option "$opt" || \
+		_notrun "$SCRATCH_DEV $FSTYP does not support -o $opt"
 	_scratch_unmount
 }
 
+# Does dax mount option work on this dev/fs?
+_require_scratch_dax()
+{
+	_require_scratch_fs_option "dax"
+}
+
 # Does norecovery support by this fs?
 _require_norecovery()
 {
-- 
2.7.4

[PATCH 2/2] overlay/04{1,3,4}: enable xino feature

[Amir Goldstein]

These tests check for constant inode number on copy up with nonsamefs
layer configuration. This problem is fixes only when opting-in with the
xino=on mount option, so let the tests enable the mount option on new
kernels and notrun on old kernels.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/041 | 15 +++++++++++----
 tests/overlay/043 | 13 ++++++++++---
 tests/overlay/044 | 15 +++++++++++----
 3 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/tests/overlay/041 b/tests/overlay/041
index 4e72348..9de61f7 100755
--- a/tests/overlay/041
+++ b/tests/overlay/041
@@ -70,7 +70,14 @@ _scratch_mkfs
 upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
 workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
 
-_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
+# enabling xino in this test requires that base filesystem inode numbers will
+# not use bit 63 in inode number of the test files, because bit 63 is used by
+# overlayfs to indicate the layer. Let's just assume that this is true for all
+# tested filesystems and if we are wrong, the test may fail
+_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o xino=on ||
+	_notrun "mount overlay with xino failed"
+_check_scratch_fs_option "xino" || \
+	_notrun "faild to enable xino"
 
 test_dir=$SCRATCH_MNT/test_dir/
 
@@ -171,7 +178,7 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
 _scratch_unmount
 
 # check overlayfs
-_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o xino=on
 
 # Verify pure lower residing in dir which has another lower layer
 middir=$OVL_BASE_TEST_DIR/$seq-ovl-mid
@@ -189,7 +196,7 @@ _scratch_mkfs
 upperdir=$OVL_BASE_SCRATCH_MNT/ovl-upper
 workdir=$OVL_BASE_SCRATCH_MNT/ovl-work
 
-_overlay_scratch_mount_dirs $middir:$lowerdir $upperdir $workdir
+_overlay_scratch_mount_dirs $middir:$lowerdir $upperdir $workdir -o xino=on
 
 # Copy up test_dir
 touch $test_dir/test_file
@@ -212,7 +219,7 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
 	echo "Pure lower in dir which has another lower layer: Invalid d_ino reported for subdir"
 
 # check overlayfs
-_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir
+_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir -o xino=on
 
 echo "Silence is golden"
 status=0
diff --git a/tests/overlay/043 b/tests/overlay/043
index 46df686..66771c0 100755
--- a/tests/overlay/043
+++ b/tests/overlay/043
@@ -83,7 +83,14 @@ _scratch_mkfs >>$seqres.full 2>&1
 upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
 workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
 
-_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
+# enabling xino in this test requires that base filesystem inode numbers will
+# not use bit 63 in inode number of the test files, because bit 63 is used by
+# overlayfs to indicate the layer. Let's just assume that this is true for all
+# tested filesystems and if we are wrong, the test may fail
+_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o xino=on || \
+	_notrun "mount overlay with xino failed"
+_check_scratch_fs_option "xino" || \
+	_notrun "faild to enable xino"
 
 FILES="dir file symlink chrdev blkdev fifo socket"
 
@@ -150,13 +157,13 @@ check_inode_numbers $testdir $tmp.after_copyup $tmp.after_move
 
 # Verify that the inode numbers survive a mount cycle
 $UMOUNT_PROG $SCRATCH_MNT
-_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
+_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o xino=on
 
 # Compare inode numbers before/after mount cycle
 check_inode_numbers $testdir $tmp.after_move $tmp.after_cycle
 
 # check overlayfs
-_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o xino=on
 
 echo "Silence is golden"
 status=0
diff --git a/tests/overlay/044 b/tests/overlay/044
index 2ab3035..0e74128 100755
--- a/tests/overlay/044
+++ b/tests/overlay/044
@@ -99,8 +99,15 @@ _scratch_mkfs >>$seqres.full 2>&1
 upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
 workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
 
-# Enable overlay index feature to prevent breaking hardlinks on copy up
-_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on
+# Enable overlay index feature to prevent breaking hardlinks on copy up.
+# enabling xino in this test requires that base filesystem inode numbers will
+# not use bit 63 in inode number of the test files, because bit 63 is used by
+# overlayfs to indicate the layer. Let's just assume that this is true for all
+# tested filesystems and if we are wrong, the test may fail
+_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on,xino=on || \
+	_notrun "mount overlay with xino failed"
+_check_scratch_fs_option "xino" || \
+	_notrun "faild to enable xino"
 
 rm -f $tmp.*
 
@@ -124,8 +131,8 @@ check_ino_nlink $SCRATCH_MNT $tmp.before $tmp.after_one
 
 # Verify that the hardlinks survive a mount cycle
 $UMOUNT_PROG $SCRATCH_MNT
-_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o index=on
-_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on
+_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o index=on,xino=on
+_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on,xino=on
 
 echo "== After mount cycle =="
 cat $FILES
-- 
2.7.4

Re: [PATCH 2/2] overlay/04{1,3,4}: enable xino feature

[Eryu Guan]

On Wed, Apr 18, 2018 at 05:16:58PM +0300, Amir Goldstein wrote:
> These tests check for constant inode number on copy up with nonsamefs
> layer configuration. This problem is fixes only when opting-in with the
> xino=on mount option, so let the tests enable the mount option on new
> kernels and notrun on old kernels.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  tests/overlay/041 | 15 +++++++++++----
>  tests/overlay/043 | 13 ++++++++++---
>  tests/overlay/044 | 15 +++++++++++----
>  3 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/tests/overlay/041 b/tests/overlay/041
> index 4e72348..9de61f7 100755
> --- a/tests/overlay/041
> +++ b/tests/overlay/041
> @@ -70,7 +70,14 @@ _scratch_mkfs
>  upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
>  workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
>  
> -_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
> +# enabling xino in this test requires that base filesystem inode numbers will
> +# not use bit 63 in inode number of the test files, because bit 63 is used by
> +# overlayfs to indicate the layer. Let's just assume that this is true for all
> +# tested filesystems and if we are wrong, the test may fail
> +_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o xino=on ||
> +	_notrun "mount overlay with xino failed"
> +_check_scratch_fs_option "xino" || \
> +	_notrun "faild to enable xino"
>  
>  test_dir=$SCRATCH_MNT/test_dir/
>  
> @@ -171,7 +178,7 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
>  _scratch_unmount
>  
>  # check overlayfs
> -_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
> +_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o xino=on
>  
>  # Verify pure lower residing in dir which has another lower layer
>  middir=$OVL_BASE_TEST_DIR/$seq-ovl-mid
> @@ -189,7 +196,7 @@ _scratch_mkfs
>  upperdir=$OVL_BASE_SCRATCH_MNT/ovl-upper
>  workdir=$OVL_BASE_SCRATCH_MNT/ovl-work
>  
> -_overlay_scratch_mount_dirs $middir:$lowerdir $upperdir $workdir
> +_overlay_scratch_mount_dirs $middir:$lowerdir $upperdir $workdir -o xino=on
>  
>  # Copy up test_dir
>  touch $test_dir/test_file
> @@ -212,7 +219,7 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
>  	echo "Pure lower in dir which has another lower layer: Invalid d_ino reported for subdir"
>  
>  # check overlayfs
> -_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir
> +_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir -o xino=on

041 fails the fsck.overlay check, but I think that's a different issue.

*** fsck.overlay output ***
fsck.overlay:[Error]: Faile to resolve upperdir:/mnt/scratch/ovl-upper:No such file or directory
Please specify correct lowerdirs and upperdir!

Usage:
        fsck.overlay [-o lowerdir=<lowers>,upperdir=<upper>,workdir=<work>] [-pnyvhV]

Options:
-o,                       specify underlying directories of overlayfs
                          multiple lower directories use ':' as separator
-p,                       automatic repair (no questions)
-n,                       make no changes to the filesystem
-y,                       assume "yes" to all questions
-v, --verbose             print more messages of overlayfs
-h, --help                display this usage of overlayfs
-V, --version             display version information
*** end fsck.overlay output

>  
>  echo "Silence is golden"
>  status=0
> diff --git a/tests/overlay/043 b/tests/overlay/043
> index 46df686..66771c0 100755
> --- a/tests/overlay/043
> +++ b/tests/overlay/043
> @@ -83,7 +83,14 @@ _scratch_mkfs >>$seqres.full 2>&1
>  upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
>  workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
>  
> -_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
> +# enabling xino in this test requires that base filesystem inode numbers will
> +# not use bit 63 in inode number of the test files, because bit 63 is used by
> +# overlayfs to indicate the layer. Let's just assume that this is true for all
> +# tested filesystems and if we are wrong, the test may fail
> +_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o xino=on || \
> +	_notrun "mount overlay with xino failed"
> +_check_scratch_fs_option "xino" || \
> +	_notrun "faild to enable xino"

043 still fails for me, kernel is v4.17-rc2

[root@fedoravm xfstests]# diff -u tests/overlay/043.out /root/workspace/xfstests/results//xfs_4k/overlay/043.out.bad
--- tests/overlay/043.out       2018-02-25 15:15:00.135387405 +0800
+++ /root/workspace/xfstests/results//xfs_4k/overlay/043.out.bad        2018-04-25 13:44:24.378262790 +0800
@@ -1,2 +1,11 @@
 QA output created by 043
+--- /tmp/29215.after_copyup    2018-04-25 13:44:23.888271267 +0800
++++ /tmp/29215.after_move      2018-04-25 13:44:24.115267339 +0800
+@@ -1,4 +1,4 @@
+-9223372036888371331 dir
++50333121 dir
+ 9223372036871553155 file
+ 9223372036871553156 symlink
+ 9223372036871553157 chrdev
+dir not found by ino 9223372036888371331 (from /tmp/29215.after_copyup)
 Silence is golden

I used ext4 as underlying filesystem, but xfs failed too.

>  
>  FILES="dir file symlink chrdev blkdev fifo socket"
>  
> @@ -150,13 +157,13 @@ check_inode_numbers $testdir $tmp.after_copyup $tmp.after_move
>  
>  # Verify that the inode numbers survive a mount cycle
>  $UMOUNT_PROG $SCRATCH_MNT
> -_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
> +_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o xino=on
>  
>  # Compare inode numbers before/after mount cycle
>  check_inode_numbers $testdir $tmp.after_move $tmp.after_cycle
>  
>  # check overlayfs
> -_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
> +_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o xino=on
>  
>  echo "Silence is golden"
>  status=0
> diff --git a/tests/overlay/044 b/tests/overlay/044
> index 2ab3035..0e74128 100755
> --- a/tests/overlay/044
> +++ b/tests/overlay/044
> @@ -99,8 +99,15 @@ _scratch_mkfs >>$seqres.full 2>&1
>  upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
>  workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
>  
> -# Enable overlay index feature to prevent breaking hardlinks on copy up
> -_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on
> +# Enable overlay index feature to prevent breaking hardlinks on copy up.
> +# enabling xino in this test requires that base filesystem inode numbers will
> +# not use bit 63 in inode number of the test files, because bit 63 is used by
> +# overlayfs to indicate the layer. Let's just assume that this is true for all
> +# tested filesystems and if we are wrong, the test may fail
> +_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on,xino=on || \
> +	_notrun "mount overlay with xino failed"
> +_check_scratch_fs_option "xino" || \
> +	_notrun "faild to enable xino"
>  
>  rm -f $tmp.*
>  
> @@ -124,8 +131,8 @@ check_ino_nlink $SCRATCH_MNT $tmp.before $tmp.after_one
>  
>  # Verify that the hardlinks survive a mount cycle
>  $UMOUNT_PROG $SCRATCH_MNT
> -_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o index=on
> -_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on
> +_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o index=on,xino=on
> +_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o index=on,xino=on

044 passes!

Thanks,
Eryu

>  
>  echo "== After mount cycle =="
>  cat $FILES
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] overlay/04{1,3,4}: enable xino feature

[Amir Goldstein]

On Tue, Apr 24, 2018 at 10:48 PM, Eryu Guan <guaneryu@gmail.com> wrote:
> On Wed, Apr 18, 2018 at 05:16:58PM +0300, Amir Goldstein wrote:
>> These tests check for constant inode number on copy up with nonsamefs
>> layer configuration. This problem is fixes only when opting-in with the
>> xino=on mount option, so let the tests enable the mount option on new
>> kernels and notrun on old kernels.
>>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  tests/overlay/041 | 15 +++++++++++----
>>  tests/overlay/043 | 13 ++++++++++---
>>  tests/overlay/044 | 15 +++++++++++----
>>  3 files changed, 32 insertions(+), 11 deletions(-)
>>
>> diff --git a/tests/overlay/041 b/tests/overlay/041
>> index 4e72348..9de61f7 100755
>> --- a/tests/overlay/041
>> +++ b/tests/overlay/041
>> @@ -70,7 +70,14 @@ _scratch_mkfs
>>  upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
>>  workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
>>
>> -_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
>> +# enabling xino in this test requires that base filesystem inode numbers will
>> +# not use bit 63 in inode number of the test files, because bit 63 is used by
>> +# overlayfs to indicate the layer. Let's just assume that this is true for all
>> +# tested filesystems and if we are wrong, the test may fail
>> +_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o xino=on ||
>> +     _notrun "mount overlay with xino failed"
>> +_check_scratch_fs_option "xino" || \
>> +     _notrun "faild to enable xino"
>>
>>  test_dir=$SCRATCH_MNT/test_dir/
>>
>> @@ -171,7 +178,7 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
>>  _scratch_unmount
>>
>>  # check overlayfs
>> -_overlay_check_scratch_dirs $lowerdir $upperdir $workdir
>> +_overlay_check_scratch_dirs $lowerdir $upperdir $workdir -o xino=on
>>
>>  # Verify pure lower residing in dir which has another lower layer
>>  middir=$OVL_BASE_TEST_DIR/$seq-ovl-mid
>> @@ -189,7 +196,7 @@ _scratch_mkfs
>>  upperdir=$OVL_BASE_SCRATCH_MNT/ovl-upper
>>  workdir=$OVL_BASE_SCRATCH_MNT/ovl-work
>>
>> -_overlay_scratch_mount_dirs $middir:$lowerdir $upperdir $workdir
>> +_overlay_scratch_mount_dirs $middir:$lowerdir $upperdir $workdir -o xino=on
>>
>>  # Copy up test_dir
>>  touch $test_dir/test_file
>> @@ -212,7 +219,7 @@ subdir_d=$($here/src/t_dir_type $pure_lower_dir $pure_lower_subdir_st_ino)
>>       echo "Pure lower in dir which has another lower layer: Invalid d_ino reported for subdir"
>>
>>  # check overlayfs
>> -_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir
>> +_overlay_check_scratch_dirs "$middir:$lowerdir" $upperdir $workdir -o xino=on
>
> 041 fails the fsck.overlay check, but I think that's a different issue.

Right. Will look into that.

>
> *** fsck.overlay output ***
> fsck.overlay:[Error]: Faile to resolve upperdir:/mnt/scratch/ovl-upper:No such file or directory
> Please specify correct lowerdirs and upperdir!
>
> Usage:
>         fsck.overlay [-o lowerdir=<lowers>,upperdir=<upper>,workdir=<work>] [-pnyvhV]
>
> Options:
> -o,                       specify underlying directories of overlayfs
>                           multiple lower directories use ':' as separator
> -p,                       automatic repair (no questions)
> -n,                       make no changes to the filesystem
> -y,                       assume "yes" to all questions
> -v, --verbose             print more messages of overlayfs
> -h, --help                display this usage of overlayfs
> -V, --version             display version information
> *** end fsck.overlay output
>
>>
>>  echo "Silence is golden"
>>  status=0
>> diff --git a/tests/overlay/043 b/tests/overlay/043
>> index 46df686..66771c0 100755
>> --- a/tests/overlay/043
>> +++ b/tests/overlay/043
>> @@ -83,7 +83,14 @@ _scratch_mkfs >>$seqres.full 2>&1
>>  upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
>>  workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
>>
>> -_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
>> +# enabling xino in this test requires that base filesystem inode numbers will
>> +# not use bit 63 in inode number of the test files, because bit 63 is used by
>> +# overlayfs to indicate the layer. Let's just assume that this is true for all
>> +# tested filesystems and if we are wrong, the test may fail
>> +_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o xino=on || \
>> +     _notrun "mount overlay with xino failed"
>> +_check_scratch_fs_option "xino" || \
>> +     _notrun "faild to enable xino"
>
> 043 still fails for me, kernel is v4.17-rc2
>
> [root@fedoravm xfstests]# diff -u tests/overlay/043.out /root/workspace/xfstests/results//xfs_4k/overlay/043.out.bad
> --- tests/overlay/043.out       2018-02-25 15:15:00.135387405 +0800
> +++ /root/workspace/xfstests/results//xfs_4k/overlay/043.out.bad        2018-04-25 13:44:24.378262790 +0800
> @@ -1,2 +1,11 @@
>  QA output created by 043
> +--- /tmp/29215.after_copyup    2018-04-25 13:44:23.888271267 +0800
> ++++ /tmp/29215.after_move      2018-04-25 13:44:24.115267339 +0800
> +@@ -1,4 +1,4 @@
> +-9223372036888371331 dir
> ++50333121 dir
> + 9223372036871553155 file
> + 9223372036871553156 symlink
> + 9223372036871553157 chrdev
> +dir not found by ino 9223372036888371331 (from /tmp/29215.after_copyup)
>  Silence is golden
>
> I used ext4 as underlying filesystem, but xfs failed too.


It's a test bug. missing _require_scratch_feature redirect_dir
Like was added to the original 017 test.
Will send a patch.

Thanks,
Amir.

Re: [PATCH 0/2] Fix failing overlay nonsamefs fstests

[Miklos Szeredi]

On Wed, Apr 18, 2018 at 4:16 PM, Amir Goldstein <amir73il@gmail.com> wrote:

> FYI, Miklos has a patch set in review to fix the last of the
> non-standard behavior tracking test that is still failing (overlay/016).
> After that patch set is merged, all overlay tests will be expected to
> pass on master - new era ;-)

Oh, we can write new tests that don't pass ;)

For example -ostrictatime mounts are not going to behave POSIX'ly (and
probably nobody cares, but still).  And we have that minor weirdness
with directory nlink being constant 1.   And probably some others.

Not that there's a hurry to test these... So yeah, having all tests
pass would be nice :)

Thanks,
Miklos

Re: [PATCH 0/2] Fix failing overlay nonsamefs fstests

[Amir Goldstein]

On Wed, Apr 18, 2018 at 5:23 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Wed, Apr 18, 2018 at 4:16 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>
>> FYI, Miklos has a patch set in review to fix the last of the
>> non-standard behavior tracking test that is still failing (overlay/016).
>> After that patch set is merged, all overlay tests will be expected to
>> pass on master - new era ;-)
>
> Oh, we can write new tests that don't pass ;)
>
> For example -ostrictatime mounts are not going to behave POSIX'ly (and
> probably nobody cares, but still).  And we have that minor weirdness
> with directory nlink being constant 1.   And probably some others.

FYI, ext4 also has directories with nlink 1 with the dir_nlink feature,
so no big deal.

Thanks,
Amir.

Re: [PATCH 0/2] Fix failing overlay nonsamefs fstests

[Miklos Szeredi]

On Wed, Apr 18, 2018 at 4:30 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, Apr 18, 2018 at 5:23 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Wed, Apr 18, 2018 at 4:16 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>
>>> FYI, Miklos has a patch set in review to fix the last of the
>>> non-standard behavior tracking test that is still failing (overlay/016).
>>> After that patch set is merged, all overlay tests will be expected to
>>> pass on master - new era ;-)
>>
>> Oh, we can write new tests that don't pass ;)

Another one:

fstat() returns incorrect st_ctime after unlink of a lower file/directory.

Alternative fixes:

1) take some attributes from whiteout

2) metacopy up to tmpfile

But first of all, we need a test case that fails, otherwise there's no
motivation to do the fix ;)

I'm wondering, would it make sense to manually add "rotation points"
to plain xfstest test cases?

Thanks,
Miklos

Re: [PATCH 0/2] Fix failing overlay nonsamefs fstests

[Amir Goldstein]

On Mon, Apr 23, 2018 at 6:37 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Wed, Apr 18, 2018 at 4:30 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> On Wed, Apr 18, 2018 at 5:23 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>> On Wed, Apr 18, 2018 at 4:16 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>>
>>>> FYI, Miklos has a patch set in review to fix the last of the
>>>> non-standard behavior tracking test that is still failing (overlay/016).
>>>> After that patch set is merged, all overlay tests will be expected to
>>>> pass on master - new era ;-)
>>>
>>> Oh, we can write new tests that don't pass ;)
>
> Another one:
>
> fstat() returns incorrect st_ctime after unlink of a lower file/directory.
>
> Alternative fixes:
>
> 1) take some attributes from whiteout
>
> 2) metacopy up to tmpfile
>
> But first of all, we need a test case that fails, otherwise there's no
> motivation to do the fix ;)
>

I admit that the failing xfstests I wrote have had a "motivating"
effect, but going forward, I feel that xfstests is not really the place
for these sort of weird behavior tests. I find it to be more suiting as
a regression tracking test suite and special test case coverage.

IMO, parent ctime fits better to a POSIX compliance test suite, such
as pdjfstest.

And besides, overlayfs now feels it is in a functional state that is worthy
of PASS. Sure, that is a very subjective observation.

> I'm wondering, would it make sense to manually add "rotation points"
> to plain xfstest test cases?

Can you give an example of an xfstest and a rotation point?
We actually had to duplicate a whole bunch of test cases, just to
test the nonsamefs flavor, so adding a 'flavor' to run all tests with is
not really the xfstests way.

I rather we add rotation related tests to unionmount testsuite
as it already supports a whole bunch of rotation techniques.
If you have any specific use cases in mind I can work on adding
those to unionmount.

Thanks,
Amir.

Re: [PATCH 0/2] Fix failing overlay nonsamefs fstests

[Miklos Szeredi]

On Mon, Apr 23, 2018 at 4:50 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Apr 23, 2018 at 6:37 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Wed, Apr 18, 2018 at 4:30 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>> On Wed, Apr 18, 2018 at 5:23 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>> On Wed, Apr 18, 2018 at 4:16 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>>>
>>>>> FYI, Miklos has a patch set in review to fix the last of the
>>>>> non-standard behavior tracking test that is still failing (overlay/016).
>>>>> After that patch set is merged, all overlay tests will be expected to
>>>>> pass on master - new era ;-)
>>>>
>>>> Oh, we can write new tests that don't pass ;)
>>
>> Another one:
>>
>> fstat() returns incorrect st_ctime after unlink of a lower file/directory.
>>
>> Alternative fixes:
>>
>> 1) take some attributes from whiteout
>>
>> 2) metacopy up to tmpfile
>>
>> But first of all, we need a test case that fails, otherwise there's no
>> motivation to do the fix ;)
>>
>
> I admit that the failing xfstests I wrote have had a "motivating"
> effect, but going forward, I feel that xfstests is not really the place
> for these sort of weird behavior tests. I find it to be more suiting as
> a regression tracking test suite and special test case coverage.
>
> IMO, parent ctime fits better to a POSIX compliance test suite, such
> as pdjfstest.

I'm not talking about parent ctime, rather victim ctime (updated due
to nlink change).

>
> And besides, overlayfs now feels it is in a functional state that is worthy
> of PASS. Sure, that is a very subjective observation.
>
>> I'm wondering, would it make sense to manually add "rotation points"
>> to plain xfstest test cases?
>
> Can you give an example of an xfstest and a rotation point?

Any test where there is a setup phase and a test phase is asking for a
rotation point (i.e. set up on lower (no-overlay) and then modify on
overlay).

I haven't thought about individual test cases, just wondering if
there's already a testcase testing st_ctime on unlink for example,
that is only not failing because there isn't a rotation of overlay
after the setup phase.

Thanks,
Miklos

Re: [PATCH 0/2] Fix failing overlay nonsamefs fstests

[Amir Goldstein]

On Mon, Apr 23, 2018 at 8:12 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Mon, Apr 23, 2018 at 4:50 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>> On Mon, Apr 23, 2018 at 6:37 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>> On Wed, Apr 18, 2018 at 4:30 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>>> On Wed, Apr 18, 2018 at 5:23 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>>> On Wed, Apr 18, 2018 at 4:16 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>>>>
>>>>>> FYI, Miklos has a patch set in review to fix the last of the
>>>>>> non-standard behavior tracking test that is still failing (overlay/016).
>>>>>> After that patch set is merged, all overlay tests will be expected to
>>>>>> pass on master - new era ;-)
>>>>>
>>>>> Oh, we can write new tests that don't pass ;)
>>>
>>> Another one:
>>>
>>> fstat() returns incorrect st_ctime after unlink of a lower file/directory.
>>>
>>> Alternative fixes:
>>>
>>> 1) take some attributes from whiteout
>>>
>>> 2) metacopy up to tmpfile

I have patches to copy up to an "orphan index":
https://github.com/amir73il/linux/commits/ovl-rocopyup

combine that with metacopy and you have a ready and
inexpensive place holder for st_ctime of unlinked file.

>>>
>>> But first of all, we need a test case that fails, otherwise there's no
>>> motivation to do the fix ;)
>>>
>>
>> I admit that the failing xfstests I wrote have had a "motivating"
>> effect, but going forward, I feel that xfstests is not really the place
>> for these sort of weird behavior tests. I find it to be more suiting as
>> a regression tracking test suite and special test case coverage.
>>
>> IMO, parent ctime fits better to a POSIX compliance test suite, such
>> as pdjfstest.
>
> I'm not talking about parent ctime, rather victim ctime (updated due
> to nlink change).
>
>>
>> And besides, overlayfs now feels it is in a functional state that is worthy
>> of PASS. Sure, that is a very subjective observation.
>>
>>> I'm wondering, would it make sense to manually add "rotation points"
>>> to plain xfstest test cases?
>>
>> Can you give an example of an xfstest and a rotation point?
>
> Any test where there is a setup phase and a test phase is asking for a
> rotation point (i.e. set up on lower (no-overlay) and then modify on
> overlay).

xfstest cases do not have a setup phase and run phase, so
I don't see a way that this could be done elegantly without
modifying all test cases.

>
> I haven't thought about individual test cases, just wondering if
> there's already a testcase testing st_ctime on unlink for example,
> that is only not failing because there isn't a rotation of overlay
> after the setup phase.
>

>From what I can see there are a few tests checking ctime update,
mainly regression tests for btrfs ctime update bugs:
$ git grep ctime.update tests/
tests/generic/221:# Check ctime updates when calling futimens without
UTIME_OMIT for the
tests/generic/236:# Check ctime updated or not if file linked
tests/generic/236:# check ctime updated
tests/generic/277:# Check if ctime update caused by chattr is written to disk
tests/generic/408:      echo "ctime updated"

generic/236 tests the link use case, but no test for unlink AFAIK.

Thanks,
Amir.

Re: [PATCH 0/2] Fix failing overlay nonsamefs fstests

[Miklos Szeredi]

On Mon, Apr 23, 2018 at 5:57 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Mon, Apr 23, 2018 at 8:12 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Mon, Apr 23, 2018 at 4:50 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>> On Mon, Apr 23, 2018 at 6:37 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>> On Wed, Apr 18, 2018 at 4:30 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>>>> On Wed, Apr 18, 2018 at 5:23 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>>>>>> On Wed, Apr 18, 2018 at 4:16 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>>>>>
>>>>>>> FYI, Miklos has a patch set in review to fix the last of the
>>>>>>> non-standard behavior tracking test that is still failing (overlay/016).
>>>>>>> After that patch set is merged, all overlay tests will be expected to
>>>>>>> pass on master - new era ;-)
>>>>>>
>>>>>> Oh, we can write new tests that don't pass ;)
>>>>
>>>> Another one:
>>>>
>>>> fstat() returns incorrect st_ctime after unlink of a lower file/directory.
>>>>
>>>> Alternative fixes:
>>>>
>>>> 1) take some attributes from whiteout
>>>>
>>>> 2) metacopy up to tmpfile
>
> I have patches to copy up to an "orphan index":
> https://github.com/amir73il/linux/commits/ovl-rocopyup
>
> combine that with metacopy and you have a ready and
> inexpensive place holder for st_ctime of unlinked file.

There are more bugs related to this.  For example the following
sequence will give a strange EEXIST error:

fd = open(path, O_RDONLY);
unlink(path);
fchmod(fd, 0400);

The reason is, fchmod will try to copy up the lower file, but finds a
whiteout on the upper layer, so the copy-up fails.

Actually, after unlink (last unlink, to be precise) we don't need a
physical file, any metadata changes can just go into the overlay inode
(ctime, atime, mode, uid, gid).  We just need to add a flag indicating
that the overlay inode is the authoritative source of metadata and
make sure any metadata changes go to the overlay inode.

And there's fsetxattr and fremovexattr.  Hmm.  We can also store
xattrs in memory, but that is becoming a bit complicated...  Oh well,
we can just postpone the tmpfile copyup until this happens.

Thanks,
Miklos