All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ext4/004: add dump/restore test
@ 2014-11-25 10:21 ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-11-25 10:21 UTC (permalink / raw)
  To: fstests, linux-ext4; +Cc: Xiaoguang Wang

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 <wangxg.fnst@cn.fujitsu.com>
---
 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`"
 
 # 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"
+}
+
 # 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
+}
+
+# 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()
+{
+	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"
+}
+
+_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
+	else
+		echo "Dump/Restore success"
+	fi
+
+	rm -rf restoresymtable
+	_diff_compare
+}
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+
+_require_scratch
+_require_dump_restore
+
+rm -f $seqres.full
+
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
+_scratch_mount || _fail "couldn't mount fs"
+
+umount $TEST_DEV
+_test_mkfs >> $seqres.full 2>&1
+_test_mount
+
+_workout
+
+status=0
+exit
diff --git a/tests/ext4/004.out b/tests/ext4/004.out
new file mode 100644
index 0000000..c9b125a
--- /dev/null
+++ b/tests/ext4/004.out
@@ -0,0 +1,7 @@
+QA output created by 004
+
+Run fsstress
+start Dump/Restore
+Dump/Restore success
+Comparing dump directory with restore directory
+Dump/Restore compare success
diff --git a/tests/ext4/group b/tests/ext4/group
index aa6a53b..e7f1f2a 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -6,6 +6,7 @@
 001 auto prealloc quick
 002 auto quick prealloc
 003 auto quick
+004 auto dump
 271 auto rw quick
 301 aio dangerous ioctl rw stress
 302 aio dangerous ioctl rw stress
@@ -14,4 +15,4 @@
 305 auto
 306 auto rw resize quick
 307 auto ioctl rw
-308 auto ioctl rw prealloc quick
\ No newline at end of file
+308 auto ioctl rw prealloc quick
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH] ext4/004: add dump/restore test
@ 2014-11-25 10:21 ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-11-25 10:21 UTC (permalink / raw)
  To: fstests, linux-ext4; +Cc: Xiaoguang Wang

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 <wangxg.fnst@cn.fujitsu.com>
---
 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`"
 
 # 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"
+}
+
 # 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
+}
+
+# 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()
+{
+	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"
+}
+
+_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
+	else
+		echo "Dump/Restore success"
+	fi
+
+	rm -rf restoresymtable
+	_diff_compare
+}
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+
+_require_scratch
+_require_dump_restore
+
+rm -f $seqres.full
+
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
+_scratch_mount || _fail "couldn't mount fs"
+
+umount $TEST_DEV
+_test_mkfs >> $seqres.full 2>&1
+_test_mount
+
+_workout
+
+status=0
+exit
diff --git a/tests/ext4/004.out b/tests/ext4/004.out
new file mode 100644
index 0000000..c9b125a
--- /dev/null
+++ b/tests/ext4/004.out
@@ -0,0 +1,7 @@
+QA output created by 004
+
+Run fsstress
+start Dump/Restore
+Dump/Restore success
+Comparing dump directory with restore directory
+Dump/Restore compare success
diff --git a/tests/ext4/group b/tests/ext4/group
index aa6a53b..e7f1f2a 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -6,6 +6,7 @@
 001 auto prealloc quick
 002 auto quick prealloc
 003 auto quick
+004 auto dump
 271 auto rw quick
 301 aio dangerous ioctl rw stress
 302 aio dangerous ioctl rw stress
@@ -14,4 +15,4 @@
 305 auto
 306 auto rw resize quick
 307 auto ioctl rw
-308 auto ioctl rw prealloc quick
\ No newline at end of file
+308 auto ioctl rw prealloc quick
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-11-25 10:21 ` Xiaoguang Wang
  (?)
@ 2014-12-01  5:45 ` Dave Chinner
  2014-12-03  2:44     ` Xiaoguang Wang
  -1 siblings, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2014-12-01  5:45 UTC (permalink / raw)
  To: Xiaoguang Wang; +Cc: fstests, linux-ext4

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 <wangxg.fnst@cn.fujitsu.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-12-01  5:45 ` Dave Chinner
@ 2014-12-03  2:44     ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-03  2:44 UTC (permalink / raw)
  To: Dave Chinner; +Cc: fstests, linux-ext4

hello,

    Thanks, I'll send a new version today according to your comments.

Regards,
Xiaoguang Wang

On 12/01/2014 01:45 PM, Dave Chinner wrote:
> 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 <wangxg.fnst@cn.fujitsu.com>
>> ---
>>  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.
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
@ 2014-12-03  2:44     ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-03  2:44 UTC (permalink / raw)
  To: Dave Chinner; +Cc: fstests, linux-ext4

hello,

    Thanks, I'll send a new version today according to your comments.

Regards,
Xiaoguang Wang

On 12/01/2014 01:45 PM, Dave Chinner wrote:
> 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 <wangxg.fnst@cn.fujitsu.com>
>> ---
>>  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.
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2] ext4/004: add dump/restore test
  2014-12-03  2:44     ` Xiaoguang Wang
@ 2014-12-03  6:40       ` Xiaoguang Wang
  -1 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-03  6:40 UTC (permalink / raw)
  To: fstests, linux-ext4; +Cc: david, Xiaoguang Wang

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 <wangxg.fnst@cn.fujitsu.com>
---
 common/config      |  2 ++
 tests/ext4/004     | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/004.out |  2 ++
 tests/ext4/group   |  3 +-
 4 files changed, 95 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..e9971fd 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 DUMP_PROG="`set_prog_path dump`"
+export RESTORE_PROG="`set_prog_path restore`"
 
 # Generate a comparable xfsprogs version number in the form of
 # major * 10000 + minor * 100 + release
diff --git a/tests/ext4/004 b/tests/ext4/004
new file mode 100755
index 0000000..00262dc
--- /dev/null
+++ b/tests/ext4/004
@@ -0,0 +1,89 @@
+#! /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 $restoredir
+	rm -rf $dumpdir
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+dump_dir=$SCRATCH_MNT/dump_restore_dir
+restore_dir=$TEST_DIR/dump_restore_dir
+
+_workout()
+{
+	echo "Run fsstress" >> $seqres.full 2>&1
+	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" >> $seqres.full 2>&1
+	cd $TEST_DIR
+
+	$DUMP_PROG -0 -f - $dump_dir 2>/dev/null | $RESTORE_PROG -urvf - >> $seqres.full 2>&1
+	if [ $? -ne 0 ];then
+		_fail "Dump/Restore failed"
+	fi
+
+	rm -rf restoresymtable
+}
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+
+_require_test
+_require_scratch
+
+_require_command $DUMP_PROG
+_require_command $RESTORE_PROG
+
+rm -f $seqres.full
+echo "Silence is golden"
+
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
+_scratch_mount
+rm -rf $TEST_DIR/*
+
+_workout
+diff -r $dump_dir $restore_dir
+
+status=0
+exit
diff --git a/tests/ext4/004.out b/tests/ext4/004.out
new file mode 100644
index 0000000..af8614a
--- /dev/null
+++ b/tests/ext4/004.out
@@ -0,0 +1,2 @@
+QA output created by 004
+Silence is golden
diff --git a/tests/ext4/group b/tests/ext4/group
index aa6a53b..e7f1f2a 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -6,6 +6,7 @@
 001 auto prealloc quick
 002 auto quick prealloc
 003 auto quick
+004 auto dump
 271 auto rw quick
 301 aio dangerous ioctl rw stress
 302 aio dangerous ioctl rw stress
@@ -14,4 +15,4 @@
 305 auto
 306 auto rw resize quick
 307 auto ioctl rw
-308 auto ioctl rw prealloc quick
\ No newline at end of file
+308 auto ioctl rw prealloc quick
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v2] ext4/004: add dump/restore test
@ 2014-12-03  6:40       ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-03  6:40 UTC (permalink / raw)
  To: fstests, linux-ext4; +Cc: david, Xiaoguang Wang

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 <wangxg.fnst@cn.fujitsu.com>
---
 common/config      |  2 ++
 tests/ext4/004     | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/004.out |  2 ++
 tests/ext4/group   |  3 +-
 4 files changed, 95 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..e9971fd 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 DUMP_PROG="`set_prog_path dump`"
+export RESTORE_PROG="`set_prog_path restore`"
 
 # Generate a comparable xfsprogs version number in the form of
 # major * 10000 + minor * 100 + release
diff --git a/tests/ext4/004 b/tests/ext4/004
new file mode 100755
index 0000000..00262dc
--- /dev/null
+++ b/tests/ext4/004
@@ -0,0 +1,89 @@
+#! /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 $restoredir
+	rm -rf $dumpdir
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+dump_dir=$SCRATCH_MNT/dump_restore_dir
+restore_dir=$TEST_DIR/dump_restore_dir
+
+_workout()
+{
+	echo "Run fsstress" >> $seqres.full 2>&1
+	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" >> $seqres.full 2>&1
+	cd $TEST_DIR
+
+	$DUMP_PROG -0 -f - $dump_dir 2>/dev/null | $RESTORE_PROG -urvf - >> $seqres.full 2>&1
+	if [ $? -ne 0 ];then
+		_fail "Dump/Restore failed"
+	fi
+
+	rm -rf restoresymtable
+}
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+
+_require_test
+_require_scratch
+
+_require_command $DUMP_PROG
+_require_command $RESTORE_PROG
+
+rm -f $seqres.full
+echo "Silence is golden"
+
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
+_scratch_mount
+rm -rf $TEST_DIR/*
+
+_workout
+diff -r $dump_dir $restore_dir
+
+status=0
+exit
diff --git a/tests/ext4/004.out b/tests/ext4/004.out
new file mode 100644
index 0000000..af8614a
--- /dev/null
+++ b/tests/ext4/004.out
@@ -0,0 +1,2 @@
+QA output created by 004
+Silence is golden
diff --git a/tests/ext4/group b/tests/ext4/group
index aa6a53b..e7f1f2a 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -6,6 +6,7 @@
 001 auto prealloc quick
 002 auto quick prealloc
 003 auto quick
+004 auto dump
 271 auto rw quick
 301 aio dangerous ioctl rw stress
 302 aio dangerous ioctl rw stress
@@ -14,4 +15,4 @@
 305 auto
 306 auto rw resize quick
 307 auto ioctl rw
-308 auto ioctl rw prealloc quick
\ No newline at end of file
+308 auto ioctl rw prealloc quick
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] ext4/004: add dump/restore test
  2014-12-03  6:40       ` Xiaoguang Wang
  (?)
@ 2014-12-03  7:49       ` Eryu Guan
  2014-12-03  9:47           ` Xiaoguang Wang
  -1 siblings, 1 reply; 23+ messages in thread
From: Eryu Guan @ 2014-12-03  7:49 UTC (permalink / raw)
  To: Xiaoguang Wang; +Cc: fstests, linux-ext4, david

On Wed, Dec 03, 2014 at 02:40:59PM +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.

For the subject "[PATCH v2] ext4/004: add dump/restore test"

The seq number can be skipped for new test.

> 
> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
> ---
>  common/config      |  2 ++
>  tests/ext4/004     | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/ext4/004.out |  2 ++
>  tests/ext4/group   |  3 +-
>  4 files changed, 95 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..e9971fd 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 DUMP_PROG="`set_prog_path dump`"
> +export RESTORE_PROG="`set_prog_path restore`"
>  
>  # Generate a comparable xfsprogs version number in the form of
>  # major * 10000 + minor * 100 + release
> diff --git a/tests/ext4/004 b/tests/ext4/004
> new file mode 100755
> index 0000000..00262dc
> --- /dev/null
> +++ b/tests/ext4/004
> @@ -0,0 +1,89 @@
> +#! /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 $restoredir
> +	rm -rf $dumpdir

$restoredir should be rm'ed before running the test, as part of test
setup not cleanup, as Dave pointed out.

$dumpdir doesn't need cleanup, scratch dev will be recreated in next
test run, Dave pointed this out too.

Also the var names should be "$restore_dir" and "$dump_dir"

> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +dump_dir=$SCRATCH_MNT/dump_restore_dir
> +restore_dir=$TEST_DIR/dump_restore_dir
> +
> +_workout()

Local function doesn't need "_" prefix

> +{
> +	echo "Run fsstress" >> $seqres.full 2>&1

2>&1 of an echo seems useless to me

> +	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" >> $seqres.full 2>&1

here too

> +	cd $TEST_DIR
> +
> +	$DUMP_PROG -0 -f - $dump_dir 2>/dev/null | $RESTORE_PROG -urvf - >> $seqres.full 2>&1
> +	if [ $? -ne 0 ];then
> +		_fail "Dump/Restore failed"
> +	fi
> +
> +	rm -rf restoresymtable
> +}
> +
> +# real QA test starts here
> +_supported_fs ext4
> +_supported_os Linux
> +
> +_require_test
> +_require_scratch
> +
> +_require_command $DUMP_PROG
> +_require_command $RESTORE_PROG
> +
> +rm -f $seqres.full
> +echo "Silence is golden"
> +
> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
> +_scratch_mount
> +rm -rf $TEST_DIR/*

Just remove $restore_dir and restoresymtable, not other files, test
dev is supposed to be aged.

Thanks,
Eryu

> +
> +_workout
> +diff -r $dump_dir $restore_dir
> +
> +status=0
> +exit
> diff --git a/tests/ext4/004.out b/tests/ext4/004.out
> new file mode 100644
> index 0000000..af8614a
> --- /dev/null
> +++ b/tests/ext4/004.out
> @@ -0,0 +1,2 @@
> +QA output created by 004
> +Silence is golden
> diff --git a/tests/ext4/group b/tests/ext4/group
> index aa6a53b..e7f1f2a 100644
> --- a/tests/ext4/group
> +++ b/tests/ext4/group
> @@ -6,6 +6,7 @@
>  001 auto prealloc quick
>  002 auto quick prealloc
>  003 auto quick
> +004 auto dump
>  271 auto rw quick
>  301 aio dangerous ioctl rw stress
>  302 aio dangerous ioctl rw stress
> @@ -14,4 +15,4 @@
>  305 auto
>  306 auto rw resize quick
>  307 auto ioctl rw
> -308 auto ioctl rw prealloc quick
> \ No newline at end of file
> +308 auto ioctl rw prealloc quick
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] ext4/004: add dump/restore test
  2014-12-03  7:49       ` Eryu Guan
@ 2014-12-03  9:47           ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-03  9:47 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, linux-ext4, david

Hi,

    Thanks for reviewing, and I have one question below, please have a check :)

On 12/03/2014 03:49 PM, Eryu Guan wrote:
> On Wed, Dec 03, 2014 at 02:40:59PM +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.
> 
> For the subject "[PATCH v2] ext4/004: add dump/restore test"
> 
> The seq number can be skipped for new test.

OK, will remove it.
> 
>>
>> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
>> ---
>>  common/config      |  2 ++
>>  tests/ext4/004     | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/ext4/004.out |  2 ++
>>  tests/ext4/group   |  3 +-
>>  4 files changed, 95 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..e9971fd 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 DUMP_PROG="`set_prog_path dump`"
>> +export RESTORE_PROG="`set_prog_path restore`"
>>  
>>  # Generate a comparable xfsprogs version number in the form of
>>  # major * 10000 + minor * 100 + release
>> diff --git a/tests/ext4/004 b/tests/ext4/004
>> new file mode 100755
>> index 0000000..00262dc
>> --- /dev/null
>> +++ b/tests/ext4/004
>> @@ -0,0 +1,89 @@
>> +#! /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 $restoredir
>> +	rm -rf $dumpdir
> 
> $restoredir should be rm'ed before running the test, as part of test
> setup not cleanup, as Dave pointed out.

I wonder why we should not rm $restoredir, in which there will be much
data generated by restore, and these data are meaningless ?
> 
> $dumpdir doesn't need cleanup, scratch dev will be recreated in next
> test run, Dave pointed this out too.

OK, sorry for missing this.
> 
> Also the var names should be "$restore_dir" and "$dump_dir"
> 
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +dump_dir=$SCRATCH_MNT/dump_restore_dir
>> +restore_dir=$TEST_DIR/dump_restore_dir
>> +
>> +_workout()
> 
> Local function doesn't need "_" prefix

OK.
> 
>> +{
>> +	echo "Run fsstress" >> $seqres.full 2>&1
> 
> 2>&1 of an echo seems useless to me
> 
>> +	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" >> $seqres.full 2>&1
> 
> here too
> 
>> +	cd $TEST_DIR
>> +
>> +	$DUMP_PROG -0 -f - $dump_dir 2>/dev/null | $RESTORE_PROG -urvf - >> $seqres.full 2>&1
>> +	if [ $? -ne 0 ];then
>> +		_fail "Dump/Restore failed"
>> +	fi
>> +
>> +	rm -rf restoresymtable
>> +}
>> +
>> +# real QA test starts here
>> +_supported_fs ext4
>> +_supported_os Linux
>> +
>> +_require_test
>> +_require_scratch
>> +
>> +_require_command $DUMP_PROG
>> +_require_command $RESTORE_PROG
>> +
>> +rm -f $seqres.full
>> +echo "Silence is golden"
>> +
>> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
>> +_scratch_mount
>> +rm -rf $TEST_DIR/*
> 
> Just remove $restore_dir and restoresymtable, not other files, test
> dev is supposed to be aged.

OK, i see, thanks.
I'll send a V3 later.

Regards,
Xiaoguang Wang

> 
> Thanks,
> Eryu
> 
>> +
>> +_workout
>> +diff -r $dump_dir $restore_dir
>> +
>> +status=0
>> +exit
>> diff --git a/tests/ext4/004.out b/tests/ext4/004.out
>> new file mode 100644
>> index 0000000..af8614a
>> --- /dev/null
>> +++ b/tests/ext4/004.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 004
>> +Silence is golden
>> diff --git a/tests/ext4/group b/tests/ext4/group
>> index aa6a53b..e7f1f2a 100644
>> --- a/tests/ext4/group
>> +++ b/tests/ext4/group
>> @@ -6,6 +6,7 @@
>>  001 auto prealloc quick
>>  002 auto quick prealloc
>>  003 auto quick
>> +004 auto dump
>>  271 auto rw quick
>>  301 aio dangerous ioctl rw stress
>>  302 aio dangerous ioctl rw stress
>> @@ -14,4 +15,4 @@
>>  305 auto
>>  306 auto rw resize quick
>>  307 auto ioctl rw
>> -308 auto ioctl rw prealloc quick
>> \ No newline at end of file
>> +308 auto ioctl rw prealloc quick
>> -- 
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] ext4/004: add dump/restore test
@ 2014-12-03  9:47           ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-03  9:47 UTC (permalink / raw)
  To: Eryu Guan; +Cc: fstests, linux-ext4, david

Hi,

    Thanks for reviewing, and I have one question below, please have a check :)

On 12/03/2014 03:49 PM, Eryu Guan wrote:
> On Wed, Dec 03, 2014 at 02:40:59PM +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.
> 
> For the subject "[PATCH v2] ext4/004: add dump/restore test"
> 
> The seq number can be skipped for new test.

OK, will remove it.
> 
>>
>> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
>> ---
>>  common/config      |  2 ++
>>  tests/ext4/004     | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/ext4/004.out |  2 ++
>>  tests/ext4/group   |  3 +-
>>  4 files changed, 95 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..e9971fd 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 DUMP_PROG="`set_prog_path dump`"
>> +export RESTORE_PROG="`set_prog_path restore`"
>>  
>>  # Generate a comparable xfsprogs version number in the form of
>>  # major * 10000 + minor * 100 + release
>> diff --git a/tests/ext4/004 b/tests/ext4/004
>> new file mode 100755
>> index 0000000..00262dc
>> --- /dev/null
>> +++ b/tests/ext4/004
>> @@ -0,0 +1,89 @@
>> +#! /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 $restoredir
>> +	rm -rf $dumpdir
> 
> $restoredir should be rm'ed before running the test, as part of test
> setup not cleanup, as Dave pointed out.

I wonder why we should not rm $restoredir, in which there will be much
data generated by restore, and these data are meaningless ?
> 
> $dumpdir doesn't need cleanup, scratch dev will be recreated in next
> test run, Dave pointed this out too.

OK, sorry for missing this.
> 
> Also the var names should be "$restore_dir" and "$dump_dir"
> 
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +dump_dir=$SCRATCH_MNT/dump_restore_dir
>> +restore_dir=$TEST_DIR/dump_restore_dir
>> +
>> +_workout()
> 
> Local function doesn't need "_" prefix

OK.
> 
>> +{
>> +	echo "Run fsstress" >> $seqres.full 2>&1
> 
> 2>&1 of an echo seems useless to me
> 
>> +	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" >> $seqres.full 2>&1
> 
> here too
> 
>> +	cd $TEST_DIR
>> +
>> +	$DUMP_PROG -0 -f - $dump_dir 2>/dev/null | $RESTORE_PROG -urvf - >> $seqres.full 2>&1
>> +	if [ $? -ne 0 ];then
>> +		_fail "Dump/Restore failed"
>> +	fi
>> +
>> +	rm -rf restoresymtable
>> +}
>> +
>> +# real QA test starts here
>> +_supported_fs ext4
>> +_supported_os Linux
>> +
>> +_require_test
>> +_require_scratch
>> +
>> +_require_command $DUMP_PROG
>> +_require_command $RESTORE_PROG
>> +
>> +rm -f $seqres.full
>> +echo "Silence is golden"
>> +
>> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
>> +_scratch_mount
>> +rm -rf $TEST_DIR/*
> 
> Just remove $restore_dir and restoresymtable, not other files, test
> dev is supposed to be aged.

OK, i see, thanks.
I'll send a V3 later.

Regards,
Xiaoguang Wang

> 
> Thanks,
> Eryu
> 
>> +
>> +_workout
>> +diff -r $dump_dir $restore_dir
>> +
>> +status=0
>> +exit
>> diff --git a/tests/ext4/004.out b/tests/ext4/004.out
>> new file mode 100644
>> index 0000000..af8614a
>> --- /dev/null
>> +++ b/tests/ext4/004.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 004
>> +Silence is golden
>> diff --git a/tests/ext4/group b/tests/ext4/group
>> index aa6a53b..e7f1f2a 100644
>> --- a/tests/ext4/group
>> +++ b/tests/ext4/group
>> @@ -6,6 +6,7 @@
>>  001 auto prealloc quick
>>  002 auto quick prealloc
>>  003 auto quick
>> +004 auto dump
>>  271 auto rw quick
>>  301 aio dangerous ioctl rw stress
>>  302 aio dangerous ioctl rw stress
>> @@ -14,4 +15,4 @@
>>  305 auto
>>  306 auto rw resize quick
>>  307 auto ioctl rw
>> -308 auto ioctl rw prealloc quick
>> \ No newline at end of file
>> +308 auto ioctl rw prealloc quick
>> -- 
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH v2] ext4/004: add dump/restore test
  2014-12-03  9:47           ` Xiaoguang Wang
  (?)
@ 2014-12-04  7:29           ` Eryu Guan
  2014-12-05  7:16               ` Xiaoguang Wang
  -1 siblings, 1 reply; 23+ messages in thread
From: Eryu Guan @ 2014-12-04  7:29 UTC (permalink / raw)
  To: Xiaoguang Wang; +Cc: fstests, linux-ext4, david

On Wed, Dec 03, 2014 at 05:47:18PM +0800, Xiaoguang Wang wrote:
> Hi,
> 
>     Thanks for reviewing, and I have one question below, please have a check :)
> 
> On 12/03/2014 03:49 PM, Eryu Guan wrote:
> > On Wed, Dec 03, 2014 at 02:40:59PM +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.
> > 
> > For the subject "[PATCH v2] ext4/004: add dump/restore test"
> > 
> > The seq number can be skipped for new test.
> 
> OK, will remove it.
> > 
> >>
> >> Signed-off-by: Xiaoguang Wang <wangxg.fnst@cn.fujitsu.com>
> >> ---
> >>  common/config      |  2 ++
> >>  tests/ext4/004     | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/ext4/004.out |  2 ++
> >>  tests/ext4/group   |  3 +-
> >>  4 files changed, 95 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..e9971fd 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 DUMP_PROG="`set_prog_path dump`"
> >> +export RESTORE_PROG="`set_prog_path restore`"
> >>  
> >>  # Generate a comparable xfsprogs version number in the form of
> >>  # major * 10000 + minor * 100 + release
> >> diff --git a/tests/ext4/004 b/tests/ext4/004
> >> new file mode 100755
> >> index 0000000..00262dc
> >> --- /dev/null
> >> +++ b/tests/ext4/004
> >> @@ -0,0 +1,89 @@
> >> +#! /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 $restoredir
> >> +	rm -rf $dumpdir
> > 
> > $restoredir should be rm'ed before running the test, as part of test
> > setup not cleanup, as Dave pointed out.
> 
> I wonder why we should not rm $restoredir, in which there will be much
> data generated by restore, and these data are meaningless ?

Usually we leave TEST_DEV populated and being used over time, so you
don't have to remove new files generated by the test.

But if the generated data takes a lot of space and fills TEST_DEV
quickly you can remove the files in cleanup, maybe add some comments
to explain why.

Thanks,
Eryu
> > 
> > $dumpdir doesn't need cleanup, scratch dev will be recreated in next
> > test run, Dave pointed this out too.
> 
> OK, sorry for missing this.
> > 
> > Also the var names should be "$restore_dir" and "$dump_dir"
> > 
> >> +}
> >> +
> >> +# get standard environment, filters and checks
> >> +. ./common/rc
> >> +. ./common/filter
> >> +
> >> +dump_dir=$SCRATCH_MNT/dump_restore_dir
> >> +restore_dir=$TEST_DIR/dump_restore_dir
> >> +
> >> +_workout()
> > 
> > Local function doesn't need "_" prefix
> 
> OK.
> > 
> >> +{
> >> +	echo "Run fsstress" >> $seqres.full 2>&1
> > 
> > 2>&1 of an echo seems useless to me
> > 
> >> +	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" >> $seqres.full 2>&1
> > 
> > here too
> > 
> >> +	cd $TEST_DIR
> >> +
> >> +	$DUMP_PROG -0 -f - $dump_dir 2>/dev/null | $RESTORE_PROG -urvf - >> $seqres.full 2>&1
> >> +	if [ $? -ne 0 ];then
> >> +		_fail "Dump/Restore failed"
> >> +	fi
> >> +
> >> +	rm -rf restoresymtable
> >> +}
> >> +
> >> +# real QA test starts here
> >> +_supported_fs ext4
> >> +_supported_os Linux
> >> +
> >> +_require_test
> >> +_require_scratch
> >> +
> >> +_require_command $DUMP_PROG
> >> +_require_command $RESTORE_PROG
> >> +
> >> +rm -f $seqres.full
> >> +echo "Silence is golden"
> >> +
> >> +_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
> >> +_scratch_mount
> >> +rm -rf $TEST_DIR/*
> > 
> > Just remove $restore_dir and restoresymtable, not other files, test
> > dev is supposed to be aged.
> 
> OK, i see, thanks.
> I'll send a V3 later.
> 
> Regards,
> Xiaoguang Wang
> 
> > 
> > Thanks,
> > Eryu
> > 
> >> +
> >> +_workout
> >> +diff -r $dump_dir $restore_dir
> >> +
> >> +status=0
> >> +exit
> >> diff --git a/tests/ext4/004.out b/tests/ext4/004.out
> >> new file mode 100644
> >> index 0000000..af8614a
> >> --- /dev/null
> >> +++ b/tests/ext4/004.out
> >> @@ -0,0 +1,2 @@
> >> +QA output created by 004
> >> +Silence is golden
> >> diff --git a/tests/ext4/group b/tests/ext4/group
> >> index aa6a53b..e7f1f2a 100644
> >> --- a/tests/ext4/group
> >> +++ b/tests/ext4/group
> >> @@ -6,6 +6,7 @@
> >>  001 auto prealloc quick
> >>  002 auto quick prealloc
> >>  003 auto quick
> >> +004 auto dump
> >>  271 auto rw quick
> >>  301 aio dangerous ioctl rw stress
> >>  302 aio dangerous ioctl rw stress
> >> @@ -14,4 +15,4 @@
> >>  305 auto
> >>  306 auto rw resize quick
> >>  307 auto ioctl rw
> >> -308 auto ioctl rw prealloc quick
> >> \ No newline at end of file
> >> +308 auto ioctl rw prealloc quick
> >> -- 
> >> 1.8.3.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe fstests" in
> >> the body of a message to majordomo@vger.kernel.org
> >> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v3] ext4: add dump/restore test
  2014-12-04  7:29           ` Eryu Guan
@ 2014-12-05  7:16               ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-05  7:16 UTC (permalink / raw)
  To: fstests, linux-ext4; +Cc: Xiaoguang Wang

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 <wangxg.fnst@cn.fujitsu.com>
---
 common/config      |  2 ++
 tests/ext4/004     | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/004.out |  2 ++
 tests/ext4/group   |  3 +-
 4 files changed, 95 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..e9971fd 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 DUMP_PROG="`set_prog_path dump`"
+export RESTORE_PROG="`set_prog_path restore`"
 
 # Generate a comparable xfsprogs version number in the form of
 # major * 10000 + minor * 100 + release
diff --git a/tests/ext4/004 b/tests/ext4/004
new file mode 100755
index 0000000..b24b627
--- /dev/null
+++ b/tests/ext4/004
@@ -0,0 +1,89 @@
+#! /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.*
+	# remove the generated data, which is much and meaningless.
+	rm -rf $restore_dir
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+dump_dir=$SCRATCH_MNT/dump_restore_dir
+restore_dir=$TEST_DIR/dump_restore_dir
+
+workout()
+{
+	echo "Run fsstress" >> $seqres.full
+	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" >> $seqres.full
+	cd $TEST_DIR
+
+	$DUMP_PROG -0 -f - $dump_dir 2>/dev/null | $RESTORE_PROG -urvf - >> $seqres.full 2>&1
+	if [ $? -ne 0 ];then
+		_fail "Dump/Restore failed"
+	fi
+
+	rm -rf restoresymtable
+}
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+
+_require_test
+_require_scratch
+
+_require_command $DUMP_PROG
+_require_command $RESTORE_PROG
+
+rm -f $seqres.full
+echo "Silence is golden"
+
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
+_scratch_mount
+rm -rf $restore_dir $TEST_DIR/restoresymtable
+
+workout
+diff -r $dump_dir $restore_dir
+
+status=0
+exit
diff --git a/tests/ext4/004.out b/tests/ext4/004.out
new file mode 100644
index 0000000..af8614a
--- /dev/null
+++ b/tests/ext4/004.out
@@ -0,0 +1,2 @@
+QA output created by 004
+Silence is golden
diff --git a/tests/ext4/group b/tests/ext4/group
index aa6a53b..e7f1f2a 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -6,6 +6,7 @@
 001 auto prealloc quick
 002 auto quick prealloc
 003 auto quick
+004 auto dump
 271 auto rw quick
 301 aio dangerous ioctl rw stress
 302 aio dangerous ioctl rw stress
@@ -14,4 +15,4 @@
 305 auto
 306 auto rw resize quick
 307 auto ioctl rw
-308 auto ioctl rw prealloc quick
\ No newline at end of file
+308 auto ioctl rw prealloc quick
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH v3] ext4: add dump/restore test
@ 2014-12-05  7:16               ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-05  7:16 UTC (permalink / raw)
  To: fstests, linux-ext4; +Cc: Xiaoguang Wang

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 <wangxg.fnst@cn.fujitsu.com>
---
 common/config      |  2 ++
 tests/ext4/004     | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/ext4/004.out |  2 ++
 tests/ext4/group   |  3 +-
 4 files changed, 95 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..e9971fd 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 DUMP_PROG="`set_prog_path dump`"
+export RESTORE_PROG="`set_prog_path restore`"
 
 # Generate a comparable xfsprogs version number in the form of
 # major * 10000 + minor * 100 + release
diff --git a/tests/ext4/004 b/tests/ext4/004
new file mode 100755
index 0000000..b24b627
--- /dev/null
+++ b/tests/ext4/004
@@ -0,0 +1,89 @@
+#! /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.*
+	# remove the generated data, which is much and meaningless.
+	rm -rf $restore_dir
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+dump_dir=$SCRATCH_MNT/dump_restore_dir
+restore_dir=$TEST_DIR/dump_restore_dir
+
+workout()
+{
+	echo "Run fsstress" >> $seqres.full
+	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" >> $seqres.full
+	cd $TEST_DIR
+
+	$DUMP_PROG -0 -f - $dump_dir 2>/dev/null | $RESTORE_PROG -urvf - >> $seqres.full 2>&1
+	if [ $? -ne 0 ];then
+		_fail "Dump/Restore failed"
+	fi
+
+	rm -rf restoresymtable
+}
+
+# real QA test starts here
+_supported_fs ext4
+_supported_os Linux
+
+_require_test
+_require_scratch
+
+_require_command $DUMP_PROG
+_require_command $RESTORE_PROG
+
+rm -f $seqres.full
+echo "Silence is golden"
+
+_scratch_mkfs_sized $((512 * 1024 * 1024)) >> $seqres.full 2>&1
+_scratch_mount
+rm -rf $restore_dir $TEST_DIR/restoresymtable
+
+workout
+diff -r $dump_dir $restore_dir
+
+status=0
+exit
diff --git a/tests/ext4/004.out b/tests/ext4/004.out
new file mode 100644
index 0000000..af8614a
--- /dev/null
+++ b/tests/ext4/004.out
@@ -0,0 +1,2 @@
+QA output created by 004
+Silence is golden
diff --git a/tests/ext4/group b/tests/ext4/group
index aa6a53b..e7f1f2a 100644
--- a/tests/ext4/group
+++ b/tests/ext4/group
@@ -6,6 +6,7 @@
 001 auto prealloc quick
 002 auto quick prealloc
 003 auto quick
+004 auto dump
 271 auto rw quick
 301 aio dangerous ioctl rw stress
 302 aio dangerous ioctl rw stress
@@ -14,4 +15,4 @@
 305 auto
 306 auto rw resize quick
 307 auto ioctl rw
-308 auto ioctl rw prealloc quick
\ No newline at end of file
+308 auto ioctl rw prealloc quick
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-11-25 10:21 ` Xiaoguang Wang
  (?)
  (?)
@ 2014-12-06 21:40 ` Theodore Ts'o
  2014-12-06 23:44   ` Dave Chinner
  2014-12-08  1:44     ` Xiaoguang Wang
  -1 siblings, 2 replies; 23+ messages in thread
From: Theodore Ts'o @ 2014-12-06 21:40 UTC (permalink / raw)
  To: Xiaoguang Wang; +Cc: fstests, linux-ext4

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 <wangxg.fnst@cn.fujitsu.com>

One question --- what is the intent of this test?  Is it to test the
kernel, or the dump/restore program?  I have not bothered putting
regression tests for e2fsprogs in xfstests, because if I'm developing
e2fsprogs, it actually makes much more sense to put the regression
tests in the e2fsprogs git tree.

If this is because it's more convenient to put this in xfsprogs
because it has fsstress, maybe we should adjust the groups that it is
in so that it's not in auto or quick, but some other group?  Or add it
to some group like "userspace" so I can exclude it when I'm mostly
interested in testing development kernels?

Thanks,

						- Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-12-06 21:40 ` [PATCH] ext4/004: " Theodore Ts'o
@ 2014-12-06 23:44   ` Dave Chinner
  2014-12-08  1:44     ` Xiaoguang Wang
  1 sibling, 0 replies; 23+ messages in thread
From: Dave Chinner @ 2014-12-06 23:44 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Xiaoguang Wang, fstests, linux-ext4

On Sat, Dec 06, 2014 at 04:40:40PM -0500, Theodore Ts'o wrote:
> 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 <wangxg.fnst@cn.fujitsu.com>
> 
> One question --- what is the intent of this test?  Is it to test the
> kernel, or the dump/restore program?  I have not bothered putting
> regression tests for e2fsprogs in xfstests, because if I'm developing
> e2fsprogs, it actually makes much more sense to put the regression
> tests in the e2fsprogs git tree.

xfstests is for testing the entire suite of filesystem software,
from userspace to kernel code. It is not a "kernel only" regression
test suite - we test all the XFS userspace tools in the xfs subdir, 
including xfsdump/xfsrestore...

> If this is because it's more convenient to put this in xfsprogs
> because it has fsstress, maybe we should adjust the groups that it is
> in so that it's not in auto or quick, but some other group?

Why? If it's testing that something works properly and runs with
hardware that everyone has available, we always want to run it as
part of the auto group. And we already have a "dump" group to
trigger running xfsdump/xfsrestore tests, so it would make sense to
use that as well.

> to some group like "userspace" so I can exclude it when I'm mostly
> interested in testing development kernels?

Which means you have no idea whether the kernel changes you are
testing breaks userspace tools or vice versa?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-12-06 21:40 ` [PATCH] ext4/004: " Theodore Ts'o
@ 2014-12-08  1:44     ` Xiaoguang Wang
  2014-12-08  1:44     ` Xiaoguang Wang
  1 sibling, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-08  1:44 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: fstests, linux-ext4

Hi,

On 12/07/2014 05:40 AM, Theodore Ts'o wrote:
> 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 <wangxg.fnst@cn.fujitsu.com>
> 
> One question --- what is the intent of this test?  Is it to test the
> kernel, or the dump/restore program?  I have not bothered putting
> regression tests for e2fsprogs in xfstests, because if I'm developing
> e2fsprogs, it actually makes much more sense to put the regression
> tests in the e2fsprogs git tree.

Yeah, my intent is to test dump/restore program, and indeed I imitate that
how xfs to test xfsdump/xfsrestore, xfs puts xfsdump/xfsrestore tests in
corresponding xfs directory.

Regards,
Xiaoguang Wang


> 
> If this is because it's more convenient to put this in xfsprogs
> because it has fsstress, maybe we should adjust the groups that it is
> in so that it's not in auto or quick, but some other group?  Or add it
> to some group like "userspace" so I can exclude it when I'm mostly
> interested in testing development kernels?
> 
> Thanks,
> 
> 						- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
@ 2014-12-08  1:44     ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-08  1:44 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: fstests, linux-ext4

Hi,

On 12/07/2014 05:40 AM, Theodore Ts'o wrote:
> 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 <wangxg.fnst@cn.fujitsu.com>
> 
> One question --- what is the intent of this test?  Is it to test the
> kernel, or the dump/restore program?  I have not bothered putting
> regression tests for e2fsprogs in xfstests, because if I'm developing
> e2fsprogs, it actually makes much more sense to put the regression
> tests in the e2fsprogs git tree.

Yeah, my intent is to test dump/restore program, and indeed I imitate that
how xfs to test xfsdump/xfsrestore, xfs puts xfsdump/xfsrestore tests in
corresponding xfs directory.

Regards,
Xiaoguang Wang


> 
> If this is because it's more convenient to put this in xfsprogs
> because it has fsstress, maybe we should adjust the groups that it is
> in so that it's not in auto or quick, but some other group?  Or add it
> to some group like "userspace" so I can exclude it when I'm mostly
> interested in testing development kernels?
> 
> Thanks,
> 
> 						- Ted
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-12-08  1:44     ` Xiaoguang Wang
  (?)
@ 2014-12-16  2:58     ` Theodore Ts'o
  2014-12-16  3:59       ` Dave Chinner
  2014-12-17  5:16         ` Xiaoguang Wang
  -1 siblings, 2 replies; 23+ messages in thread
From: Theodore Ts'o @ 2014-12-16  2:58 UTC (permalink / raw)
  To: Xiaoguang Wang; +Cc: fstests, linux-ext4

On Mon, Dec 08, 2014 at 09:44:48AM +0800, Xiaoguang Wang wrote:
> 
> Yeah, my intent is to test dump/restore program, and indeed I imitate that
> how xfs to test xfsdump/xfsrestore, xfs puts xfsdump/xfsrestore tests in
> corresponding xfs directory.

I'm finding that the test takes 6-7 minutes to run, partially because
it's writing close to half a gigabyte worth of data for the
dump/restore.  Is this really necessary?  Can we perhaps cut down the
amount of data generated by running fsstress?  The time to run the
full set of tests is taking longer and longer, and one answer might be
that for tests that are irrelevant for kernel and which take a long
time, I'll just supress them in my test runs.  But maybe we can just
significantly cut back the amount of data to be backed up and
restored?  How much do we really need to create in order for you to
feel that you've adequately tested dump/restore?

And I'll note that using the current fsstress arguments, you are only
creating regular files and directories, and there are no symlinks,
device nodes, or FIFO's being created to test whether those files are
correctly being backed up and restored.

Cheers,

						- Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-12-16  2:58     ` Theodore Ts'o
@ 2014-12-16  3:59       ` Dave Chinner
  2014-12-16 15:53         ` Theodore Ts'o
  2014-12-17  5:16         ` Xiaoguang Wang
  1 sibling, 1 reply; 23+ messages in thread
From: Dave Chinner @ 2014-12-16  3:59 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Xiaoguang Wang, fstests, linux-ext4

On Mon, Dec 15, 2014 at 09:58:27PM -0500, Theodore Ts'o wrote:
> On Mon, Dec 08, 2014 at 09:44:48AM +0800, Xiaoguang Wang wrote:
> > 
> > Yeah, my intent is to test dump/restore program, and indeed I imitate that
> > how xfs to test xfsdump/xfsrestore, xfs puts xfsdump/xfsrestore tests in
> > corresponding xfs directory.
> 
> I'm finding that the test takes 6-7 minutes to run, partially because
> it's writing close to half a gigabyte worth of data for the
> dump/restore.  Is this really necessary?  Can we perhaps cut down the
> amount of data generated by running fsstress?

Yes, that is too long for an "auto" test. A couple of minutes is
about the limit we should be trying to stick to for auto tests; we
don't really add any extra coverage by making such tests run for a
long time.

As it is, most of the xfsdump/restore tests take around 30-60s to
run, so that's probably a good guide to follow for this.

> And I'll note that using the current fsstress arguments, you are only
> creating regular files and directories, and there are no symlinks,
> device nodes, or FIFO's being created to test whether those files are
> correctly being backed up and restored.

Probably a good idea, too. Thanks for looking at this, Ted.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-12-16  3:59       ` Dave Chinner
@ 2014-12-16 15:53         ` Theodore Ts'o
  2014-12-16 19:36           ` Dave Chinner
  0 siblings, 1 reply; 23+ messages in thread
From: Theodore Ts'o @ 2014-12-16 15:53 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Xiaoguang Wang, fstests, linux-ext4

On Tue, Dec 16, 2014 at 02:59:43PM +1100, Dave Chinner wrote:
> 
> Yes, that is too long for an "auto" test. A couple of minutes is
> about the limit we should be trying to stick to for auto tests; we
> don't really add any extra coverage by making such tests run for a
> long time.
> 
> As it is, most of the xfsdump/restore tests take around 30-60s to
> run, so that's probably a good guide to follow for this.

I'm currently running with a patch that cuts it down to ~22 seconds
(and about 60 megs worth of data in the dump and restore directory).
I'll send a patch....

> > And I'll note that using the current fsstress arguments, you are only
> > creating regular files and directories, and there are no symlinks,
> > device nodes, or FIFO's being created to test whether those files are
> > correctly being backed up and restored.
> 
> Probably a good idea, too. Thanks for looking at this, Ted.

I looked a bit more closely at this, and unfortunately it's not a
quick fix.  The issue is that the test is currently using diff -r to
verify that the restore directory == the dump directory, and diff
doesn't handle special files.

Doing this right might require writing a special case directory
comparison script in perl, or some such; if we do this, then we can
also have the directory comparison tool also check to make sure the
uid/gid/mode bits match, which diff also doesn't handle.

					- Ted

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-12-16 15:53         ` Theodore Ts'o
@ 2014-12-16 19:36           ` Dave Chinner
  0 siblings, 0 replies; 23+ messages in thread
From: Dave Chinner @ 2014-12-16 19:36 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: Xiaoguang Wang, fstests, linux-ext4

On Tue, Dec 16, 2014 at 10:53:28AM -0500, Theodore Ts'o wrote:
> On Tue, Dec 16, 2014 at 02:59:43PM +1100, Dave Chinner wrote:
> > 
> > Yes, that is too long for an "auto" test. A couple of minutes is
> > about the limit we should be trying to stick to for auto tests; we
> > don't really add any extra coverage by making such tests run for a
> > long time.
> > 
> > As it is, most of the xfsdump/restore tests take around 30-60s to
> > run, so that's probably a good guide to follow for this.
> 
> I'm currently running with a patch that cuts it down to ~22 seconds
> (and about 60 megs worth of data in the dump and restore directory).
> I'll send a patch....
> 
> > > And I'll note that using the current fsstress arguments, you are only
> > > creating regular files and directories, and there are no symlinks,
> > > device nodes, or FIFO's being created to test whether those files are
> > > correctly being backed up and restored.
> > 
> > Probably a good idea, too. Thanks for looking at this, Ted.
> 
> I looked a bit more closely at this, and unfortunately it's not a
> quick fix.  The issue is that the test is currently using diff -r to
> verify that the restore directory == the dump directory, and diff
> doesn't handle special files.
> 
> Doing this right might require writing a special case directory
> comparison script in perl, or some such; if we do this, then we can
> also have the directory comparison tool also check to make sure the
> uid/gid/mode bits match, which diff also doesn't handle.

See common/dump, specifically _ls_compare_sub,
_ls_nodate_compare_sub, _diff_compare_sub and _diff_compare_eas.
THere are also routines for creating directory structures with
symlinks, hardlinks and xattrs for dump/restore testing. Yes, it's
all xfsdump/restore specific, but we already have a wheel ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
  2014-12-16  2:58     ` Theodore Ts'o
@ 2014-12-17  5:16         ` Xiaoguang Wang
  2014-12-17  5:16         ` Xiaoguang Wang
  1 sibling, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-17  5:16 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: fstests, linux-ext4, Dave Chinner

Hi,

On 12/16/2014 10:58 AM, Theodore Ts'o wrote:
> On Mon, Dec 08, 2014 at 09:44:48AM +0800, Xiaoguang Wang wrote:
>>
>> Yeah, my intent is to test dump/restore program, and indeed I imitate that
>> how xfs to test xfsdump/xfsrestore, xfs puts xfsdump/xfsrestore tests in
>> corresponding xfs directory.
> 
> I'm finding that the test takes 6-7 minutes to run, partially because
> it's writing close to half a gigabyte worth of data for the
> dump/restore.  Is this really necessary?  Can we perhaps cut down the
> amount of data generated by running fsstress?  The time to run the
> full set of tests is taking longer and longer, and one answer might be
> that for tests that are irrelevant for kernel and which take a long
> time, I'll just supress them in my test runs.  But maybe we can just
> significantly cut back the amount of data to be backed up and
> restored?  How much do we really need to create in order for you to
> feel that you've adequately tested dump/restore?

I choose to make a 512MB file system and the arguments "-z -f creat=5 -f write=20
-f mkdir=5 -n 1000 -p 15" is to make sure that the dump operation would be against
a full file system. I have read your patch "ext4/004: limit the amount of data written
so test runs faster", it looks OK to me, sorry for the bothering.

> 
> And I'll note that using the current fsstress arguments, you are only
> creating regular files and directories, and there are no symlinks,
> device nodes, or FIFO's being created to test whether those files are
> correctly being backed up and restored.

Yeah, I skipped these special files deliberately. Originally I used the arguments
"-n 1000 -p 15". In RHEL7.0GA, restore tool will trigger a segmentation fault,
the call trace is:
---------------------------------------------------------------------
Program terminated with signal 11, Segmentation fault.
#0  0x000000000040df24 in readxattr ()
...
(gdb) bt
#0  0x000000000040df24 in readxattr ()
#1  0x000000000040e160 in extractattr ()
#2  0x000000000040e4e2 in extractfile ()
#3  0x00000000004097b0 in createleaves ()
#4  0x0000000000403ae6 in main ()
--------------------------------------------------------------------- 
The dump/restore version is 0.4b44, which is already latest. I have sent a bug
report to Stelian Pop, maintainer of dump/restore. Later if this issue is fixed,
I'll send patch to improve this 004 case, let it handle types of files, sorry.

Regards,
Xiaoguang Wang 
> 
> Cheers,
> 
> 						- Ted
> .
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] ext4/004: add dump/restore test
@ 2014-12-17  5:16         ` Xiaoguang Wang
  0 siblings, 0 replies; 23+ messages in thread
From: Xiaoguang Wang @ 2014-12-17  5:16 UTC (permalink / raw)
  To: Theodore Ts'o; +Cc: fstests, linux-ext4, Dave Chinner

Hi,

On 12/16/2014 10:58 AM, Theodore Ts'o wrote:
> On Mon, Dec 08, 2014 at 09:44:48AM +0800, Xiaoguang Wang wrote:
>>
>> Yeah, my intent is to test dump/restore program, and indeed I imitate that
>> how xfs to test xfsdump/xfsrestore, xfs puts xfsdump/xfsrestore tests in
>> corresponding xfs directory.
> 
> I'm finding that the test takes 6-7 minutes to run, partially because
> it's writing close to half a gigabyte worth of data for the
> dump/restore.  Is this really necessary?  Can we perhaps cut down the
> amount of data generated by running fsstress?  The time to run the
> full set of tests is taking longer and longer, and one answer might be
> that for tests that are irrelevant for kernel and which take a long
> time, I'll just supress them in my test runs.  But maybe we can just
> significantly cut back the amount of data to be backed up and
> restored?  How much do we really need to create in order for you to
> feel that you've adequately tested dump/restore?

I choose to make a 512MB file system and the arguments "-z -f creat=5 -f write=20
-f mkdir=5 -n 1000 -p 15" is to make sure that the dump operation would be against
a full file system. I have read your patch "ext4/004: limit the amount of data written
so test runs faster", it looks OK to me, sorry for the bothering.

> 
> And I'll note that using the current fsstress arguments, you are only
> creating regular files and directories, and there are no symlinks,
> device nodes, or FIFO's being created to test whether those files are
> correctly being backed up and restored.

Yeah, I skipped these special files deliberately. Originally I used the arguments
"-n 1000 -p 15". In RHEL7.0GA, restore tool will trigger a segmentation fault,
the call trace is:
---------------------------------------------------------------------
Program terminated with signal 11, Segmentation fault.
#0  0x000000000040df24 in readxattr ()
...
(gdb) bt
#0  0x000000000040df24 in readxattr ()
#1  0x000000000040e160 in extractattr ()
#2  0x000000000040e4e2 in extractfile ()
#3  0x00000000004097b0 in createleaves ()
#4  0x0000000000403ae6 in main ()
--------------------------------------------------------------------- 
The dump/restore version is 0.4b44, which is already latest. I have sent a bug
report to Stelian Pop, maintainer of dump/restore. Later if this issue is fixed,
I'll send patch to improve this 004 case, let it handle types of files, sorry.

Regards,
Xiaoguang Wang 
> 
> Cheers,
> 
> 						- Ted
> .
> 


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-12-17  5:19 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-25 10:21 [PATCH] ext4/004: add dump/restore test Xiaoguang Wang
2014-11-25 10:21 ` Xiaoguang Wang
2014-12-01  5:45 ` Dave Chinner
2014-12-03  2:44   ` Xiaoguang Wang
2014-12-03  2:44     ` Xiaoguang Wang
2014-12-03  6:40     ` [PATCH v2] " Xiaoguang Wang
2014-12-03  6:40       ` Xiaoguang Wang
2014-12-03  7:49       ` Eryu Guan
2014-12-03  9:47         ` Xiaoguang Wang
2014-12-03  9:47           ` Xiaoguang Wang
2014-12-04  7:29           ` Eryu Guan
2014-12-05  7:16             ` [PATCH v3] ext4: " Xiaoguang Wang
2014-12-05  7:16               ` Xiaoguang Wang
2014-12-06 21:40 ` [PATCH] ext4/004: " Theodore Ts'o
2014-12-06 23:44   ` Dave Chinner
2014-12-08  1:44   ` Xiaoguang Wang
2014-12-08  1:44     ` Xiaoguang Wang
2014-12-16  2:58     ` Theodore Ts'o
2014-12-16  3:59       ` Dave Chinner
2014-12-16 15:53         ` Theodore Ts'o
2014-12-16 19:36           ` Dave Chinner
2014-12-17  5:16       ` Xiaoguang Wang
2014-12-17  5:16         ` Xiaoguang Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.