On Mon, Dec 12, 2016 at 09:47:47AM +0200, Amir Goldstein wrote:
> On Mon, Dec 12, 2016 at 7:44 AM, Eryu Guan <eguan@xxxxxxxxxx> wrote:
> > On Sun, Dec 11, 2016 at 06:38:00PM +0200, Amir Goldstein wrote:
> >> Introduce a new test to demonstrate 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.
> >>
> >> There is a POC patch by Miklos Szeredi that fixes this issue.
> >> ---
> >> tests/overlay/018 | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >> tests/overlay/018.out | 5 +++
> >> tests/overlay/group | 1 +
> >> 3 files changed, 102 insertions(+)
> >> create mode 100755 tests/overlay/018
> >> create mode 100644 tests/overlay/018.out
> >>
> >> diff --git a/tests/overlay/018 b/tests/overlay/018
> >> new file mode 100755
> >> index 0000000..4b358e5
> >> --- /dev/null
> >> +++ b/tests/overlay/018
> >> @@ -0,0 +1,96 @@
> >> +#! /bin/bash
> >> +# FSQA Test No. 018
> >> +#
> >> +# Test unstable inode number
> >> +#
> >> +# 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.
> >> +#
> >> +#-----------------------------------------------------------------------
> >> +#
> >> +# Copyright (C) 2016 CTERA Networks. All Rights Reserved.
> >> +# Author: Amir Goldstein <amir73il@xxxxxxxxx>
> >> +#
> >> +# 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"
> >> +
> >> +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
> >
> > Need a "_require_user" here.
>
> Right, so I'll use an arbitrary uid instead, cause the value of uid is
> not the issue.
> v2 coming right up.
>
> >
> >> +_require_scratch
> >> +
> >> +rm -f $seqres.full
> >> +
> >> +_scratch_mkfs >>$seqres.full 2>&1
> >> +
> >> +# Create our test files
> >> +lowerdir=$SCRATCH_DEV/$OVERLAY_LOWER_DIR
> >> +mkdir -p $lowerdir
> >> +mkdir $lowerdir/dir
> >> +touch $lowerdir/file
> >> +ln -s $lowerdir/file $lowerdir/symlink
> >> +mkfifo $lowerdir/fifo
> >
> > Should hardlink and device-special be tested too?
> >
>
> Hardlink is a more complex issue, which is not solved by the POC patch
> so I am leaving it to a test of its own.
> Hardlinks should not only preserve inode number, but also nlink and actually
> refer to same upper inode. The POC patch even breaks that brutally, by
I think the hardlinks are known to be broken on copy-up,
Documentation/filesystems/overlayfs.txt documented this behavior
"
If a file with multiple hard links is copied up, then this will
"break" the link. Changes will not be propagated to other names
referring to the same inode.
"
> making all broken hardlink clones apear to have the same inode number,
> but actually they are completely different files/inodes.
So after the POC patch, the hardlinks are still broken but having the
same inode number and making the situation more complext :)
How about adding some comments on why hardlinks are not tested?
>
> fifo is a representative of the device special files.
> Since overlayfs only ever tests for
> special_file(m) := (S_ISCHR(m)||S_ISBLK(m)||S_ISFIFO(m)||S_ISSOCK(m))
> and treats all special files the same,
> it seemed excessive to test all of those types individually.
The handles of these file types might be changed in future, and adding
tests of these file types are easy to do, I'd prefer having a full test
coverage of them.
Thanks,
Eryu
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
