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.
> +_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
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.
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 #).
> +
> +file_count=10
> +link_max=2
> +link_count=2
> +_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
> +
> +# Case4:
> +# Setting whiteout_link_max=10 will share inode
> +# with 9 whiteout files and meanwhile the inode
> +# will also share with tmpfile(in workdir) which
> +# is used for creating whiteout file.
> +
Same here. nlink 10 is what I would expect, not 9.
Thanks,
Amir.
