diff mbox

[V2] overlay: Test consistent st_ino numbers for non-samefs scenario

Message ID 20171114110948.31345-1-chandan@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chandan Rajendra Nov. 14, 2017, 11:09 a.m. UTC
This commit adds a test to verify consistent st_ino feature when
the overlayfs instance is composed of two different underlying
filesystem instances.

For example,
$ mount -t xfs /dev/loop0 /mnt/test
$ mount -t xfs /dev/loop1 /mnt/scratch
$ mkdir /mnt/scratch/upper
$ mkdir /mnt/scratch/work
$ mount -t overlay overlay -o lowerdir=/mnt/test \
        -o upperdir=/mnt/scratch/upper \
        -o workdir=/mnt/scratch/work /mnt/merge

The goal of this test is to verify that overlayfs returns consistent
st_ino for the following scenarios,
- Copy-up of lowerdir files
- Rename files and drop dentry/inode cache
- Remount the overlayfs instance

Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
---
Changelog:
v1->v2:
Address review comments provided by Amir.
1. To succeed, the test now requires the underlying base filesystems to provide
   32-bit inode numbers or the user to provide "xino" mount option. The test
   also requires that either overlayfs 'index' config feature to be enabled by
   default or to be enabled by using the 'index=on' mount option.
2. Require $TEST_DEV for use as Overlayfs' lowerdir.
3. Pass $OVERLAY_MOUNT_OPTIONS as argument to _overlay_scratch_mount_dirs().
   
 tests/overlay/043     | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/043.out |   2 +
 tests/overlay/group   |   1 +
 3 files changed, 170 insertions(+)
 create mode 100755 tests/overlay/043
 create mode 100644 tests/overlay/043.out

Comments

Amir Goldstein Nov. 14, 2017, 1:45 p.m. UTC | #1
On Tue, Nov 14, 2017 at 1:09 PM, Chandan Rajendra
<chandan@linux.vnet.ibm.com> wrote:
> This commit adds a test to verify consistent st_ino feature when
> the overlayfs instance is composed of two different underlying
> filesystem instances.
>
> For example,
> $ mount -t xfs /dev/loop0 /mnt/test
> $ mount -t xfs /dev/loop1 /mnt/scratch
> $ mkdir /mnt/scratch/upper
> $ mkdir /mnt/scratch/work
> $ mount -t overlay overlay -o lowerdir=/mnt/test \
>         -o upperdir=/mnt/scratch/upper \
>         -o workdir=/mnt/scratch/work /mnt/merge
>
> The goal of this test is to verify that overlayfs returns consistent
> st_ino for the following scenarios,
> - Copy-up of lowerdir files
> - Rename files and drop dentry/inode cache
> - Remount the overlayfs instance
>
> Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> ---
> Changelog:
> v1->v2:
> Address review comments provided by Amir.
> 1. To succeed, the test now requires the underlying base filesystems to provide
>    32-bit inode numbers or the user to provide "xino" mount option. The test
>    also requires that either overlayfs 'index' config feature to be enabled by
>    default or to be enabled by using the 'index=on' mount option.
> 2. Require $TEST_DEV for use as Overlayfs' lowerdir.
> 3. Pass $OVERLAY_MOUNT_OPTIONS as argument to _overlay_scratch_mount_dirs().
>
>  tests/overlay/043     | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/043.out |   2 +
>  tests/overlay/group   |   1 +
>  3 files changed, 170 insertions(+)
>  create mode 100755 tests/overlay/043
>  create mode 100644 tests/overlay/043.out
>
> diff --git a/tests/overlay/043 b/tests/overlay/043
> new file mode 100755
> index 0000000..5778564
> --- /dev/null
> +++ b/tests/overlay/043
> @@ -0,0 +1,167 @@
> +#! /bin/bash
> +# FSQA Test No. 043
> +#
> +# Test constant inode numbers on non-samefs setup
> +# This is a variant of overlay/017 to test constant st_ino numbers for
> +# non-samefs setup.
> +#
> +# This simple test demonstrates a known issue with overlayfs:
> +# - stat file A shows inode number X
> +# - 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
> +# and if inode numbers persist after rename, drop caches and
> +# mount cycle.
> +#
> +# This test succeeds when either the underlying filesystems provide 32-bit
> +# inode numbers or when the xino mount option is provided. This test
> +# also requires that either overlayfs 'index' config feature to be enabled by
> +# default or to be enabled by using the 'index=on' mount option.
> +#


Test looks good, but I am not happy with merging this text, even though I wrote
part of it...

Regarding the claim that the test succeeds on 32bit ino file systems,
this is not
yet true and may not be true in the future. We won't know how 'xino' will be
call or configured until the feature is merged, so I suggest to leave
this paragraph
out of the test description before merging.

Regarding the 'index' feature text, I don't like the situation as it is.
I would like to get a feedback from Eryu after he understands the options,
but my proposal is as follows:

- Test overlay/017 (constant st_ino/d_ino) should be independent of index
  so remove "hardlink1 hardlink2" from overlay/017 FILES, because the
  hardlinks use case is already covered by the more specific 018 test
  which does require and enable index.
- Add the d_ino verification from 017 also to 018, so test coverage for
  hardlinks doesn't change
- That makes 017 a regression test for constant st_ino/d_ino (excluding
  hardlinks), even if index is disabled and 018 a regression test for hardlinks
  (including st_ino/d_ino) which depends on index feature

If that is acceptable then the new variants 017 should not require index
and a nonsamefs variant of 018 which does require index would be nice.

Thinking out loud: would it have been better if we added a general way
to configure all tests to tun in non-samefs configuration instead of duplicating
individual test? I don't have any good ideas how that would work.

Eryu,

What do you think about all this?

Amir.
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eryu Guan Nov. 15, 2017, 6:36 a.m. UTC | #2
On Tue, Nov 14, 2017 at 03:45:17PM +0200, Amir Goldstein wrote:
> On Tue, Nov 14, 2017 at 1:09 PM, Chandan Rajendra
> <chandan@linux.vnet.ibm.com> wrote:
> > This commit adds a test to verify consistent st_ino feature when
> > the overlayfs instance is composed of two different underlying
> > filesystem instances.
> >
> > For example,
> > $ mount -t xfs /dev/loop0 /mnt/test
> > $ mount -t xfs /dev/loop1 /mnt/scratch
> > $ mkdir /mnt/scratch/upper
> > $ mkdir /mnt/scratch/work
> > $ mount -t overlay overlay -o lowerdir=/mnt/test \
> >         -o upperdir=/mnt/scratch/upper \
> >         -o workdir=/mnt/scratch/work /mnt/merge
> >
> > The goal of this test is to verify that overlayfs returns consistent
> > st_ino for the following scenarios,
> > - Copy-up of lowerdir files
> > - Rename files and drop dentry/inode cache
> > - Remount the overlayfs instance
> >
> > Signed-off-by: Chandan Rajendra <chandan@linux.vnet.ibm.com>
> > ---
> > Changelog:
> > v1->v2:
> > Address review comments provided by Amir.
> > 1. To succeed, the test now requires the underlying base filesystems to provide
> >    32-bit inode numbers or the user to provide "xino" mount option. The test
> >    also requires that either overlayfs 'index' config feature to be enabled by
> >    default or to be enabled by using the 'index=on' mount option.
> > 2. Require $TEST_DEV for use as Overlayfs' lowerdir.
> > 3. Pass $OVERLAY_MOUNT_OPTIONS as argument to _overlay_scratch_mount_dirs().
> >
> >  tests/overlay/043     | 167 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/overlay/043.out |   2 +
> >  tests/overlay/group   |   1 +
> >  3 files changed, 170 insertions(+)
> >  create mode 100755 tests/overlay/043
> >  create mode 100644 tests/overlay/043.out
> >
> > diff --git a/tests/overlay/043 b/tests/overlay/043
> > new file mode 100755
> > index 0000000..5778564
> > --- /dev/null
> > +++ b/tests/overlay/043
> > @@ -0,0 +1,167 @@
> > +#! /bin/bash
> > +# FSQA Test No. 043
> > +#
> > +# Test constant inode numbers on non-samefs setup
> > +# This is a variant of overlay/017 to test constant st_ino numbers for
> > +# non-samefs setup.
> > +#
> > +# This simple test demonstrates a known issue with overlayfs:
> > +# - stat file A shows inode number X
> > +# - 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
> > +# and if inode numbers persist after rename, drop caches and
> > +# mount cycle.
> > +#
> > +# This test succeeds when either the underlying filesystems provide 32-bit
> > +# inode numbers or when the xino mount option is provided. This test
> > +# also requires that either overlayfs 'index' config feature to be enabled by
> > +# default or to be enabled by using the 'index=on' mount option.
> > +#
> 
> 
> Test looks good, but I am not happy with merging this text, even though I wrote
> part of it...
> 
> Regarding the claim that the test succeeds on 32bit ino file systems,
> this is not
> yet true and may not be true in the future. We won't know how 'xino' will be
> call or configured until the feature is merged, so I suggest to leave
> this paragraph
> out of the test description before merging.
> 
> Regarding the 'index' feature text, I don't like the situation as it is.
> I would like to get a feedback from Eryu after he understands the options,
> but my proposal is as follows:
> 
> - Test overlay/017 (constant st_ino/d_ino) should be independent of index
>   so remove "hardlink1 hardlink2" from overlay/017 FILES, because the
>   hardlinks use case is already covered by the more specific 018 test
>   which does require and enable index.
> - Add the d_ino verification from 017 also to 018, so test coverage for
>   hardlinks doesn't change
> - That makes 017 a regression test for constant st_ino/d_ino (excluding
>   hardlinks), even if index is disabled and 018 a regression test for hardlinks
>   (including st_ino/d_ino) which depends on index feature
> 
> If that is acceptable then the new variants 017 should not require index
> and a nonsamefs variant of 018 which does require index would be nice.

After re-reading the patch description that introduced index feature and
overlay/017,8, I agreed with above proposal, so that 017 and 018 become
well separated/defined tests to cover the whole constant inode feature
in samefs setup.

> 
> Thinking out loud: would it have been better if we added a general way
> to configure all tests to tun in non-samefs configuration instead of duplicating
> individual test? I don't have any good ideas how that would work.

I think duplicating some of the tests with a slightly different setup
should be fine, we already have several examples in fstests now.

Thanks,
Eryu
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tests/overlay/043 b/tests/overlay/043
new file mode 100755
index 0000000..5778564
--- /dev/null
+++ b/tests/overlay/043
@@ -0,0 +1,167 @@ 
+#! /bin/bash
+# FSQA Test No. 043
+#
+# Test constant inode numbers on non-samefs setup
+# This is a variant of overlay/017 to test constant st_ino numbers for
+# non-samefs setup.
+#
+# This simple test demonstrates a known issue with overlayfs:
+# - stat file A shows inode number X
+# - 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
+# and if inode numbers persist after rename, drop caches and
+# mount cycle.
+#
+# This test succeeds when either the underlying filesystems provide 32-bit
+# inode numbers or when the xino mount option is provided. This test
+# also requires that either overlayfs 'index' config feature to be enabled by
+# default or to be enabled by using the 'index=on' mount option.
+#
+#-----------------------------------------------------------------------
+#
+# Copyright (C) 2017 IBM Corporation. All Rights Reserved.
+# Author: Chandan Rajendra <chandan@linux.vnet.ibm.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+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
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs overlay
+_supported_os Linux
+_require_scratch
+_require_test
+_require_test_program "af_unix"
+_require_test_program "t_dir_type"
+
+rm -f $seqres.full
+
+lowerdir=$OVL_BASE_TEST_DIR/$seq-ovl-lower
+rm -rf $lowerdir
+mkdir $lowerdir
+
+# Create our test files.
+mkdir $lowerdir/dir
+touch $lowerdir/file
+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
+touch $lowerdir/hardlink1
+ln $lowerdir/hardlink1 $lowerdir/hardlink2
+
+_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 \
+			    $OVERLAY_MOUNT_OPTIONS
+
+FILES="dir file symlink chrdev blkdev fifo socket hardlink1 hardlink2"
+
+# 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=$3
+
+	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
+		$here/src/t_dir_type $dir $ino | grep -q $f || \
+			echo "$f not found by ino $ino (from $before)"
+	done
+}
+
+rm -f $tmp.*
+testdir=$SCRATCH_MNT/test
+mkdir -p $testdir
+
+# Record inode numbers before copy up
+record_inode_numbers $SCRATCH_MNT $tmp.before
+
+for f in $FILES; do
+	# chown -h modifies all those file types
+	chown -h 100 $SCRATCH_MNT/$f
+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
+
+# Verify that the inode numbers survive a mount cycle
+$UMOUNT_PROG $SCRATCH_MNT
+_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir \
+			    $OVERLAY_MOUNT_OPTIONS
+
+# Compare inode numbers before/after mount cycle
+check_inode_numbers $testdir $tmp.after_move $tmp.after_cycle
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/overlay/043.out b/tests/overlay/043.out
new file mode 100644
index 0000000..f90f0a5
--- /dev/null
+++ b/tests/overlay/043.out
@@ -0,0 +1,2 @@ 
+QA output created by 043
+Silence is golden
diff --git a/tests/overlay/group b/tests/overlay/group
index 782c39f..a99ff07 100644
--- a/tests/overlay/group
+++ b/tests/overlay/group
@@ -45,3 +45,4 @@ 
 040 auto quick
 041 auto quick copyup nonsamefs
 042 auto quick copyup hardlink
+043 auto quick copyup nonsamefs