From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ipmail07.adl2.internode.on.net ([150.101.137.131]:32832 "EHLO ipmail07.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751627AbaLAFpa (ORCPT ); Mon, 1 Dec 2014 00:45:30 -0500 Date: Mon, 1 Dec 2014 16:45:14 +1100 From: Dave Chinner Subject: Re: [PATCH] ext4/004: add dump/restore test Message-ID: <20141201054514.GL16151@dastard> References: <1416910869-28538-1-git-send-email-wangxg.fnst@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1416910869-28538-1-git-send-email-wangxg.fnst@cn.fujitsu.com> Sender: fstests-owner@vger.kernel.org To: Xiaoguang Wang Cc: fstests@vger.kernel.org, linux-ext4@vger.kernel.org List-ID: On Tue, Nov 25, 2014 at 06:21:09PM +0800, Xiaoguang Wang wrote: > This test case will first use fsstress to fill a file system, then > dump it to standard output and restore it from standard input, finally > check that the original contents and the new contents generated by > restore tool will be same. > > Signed-off-by: Xiaoguang Wang > --- > common/config | 2 ++ > common/rc | 7 ++++ > tests/ext4/004 | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/ext4/004.out | 7 ++++ > tests/ext4/group | 3 +- > 5 files changed, 122 insertions(+), 1 deletion(-) > create mode 100755 tests/ext4/004 > create mode 100644 tests/ext4/004.out > > diff --git a/common/config b/common/config > index 1cb08c0..8ea70ec 100644 > --- a/common/config > +++ b/common/config > @@ -188,6 +188,8 @@ export LOGGER_PROG="`set_prog_path logger`" > export DBENCH_PROG="`set_prog_path dbench`" > export DMSETUP_PROG="`set_prog_path dmsetup`" > export WIPEFS_PROG="`set_prog_path wipefs`" > +export E2FS_DUMP_PROG="`set_prog_path dump`" > +export E2FS_RESTORE_PROG="`set_prog_path restore`" So now we have DUMPE2FS_PROG and E2FS_DUMP_PROG, which are different programs altogether but are going to be very easy to confuse. I would suggest that follow the convention of all the other *_PROG variables dump -> $DUMP_PROG is probably a good idea to avoid confusion. > # Generate a comparable xfsprogs version number in the form of > # major * 10000 + minor * 100 + release > diff --git a/common/rc b/common/rc > index d5e3aff..ef7939d 100644 > --- a/common/rc > +++ b/common/rc > @@ -2258,6 +2258,13 @@ _require_fio() > [ $? -eq 0 ] || _notrun "$FIO_PROG too old, see $seqres.full" > } > > +# Check that dump/restore is present > +_require_dump_restore() > +{ > + _require_command $E2FS_DUMP_PROG "dump" > + _require_command $E2FS_RESTORE_PROG "restore" > +} No need for the wrapper, just call _require_command directly. > + > # Does freeze work on this fs? > _require_freeze() > { > diff --git a/tests/ext4/004 b/tests/ext4/004 > new file mode 100755 > index 0000000..cbf35e5 > --- /dev/null > +++ b/tests/ext4/004 > @@ -0,0 +1,104 @@ > +#! /bin/bash > +# FSQA Test No. 004 > +# > +# Test "dump | restore"(as opposed to a tape) > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2014 Fujitsu. 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=1 # failure is the default! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $tmp.* > + rm -rf $restore_dir > + rm -rf $dump_dir > + _scratch_unmount scratch does not need to be unmounted here, nor does it need to be cleaned up. Also, you have to assume that the previous run of the test did not clean up (i.e. system crashed before cleanup ran), so you are going to want to do the rm on $restore_dir as part of the test setup, not cleanup.... > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +dump_dir=$SCRATCH_MNT/dump_restore_dir > +restore_dir=$TEST_DIR/dump_restore_dir > + > +_diff_compare() "_" prefix is for library functions (i.e. from common/...), not local functions. > +{ > + echo "Comparing dump directory with restore directory" > + diff -rs $dump_dir $restore_dir >> $seqres.full 2>&1 > + if [ $? -ne 0 ];then > + echo "Dump/Restore compare failed" > + exit > + fi > + echo "Dump/Restore compare success" > +} There is no need for this entire function. just running: diff -r $dump_dir $restore_dir will tell us everything. i.e. it is silent if both directories are the same, but will throw output if there are differences. That will output will cause the golden image match (and hence the test) to fail.... > + > +_workout() > +{ > + echo "" > + echo "Run fsstress" > + args=`_scale_fsstress_args -z -f creat=5 -f write=20 -f mkdir=5 -n 1000 -p 15 -d $dump_dir` > + echo "fsstress $args" >> $seqres.full > + > + $FSSTRESS_PROG $args >> $seqres.full 2>&1 > + > + echo "start Dump/Restore" > + cd $TEST_DIR > + > + $E2FS_DUMP_PROG -0 -f - $dump_dir 2>/dev/null | $E2FS_RESTORE_PROG -urvf - >> $seqres.full 2>&1 > + if [ $? -ne 0 ];then > + echo "Dump/Restore failed" > + exit _fail? > + else > + echo "Dump/Restore success" > + fi And there's no need to echo anything on success. > + rm -rf restoresymtable > + _diff_compare > +} No need to put all of this in a function. > +# real QA test starts here > +_supported_fs ext4 > +_supported_os Linux > + > +_require_scratch > +_require_dump_restore _require_test > + > +rm -f $seqres.full > + > +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1 > +_scratch_mount || _fail "couldn't mount fs" No need to check for mount failure. > + > +umount $TEST_DEV > +_test_mkfs >> $seqres.full 2>&1 > +_test_mount No. That is just not allowed. The test device should *never* be mkfs'd by a test. It shouldn't even be unmounted. Prep should be just an rm... Cheers, Dave. -- Dave Chinner david@fromorbit.com