[PATCH v2 0/6] fstests: more tests for overlay constant inode numbers

[PATCH v2 0/6] fstests: more tests for overlay constant inode numbers

[Amir Goldstein]

Hi Eryu,

This series contains enhancements to overlay/017, which I used to test
my work on overlayfs constant inode numbers. [1]

The original test was a bit naiive, not taking into account renames,
drop caches and mount cycle. All those are added by this series.
Since the original test does not pass on stable kernel nor will
it pass with the changes by this series, these changes should cause
no regressions to testers.

My work covers only the inode numbers returned from stat(2) and not
the inode numbers returned in d_ino from readdir(3), so the 'find -inum'
part of this test could still fail with my overlayfs patches.

However, I ran my tests in kvm-xfstests VM, where 'find -inum' called
stat(2) for each entry, so the test did pass.

I will dig deeper into this behavior when I work on fixing d_ino
values in the next part of my work.

Changes since v1:
- Re-factor check_inode_numbers
- Move -maxdepth option position
- Fix somme comments
- Blurb more in commit messages

Amir.

[1] https://marc.info/?l=linux-unionfs&m=149324252301397&w=2


Amir Goldstein (6):
  overlay/017: silence test output
  overlay/017: fix some comments
  overlay/017: use af_unix to create socket test file
  overlay/017: create helpers to record and check inode numbers
  overlay/017: verify constant inode number after rename
  overlay/017: test persistent inode numbers after mount cycle

 tests/overlay/017     | 86 +++++++++++++++++++++++++++++++++++++++++----------
 tests/overlay/017.out |  7 +----
 2 files changed, 70 insertions(+), 23 deletions(-)

-- 
2.7.4

[PATCH v2 1/6] overlay/017: silence test output

[Amir Goldstein]

Change test to output golden silence on success.

We are going to run the same check several times,
so instead of cloning the test output, cloning the
silence will be more conveniet.

Generalize cleanup of temp files for the same reason.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/017     | 7 +++++--
 tests/overlay/017.out | 7 +------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/tests/overlay/017 b/tests/overlay/017
index 5ca3040..7c1430d 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -40,6 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
+	cd /
 	rm -f $tmp.*
 }
 
@@ -73,7 +74,7 @@ mknod $lowerdir/fifo p
 _scratch_mount
 
 
-rm -f $tmp.before $tmp.after
+rm -f $tmp.*
 
 # Test stable stat(2) st_ino
 
@@ -89,11 +90,13 @@ done
 
 # find by inode number - expect to find file by inode number
 cat $tmp.before | while read ino f; do
-	find $f -inum $ino -maxdepth 0 | _filter_scratch
+	find $SCRATCH_MNT/ -maxdepth 1 -inum $ino | grep -q $f || \
+		echo "$f not found by ino $ino"
 done
 
 # Compare before..after - expect silence
 diff $tmp.before $tmp.after
 
+echo "Silence is golden"
 status=0
 exit
diff --git a/tests/overlay/017.out b/tests/overlay/017.out
index 1f277c5..8222844 100644
--- a/tests/overlay/017.out
+++ b/tests/overlay/017.out
@@ -1,7 +1,2 @@
 QA output created by 017
-SCRATCH_MNT/dir
-SCRATCH_MNT/file
-SCRATCH_MNT/symlink
-SCRATCH_MNT/chrdev
-SCRATCH_MNT/blkdev
-SCRATCH_MNT/fifo
+Silence is golden
-- 
2.7.4

[PATCH v2 2/6] overlay/017: fix some comments

[Amir Goldstein]

Align all comments to the term 'constant inode numbers' and
explain why hardlinks are excluded from this test.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/017 | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tests/overlay/017 b/tests/overlay/017
index 7c1430d..e04690c 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -1,7 +1,7 @@
 #! /bin/bash
 # FSQA Test No. 017
 #
-# Test unstable inode number
+# Test constant inode numbers
 #
 # This simple test demonstrates a known issue with overlayfs:
 # - stat file A shows inode number X
@@ -58,9 +58,8 @@ rm -f $seqres.full
 _scratch_mkfs >>$seqres.full 2>&1
 
 # Create our test files.
-# Not dealing with hardlinks here, because there is more to test
-# then stable inode number.
-# Hardlinks will get a test of their own.
+# Not dealing with hardlinks here, when hardlinks are broken they
+# should not preserve the inode number.
 lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
 mkdir -p $lowerdir
 mkdir $lowerdir/dir
-- 
2.7.4

[PATCH v2 3/6] overlay/017: use af_unix to create socket test file

[Amir Goldstein]

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/017 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/overlay/017 b/tests/overlay/017
index e04690c..779907d 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -34,6 +34,7 @@ seq=`basename $0`
 seqres=$RESULT_DIR/$seq
 echo "QA output created by $seq"
 
+here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
 trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -52,6 +53,7 @@ _cleanup()
 _supported_fs overlay
 _supported_os Linux
 _require_scratch
+_require_test_program "af_unix"
 
 rm -f $seqres.full
 
@@ -68,6 +70,7 @@ ln -s $lowerdir/file $lowerdir/symlink
 mknod $lowerdir/chrdev c 1 1
 mknod $lowerdir/blkdev b 1 1
 mknod $lowerdir/fifo p
+$here/src/af_unix $lowerdir/socket
 
 
 _scratch_mount
@@ -78,7 +81,7 @@ rm -f $tmp.*
 # Test stable stat(2) st_ino
 
 # Record inode numbers before and after copy up
-for f in dir file symlink chrdev blkdev fifo; do
+for f in dir file symlink chrdev blkdev fifo socket; do
 	ls -id $SCRATCH_MNT/$f >> $tmp.before
 	# chown -h modifies all those file types
 	chown -h 100 $SCRATCH_MNT/$f
-- 
2.7.4

[PATCH v2 4/6] overlay/017: create helpers to record and check inode numbers

[Amir Goldstein]

Use helpers to records and check inode numbers so we can repeat
the same test after rename and mount cycle.

Suggested-by: Eryu Guan <eguan@redhat.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/017 | 57 ++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/tests/overlay/017 b/tests/overlay/017
index 779907d..3ff429f 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -72,32 +72,59 @@ mknod $lowerdir/blkdev b 1 1
 mknod $lowerdir/fifo p
 $here/src/af_unix $lowerdir/socket
 
+FILES="dir file symlink chrdev blkdev fifo socket"
+
+# Record inode numbers in format <ino> <basename>
+function record_inode_numbers()
+{
+	dir=$1
+	outfile=$2
+
+	for f in $FILES; do
+		ls -id $dir/$f
+	done | \
+	while read ino file; do
+		echo $ino `basename $file` >> $outfile
+	done
+}
+
+# Check inode numbers match recorder inode numbers
+function check_inode_numbers()
+{
+	dir=$1
+	before=$2
+	after=$2
+
+	record_inode_numbers $dir $after
+
+	# Test constant stat(2) st_ino -
+	#   Compare before..after - expect silence
+	# We use diff -u so out.bad will tell us which stage failed
+	diff -u $before $after
+
+	# Test constant readdir(3)/getdents(2) d_ino -
+	#   Expect to find file by inode number
+	cat $before | while read ino f; do
+		find $dir/ -maxdepth 1 -inum $ino | grep -q $f || \
+			echo "$f not found by ino $ino (from $before)"
+	done
+}
 
 _scratch_mount
 
 
 rm -f $tmp.*
 
-# Test stable stat(2) st_ino
+# Record inode numbers before copy up
+record_inode_numbers $SCRATCH_MNT $tmp.before
 
-# Record inode numbers before and after copy up
-for f in dir file symlink chrdev blkdev fifo socket; do
-	ls -id $SCRATCH_MNT/$f >> $tmp.before
+for f in $FILES; do
 	# chown -h modifies all those file types
 	chown -h 100 $SCRATCH_MNT/$f
-	ls -id $SCRATCH_MNT/$f >> $tmp.after
-done
-
-# Test stable readdir(3)/getdents(2) d_ino
-
-# find by inode number - expect to find file by inode number
-cat $tmp.before | while read ino f; do
-	find $SCRATCH_MNT/ -maxdepth 1 -inum $ino | grep -q $f || \
-		echo "$f not found by ino $ino"
 done
 
-# Compare before..after - expect silence
-diff $tmp.before $tmp.after
+# Compare inode numbers before/after copy up
+check_inode_numbers $SCRATCH_MNT $tmp.before $tmp.after_copyup
 
 echo "Silence is golden"
 status=0
-- 
2.7.4

[PATCH v2 5/6] overlay/017: verify constant inode number after rename

[Amir Goldstein]

The test verifies constant inode number after copy up.

Verify that inode number remains constant also after rename
and drop caches (when overlayfs needs to find the lower
inodes in another location).

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/017 | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/tests/overlay/017 b/tests/overlay/017
index 3ff429f..fe2518b 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -8,7 +8,8 @@
 # - modify A to trigger copy up
 # - stat file A shows inode number Y != X
 #
-# Also test if d_ino of readdir entries changes after copy up.
+# Also test if d_ino of readdir entries changes after copy up
+# and if inode numbers persist after rename and drop caches.
 #
 #-----------------------------------------------------------------------
 #
@@ -114,6 +115,8 @@ _scratch_mount
 
 
 rm -f $tmp.*
+testdir=$SCRATCH_MNT/test
+mkdir -p $testdir
 
 # Record inode numbers before copy up
 record_inode_numbers $SCRATCH_MNT $tmp.before
@@ -126,6 +129,16 @@ done
 # Compare inode numbers before/after copy up
 check_inode_numbers $SCRATCH_MNT $tmp.before $tmp.after_copyup
 
+for f in $FILES; do
+	# move to another dir
+	mv $SCRATCH_MNT/$f $testdir/
+done
+
+echo 3 > /proc/sys/vm/drop_caches
+
+# Compare inode numbers before/after rename and drop caches
+check_inode_numbers $testdir $tmp.after_copyup $tmp.after_move
+
 echo "Silence is golden"
 status=0
 exit
-- 
2.7.4

[PATCH v2 6/6] overlay/017: test persistent inode numbers after mount cycle

[Amir Goldstein]

Overlayfs directory inodes are constant across copy up,
but not persistent on mount cycle.

Compare the inode numbers before and after mount cycle.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 tests/overlay/017 | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/overlay/017 b/tests/overlay/017
index fe2518b..c5671ec 100755
--- a/tests/overlay/017
+++ b/tests/overlay/017
@@ -9,7 +9,8 @@
 # - stat file A shows inode number Y != X
 #
 # Also test if d_ino of readdir entries changes after copy up
-# and if inode numbers persist after rename and drop caches.
+# and if inode numbers persist after rename, drop caches and
+# mount cycle.
 #
 #-----------------------------------------------------------------------
 #
@@ -139,6 +140,12 @@ echo 3 > /proc/sys/vm/drop_caches
 # Compare inode numbers before/after rename and drop caches
 check_inode_numbers $testdir $tmp.after_copyup $tmp.after_move
 
+# Verify that the inode numbers survive a mount cycle
+_scratch_cycle_mount
+
+# Compare inode numbers before/after mount cycle
+check_inode_numbers $testdir $tmp.after_move $tmp.after_cycle
+
 echo "Silence is golden"
 status=0
 exit
-- 
2.7.4

Re: [PATCH v2 4/6] overlay/017: create helpers to record and check inode numbers

[Eryu Guan]

On Fri, Apr 28, 2017 at 11:24:46AM +0300, Amir Goldstein wrote:
> Use helpers to records and check inode numbers so we can repeat
> the same test after rename and mount cycle.
> 
> Suggested-by: Eryu Guan <eguan@redhat.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  tests/overlay/017 | 57 ++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/overlay/017 b/tests/overlay/017
> index 779907d..3ff429f 100755
> --- a/tests/overlay/017
> +++ b/tests/overlay/017
> @@ -72,32 +72,59 @@ mknod $lowerdir/blkdev b 1 1
>  mknod $lowerdir/fifo p
>  $here/src/af_unix $lowerdir/socket
>  
> +FILES="dir file symlink chrdev blkdev fifo socket"
> +
> +# Record inode numbers in format <ino> <basename>
> +function record_inode_numbers()
> +{
> +	dir=$1
> +	outfile=$2
> +
> +	for f in $FILES; do
> +		ls -id $dir/$f
> +	done | \
> +	while read ino file; do
> +		echo $ino `basename $file` >> $outfile
> +	done
> +}
> +
> +# Check inode numbers match recorder inode numbers
> +function check_inode_numbers()
> +{
> +	dir=$1
> +	before=$2
> +	after=$2
             ^^^ $3
Otherwise looks good to me. I can fix this typo at commit time.

But I've already kicked off release testing for this week's update,
I'll queue these patches for next update.

Thanks,
Eryu

Re: [PATCH v2 4/6] overlay/017: create helpers to record and check inode numbers

[Amir Goldstein]

On Fri, Apr 28, 2017 at 12:24 PM, Eryu Guan <eguan@redhat.com> wrote:
> On Fri, Apr 28, 2017 at 11:24:46AM +0300, Amir Goldstein wrote:
>> Use helpers to records and check inode numbers so we can repeat
>> the same test after rename and mount cycle.
>>
>> Suggested-by: Eryu Guan <eguan@redhat.com>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  tests/overlay/017 | 57 ++++++++++++++++++++++++++++++++++++++++---------------
>>  1 file changed, 42 insertions(+), 15 deletions(-)
>>
>> diff --git a/tests/overlay/017 b/tests/overlay/017
>> index 779907d..3ff429f 100755
>> --- a/tests/overlay/017
>> +++ b/tests/overlay/017
>> @@ -72,32 +72,59 @@ mknod $lowerdir/blkdev b 1 1
>>  mknod $lowerdir/fifo p
>>  $here/src/af_unix $lowerdir/socket
>>
>> +FILES="dir file symlink chrdev blkdev fifo socket"
>> +
>> +# Record inode numbers in format <ino> <basename>
>> +function record_inode_numbers()
>> +{
>> +     dir=$1
>> +     outfile=$2
>> +
>> +     for f in $FILES; do
>> +             ls -id $dir/$f
>> +     done | \
>> +     while read ino file; do
>> +             echo $ino `basename $file` >> $outfile
>> +     done
>> +}
>> +
>> +# Check inode numbers match recorder inode numbers
>> +function check_inode_numbers()
>> +{
>> +     dir=$1
>> +     before=$2
>> +     after=$2
>              ^^^ $3
> Otherwise looks good to me. I can fix this typo at commit time.

Oh boy! That's a lousy test :-)
Now I wonder why the test failed when I ran it on kernel v4.10
(when feeding after to find -inum)... something for me to check.

>
> But I've already kicked off release testing for this week's update,
> I'll queue these patches for next update.
>

Sure. No rush on my end.

Thanks,
Amir.

Re: [PATCH v2 4/6] overlay/017: create helpers to record and check inode numbers

[Amir Goldstein]

On Fri, Apr 28, 2017 at 1:04 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Fri, Apr 28, 2017 at 12:24 PM, Eryu Guan <eguan@redhat.com> wrote:
>> On Fri, Apr 28, 2017 at 11:24:46AM +0300, Amir Goldstein wrote:
>>> Use helpers to records and check inode numbers so we can repeat
>>> the same test after rename and mount cycle.
>>>
>>> Suggested-by: Eryu Guan <eguan@redhat.com>
>>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>>> ---
>>>  tests/overlay/017 | 57 ++++++++++++++++++++++++++++++++++++++++---------------
>>>  1 file changed, 42 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/tests/overlay/017 b/tests/overlay/017
>>> index 779907d..3ff429f 100755
>>> --- a/tests/overlay/017
>>> +++ b/tests/overlay/017
>>> @@ -72,32 +72,59 @@ mknod $lowerdir/blkdev b 1 1
>>>  mknod $lowerdir/fifo p
>>>  $here/src/af_unix $lowerdir/socket
>>>
>>> +FILES="dir file symlink chrdev blkdev fifo socket"
>>> +
>>> +# Record inode numbers in format <ino> <basename>
>>> +function record_inode_numbers()
>>> +{
>>> +     dir=$1
>>> +     outfile=$2
>>> +
>>> +     for f in $FILES; do
>>> +             ls -id $dir/$f
>>> +     done | \
>>> +     while read ino file; do
>>> +             echo $ino `basename $file` >> $outfile
>>> +     done
>>> +}
>>> +
>>> +# Check inode numbers match recorder inode numbers
>>> +function check_inode_numbers()
>>> +{
>>> +     dir=$1
>>> +     before=$2
>>> +     after=$2
>>              ^^^ $3
>> Otherwise looks good to me. I can fix this typo at commit time.
>
> Oh boy! That's a lousy test :-)
> Now I wonder why the test failed when I ran it on kernel v4.10
> (when feeding after to find -inum)... something for me to check.
>

Ah. got it. 'after' inode number where appended to $tmp.before,
so find was also looking by the before inodes.