On Mon, Oct 29, 2018 at 4:51 PM Jayashree Mohan <jayashree2912@xxxxxxxxx> wrote:
>
> Hi all,
>
> As discussed previously, I have ported a few CrashMonkey tests into
> xfstest test-case for your review. I have written a patch for testing
> simple "hard link" behaviour. This patch batches 37 test cases
> generated by CrashMonkey into one xfstest test, all of them testing
> hard-link behaviour. These 37 tests have been grouped based on the
> similarities; there are comments in the patch to
> describe what each group is testing for. On a high-level, we are
> testing the creation of hard links between files in same directory and
> files across two directories, while allowing fsync of either the files
> involved, their parent directories, or unrelated sibling files.
>
> We aim to convert the other CrashMonkey tests in the same way, batched
> by the type of file-system operation we test. This particular test
> took 10 seconds to run on my VM (Ubuntu 16.04, 2GB RAM, single core)
> on 100MB scratch partition. I have added per test case timer in this
> patch, which shows each test case takes about 0.25 seconds (to run,
> flakey_remount, test and cleanup). This test passes clean on ext4, xfs
> and F2FS. It will show three failed cases as of kernel 4.16 on btrfs
> (I think it's not yet patched on 4.19, so you should see the failure
> on the newest kernel as well).
>
> Thinking more about it, none of the CrashMonkey test cases check for
> resource exhaustion and hence we don't need more than 100MB of scratch
> device. If the per-test external overhead due to fsck/scrub etc
> depends on the scratch partition size, we can speed up the CrashMonkey
> tests by allowing a new device like scratch(much smaller in size -
> about 100-200MB). Additionally, our tests also do not need fsck to be
> run at the end. Is there a way to skip performing fsck after each test
> (if its something configurable in the test file) ?
Just use _require_scratch_nocheck() instead of _require_scratch()
For this type of tests, I think it's a good idea to let fsck run.
Even if all of the links are persisted, the log/journal replay might
have caused metadata inconsistencies in the fs for example - this was
true for many cases I fixed over the years in btrfs.
Even if fsck doesn't report any problem now, it's still good to run
it, to help prevent future regressions.
Plus this test creates a very small fs, it's not like fsck will take a
significant time to run.
So for all these reasons I would unmount and fsck after each test.
The test looks good to me, only some coding style comments below.
>
> If this sort of patch seems fine to you, I can go ahead and port the
> other CrashMonkey tests in the same way. After batching, I hope there
> would be around 10-12 test files like the one attached here.
>
> Patch below:
> ---
>
> diff --git a/tests/generic/517-link b/tests/generic/517-link
> new file mode 100755
> index 0000000..791b6c0
> --- /dev/null
> +++ b/tests/generic/517-link
> @@ -0,0 +1,162 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2018 The University of Texas at Austin. All Rights Reserved.
> +#
> +# FS QA Test 517-link
> +#
> +# Test if we create a hard link to a file and persist either of the
> files, all the names persist.
> +#
> +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()
> +{
> + _cleanup_flakey
> + cd /
> + rm -f $tmp.*
> +}
> +
> +init_start_time=$(date +%s.%N)
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmflakey
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +_supported_fs generic
> +_supported_os Linux
> +_require_scratch
> +_require_dm_target flakey
> +
> +# initialize scratch device
> +_scratch_mkfs >>$seqres.full 2>&1
> +_require_metadata_journaling $SCRATCH_DEV
> +_init_flakey
> +_mount_flakey
> +init_end_time=$(date +%s.%N)
> +init_time_elapsed=$(echo "$init_end_time-$init_start_time" | bc)
> +echo "Initialization time = $init_time_elapsed" >> $seqres.full
> +
> +stat_opt='-c "blocks: %b size: %s inode: %i links: %h"'
> +test_num=0
> +total_time=0
> +
> +cleanup()
> +{
> + rm -rf $SCRATCH_MNT/*
> + sync
> +}
> +
> +# create a hard link $2 to file $1, and fsync $3, followed by power-cu
> +test_link()
> +{
> + start_time=$(date +%s.%N)
> + test_num=$((test_num + 1))
> + sibling=0
> + before=""
> + after=""
Several of these are local variable, so prefix them with the 'local' keyword.
> + echo -ne "\n=== Test $test_num : link $1 $2 " | tee -a $seqres.full
You need to use the _filter_scratch filter here and then adjust the
golden output (the .out file).
Because if not, then the test will fail for anyone with a different
mount path for the scratch device.
Yours is /mnt/scratch but mine is /home/fdmanana/scratch_1 for example.
> +
> + # now execute the workload
> + mkdir -p "${1%/*}" && mkdir -p "${2%/*}" && touch "$1"
> + ln $1 $2
> +
> + if [ -z "$3" ]
> + then
The fstests coding style is like:
if [ whatever ]; then
fi
This applies to all if statements below.
> + echo -ne " with sync ===\n"
> + sync
> + before=`stat "$stat_opt" $1`
> + else
> + echo " with fsync $3 ==="
> +
> + # If the file being persisted is a sibling, create it first
> + if [ ! -f $3 ]
> + then
> + sibling=1
> + touch $3
> + fi
> +
> + $XFS_IO_PROG -c "fsync" $3 | _filter_xfs_io
No need to use the filter. The fsync command, unlike pwrite for
example, doesn't output rates and time.
> +
> + if [ $sibling -ne 1 ]
> + then
> + before=`stat "$stat_opt" $1`
> + fi
> + fi
> +
> + _flakey_drop_and_remount | tee -a $seqres.full
> +
> + if [ -f $1 ]
> + then
> + after=`stat "$stat_opt" $1`
> + fi
> +
> + if [ "$before" != "$after" ] && [ $sibling -ne 1 ]
> + then
> + echo "Before: $before" | tee -a $seqres.full
> + echo "After: $after" | tee -a $seqres.full
> + fi
> +
> + cleanup
> + end_time=$(date +%s.%N)
> + time_elapsed=$(echo "$end_time-$start_time" | bc)
> + echo " Elapsed time : $time_elapsed" >> $seqres.full
> + total_time=$(echo "$total_time+$time_elapsed" | bc)
Add one space before the + and another after, for readability and to
conform with the code base.
> + echo " Total time : $total_time" >> $seqres.full
> +}
> +
> +# run the link test for different combinations
> +
> +test_start_time=$(date +%s.%N)
> +# Group 1: Both files within root directory
> +for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar; do
> + test_link $SCRATCH_MNT/foo $SCRATCH_MNT/bar $file_name
> +done
> +test_link $SCRATCH_MNT/foo $SCRATCH_MNT/bar
> +
> +# Group 2: Create hard link in a sub directory
> +for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar
> $SCRATCH_MNT/A $SCRATCH_MNT/A/bar $SCRATCH_MNT/A/foo; do
> + test_link $SCRATCH_MNT/foo $SCRATCH_MNT/A/bar $file_name
> +done
> +test_link $SCRATCH_MNT/foo $SCRATCH_MNT/A/bar
> +
> +# Group 3: Create hard link in parent directory
> +for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar
> $SCRATCH_MNT/A $SCRATCH_MNT/A/bar $SCRATCH_MNT/A/foo; do
> + test_link $SCRATCH_MNT/A/foo $SCRATCH_MNT/bar $file_name
> +done
> +test_link $SCRATCH_MNT/A/foo $SCRATCH_MNT/bar
> +
> +# Group 4: Both files within a directory other than root
> +for file_name in $SCRATCH_MNT $SCRATCH_MNT/A $SCRATCH_MNT/A/bar
> $SCRATCH_MNT/A/foo; do
> + test_link $SCRATCH_MNT/A/foo $SCRATCH_MNT/A/bar $file_name
> +done
> +test_link $SCRATCH_MNT/A/foo $SCRATCH_MNT/A/bar
> +
> +#Group 5: Exercise name reuse : Link file in sub-directory
> +for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar
> $SCRATCH_MNT/A $SCRATCH_MNT/A/bar $SCRATCH_MNT/A/foo; do
> + test_link $SCRATCH_MNT/bar $SCRATCH_MNT/A/bar $file_name
> +done
> +test_link $SCRATCH_MNT/bar $SCRATCH_MNT/A/bar
> +
> +#Group 6: Exercise name reuse : Link file in parent directory
> +for file_name in $SCRATCH_MNT $SCRATCH_MNT/foo $SCRATCH_MNT/bar
> $SCRATCH_MNT/A $SCRATCH_MNT/A/bar $SCRATCH_MNT/A/foo; do
> + test_link $SCRATCH_MNT/A/bar $SCRATCH_MNT/bar $file_name
> +done
> +test_link $SCRATCH_MNT/A/bar $SCRATCH_MNT/bar
> +
> +test_end_time=$(date +%s.%N)
> +test_time_elapsed=$(echo "$test_end_time-$test_start_time" | bc)
> +echo "Test Elapsed time : $test_time_elapsed" >> $seqres.full
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/517-link.out b/tests/generic/517-link.out
> new file mode 100644
> index 0000000..381b644
> --- /dev/null
> +++ b/tests/generic/517-link.out
> @@ -0,0 +1,75 @@
> +QA output created by 517-link
> +
> +=== Test 1 : link /mnt/scratch/foo /mnt/scratch/bar with fsync
> /mnt/scratch ===
> +
> +=== Test 2 : link /mnt/scratch/foo /mnt/scratch/bar with fsync
> /mnt/scratch/foo ===
> +
> +=== Test 3 : link /mnt/scratch/foo /mnt/scratch/bar with fsync
> /mnt/scratch/bar ===
> +
> +=== Test 4 : link /mnt/scratch/foo /mnt/scratch/bar with sync ===
> +
> +=== Test 5 : link /mnt/scratch/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch ===
> +
> +=== Test 6 : link /mnt/scratch/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch/foo ===
> +
> +=== Test 7 : link /mnt/scratch/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch/bar ===
> +
> +=== Test 8 : link /mnt/scratch/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch/A ===
> +
> +=== Test 9 : link /mnt/scratch/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch/A/bar ===
> +
> +=== Test 10 : link /mnt/scratch/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch/A/foo ===
> +
> +=== Test 11 : link /mnt/scratch/foo /mnt/scratch/A/bar with sync ===
> +
> +=== Test 12 : link /mnt/scratch/A/foo /mnt/scratch/bar with fsync
> /mnt/scratch ===
> +
> +=== Test 13 : link /mnt/scratch/A/foo /mnt/scratch/bar with fsync
> /mnt/scratch/foo ===
> +
> +=== Test 14 : link /mnt/scratch/A/foo /mnt/scratch/bar with fsync
> /mnt/scratch/bar ===
> +
> +=== Test 15 : link /mnt/scratch/A/foo /mnt/scratch/bar with fsync
> /mnt/scratch/A ===
> +
> +=== Test 16 : link /mnt/scratch/A/foo /mnt/scratch/bar with fsync
> /mnt/scratch/A/bar ===
> +
> +=== Test 17 : link /mnt/scratch/A/foo /mnt/scratch/bar with fsync
> /mnt/scratch/A/foo ===
> +
> +=== Test 18 : link /mnt/scratch/A/foo /mnt/scratch/bar with sync ===
> +
> +=== Test 19 : link /mnt/scratch/A/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch ===
> +
> +=== Test 20 : link /mnt/scratch/A/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch/A ===
> +
> +=== Test 21 : link /mnt/scratch/A/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch/A/bar ===
> +
> +=== Test 22 : link /mnt/scratch/A/foo /mnt/scratch/A/bar with fsync
> /mnt/scratch/A/foo ===
> +
> +=== Test 23 : link /mnt/scratch/A/foo /mnt/scratch/A/bar with sync ===
> +
> +=== Test 24 : link /mnt/scratch/bar /mnt/scratch/A/bar with fsync
> /mnt/scratch ===
> +
> +=== Test 25 : link /mnt/scratch/bar /mnt/scratch/A/bar with fsync
> /mnt/scratch/foo ===
> +
> +=== Test 26 : link /mnt/scratch/bar /mnt/scratch/A/bar with fsync
> /mnt/scratch/bar ===
> +
> +=== Test 27 : link /mnt/scratch/bar /mnt/scratch/A/bar with fsync
> /mnt/scratch/A ===
> +
> +=== Test 28 : link /mnt/scratch/bar /mnt/scratch/A/bar with fsync
> /mnt/scratch/A/bar ===
> +
> +=== Test 29 : link /mnt/scratch/bar /mnt/scratch/A/bar with fsync
> /mnt/scratch/A/foo ===
> +
> +=== Test 30 : link /mnt/scratch/bar /mnt/scratch/A/bar with sync ===
> +
> +=== Test 31 : link /mnt/scratch/A/bar /mnt/scratch/bar with fsync
> /mnt/scratch ===
> +
> +=== Test 32 : link /mnt/scratch/A/bar /mnt/scratch/bar with fsync
> /mnt/scratch/foo ===
> +
> +=== Test 33 : link /mnt/scratch/A/bar /mnt/scratch/bar with fsync
> /mnt/scratch/bar ===
> +
> +=== Test 34 : link /mnt/scratch/A/bar /mnt/scratch/bar with fsync
> /mnt/scratch/A ===
> +
> +=== Test 35 : link /mnt/scratch/A/bar /mnt/scratch/bar with fsync
> /mnt/scratch/A/bar ===
> +
> +=== Test 36 : link /mnt/scratch/A/bar /mnt/scratch/bar with fsync
> /mnt/scratch/A/foo ===
> +
> +=== Test 37 : link /mnt/scratch/A/bar /mnt/scratch/bar with sync ===
> diff --git a/tests/generic/group b/tests/generic/group
> index 47de978..dc04152 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -519,3 +519,4 @@
> 514 auto quick clone
> 515 auto quick clone
> 516 auto quick dedupe clone
> +517-link crash
Should also be in the 'quick' and 'log' groups.
Not sure if it's worth adding the new 'crash' group. The test isn't
fundamentally different from other power failure tests.
Thanks.
>
>
> Thanks,
> Jayashree Mohan
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
