---- 在 星期五, 2020-04-10 15:21:51 Amir Goldstein <amir73il@xxxxxxxxx> 撰写 ----
> On Fri, Apr 10, 2020 at 4:21 AM Chengguang Xu <cgxu519@xxxxxxxxxxxx> wrote:
> >
> > This is a test for whiteout inode sharing feature.
> >
> > Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxxxx>
> > ---
> > Hi Eryu,
> >
> > Kernel patch of this feature is still in review but I hope to merge
> > test case first, so that we can check the correctness in a convenient
> > way. The test case will carefully check new module param and skip the
> > test if the param does not exist.
> >
> >
> > tests/overlay/072 | 148 ++++++++++++++++++++++++++++++++++++++++++
> > tests/overlay/072.out | 2 +
> > tests/overlay/group | 1 +
> > 3 files changed, 151 insertions(+)
> > create mode 100755 tests/overlay/072
> > create mode 100644 tests/overlay/072.out
> >
> > diff --git a/tests/overlay/072 b/tests/overlay/072
> > new file mode 100755
> > index 00000000..1cff386d
> > --- /dev/null
> > +++ b/tests/overlay/072
> > @@ -0,0 +1,148 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (c) 2020 Chengguang Xu <cgxu519@xxxxxxxxxxxx>.
> > +# All Rights Reserved.
> > +#
> > +# FS QA Test 072
> > +#
> > +# This is a test for inode sharing with whiteout files.
> > +#
> > +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
> > +
> > +# remove previous $seqres.full before test
> > +rm -f $seqres.full
> > +
> > +# real QA test starts here
> > +_supported_fs overlay
> > +_supported_os Linux
> > +_require_test
> > +_require_scratch
> > +
> > +param_name="whiteout_link_max"
> > +check_whiteout_link_max()
> > +{
> > + local param_value=`_get_fs_module_param ${param_name}`
>
> Keep this value and set it back on _cleanup()
>
> > + if [ -z ${param_value} ]; then
> > + _notrun "${FSTYP} module param ${param_name} does not exist"
>
> This message will be more helpful:
> "overlayfs does not support whiteout inode sharing"
>
> > + fi
> > +}
> > +
> > +lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
> > +upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
> > +merged=$OVL_BASE_SCRATCH_MNT/$OVL_MNT
>
> Best if you use $SCRATCH_MNT instead of $merged
>
> > +
> > +#Make some files in lowerdir.
> > +make_lower_files()
> > +{
> > + seq 1 $file_count | while read line; do
>
> I think that a for statement would be more readable here.
>
> > + `touch $lowerdir/test${line} 1>&2 2>/dev/null`
> > + done
> > +}
> > +
> > +#Delete all copy-uped files in upperdir.
> > +make_whiteout_files()
> > +{
> > + rm -f $merged/* 1>&2 2>/dev/null
> > +}
> > +
> > +#Check link count of whiteout files.
> > +check_whiteout_files()
> > +{
> > + seq 1 $file_count | while read line; do
> > + local real_count=`stat -c %h $upperdir/test${line} 2>/dev/null`
> > + if [[ $link_count != $real_count ]]; then
> > + echo "Expected whiteout link count is $link_count but real count is $real_count"
> > + fi
> > + done
> > +}
> > +
> > +check_whiteout_link_max
> > +
> > +# Case1:
> > +# Setting whiteout_link_max=0 will not share inode
> > +# with whiteout files, it means each whiteout file
> > +# will has it's own inode.
> > +
> > +file_count=10
> > +link_max=0
> > +link_count=1
>
> Would be nicer to put all the below in a run_test_case() function
> with above as arguments.
>
Something like below?
---------
# Arguments:
# $1: Maximum link count
# $2: Testing file number
# $3: Expected link count
run_test_case()
{
_scratch_mkfs
_set_fs_module_param $param_name ${1}
make_lower_files ${2}
_scratch_mount
make_whiteout_files
check_whiteout_files ${2} ${3}
$UMOUNT_PROG $SCRATCH_MNT
}
link_max=1
file_count=10
link_count=1
run_test_case $link_max $file_count $link_count
---------
> > +_scratch_mkfs
> > +_set_fs_module_param $param_name $link_max
> > +make_lower_files
> > +_scratch_mount
> > +make_whiteout_files
> > +check_whiteout_files
> > +$UMOUNT_PROG $OVL_BASE_SCRATCH_MNT/$OVL_MNT
>
> Better:
> $UMOUNT_PROG $SCRATCH_MNT
>
> Even better:
> _scratch_umount
I haven't found the definition of _scratch_umount,
have we implemented it?
>
> The difference is that if the test case has reference leaks on inode or
> dentry, _overlay_base_unmount() will detect them.
>
> > +
> > +# Case2:
> > +# Setting whiteout_link_max=1 will not share inode
> > +# with whiteout files, it means each whiteout file
> > +# will has it's own inode.
> > +
> > +file_count=10
> > +link_max=1
> > +link_count=1
> > +_scratch_mkfs
> > +_set_fs_module_param $param_name $link_max
> > +make_lower_files
> > +_scratch_mount
> > +make_whiteout_files
> > +check_whiteout_files $link_count
> > +$UMOUNT_PROG $OVL_BASE_SCRATCH_MNT/$OVL_MNT
> > +
> > +# Case3:
> > +# Setting whiteout_link_max=2 will not share inode
> > +# with whiteout files, it means each whiteout file
> > +# will has it's own inode. However, the inode will
> > +# be shared with tmpfile(in workdir) which is used
> > +# for creating whiteout file.
>
> First, that is strange outcome of whiteout_link_max=2
> I would not expect it.
> Second, how can every whiteout be shared with tmpfile?
> There should be at most one tmpfile at all times, so the
> whiteouts that already reached whiteout_link_max should
> not be linked to any tmpfile.
I think I misunderstood your comment in my kernel patch, so I changed
the logic to keep all tmpfiles in workdir and cleanup them during next mount.
I'll fix it in V3 kernel patch.
>
> Please add to test_case() verification that work dir contains
> at most one tmpfile.
> But please make sure that the test is clever enough to check
> both work and index dirs for tmpfiles (names beginning with #).
>
So should I check module params of index and nfs_export for checking
tmpfile in index dir?
Thanks,
cgxu
