On Mon, Dec 12, 2016 at 10:16 AM, Eryu Guan <eguan@xxxxxxxxxx> wrote:
> 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?
>
Sure.
>>
>> 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.
>
OK.
--
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
