> 在 2017年12月6日，上午4:38，Amir Goldstein <amir73il@xxxxxxxxx> 写道：
>
> On Tue, Dec 5, 2017 at 5:50 PM, Chengguang Xu <cgxu519@xxxxxxxxxx> wrote:
>> Run syncfs and shutdown upper filesystem to check syncfs result.
>>
>
> Sorry to be zigzagging you from generic to overlay and back, but the test
> you wrote is completely generic, which is a good thing, so it belongs in
> generic IMO.
> Eryu?
If Eryu thinks it’s acceptable, I can move to generic again.
>
>> Signed-off-by: Chengguang Xu <cgxu519@xxxxxxxxxx>
>> ---
>> tests/overlay/045 | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> tests/overlay/045.out | 2 +
>> tests/overlay/group | 1 +
>> 3 files changed, 110 insertions(+)
>> create mode 100755 tests/overlay/045
>> create mode 100644 tests/overlay/045.out
>>
>> diff --git a/tests/overlay/045 b/tests/overlay/045
>> new file mode 100755
>> index 0000000..402a955
>> --- /dev/null
>> +++ b/tests/overlay/045
>> @@ -0,0 +1,107 @@
>> +#! /bin/bash
>> +# FS QA Test 045
>> +#
>> +# Inspired by syncfs bug of overlayfs.
>> +# Run syncfs and shutdown upper filesystem to check syncfs result.
>> +# Test will be skipped if underlying upper filesystem does not support
>> +# shutdown or syncfs syscall.
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2017 Chengguang Xu <cgxu519@xxxxxxxxxx>.
>> +# All Rights Reserved.
>> +#
>> +# 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"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=0
>> +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
>> +
>> +# Modify as appropriate.
>> +_supported_fs overlay
>> +_supported_os Linux
>> +_require_test
>> +_require_scratch
>> +_require_scratch_shutdown
>> +_require_xfs_io_command "syncfs"
>> +
>> +# if error
>> +
>> +PREFIX=${seq}-testfile
>> +TESTFILES=${PREFIX}-*
>> +FCNT=1000
>> +
>> +write_data()
>> +{
>> + if [ $1 -eq 0 ]; then
>> + TARGET=$TEST_DIR
>> + else
>> + TARGET=$SCRATCH_MNT
>> + fi
>> +
>> + rm -f $TARGET/$TESTFILES >/dev/null 2>&1
>> + for i in `seq 1 $FCNT`; do
>> + $XFS_IO_PROG -f -c "pwrite 1K 1K" \
>> + ${TARGET}/${PREFIX}-$i >/dev/null 2>&1
>> + done
>> +}
>> +
>> +
>> +check_data()
>> +{
>> + diff -r $TEST_DIR $SCRATCH_MNT
>
> TEST_DIR is typically filled with other stuff, so this won't work.
> I would have gone for md5sum -c $TEST_DIR/$seq-md5, but diff
> is fine as long as you keep it contained inside $TEST_DIR/$seq-dir
> or something.
In my test it works fine. but as you said it’s better make test dir inside $TEST_DIR.
>
>> + if [ $? -ne 0 ]; then
>> + status=1
>> + fi
>> +}
>> +
>> +_scratch_mkfs >/dev/null 2>&1
>> +_scratch_mount
>> +
>> +write_data 0
>
> Why 0/1? pass the TARGET path as argument.
> Or use md5sum instead or writing twice.
Need modify as your advice.
>
>> +echo 3 > /proc/sys/vm/drop_caches
>
> Why is this needed? if it is really needed add a comment why
Main purpose is to mitigate interference from write-back,
but it seems having side effect to outside, I think if
we have enough test files then avoid dropping caches is reasonable.
This is also the reason why I’m using diff two directories instead of
using md5sum to one test file.
>
>> +
>> +write_data 1
>> +$XFS_IO_PROG -c "syncfs" $SCRATCH_MNT/${PREFIX}-${FCNT} >/dev/null 2>&1
>> +
>> +_scratch_shutdown
>> +_scratch_cycle_mount
>> +check_data
>> +
>> +rm -f $TEST_DIR/$TESTFILES >/dev/null 2>&1
>> +rm -f $SCRATCH_MNT/$TESTFILES >/dev/null 2>&1
>
> In TEST_DIR we leave garbage behind on purpose, clean the dir (e.g. $seq-dir)
> on test start.
> scratch gets formatted so you don't need to clean it.
You are right, I did too much here.
>
>> +
>> +echo "Silence is golden"
>> +exit
>> diff --git a/tests/overlay/045.out b/tests/overlay/045.out
>> new file mode 100644
>> index 0000000..66276cf
>> --- /dev/null
>> +++ b/tests/overlay/045.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 045
>> +Silence is golden
>> diff --git a/tests/overlay/group b/tests/overlay/group
>> index b32c6cd..eccd7d0 100644
>> --- a/tests/overlay/group
>> +++ b/tests/overlay/group
>> @@ -47,3 +47,4 @@
>> 042 auto quick copyup hardlink
>> 043 auto quick copyup nonsamefs
>> 044 auto quick copyup hardlink nonsamefs
>> +045 auto quick sync
>
> Shutdown group seems in order.
I think shutdown is a way or method to make this test work,
but it seems not purpose of test itself.
>
> Thanks,
> Amir.
> --
> 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
--
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
