diff mbox

[RFC,3/3] fstests: generic: Check the fs after each FUA writes

Message ID 20180314090230.25055-3-wqu@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Qu Wenruo March 14, 2018, 9:02 a.m. UTC
Basic test case which triggers fsstress with dm-log-writes, and then
check the fs after each FUA writes.
With needed infrastructure and special handlers for journal based fs.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
In my test, xfs and btrfs survives while ext4 would report error during fsck.

My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
ourselves. Not sure if it's allowed.
---
 common/dmlogwrites    | 119 ++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/481     | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/generic/481.out |   2 +
 tests/generic/group   |   1 +
 4 files changed, 246 insertions(+)
 create mode 100755 tests/generic/481
 create mode 100644 tests/generic/481.out

Comments

Amir Goldstein March 14, 2018, 10:27 a.m. UTC | #1
On Wed, Mar 14, 2018 at 11:02 AM, Qu Wenruo <wqu@suse.com> wrote:
> Basic test case which triggers fsstress with dm-log-writes, and then
> check the fs after each FUA writes.
> With needed infrastructure and special handlers for journal based fs.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> In my test, xfs and btrfs survives while ext4 would report error during fsck.
>
> My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
> ourselves. Not sure if it's allowed.

Definitely not allowed.

I suppose you could either create an LVM volume on scratch for the tested
fs and leave room for a snapshot target on scratch or do something similar
with LOGWRITES_DEV, which may be more appropriate in the context of
common/dmlogwrites helper.

I see you are posting this generic series independently from the btrsf specific
series, but the two series have conflicting changes. How do intent for
the final merged result to look like?


> ---
>  common/dmlogwrites    | 119 ++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/481     | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/481.out |   2 +
>  tests/generic/group   |   1 +
>  4 files changed, 246 insertions(+)
>  create mode 100755 tests/generic/481
>  create mode 100644 tests/generic/481.out
>
> diff --git a/common/dmlogwrites b/common/dmlogwrites
> index 467b872e..258f5887 100644
> --- a/common/dmlogwrites
> +++ b/common/dmlogwrites
> @@ -126,3 +126,122 @@ _log_writes_cleanup()
>         $UDEV_SETTLE_PROG >/dev/null 2>&1
>         _log_writes_remove
>  }
> +
> +# Convert log writes mark to entry number
> +# Result entry number is output to stdout, could be empty if not found
> +_log_writes_mark_to_entry_number()
> +{
> +       local _mark=$1
> +       local ret
> +
> +       [ -z "$_mark" ] && _fatal \
> +               "mark must be given for _log_writes_mark_to_entry_number"
> +
> +       ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
> +               --end-mark $_mark 2> /dev/null)
> +       [ -z "$ret" ] && return
> +       ret=$(echo "$ret" | cut -f1 -d\@)
> +       echo "mark $_mark has entry number $ret" >> $seqres.full
> +       echo "$ret"
> +}
> +
> +# Find next fua write entry number
> +# Result entry number is output to stdout, could be empty if not found
> +_log_writes_find_next_fua()
> +{
> +       local _start_entry=$1
> +       local ret
> +
> +       [ -z "$_start_entry" ] && _start_entry=0
> +       ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
> +             --next-fua --start-entry $_start_entry 2> /dev/null)
> +       [ -z "$ret" ] && return
> +
> +       ret=$(echo "$ret" | cut -f1 -d\@)
> +       echo "next fua is entry number $ret" >> $seqres.full
> +       echo "$ret"
> +}
> +
> +# Replay log range to specified entry
> +# $1:  End entry. The entry with this number *WILL* be replayed
> +# $2:  Start entry. If not specified, start from the first entry.
> +# $3:  Verbose. If set to 'y' do verbose output
> +_log_writes_replay_log_entry_range()
> +{
> +       local _end=$1
> +       local _start=$2
> +       local _verbose=$3
> +
> +       [ -z "$_end" ] && _fatal \
> +       "end entry must be specified for _log_writes_replay_log_entry_range"
> +
> +       [ "x$verbose" == "xy" ] && _verbose="-v"

Those xy tricks are really not needed with bash.
"$verbose" == "y" is nicer and better.


> +       [ -z "$_start" ] && _start=0
> +       [ "$_start" -gt "$_end" ] && _fatal \
> +               "wrong parameter order for _log_writes_replay_log_entry_range:start=$_start end=$_end"
> +
> +       # To ensure we replay the last entry, for _start == 0 case,
> +       # we need to manually increase the end entry number to ensure
> +       # it's played
> +       echo "=== replay from $_start to $_end ===" >> $seqres.full
> +       $here/src/log-writes/replay-log --log $LOGWRITES_DEV \
> +               --replay $SCRATCH_DEV --start-entry $_start \
> +               --limit $(($_end - $_start + 1)) $_verbose \
> +               >> $seqres.full 2>&1
> +       [ $? -ne 0 ] && _fatal "replay failed"
> +}
> +
> +_log_writes_cleanup_snapshot()
> +{
> +       $UDEV_SETTLE_PROG > /dev/null 2>&1
> +       $DMSETUP_PROG remove "$DMLOGWRITES_SNAPSHOT_NAME" > /dev/null 2>&1
> +       $DMSETUP_PROG remove "$DMLOGWRITES_ORIGIN_NAME" > /dev/null 2>&1
> +}
> +
> +# Helper to create snapshot of a the replayed device
> +# Useful for journal based filesystem such as XFS and Ext4 to replay
> +# their journal without touching the replay device, so that we can
> +# continue replaying other than replay from the beginning.
> +# $1:  Snapshot device
> +_log_writes_create_snapshot()
> +{
> +       _require_dm_target snapshot
> +
> +       local snapshot_dev=$1
> +       local cow_base=""
> +
> +       [ -z "$snapshot_dev" ] && _fatal \
> +               "@device must be specified for _log_writes_create_snapshot"
> +       local size=$(blockdev --getsz $SCRATCH_DEV)
> +       [ -z "$size" ] && _fatal \
> +               "failed to get device size for _log_writes_create_snapshot"
> +
> +       _log_writes_cleanup_snapshot
> +
> +       DMLOGWRITES_ORIGIN_NAME="log-writes-origin"
> +       DMLOGWRITES_SNAPSHOT_NAME="log-writes-snapshot"
> +       DMLOGWRITES_ORIGIN_TABLE="0 $size snapshot-origin $SCRATCH_DEV"
> +       DMLOGWRITES_SNAPSHOT_TABLE="0 $size snapshot /dev/mapper/$DMLOGWRITES_ORIGIN_NAME $snapshot_dev N 512"
> +
> +       $UDEV_SETTLE_PROG >/dev/null 2>&1
> +       $DMSETUP_PROG create $DMLOGWRITES_ORIGIN_NAME --table "$DMLOGWRITES_ORIGIN_TABLE" ||\
> +               _fatal "failed to create snapshot-origin of log writes target"
> +       $DMSETUP_PROG mknodes > /dev/null 2>&1
> +       $UDEV_SETTLE_PROG >/dev/null 2>&1
> +       $DMSETUP_PROG create $DMLOGWRITES_SNAPSHOT_NAME --table "$DMLOGWRITES_SNAPSHOT_TABLE" ||\
> +               _fatal "failed to create snapshot of log writes target"
> +       $DMSETUP_PROG mknodes > /dev/null 2>&1
> +}
> +
> +_log_writes_mount_snapshot()
> +{
> +       _scratch_options mount
> +       $MOUNT_PROG -t $FSTYP `_common_dev_mount_options $*` $SCRATCH_OPTIONS \
> +               "/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME" $SCRATCH_MNT
> +}
> +
> +_log_writes_unmount_snapshot()
> +{
> +       $UMOUNT_PROG $SCRATCH_MNT
> +}
> +
> diff --git a/tests/generic/481 b/tests/generic/481
> new file mode 100755
> index 00000000..3985493d
> --- /dev/null
> +++ b/tests/generic/481
> @@ -0,0 +1,124 @@
> +#! /bin/bash
> +# FS QA Test 481
> +#
> +# Test filesystem consistency after each FUA operation
> +#
> +# Will do log replay and check the filesystem.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2018 SuSE.  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 /
> +       $KILLALL_PROG -KILL -q $FSSTRESS_PROG &> /dev/null
> +       _log_writes_cleanup_snapshot &> /dev/null
> +       _log_writes_cleanup &> /dev/null
> +
> +       # We use $TEST_DEV as snapshot COW device, which can't be
> +       # mounted/recognized as normal fs, need to recreate it
> +       # or fstest will complain about it
> +       _mkfs_dev $TEST_DEV > /dev/null 2>&1
> +       rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmlogwrites
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +
> +_require_command "$KILLALL_PROG" killall
> +# Use $TEST_DEV as snapshot CoW device
> +_require_test
> +# Use $SCRATCH_DEV as replay device
> +_require_scratch
> +# and we need extra device as log device
> +_require_log_writes
> +
> +
> +workload=$(( 512 * $LOAD_FACTOR ))
> +nr_threads=$(($($here/src/feature -o) * $LOAD_FACTOR))
> +
> +_test_unmount
> +_log_writes_init
> +_log_writes_mkfs >> $seqres.full 2>&1
> +_log_writes_mark mkfs
> +
> +_log_writes_mount
> +run_check $FSSTRESS_PROG -w -n $workload -p $nr_threads -d $SCRATCH_MNT \
> +       $FSSTRESS_AVOID > /dev/null 2>&1
> +_log_writes_unmount
> +
> +_log_writes_remove
> +prev=$(_log_writes_mark_to_entry_number mkfs)
> +[ -z "$prev" ] && _fail "failed to locate entry mark 'mkfs'"
> +cur=$(_log_writes_find_next_fua $prev)
> +[ -z "$cur" ] && _fail "failed to locate next FUA write"
> +
> +_log_writes_replay_log_entry_range $prev
> +while [ ! -z "$cur" ]; do
> +       _log_writes_replay_log_entry_range $cur $prev >> $seqres.full
> +
> +       echo "=== Replay to entry number $cur ===" >> $seqres.full
> +       # Here we need extra mount to replay the log, mainly for journal based
> +       # fs, as their fsck will report dirty log as error. So do snapshot
> +       # to replay, so we can still continue replaying
> +       _log_writes_create_snapshot $TEST_DEV
> +       _log_writes_mount_snapshot
> +       _log_writes_unmount_snapshot
> +       _check_generic_filesystem "/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME"

Maybe _log_writes_check_snapshot helper?

Thanks,
Amir.
--
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
Qu Wenruo March 14, 2018, 10:33 a.m. UTC | #2
On 2018年03月14日 18:27, Amir Goldstein wrote:
> On Wed, Mar 14, 2018 at 11:02 AM, Qu Wenruo <wqu@suse.com> wrote:
>> Basic test case which triggers fsstress with dm-log-writes, and then
>> check the fs after each FUA writes.
>> With needed infrastructure and special handlers for journal based fs.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> In my test, xfs and btrfs survives while ext4 would report error during fsck.
>>
>> My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
>> ourselves. Not sure if it's allowed.
> 
> Definitely not allowed.
> 
> I suppose you could either create an LVM volume on scratch for the tested
> fs and leave room for a snapshot target on scratch or do something similar
> with LOGWRITES_DEV, which may be more appropriate in the context of
> common/dmlogwrites helper.

This sounds much better.

> 
> I see you are posting this generic series independently from the btrsf specific
> series, but the two series have conflicting changes. How do intent for
> the final merged result to look like?

Btrfs specific one will be discard and merged into this.
The found btrfs specific problem proves to be harmless now.

Thanks,
Qu

> 
> 
>> ---
>>  common/dmlogwrites    | 119 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/generic/481     | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/generic/481.out |   2 +
>>  tests/generic/group   |   1 +
>>  4 files changed, 246 insertions(+)
>>  create mode 100755 tests/generic/481
>>  create mode 100644 tests/generic/481.out
>>
>> diff --git a/common/dmlogwrites b/common/dmlogwrites
>> index 467b872e..258f5887 100644
>> --- a/common/dmlogwrites
>> +++ b/common/dmlogwrites
>> @@ -126,3 +126,122 @@ _log_writes_cleanup()
>>         $UDEV_SETTLE_PROG >/dev/null 2>&1
>>         _log_writes_remove
>>  }
>> +
>> +# Convert log writes mark to entry number
>> +# Result entry number is output to stdout, could be empty if not found
>> +_log_writes_mark_to_entry_number()
>> +{
>> +       local _mark=$1
>> +       local ret
>> +
>> +       [ -z "$_mark" ] && _fatal \
>> +               "mark must be given for _log_writes_mark_to_entry_number"
>> +
>> +       ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
>> +               --end-mark $_mark 2> /dev/null)
>> +       [ -z "$ret" ] && return
>> +       ret=$(echo "$ret" | cut -f1 -d\@)
>> +       echo "mark $_mark has entry number $ret" >> $seqres.full
>> +       echo "$ret"
>> +}
>> +
>> +# Find next fua write entry number
>> +# Result entry number is output to stdout, could be empty if not found
>> +_log_writes_find_next_fua()
>> +{
>> +       local _start_entry=$1
>> +       local ret
>> +
>> +       [ -z "$_start_entry" ] && _start_entry=0
>> +       ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
>> +             --next-fua --start-entry $_start_entry 2> /dev/null)
>> +       [ -z "$ret" ] && return
>> +
>> +       ret=$(echo "$ret" | cut -f1 -d\@)
>> +       echo "next fua is entry number $ret" >> $seqres.full
>> +       echo "$ret"
>> +}
>> +
>> +# Replay log range to specified entry
>> +# $1:  End entry. The entry with this number *WILL* be replayed
>> +# $2:  Start entry. If not specified, start from the first entry.
>> +# $3:  Verbose. If set to 'y' do verbose output
>> +_log_writes_replay_log_entry_range()
>> +{
>> +       local _end=$1
>> +       local _start=$2
>> +       local _verbose=$3
>> +
>> +       [ -z "$_end" ] && _fatal \
>> +       "end entry must be specified for _log_writes_replay_log_entry_range"
>> +
>> +       [ "x$verbose" == "xy" ] && _verbose="-v"
> 
> Those xy tricks are really not needed with bash.
> "$verbose" == "y" is nicer and better.
> 
> 
>> +       [ -z "$_start" ] && _start=0
>> +       [ "$_start" -gt "$_end" ] && _fatal \
>> +               "wrong parameter order for _log_writes_replay_log_entry_range:start=$_start end=$_end"
>> +
>> +       # To ensure we replay the last entry, for _start == 0 case,
>> +       # we need to manually increase the end entry number to ensure
>> +       # it's played
>> +       echo "=== replay from $_start to $_end ===" >> $seqres.full
>> +       $here/src/log-writes/replay-log --log $LOGWRITES_DEV \
>> +               --replay $SCRATCH_DEV --start-entry $_start \
>> +               --limit $(($_end - $_start + 1)) $_verbose \
>> +               >> $seqres.full 2>&1
>> +       [ $? -ne 0 ] && _fatal "replay failed"
>> +}
>> +
>> +_log_writes_cleanup_snapshot()
>> +{
>> +       $UDEV_SETTLE_PROG > /dev/null 2>&1
>> +       $DMSETUP_PROG remove "$DMLOGWRITES_SNAPSHOT_NAME" > /dev/null 2>&1
>> +       $DMSETUP_PROG remove "$DMLOGWRITES_ORIGIN_NAME" > /dev/null 2>&1
>> +}
>> +
>> +# Helper to create snapshot of a the replayed device
>> +# Useful for journal based filesystem such as XFS and Ext4 to replay
>> +# their journal without touching the replay device, so that we can
>> +# continue replaying other than replay from the beginning.
>> +# $1:  Snapshot device
>> +_log_writes_create_snapshot()
>> +{
>> +       _require_dm_target snapshot
>> +
>> +       local snapshot_dev=$1
>> +       local cow_base=""
>> +
>> +       [ -z "$snapshot_dev" ] && _fatal \
>> +               "@device must be specified for _log_writes_create_snapshot"
>> +       local size=$(blockdev --getsz $SCRATCH_DEV)
>> +       [ -z "$size" ] && _fatal \
>> +               "failed to get device size for _log_writes_create_snapshot"
>> +
>> +       _log_writes_cleanup_snapshot
>> +
>> +       DMLOGWRITES_ORIGIN_NAME="log-writes-origin"
>> +       DMLOGWRITES_SNAPSHOT_NAME="log-writes-snapshot"
>> +       DMLOGWRITES_ORIGIN_TABLE="0 $size snapshot-origin $SCRATCH_DEV"
>> +       DMLOGWRITES_SNAPSHOT_TABLE="0 $size snapshot /dev/mapper/$DMLOGWRITES_ORIGIN_NAME $snapshot_dev N 512"
>> +
>> +       $UDEV_SETTLE_PROG >/dev/null 2>&1
>> +       $DMSETUP_PROG create $DMLOGWRITES_ORIGIN_NAME --table "$DMLOGWRITES_ORIGIN_TABLE" ||\
>> +               _fatal "failed to create snapshot-origin of log writes target"
>> +       $DMSETUP_PROG mknodes > /dev/null 2>&1
>> +       $UDEV_SETTLE_PROG >/dev/null 2>&1
>> +       $DMSETUP_PROG create $DMLOGWRITES_SNAPSHOT_NAME --table "$DMLOGWRITES_SNAPSHOT_TABLE" ||\
>> +               _fatal "failed to create snapshot of log writes target"
>> +       $DMSETUP_PROG mknodes > /dev/null 2>&1
>> +}
>> +
>> +_log_writes_mount_snapshot()
>> +{
>> +       _scratch_options mount
>> +       $MOUNT_PROG -t $FSTYP `_common_dev_mount_options $*` $SCRATCH_OPTIONS \
>> +               "/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME" $SCRATCH_MNT
>> +}
>> +
>> +_log_writes_unmount_snapshot()
>> +{
>> +       $UMOUNT_PROG $SCRATCH_MNT
>> +}
>> +
>> diff --git a/tests/generic/481 b/tests/generic/481
>> new file mode 100755
>> index 00000000..3985493d
>> --- /dev/null
>> +++ b/tests/generic/481
>> @@ -0,0 +1,124 @@
>> +#! /bin/bash
>> +# FS QA Test 481
>> +#
>> +# Test filesystem consistency after each FUA operation
>> +#
>> +# Will do log replay and check the filesystem.
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2018 SuSE.  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 /
>> +       $KILLALL_PROG -KILL -q $FSSTRESS_PROG &> /dev/null
>> +       _log_writes_cleanup_snapshot &> /dev/null
>> +       _log_writes_cleanup &> /dev/null
>> +
>> +       # We use $TEST_DEV as snapshot COW device, which can't be
>> +       # mounted/recognized as normal fs, need to recreate it
>> +       # or fstest will complain about it
>> +       _mkfs_dev $TEST_DEV > /dev/null 2>&1
>> +       rm -f $tmp.*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +. ./common/dmlogwrites
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +# real QA test starts here
>> +
>> +# Modify as appropriate.
>> +_supported_fs generic
>> +_supported_os Linux
>> +
>> +_require_command "$KILLALL_PROG" killall
>> +# Use $TEST_DEV as snapshot CoW device
>> +_require_test
>> +# Use $SCRATCH_DEV as replay device
>> +_require_scratch
>> +# and we need extra device as log device
>> +_require_log_writes
>> +
>> +
>> +workload=$(( 512 * $LOAD_FACTOR ))
>> +nr_threads=$(($($here/src/feature -o) * $LOAD_FACTOR))
>> +
>> +_test_unmount
>> +_log_writes_init
>> +_log_writes_mkfs >> $seqres.full 2>&1
>> +_log_writes_mark mkfs
>> +
>> +_log_writes_mount
>> +run_check $FSSTRESS_PROG -w -n $workload -p $nr_threads -d $SCRATCH_MNT \
>> +       $FSSTRESS_AVOID > /dev/null 2>&1
>> +_log_writes_unmount
>> +
>> +_log_writes_remove
>> +prev=$(_log_writes_mark_to_entry_number mkfs)
>> +[ -z "$prev" ] && _fail "failed to locate entry mark 'mkfs'"
>> +cur=$(_log_writes_find_next_fua $prev)
>> +[ -z "$cur" ] && _fail "failed to locate next FUA write"
>> +
>> +_log_writes_replay_log_entry_range $prev
>> +while [ ! -z "$cur" ]; do
>> +       _log_writes_replay_log_entry_range $cur $prev >> $seqres.full
>> +
>> +       echo "=== Replay to entry number $cur ===" >> $seqres.full
>> +       # Here we need extra mount to replay the log, mainly for journal based
>> +       # fs, as their fsck will report dirty log as error. So do snapshot
>> +       # to replay, so we can still continue replaying
>> +       _log_writes_create_snapshot $TEST_DEV
>> +       _log_writes_mount_snapshot
>> +       _log_writes_unmount_snapshot
>> +       _check_generic_filesystem "/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME"
> 
> Maybe _log_writes_check_snapshot helper?
> 
> Thanks,
> Amir.
> --
> 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
>
Eryu Guan March 16, 2018, 4:01 a.m. UTC | #3
On Wed, Mar 14, 2018 at 05:02:30PM +0800, Qu Wenruo wrote:
> Basic test case which triggers fsstress with dm-log-writes, and then
> check the fs after each FUA writes.
> With needed infrastructure and special handlers for journal based fs.

It's not clear to me why the existing infrastructure is not sufficient
for the test. It'd be great if you could provide more information and/or
background in commit log.

> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> In my test, xfs and btrfs survives while ext4 would report error during fsck.
> 
> My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
> ourselves. Not sure if it's allowed.

As Amir already replied, that's not allowed, any destructive operations
should be done on $SCRATCH_DEV.

> ---
>  common/dmlogwrites    | 119 ++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/481     | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/481.out |   2 +
>  tests/generic/group   |   1 +
>  4 files changed, 246 insertions(+)
>  create mode 100755 tests/generic/481
>  create mode 100644 tests/generic/481.out
> 
> diff --git a/common/dmlogwrites b/common/dmlogwrites
> index 467b872e..258f5887 100644
> --- a/common/dmlogwrites
> +++ b/common/dmlogwrites
> @@ -126,3 +126,122 @@ _log_writes_cleanup()
>  	$UDEV_SETTLE_PROG >/dev/null 2>&1
>  	_log_writes_remove
>  }
> +
> +# Convert log writes mark to entry number
> +# Result entry number is output to stdout, could be empty if not found
> +_log_writes_mark_to_entry_number()
> +{
> +	local _mark=$1

No need to name a local variable with leading underscore.

> +	local ret
> +
> +	[ -z "$_mark" ] && _fatal \
> +		"mark must be given for _log_writes_mark_to_entry_number"

Please use _fail instead of _fatal (and all the other places in this
patch). I think _fatal is only used at the initializing phase of the
test harness (e.g. check required tools, env etc.) and abort the test
run in early stage. _fail is used to indicate a failure in test phase,
it also logs the error messages in $seqres.full.

> +
> +	ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
> +		--end-mark $_mark 2> /dev/null)

Is this output long or short? If it's a short one, e.g. a number or a
simple string, I think it's OK to save it in a variable. But if it's
long/complex enough and/or has multiple lines, I think we'd better to
save it with a tmp file, e.g. $tmp.<suffix>.

> +	[ -z "$ret" ] && return
> +	ret=$(echo "$ret" | cut -f1 -d\@)

Because I want to avoid echoing such a variable to a pipe (even it's
quoted), it used to cause subtle problems if not quoted properly.

Also, it'd be better to give a sample output of the replay-log command
in comment if it's short enough, so people could have an idea what it
looks like and know what you're looking for in the subsequent steps
(cut -f1 -d\@, in this example), and it's easier to review.

Above comments apply to other helper functions too.

Thanks for all the work!

Eryu
--
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
Qu Wenruo March 16, 2018, 5:17 a.m. UTC | #4
On 2018年03月16日 12:01, Eryu Guan wrote:
> On Wed, Mar 14, 2018 at 05:02:30PM +0800, Qu Wenruo wrote:
>> Basic test case which triggers fsstress with dm-log-writes, and then
>> check the fs after each FUA writes.
>> With needed infrastructure and special handlers for journal based fs.
> 
> It's not clear to me why the existing infrastructure is not sufficient
> for the test. It'd be great if you could provide more information and/or
> background in commit log.

The main problem of current infrastructure is we don't have the
following things:

1) Way to take full advantage of dm-log-writes
   The main thing is, we don't have test cases to check each FUA (this
   patch) and flush (later patch after clearing all the RFC comments).

   We have some dm-flakey test cases to emulate power loss, but they are
   mostly for fsync.
   Here we are not only testing fsync, but also every superblock update.
   Which should be a super set of dm-flakey tests.

2) Workaround for journal replay
   In fact, if we only test btrfs, we don't even need such complicated
   work, just 'replay-log --fsck "btrfs check" --check fua' will be
   enough. As btrfs check doesn't report dirty journal (log tree) as
   problem.
   But for journal based fses, their fsck all report dirty journal as
   error, which needs current snapshot works to replay them before
   running fsck.

I would add them in next version if there is no extra comment on this.

> 
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> In my test, xfs and btrfs survives while ext4 would report error during fsck.
>>
>> My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
>> ourselves. Not sure if it's allowed.
> 
> As Amir already replied, that's not allowed, any destructive operations
> should be done on $SCRATCH_DEV.

Yep, I'm looking for similar case who uses $SCRATCH_DEV as LVM pv do get
extra device.

Or can we reuse the scratch_dev_pool even for ext4/xfs?

> 
>> ---
>>  common/dmlogwrites    | 119 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/generic/481     | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/generic/481.out |   2 +
>>  tests/generic/group   |   1 +
>>  4 files changed, 246 insertions(+)
>>  create mode 100755 tests/generic/481
>>  create mode 100644 tests/generic/481.out
>>
>> diff --git a/common/dmlogwrites b/common/dmlogwrites
>> index 467b872e..258f5887 100644
>> --- a/common/dmlogwrites
>> +++ b/common/dmlogwrites
>> @@ -126,3 +126,122 @@ _log_writes_cleanup()
>>  	$UDEV_SETTLE_PROG >/dev/null 2>&1
>>  	_log_writes_remove
>>  }
>> +
>> +# Convert log writes mark to entry number
>> +# Result entry number is output to stdout, could be empty if not found
>> +_log_writes_mark_to_entry_number()
>> +{
>> +	local _mark=$1
> 
> No need to name a local variable with leading underscore.
> 
>> +	local ret
>> +
>> +	[ -z "$_mark" ] && _fatal \
>> +		"mark must be given for _log_writes_mark_to_entry_number"
> 
> Please use _fail instead of _fatal (and all the other places in this
> patch). I think _fatal is only used at the initializing phase of the
> test harness (e.g. check required tools, env etc.) and abort the test
> run in early stage. _fail is used to indicate a failure in test phase,
> it also logs the error messages in $seqres.full.

Glad to know the difference.

> 
>> +
>> +	ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
>> +		--end-mark $_mark 2> /dev/null)
> 
> Is this output long or short? If it's a short one, e.g. a number or a
> simple string, I think it's OK to save it in a variable. But if it's
> long/complex enough and/or has multiple lines, I think we'd better to
> save it with a tmp file, e.g. $tmp.<suffix>.

Very very short, just one line like '1024@2048'.

> 
>> +	[ -z "$ret" ] && return
>> +	ret=$(echo "$ret" | cut -f1 -d\@)
> 
> Because I want to avoid echoing such a variable to a pipe (even it's
> quoted), it used to cause subtle problems if not quoted properly.
> 
> Also, it'd be better to give a sample output of the replay-log command
> in comment if it's short enough, so people could have an idea what it
> looks like and know what you're looking for in the subsequent steps
> (cut -f1 -d\@, in this example), and it's easier to review.
> 
> Above comments apply to other helper functions too.
> 
> Thanks for all the work!

Thanks for your review too,
Qu

> 
> Eryu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Eryu Guan March 16, 2018, 8:19 a.m. UTC | #5
On Fri, Mar 16, 2018 at 01:17:07PM +0800, Qu Wenruo wrote:
> 
> 
> On 2018年03月16日 12:01, Eryu Guan wrote:
> > On Wed, Mar 14, 2018 at 05:02:30PM +0800, Qu Wenruo wrote:
> >> Basic test case which triggers fsstress with dm-log-writes, and then
> >> check the fs after each FUA writes.
> >> With needed infrastructure and special handlers for journal based fs.
> > 
> > It's not clear to me why the existing infrastructure is not sufficient
> > for the test. It'd be great if you could provide more information and/or
> > background in commit log.
> 
> The main problem of current infrastructure is we don't have the
> following things:
> 
> 1) Way to take full advantage of dm-log-writes
>    The main thing is, we don't have test cases to check each FUA (this
>    patch) and flush (later patch after clearing all the RFC comments).
> 
>    We have some dm-flakey test cases to emulate power loss, but they are
>    mostly for fsync.
>    Here we are not only testing fsync, but also every superblock update.
>    Which should be a super set of dm-flakey tests.
> 
> 2) Workaround for journal replay
>    In fact, if we only test btrfs, we don't even need such complicated
>    work, just 'replay-log --fsck "btrfs check" --check fua' will be
>    enough. As btrfs check doesn't report dirty journal (log tree) as
>    problem.
>    But for journal based fses, their fsck all report dirty journal as
>    error, which needs current snapshot works to replay them before
>    running fsck.

And replay-to-fua doesn't guarantee a consistent filesystem state,
that's why we need to mount/umount the target device to replay the
filesystem journal, and to avoid replaying already-replayed-log over and
over again, we create a snapshot of the target device and mount cycle &
fsck the snapshot, right?

I'm wondering if the overhead of repeatly create & destroy snapshots is
larger than replaying log from start. Maybe snapshots take more time?

> 
> I would add them in next version if there is no extra comment on this.
> 
> > 
> >>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >> ---
> >> In my test, xfs and btrfs survives while ext4 would report error during fsck.
> >>
> >> My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
> >> ourselves. Not sure if it's allowed.
> > 
> > As Amir already replied, that's not allowed, any destructive operations
> > should be done on $SCRATCH_DEV.
> 
> Yep, I'm looking for similar case who uses $SCRATCH_DEV as LVM pv do get
> extra device.
> 
> Or can we reuse the scratch_dev_pool even for ext4/xfs?

I think so, IMO pool devices are not limited to btrfs. But I think we
could use a loop device reside on $TEST_DIR? Or if snapshots take longer
time, then we don't need this extra device at all :)

I have some other comments, will reply to the RFC patch in another
email.

Thanks,
Eryu
--
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
Amir Goldstein March 16, 2018, 8:29 a.m. UTC | #6
On Fri, Mar 16, 2018 at 10:19 AM, Eryu Guan <guaneryu@gmail.com> wrote:
> On Fri, Mar 16, 2018 at 01:17:07PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2018年03月16日 12:01, Eryu Guan wrote:
>> > On Wed, Mar 14, 2018 at 05:02:30PM +0800, Qu Wenruo wrote:
>> >> Basic test case which triggers fsstress with dm-log-writes, and then
>> >> check the fs after each FUA writes.
>> >> With needed infrastructure and special handlers for journal based fs.
>> >
>> > It's not clear to me why the existing infrastructure is not sufficient
>> > for the test. It'd be great if you could provide more information and/or
>> > background in commit log.
>>
>> The main problem of current infrastructure is we don't have the
>> following things:
>>
>> 1) Way to take full advantage of dm-log-writes
>>    The main thing is, we don't have test cases to check each FUA (this
>>    patch) and flush (later patch after clearing all the RFC comments).
>>
>>    We have some dm-flakey test cases to emulate power loss, but they are
>>    mostly for fsync.
>>    Here we are not only testing fsync, but also every superblock update.
>>    Which should be a super set of dm-flakey tests.
>>
>> 2) Workaround for journal replay
>>    In fact, if we only test btrfs, we don't even need such complicated
>>    work, just 'replay-log --fsck "btrfs check" --check fua' will be
>>    enough. As btrfs check doesn't report dirty journal (log tree) as
>>    problem.
>>    But for journal based fses, their fsck all report dirty journal as
>>    error, which needs current snapshot works to replay them before
>>    running fsck.
>
> And replay-to-fua doesn't guarantee a consistent filesystem state,
> that's why we need to mount/umount the target device to replay the
> filesystem journal, and to avoid replaying already-replayed-log over and
> over again, we create a snapshot of the target device and mount cycle &
> fsck the snapshot, right?
>
> I'm wondering if the overhead of repeatly create & destroy snapshots is
> larger than replaying log from start. Maybe snapshots take more time?
>

FYI, the snapshots flavor comes from Josef's scripts and it is called fast***
I suppose this means the non-snapshot flavor is the original implementation
and it is slower. Josef?

>>
>> I would add them in next version if there is no extra comment on this.
>>
>> >
>> >>
>> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> >> ---
>> >> In my test, xfs and btrfs survives while ext4 would report error during fsck.
>> >>
>> >> My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
>> >> ourselves. Not sure if it's allowed.
>> >
>> > As Amir already replied, that's not allowed, any destructive operations
>> > should be done on $SCRATCH_DEV.
>>
>> Yep, I'm looking for similar case who uses $SCRATCH_DEV as LVM pv do get
>> extra device.
>>
>> Or can we reuse the scratch_dev_pool even for ext4/xfs?
>
> I think so, IMO pool devices are not limited to btrfs. But I think we
> could use a loop device reside on $TEST_DIR? Or if snapshots take longer
> time, then we don't need this extra device at all :)
>
> I have some other comments, will reply to the RFC patch in another
> email.
>
> Thanks,
> Eryu
--
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
Eryu Guan March 16, 2018, 8:33 a.m. UTC | #7
On Wed, Mar 14, 2018 at 05:02:30PM +0800, Qu Wenruo wrote:
> Basic test case which triggers fsstress with dm-log-writes, and then
> check the fs after each FUA writes.
> With needed infrastructure and special handlers for journal based fs.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> In my test, xfs and btrfs survives while ext4 would report error during fsck.
> 
> My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
> ourselves. Not sure if it's allowed.
> ---
>  common/dmlogwrites    | 119 ++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/481     | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/481.out |   2 +
>  tests/generic/group   |   1 +
>  4 files changed, 246 insertions(+)
>  create mode 100755 tests/generic/481
>  create mode 100644 tests/generic/481.out
> 
> diff --git a/common/dmlogwrites b/common/dmlogwrites
> index 467b872e..258f5887 100644
> --- a/common/dmlogwrites
> +++ b/common/dmlogwrites
> @@ -126,3 +126,122 @@ _log_writes_cleanup()
>  	$UDEV_SETTLE_PROG >/dev/null 2>&1
>  	_log_writes_remove
>  }
> +
> +# Convert log writes mark to entry number
> +# Result entry number is output to stdout, could be empty if not found
> +_log_writes_mark_to_entry_number()
> +{
> +	local _mark=$1
> +	local ret
> +
> +	[ -z "$_mark" ] && _fatal \
> +		"mark must be given for _log_writes_mark_to_entry_number"
> +
> +	ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
> +		--end-mark $_mark 2> /dev/null)
> +	[ -z "$ret" ] && return
> +	ret=$(echo "$ret" | cut -f1 -d\@)
> +	echo "mark $_mark has entry number $ret" >> $seqres.full
> +	echo "$ret"
> +}
> +
> +# Find next fua write entry number
> +# Result entry number is output to stdout, could be empty if not found
> +_log_writes_find_next_fua()
> +{
> +	local _start_entry=$1
> +	local ret
> +
> +	[ -z "$_start_entry" ] && _start_entry=0
> +	ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
> +	      --next-fua --start-entry $_start_entry 2> /dev/null)
> +	[ -z "$ret" ] && return
> +
> +	ret=$(echo "$ret" | cut -f1 -d\@)
> +	echo "next fua is entry number $ret" >> $seqres.full
> +	echo "$ret"
> +}
> +
> +# Replay log range to specified entry
> +# $1:	End entry. The entry with this number *WILL* be replayed
> +# $2:	Start entry. If not specified, start from the first entry.
> +# $3:	Verbose. If set to 'y' do verbose output
> +_log_writes_replay_log_entry_range()

_log_writes_replay_log_range() would be fine.

> +{
> +	local _end=$1
> +	local _start=$2

This arguments order ($end comes first) makes me confused when I first
read the test code. Reverse the order?

> +	local _verbose=$3
> +
> +	[ -z "$_end" ] && _fatal \
> +	"end entry must be specified for _log_writes_replay_log_entry_range"
> +
> +	[ "x$verbose" == "xy" ] && _verbose="-v"
> +	[ -z "$_start" ] && _start=0
> +	[ "$_start" -gt "$_end" ] && _fatal \
> +		"wrong parameter order for _log_writes_replay_log_entry_range:start=$_start end=$_end"
> +
> +	# To ensure we replay the last entry, for _start == 0 case,
> +	# we need to manually increase the end entry number to ensure
> +	# it's played
> +	echo "=== replay from $_start to $_end ===" >> $seqres.full
> +	$here/src/log-writes/replay-log --log $LOGWRITES_DEV \
> +		--replay $SCRATCH_DEV --start-entry $_start \
> +		--limit $(($_end - $_start + 1)) $_verbose \
> +		>> $seqres.full 2>&1
> +	[ $? -ne 0 ] && _fatal "replay failed"
> +}
> +
> +_log_writes_cleanup_snapshot()
> +{
> +	$UDEV_SETTLE_PROG > /dev/null 2>&1
> +	$DMSETUP_PROG remove "$DMLOGWRITES_SNAPSHOT_NAME" > /dev/null 2>&1
> +	$DMSETUP_PROG remove "$DMLOGWRITES_ORIGIN_NAME" > /dev/null 2>&1
> +}
> +
> +# Helper to create snapshot of a the replayed device
> +# Useful for journal based filesystem such as XFS and Ext4 to replay
> +# their journal without touching the replay device, so that we can
> +# continue replaying other than replay from the beginning.
> +# $1:	Snapshot device
> +_log_writes_create_snapshot()
> +{
> +	_require_dm_target snapshot

This doesn't belong here, call it in the test.

> +
> +	local snapshot_dev=$1
> +	local cow_base=""

Unused variable.

> +
> +	[ -z "$snapshot_dev" ] && _fatal \
> +		"@device must be specified for _log_writes_create_snapshot"
> +	local size=$(blockdev --getsz $SCRATCH_DEV)
> +	[ -z "$size" ] && _fatal \
> +		"failed to get device size for _log_writes_create_snapshot"
> +
> +	_log_writes_cleanup_snapshot
> +
> +	DMLOGWRITES_ORIGIN_NAME="log-writes-origin"
> +	DMLOGWRITES_SNAPSHOT_NAME="log-writes-snapshot"
> +	DMLOGWRITES_ORIGIN_TABLE="0 $size snapshot-origin $SCRATCH_DEV"
> +	DMLOGWRITES_SNAPSHOT_TABLE="0 $size snapshot /dev/mapper/$DMLOGWRITES_ORIGIN_NAME $snapshot_dev N 512"
> +
> +	$UDEV_SETTLE_PROG >/dev/null 2>&1
> +	$DMSETUP_PROG create $DMLOGWRITES_ORIGIN_NAME --table "$DMLOGWRITES_ORIGIN_TABLE" ||\
> +		_fatal "failed to create snapshot-origin of log writes target"
> +	$DMSETUP_PROG mknodes > /dev/null 2>&1
> +	$UDEV_SETTLE_PROG >/dev/null 2>&1
> +	$DMSETUP_PROG create $DMLOGWRITES_SNAPSHOT_NAME --table "$DMLOGWRITES_SNAPSHOT_TABLE" ||\
> +		_fatal "failed to create snapshot of log writes target"
> +	$DMSETUP_PROG mknodes > /dev/null 2>&1
> +}
> +
> +_log_writes_mount_snapshot()
> +{
> +	_scratch_options mount
> +	$MOUNT_PROG -t $FSTYP `_common_dev_mount_options $*` $SCRATCH_OPTIONS \
> +		"/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME" $SCRATCH_MNT
> +}
> +
> +_log_writes_unmount_snapshot()
> +{
> +	$UMOUNT_PROG $SCRATCH_MNT
> +}
> +
> diff --git a/tests/generic/481 b/tests/generic/481
> new file mode 100755
> index 00000000..3985493d
> --- /dev/null
> +++ b/tests/generic/481
> @@ -0,0 +1,124 @@
> +#! /bin/bash
> +# FS QA Test 481
> +#
> +# Test filesystem consistency after each FUA operation
> +#
> +# Will do log replay and check the filesystem.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2018 SuSE.  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 /
> +	$KILLALL_PROG -KILL -q $FSSTRESS_PROG &> /dev/null
> +	_log_writes_cleanup_snapshot &> /dev/null
> +	_log_writes_cleanup &> /dev/null
> +
> +	# We use $TEST_DEV as snapshot COW device, which can't be
> +	# mounted/recognized as normal fs, need to recreate it
> +	# or fstest will complain about it
> +	_mkfs_dev $TEST_DEV > /dev/null 2>&1
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/dmlogwrites
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_fs generic
> +_supported_os Linux
> +
> +_require_command "$KILLALL_PROG" killall
> +# Use $TEST_DEV as snapshot CoW device
> +_require_test
> +# Use $SCRATCH_DEV as replay device
> +_require_scratch
> +# and we need extra device as log device
> +_require_log_writes
> +
> +
> +workload=$(( 512 * $LOAD_FACTOR ))
> +nr_threads=$(($($here/src/feature -o) * $LOAD_FACTOR))
> +
> +_test_unmount
> +_log_writes_init
> +_log_writes_mkfs >> $seqres.full 2>&1
> +_log_writes_mark mkfs
> +
> +_log_writes_mount
> +run_check $FSSTRESS_PROG -w -n $workload -p $nr_threads -d $SCRATCH_MNT \
> +	$FSSTRESS_AVOID > /dev/null 2>&1

Use _scale_fsstress_args to scale the load.

And I don't think checking the result of fsstress is necessary, it's
used as a load generator and we check fs consistency anyway.

> +_log_writes_unmount
> +
> +_log_writes_remove
> +prev=$(_log_writes_mark_to_entry_number mkfs)
> +[ -z "$prev" ] && _fail "failed to locate entry mark 'mkfs'"
> +cur=$(_log_writes_find_next_fua $prev)
> +[ -z "$cur" ] && _fail "failed to locate next FUA write"
> +
> +_log_writes_replay_log_entry_range $prev
> +while [ ! -z "$cur" ]; do
> +	_log_writes_replay_log_entry_range $cur $prev >> $seqres.full
> +
> +	echo "=== Replay to entry number $cur ===" >> $seqres.full
> +	# Here we need extra mount to replay the log, mainly for journal based
> +	# fs, as their fsck will report dirty log as error. So do snapshot
> +	# to replay, so we can still continue replaying
> +	_log_writes_create_snapshot $TEST_DEV
> +	_log_writes_mount_snapshot
> +	_log_writes_unmount_snapshot
> +	_check_generic_filesystem "/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME"

Use _check_scratch_fs $some_dev, _check_generic_filesystem doesn't work
for xfs/btrfs/udf/overlay.

> +	if [ $? -ne 0 ]; then
> +		_log_writes_replay_log_entry_range $cur $prev y >> $seqres.full
> +		_fail "error found in fsck after log replay"
> +	fi

And _check_scratch_fs/_check_generic_filesystem exits directly on fsck
failure, you can't do the return status check.

> +	_log_writes_cleanup_snapshot
> +
> +	prev=$cur
> +	cur=$(_log_writes_find_next_fua $(($cur + 1)))
> +	[ -z "$cur" ] && break
> +done
> +_log_writes_cleanup_snapshot
> +_log_writes_cleanup
> +
> +# mkfs before ending the test case, or $TEST_DEV doesn't contain any
> +# valid fs
> +_mkfs_dev $TEST_DEV
> +
> +echo "Silence is golden"
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/generic/481.out b/tests/generic/481.out
> new file mode 100644
> index 00000000..206e1163
> --- /dev/null
> +++ b/tests/generic/481.out
> @@ -0,0 +1,2 @@
> +QA output created by 481
> +Silence is golden
> diff --git a/tests/generic/group b/tests/generic/group
> index ea2056b1..1de053a6 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -483,3 +483,4 @@
>  478 auto quick
>  479 auto quick metadata
>  480 auto quick metadata
> +481 auto

Add 'replay' group, as all other tests using dm-log-writes.

Thanks,
Eryu
--
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
Qu Wenruo March 16, 2018, 8:44 a.m. UTC | #8
On 2018年03月16日 16:19, Eryu Guan wrote:
> On Fri, Mar 16, 2018 at 01:17:07PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2018年03月16日 12:01, Eryu Guan wrote:
>>> On Wed, Mar 14, 2018 at 05:02:30PM +0800, Qu Wenruo wrote:
>>>> Basic test case which triggers fsstress with dm-log-writes, and then
>>>> check the fs after each FUA writes.
>>>> With needed infrastructure and special handlers for journal based fs.
>>>
>>> It's not clear to me why the existing infrastructure is not sufficient
>>> for the test. It'd be great if you could provide more information and/or
>>> background in commit log.
>>
>> The main problem of current infrastructure is we don't have the
>> following things:
>>
>> 1) Way to take full advantage of dm-log-writes
>>    The main thing is, we don't have test cases to check each FUA (this
>>    patch) and flush (later patch after clearing all the RFC comments).
>>
>>    We have some dm-flakey test cases to emulate power loss, but they are
>>    mostly for fsync.
>>    Here we are not only testing fsync, but also every superblock update.
>>    Which should be a super set of dm-flakey tests.
>>
>> 2) Workaround for journal replay
>>    In fact, if we only test btrfs, we don't even need such complicated
>>    work, just 'replay-log --fsck "btrfs check" --check fua' will be
>>    enough. As btrfs check doesn't report dirty journal (log tree) as
>>    problem.
>>    But for journal based fses, their fsck all report dirty journal as
>>    error, which needs current snapshot works to replay them before
>>    running fsck.
> 
> And replay-to-fua doesn't guarantee a consistent filesystem state,
> that's why we need to mount/umount the target device to replay the
> filesystem journal, and to avoid replaying already-replayed-log over and
> over again, we create a snapshot of the target device and mount cycle &
> fsck the snapshot, right?
> 
> I'm wondering if the overhead of repeatly create & destroy snapshots is
> larger than replaying log from start. Maybe snapshots take more time?

This is the interesting part.

Repeatedly creating snapshot will become slower and slower, and in fact
for ext4/xfs it's pretty slow, so I use the small nops for fsstress
(512), as for later snapshot creation it's taking quite a long time.

But if we switch back to no snapshot, but pure replay-log method,
replay-log itself will become slow too.
But at least it can go much further (nops=2048) and still get a somewhat
acceptable speed.

Either way, we will get slower and slower when the number of operation grow.


However personally speaking, the pure replay-log one looks a little
better, as I don't need to use LVM to create snapshot. :)

> 
>>
>> I would add them in next version if there is no extra comment on this.
>>
>>>
>>>>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>> ---
>>>> In my test, xfs and btrfs survives while ext4 would report error during fsck.
>>>>
>>>> My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
>>>> ourselves. Not sure if it's allowed.
>>>
>>> As Amir already replied, that's not allowed, any destructive operations
>>> should be done on $SCRATCH_DEV.
>>
>> Yep, I'm looking for similar case who uses $SCRATCH_DEV as LVM pv do get
>> extra device.
>>
>> Or can we reuse the scratch_dev_pool even for ext4/xfs?
> 
> I think so, IMO pool devices are not limited to btrfs. But I think we
> could use a loop device reside on $TEST_DIR? Or if snapshots take longer
> time, then we don't need this extra device at all :)

That would be much better!

Both common/dmlogwrites and the test case will be much simpler.

Thanks,
Qu

> 
> I have some other comments, will reply to the RFC patch in another
> email.
> 
> Thanks,
> Eryu
> --
> 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
>
Qu Wenruo March 16, 2018, 8:50 a.m. UTC | #9
On 2018年03月16日 16:33, Eryu Guan wrote:
> On Wed, Mar 14, 2018 at 05:02:30PM +0800, Qu Wenruo wrote:
>> Basic test case which triggers fsstress with dm-log-writes, and then
>> check the fs after each FUA writes.
>> With needed infrastructure and special handlers for journal based fs.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> In my test, xfs and btrfs survives while ext4 would report error during fsck.
>>
>> My current biggest concern is, we abuse $TEST_DEV and mkfs on it all by
>> ourselves. Not sure if it's allowed.
>> ---
>>  common/dmlogwrites    | 119 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/generic/481     | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/generic/481.out |   2 +
>>  tests/generic/group   |   1 +
>>  4 files changed, 246 insertions(+)
>>  create mode 100755 tests/generic/481
>>  create mode 100644 tests/generic/481.out
>>
>> diff --git a/common/dmlogwrites b/common/dmlogwrites
>> index 467b872e..258f5887 100644
>> --- a/common/dmlogwrites
>> +++ b/common/dmlogwrites
>> @@ -126,3 +126,122 @@ _log_writes_cleanup()
>>  	$UDEV_SETTLE_PROG >/dev/null 2>&1
>>  	_log_writes_remove
>>  }
>> +
>> +# Convert log writes mark to entry number
>> +# Result entry number is output to stdout, could be empty if not found
>> +_log_writes_mark_to_entry_number()
>> +{
>> +	local _mark=$1
>> +	local ret
>> +
>> +	[ -z "$_mark" ] && _fatal \
>> +		"mark must be given for _log_writes_mark_to_entry_number"
>> +
>> +	ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
>> +		--end-mark $_mark 2> /dev/null)
>> +	[ -z "$ret" ] && return
>> +	ret=$(echo "$ret" | cut -f1 -d\@)
>> +	echo "mark $_mark has entry number $ret" >> $seqres.full
>> +	echo "$ret"
>> +}
>> +
>> +# Find next fua write entry number
>> +# Result entry number is output to stdout, could be empty if not found
>> +_log_writes_find_next_fua()
>> +{
>> +	local _start_entry=$1
>> +	local ret
>> +
>> +	[ -z "$_start_entry" ] && _start_entry=0
>> +	ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
>> +	      --next-fua --start-entry $_start_entry 2> /dev/null)
>> +	[ -z "$ret" ] && return
>> +
>> +	ret=$(echo "$ret" | cut -f1 -d\@)
>> +	echo "next fua is entry number $ret" >> $seqres.full
>> +	echo "$ret"
>> +}
>> +
>> +# Replay log range to specified entry
>> +# $1:	End entry. The entry with this number *WILL* be replayed
>> +# $2:	Start entry. If not specified, start from the first entry.
>> +# $3:	Verbose. If set to 'y' do verbose output
>> +_log_writes_replay_log_entry_range()
> 
> _log_writes_replay_log_range() would be fine.
> 
>> +{
>> +	local _end=$1
>> +	local _start=$2
> 
> This arguments order ($end comes first) makes me confused when I first
> read the test code. Reverse the order?

Yep, the order is confusing.

However the confusing parameter order is here because some parameter is
optional.

As we can call _log_writes_replay_log_range() like:

_log_writes_replay_log_range 1024	# Replay to number 1024
_log_writes_replay_log_range 1024 2048	# Replay from 1024 to 2048
_log_writes_replay_log_range 128 512 y  # verbose replay

So this makes order pretty hard to reverse.

(Or, we discard the ability to replay from $start to $end, and only
 support replay to $end?)

Anyway, I would use the pure replay-log method to avoid extra device,
and see if it would make test case easier.

Thanks,
Qu

> 
>> +	local _verbose=$3
>> +
>> +	[ -z "$_end" ] && _fatal \
>> +	"end entry must be specified for _log_writes_replay_log_entry_range"
>> +
>> +	[ "x$verbose" == "xy" ] && _verbose="-v"
>> +	[ -z "$_start" ] && _start=0
>> +	[ "$_start" -gt "$_end" ] && _fatal \
>> +		"wrong parameter order for _log_writes_replay_log_entry_range:start=$_start end=$_end"
>> +
>> +	# To ensure we replay the last entry, for _start == 0 case,
>> +	# we need to manually increase the end entry number to ensure
>> +	# it's played
>> +	echo "=== replay from $_start to $_end ===" >> $seqres.full
>> +	$here/src/log-writes/replay-log --log $LOGWRITES_DEV \
>> +		--replay $SCRATCH_DEV --start-entry $_start \
>> +		--limit $(($_end - $_start + 1)) $_verbose \
>> +		>> $seqres.full 2>&1
>> +	[ $? -ne 0 ] && _fatal "replay failed"
>> +}
>> +
>> +_log_writes_cleanup_snapshot()
>> +{
>> +	$UDEV_SETTLE_PROG > /dev/null 2>&1
>> +	$DMSETUP_PROG remove "$DMLOGWRITES_SNAPSHOT_NAME" > /dev/null 2>&1
>> +	$DMSETUP_PROG remove "$DMLOGWRITES_ORIGIN_NAME" > /dev/null 2>&1
>> +}
>> +
>> +# Helper to create snapshot of a the replayed device
>> +# Useful for journal based filesystem such as XFS and Ext4 to replay
>> +# their journal without touching the replay device, so that we can
>> +# continue replaying other than replay from the beginning.
>> +# $1:	Snapshot device
>> +_log_writes_create_snapshot()
>> +{
>> +	_require_dm_target snapshot
> 
> This doesn't belong here, call it in the test.
> 
>> +
>> +	local snapshot_dev=$1
>> +	local cow_base=""
> 
> Unused variable.
> 
>> +
>> +	[ -z "$snapshot_dev" ] && _fatal \
>> +		"@device must be specified for _log_writes_create_snapshot"
>> +	local size=$(blockdev --getsz $SCRATCH_DEV)
>> +	[ -z "$size" ] && _fatal \
>> +		"failed to get device size for _log_writes_create_snapshot"
>> +
>> +	_log_writes_cleanup_snapshot
>> +
>> +	DMLOGWRITES_ORIGIN_NAME="log-writes-origin"
>> +	DMLOGWRITES_SNAPSHOT_NAME="log-writes-snapshot"
>> +	DMLOGWRITES_ORIGIN_TABLE="0 $size snapshot-origin $SCRATCH_DEV"
>> +	DMLOGWRITES_SNAPSHOT_TABLE="0 $size snapshot /dev/mapper/$DMLOGWRITES_ORIGIN_NAME $snapshot_dev N 512"
>> +
>> +	$UDEV_SETTLE_PROG >/dev/null 2>&1
>> +	$DMSETUP_PROG create $DMLOGWRITES_ORIGIN_NAME --table "$DMLOGWRITES_ORIGIN_TABLE" ||\
>> +		_fatal "failed to create snapshot-origin of log writes target"
>> +	$DMSETUP_PROG mknodes > /dev/null 2>&1
>> +	$UDEV_SETTLE_PROG >/dev/null 2>&1
>> +	$DMSETUP_PROG create $DMLOGWRITES_SNAPSHOT_NAME --table "$DMLOGWRITES_SNAPSHOT_TABLE" ||\
>> +		_fatal "failed to create snapshot of log writes target"
>> +	$DMSETUP_PROG mknodes > /dev/null 2>&1
>> +}
>> +
>> +_log_writes_mount_snapshot()
>> +{
>> +	_scratch_options mount
>> +	$MOUNT_PROG -t $FSTYP `_common_dev_mount_options $*` $SCRATCH_OPTIONS \
>> +		"/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME" $SCRATCH_MNT
>> +}
>> +
>> +_log_writes_unmount_snapshot()
>> +{
>> +	$UMOUNT_PROG $SCRATCH_MNT
>> +}
>> +
>> diff --git a/tests/generic/481 b/tests/generic/481
>> new file mode 100755
>> index 00000000..3985493d
>> --- /dev/null
>> +++ b/tests/generic/481
>> @@ -0,0 +1,124 @@
>> +#! /bin/bash
>> +# FS QA Test 481
>> +#
>> +# Test filesystem consistency after each FUA operation
>> +#
>> +# Will do log replay and check the filesystem.
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2018 SuSE.  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 /
>> +	$KILLALL_PROG -KILL -q $FSSTRESS_PROG &> /dev/null
>> +	_log_writes_cleanup_snapshot &> /dev/null
>> +	_log_writes_cleanup &> /dev/null
>> +
>> +	# We use $TEST_DEV as snapshot COW device, which can't be
>> +	# mounted/recognized as normal fs, need to recreate it
>> +	# or fstest will complain about it
>> +	_mkfs_dev $TEST_DEV > /dev/null 2>&1
>> +	rm -f $tmp.*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +. ./common/dmlogwrites
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +# real QA test starts here
>> +
>> +# Modify as appropriate.
>> +_supported_fs generic
>> +_supported_os Linux
>> +
>> +_require_command "$KILLALL_PROG" killall
>> +# Use $TEST_DEV as snapshot CoW device
>> +_require_test
>> +# Use $SCRATCH_DEV as replay device
>> +_require_scratch
>> +# and we need extra device as log device
>> +_require_log_writes
>> +
>> +
>> +workload=$(( 512 * $LOAD_FACTOR ))
>> +nr_threads=$(($($here/src/feature -o) * $LOAD_FACTOR))
>> +
>> +_test_unmount
>> +_log_writes_init
>> +_log_writes_mkfs >> $seqres.full 2>&1
>> +_log_writes_mark mkfs
>> +
>> +_log_writes_mount
>> +run_check $FSSTRESS_PROG -w -n $workload -p $nr_threads -d $SCRATCH_MNT \
>> +	$FSSTRESS_AVOID > /dev/null 2>&1
> 
> Use _scale_fsstress_args to scale the load.
> 
> And I don't think checking the result of fsstress is necessary, it's
> used as a load generator and we check fs consistency anyway.
> 
>> +_log_writes_unmount
>> +
>> +_log_writes_remove
>> +prev=$(_log_writes_mark_to_entry_number mkfs)
>> +[ -z "$prev" ] && _fail "failed to locate entry mark 'mkfs'"
>> +cur=$(_log_writes_find_next_fua $prev)
>> +[ -z "$cur" ] && _fail "failed to locate next FUA write"
>> +
>> +_log_writes_replay_log_entry_range $prev
>> +while [ ! -z "$cur" ]; do
>> +	_log_writes_replay_log_entry_range $cur $prev >> $seqres.full
>> +
>> +	echo "=== Replay to entry number $cur ===" >> $seqres.full
>> +	# Here we need extra mount to replay the log, mainly for journal based
>> +	# fs, as their fsck will report dirty log as error. So do snapshot
>> +	# to replay, so we can still continue replaying
>> +	_log_writes_create_snapshot $TEST_DEV
>> +	_log_writes_mount_snapshot
>> +	_log_writes_unmount_snapshot
>> +	_check_generic_filesystem "/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME"
> 
> Use _check_scratch_fs $some_dev, _check_generic_filesystem doesn't work
> for xfs/btrfs/udf/overlay.
> 
>> +	if [ $? -ne 0 ]; then
>> +		_log_writes_replay_log_entry_range $cur $prev y >> $seqres.full
>> +		_fail "error found in fsck after log replay"
>> +	fi
> 
> And _check_scratch_fs/_check_generic_filesystem exits directly on fsck
> failure, you can't do the return status check.
> 
>> +	_log_writes_cleanup_snapshot
>> +
>> +	prev=$cur
>> +	cur=$(_log_writes_find_next_fua $(($cur + 1)))
>> +	[ -z "$cur" ] && break
>> +done
>> +_log_writes_cleanup_snapshot
>> +_log_writes_cleanup
>> +
>> +# mkfs before ending the test case, or $TEST_DEV doesn't contain any
>> +# valid fs
>> +_mkfs_dev $TEST_DEV
>> +
>> +echo "Silence is golden"
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/generic/481.out b/tests/generic/481.out
>> new file mode 100644
>> index 00000000..206e1163
>> --- /dev/null
>> +++ b/tests/generic/481.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 481
>> +Silence is golden
>> diff --git a/tests/generic/group b/tests/generic/group
>> index ea2056b1..1de053a6 100644
>> --- a/tests/generic/group
>> +++ b/tests/generic/group
>> @@ -483,3 +483,4 @@
>>  478 auto quick
>>  479 auto quick metadata
>>  480 auto quick metadata
>> +481 auto
> 
> Add 'replay' group, as all other tests using dm-log-writes.
> 
> Thanks,
> Eryu
> --
> 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
>
diff mbox

Patch

diff --git a/common/dmlogwrites b/common/dmlogwrites
index 467b872e..258f5887 100644
--- a/common/dmlogwrites
+++ b/common/dmlogwrites
@@ -126,3 +126,122 @@  _log_writes_cleanup()
 	$UDEV_SETTLE_PROG >/dev/null 2>&1
 	_log_writes_remove
 }
+
+# Convert log writes mark to entry number
+# Result entry number is output to stdout, could be empty if not found
+_log_writes_mark_to_entry_number()
+{
+	local _mark=$1
+	local ret
+
+	[ -z "$_mark" ] && _fatal \
+		"mark must be given for _log_writes_mark_to_entry_number"
+
+	ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
+		--end-mark $_mark 2> /dev/null)
+	[ -z "$ret" ] && return
+	ret=$(echo "$ret" | cut -f1 -d\@)
+	echo "mark $_mark has entry number $ret" >> $seqres.full
+	echo "$ret"
+}
+
+# Find next fua write entry number
+# Result entry number is output to stdout, could be empty if not found
+_log_writes_find_next_fua()
+{
+	local _start_entry=$1
+	local ret
+
+	[ -z "$_start_entry" ] && _start_entry=0
+	ret=$($here/src/log-writes/replay-log --find --log $LOGWRITES_DEV \
+	      --next-fua --start-entry $_start_entry 2> /dev/null)
+	[ -z "$ret" ] && return
+
+	ret=$(echo "$ret" | cut -f1 -d\@)
+	echo "next fua is entry number $ret" >> $seqres.full
+	echo "$ret"
+}
+
+# Replay log range to specified entry
+# $1:	End entry. The entry with this number *WILL* be replayed
+# $2:	Start entry. If not specified, start from the first entry.
+# $3:	Verbose. If set to 'y' do verbose output
+_log_writes_replay_log_entry_range()
+{
+	local _end=$1
+	local _start=$2
+	local _verbose=$3
+
+	[ -z "$_end" ] && _fatal \
+	"end entry must be specified for _log_writes_replay_log_entry_range"
+
+	[ "x$verbose" == "xy" ] && _verbose="-v"
+	[ -z "$_start" ] && _start=0
+	[ "$_start" -gt "$_end" ] && _fatal \
+		"wrong parameter order for _log_writes_replay_log_entry_range:start=$_start end=$_end"
+
+	# To ensure we replay the last entry, for _start == 0 case,
+	# we need to manually increase the end entry number to ensure
+	# it's played
+	echo "=== replay from $_start to $_end ===" >> $seqres.full
+	$here/src/log-writes/replay-log --log $LOGWRITES_DEV \
+		--replay $SCRATCH_DEV --start-entry $_start \
+		--limit $(($_end - $_start + 1)) $_verbose \
+		>> $seqres.full 2>&1
+	[ $? -ne 0 ] && _fatal "replay failed"
+}
+
+_log_writes_cleanup_snapshot()
+{
+	$UDEV_SETTLE_PROG > /dev/null 2>&1
+	$DMSETUP_PROG remove "$DMLOGWRITES_SNAPSHOT_NAME" > /dev/null 2>&1
+	$DMSETUP_PROG remove "$DMLOGWRITES_ORIGIN_NAME" > /dev/null 2>&1
+}
+
+# Helper to create snapshot of a the replayed device
+# Useful for journal based filesystem such as XFS and Ext4 to replay
+# their journal without touching the replay device, so that we can
+# continue replaying other than replay from the beginning.
+# $1:	Snapshot device
+_log_writes_create_snapshot()
+{
+	_require_dm_target snapshot
+
+	local snapshot_dev=$1
+	local cow_base=""
+
+	[ -z "$snapshot_dev" ] && _fatal \
+		"@device must be specified for _log_writes_create_snapshot"
+	local size=$(blockdev --getsz $SCRATCH_DEV)
+	[ -z "$size" ] && _fatal \
+		"failed to get device size for _log_writes_create_snapshot"
+
+	_log_writes_cleanup_snapshot
+
+	DMLOGWRITES_ORIGIN_NAME="log-writes-origin"
+	DMLOGWRITES_SNAPSHOT_NAME="log-writes-snapshot"
+	DMLOGWRITES_ORIGIN_TABLE="0 $size snapshot-origin $SCRATCH_DEV"
+	DMLOGWRITES_SNAPSHOT_TABLE="0 $size snapshot /dev/mapper/$DMLOGWRITES_ORIGIN_NAME $snapshot_dev N 512"
+
+	$UDEV_SETTLE_PROG >/dev/null 2>&1
+	$DMSETUP_PROG create $DMLOGWRITES_ORIGIN_NAME --table "$DMLOGWRITES_ORIGIN_TABLE" ||\
+		_fatal "failed to create snapshot-origin of log writes target"
+	$DMSETUP_PROG mknodes > /dev/null 2>&1
+	$UDEV_SETTLE_PROG >/dev/null 2>&1
+	$DMSETUP_PROG create $DMLOGWRITES_SNAPSHOT_NAME --table "$DMLOGWRITES_SNAPSHOT_TABLE" ||\
+		_fatal "failed to create snapshot of log writes target"
+	$DMSETUP_PROG mknodes > /dev/null 2>&1
+}
+
+_log_writes_mount_snapshot()
+{
+	_scratch_options mount
+	$MOUNT_PROG -t $FSTYP `_common_dev_mount_options $*` $SCRATCH_OPTIONS \
+		"/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME" $SCRATCH_MNT
+}
+
+_log_writes_unmount_snapshot()
+{
+	$UMOUNT_PROG $SCRATCH_MNT
+}
+
diff --git a/tests/generic/481 b/tests/generic/481
new file mode 100755
index 00000000..3985493d
--- /dev/null
+++ b/tests/generic/481
@@ -0,0 +1,124 @@ 
+#! /bin/bash
+# FS QA Test 481
+#
+# Test filesystem consistency after each FUA operation
+#
+# Will do log replay and check the filesystem.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2018 SuSE.  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 /
+	$KILLALL_PROG -KILL -q $FSSTRESS_PROG &> /dev/null
+	_log_writes_cleanup_snapshot &> /dev/null
+	_log_writes_cleanup &> /dev/null
+
+	# We use $TEST_DEV as snapshot COW device, which can't be
+	# mounted/recognized as normal fs, need to recreate it
+	# or fstest will complain about it
+	_mkfs_dev $TEST_DEV > /dev/null 2>&1
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/dmlogwrites
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_fs generic
+_supported_os Linux
+
+_require_command "$KILLALL_PROG" killall
+# Use $TEST_DEV as snapshot CoW device
+_require_test
+# Use $SCRATCH_DEV as replay device
+_require_scratch
+# and we need extra device as log device
+_require_log_writes
+
+
+workload=$(( 512 * $LOAD_FACTOR ))
+nr_threads=$(($($here/src/feature -o) * $LOAD_FACTOR))
+
+_test_unmount
+_log_writes_init
+_log_writes_mkfs >> $seqres.full 2>&1
+_log_writes_mark mkfs
+
+_log_writes_mount
+run_check $FSSTRESS_PROG -w -n $workload -p $nr_threads -d $SCRATCH_MNT \
+	$FSSTRESS_AVOID > /dev/null 2>&1
+_log_writes_unmount
+
+_log_writes_remove
+prev=$(_log_writes_mark_to_entry_number mkfs)
+[ -z "$prev" ] && _fail "failed to locate entry mark 'mkfs'"
+cur=$(_log_writes_find_next_fua $prev)
+[ -z "$cur" ] && _fail "failed to locate next FUA write"
+
+_log_writes_replay_log_entry_range $prev
+while [ ! -z "$cur" ]; do
+	_log_writes_replay_log_entry_range $cur $prev >> $seqres.full
+
+	echo "=== Replay to entry number $cur ===" >> $seqres.full
+	# Here we need extra mount to replay the log, mainly for journal based
+	# fs, as their fsck will report dirty log as error. So do snapshot
+	# to replay, so we can still continue replaying
+	_log_writes_create_snapshot $TEST_DEV
+	_log_writes_mount_snapshot
+	_log_writes_unmount_snapshot
+	_check_generic_filesystem "/dev/mapper/$DMLOGWRITES_SNAPSHOT_NAME"
+	if [ $? -ne 0 ]; then
+		_log_writes_replay_log_entry_range $cur $prev y >> $seqres.full
+		_fail "error found in fsck after log replay"
+	fi
+	_log_writes_cleanup_snapshot
+
+	prev=$cur
+	cur=$(_log_writes_find_next_fua $(($cur + 1)))
+	[ -z "$cur" ] && break
+done
+_log_writes_cleanup_snapshot
+_log_writes_cleanup
+
+# mkfs before ending the test case, or $TEST_DEV doesn't contain any
+# valid fs
+_mkfs_dev $TEST_DEV
+
+echo "Silence is golden"
+
+# success, all done
+status=0
+exit
diff --git a/tests/generic/481.out b/tests/generic/481.out
new file mode 100644
index 00000000..206e1163
--- /dev/null
+++ b/tests/generic/481.out
@@ -0,0 +1,2 @@ 
+QA output created by 481
+Silence is golden
diff --git a/tests/generic/group b/tests/generic/group
index ea2056b1..1de053a6 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -483,3 +483,4 @@ 
 478 auto quick
 479 auto quick metadata
 480 auto quick metadata
+481 auto