xfs
[Top] [All Lists]

Re: [PATCH 2/2] xfstests: Check fs consistency on TEST_DEV only when nee

To: Lukas Czerner <lczerner@xxxxxxxxxx>
Subject: Re: [PATCH 2/2] xfstests: Check fs consistency on TEST_DEV only when needed
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 10 Jul 2014 11:07:11 +1000
Cc: fstests@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1403617014-17870-2-git-send-email-lczerner@xxxxxxxxxx>
References: <1403617014-17870-1-git-send-email-lczerner@xxxxxxxxxx> <1403617014-17870-2-git-send-email-lczerner@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Jun 24, 2014 at 03:36:54PM +0200, Lukas Czerner wrote:
> Currently we're checking file system consistency on TEST_DEV after every
> successful test run even though the TEST_DEV might not even be used in
> that test.

It gets mounted/unmounted for each test, so even if it is not used,
it might still get modified...

> Fix it by checking for number of write io on TEST_DEV device before the
> test and compare it with the value after the test.

Doesn't work for ram disks:

# ./setup 
TEST: DIR=/mnt/test DEV=/dev/ram0 rt=[] log=[]
TAPE: dev=[] rmt=[] rmtirix=[@]
SCRATCH: MNT=/mnt/scr DEV=/dev/ram1 rt=[] log=[]
VARIABLES: external=no largeblk=no fstyp=xfs
           large_scratch_dev=no attrsecure=no
# ls -l /dev/ram0
brw-rw---- 1 root disk 1, 0 Jul  9 15:51 /dev/ram0
# cat /sys/dev/block/1\:0/stat
       0        0        0        0        0        0        0        0        
0        0        0
#

Nor does it work on loop devices.

> Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx>
> ---
>  check     |  6 +++++-
>  common/rc | 11 +++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/check b/check
> index 40f99d7..66d0f72 100755
> --- a/check
> +++ b/check
> @@ -544,6 +544,7 @@ for section in $HOST_OPTIONS_SECTIONS; do
>               rm -f core $seqres.notrun
>  
>               scratch_atime=`stat -c %X $SCRATCH_DEV`
> +             test_write_io=`_get_write_io_count $TEST_DEV`
>               start=`_wallclock`
>               $timestamp && echo -n " ["`date "+%T"`"]"
>               [ ! -x $seq ] && chmod u+x $seq # ensure we can run it
> @@ -626,7 +627,10 @@ for section in $HOST_OPTIONS_SECTIONS; do
>           then
>               try="$try $seqnum"
>               n_try=`expr $n_try + 1`
> -             _check_test_fs
> +             write_io=`_get_write_io_count $TEST_DEV`
> +             if [ -z $test_write_io ] || [ $write_io -ne $test_write_io ]; 
> then
> +                     _check_test_fs
> +             fi

Which you work around without mentioning the reason for the zero
value check. ;)

Personally, I want the test devices on my systems checked regardless
of whether the test tries to modify them or not. I, quite frankly,
don't trust the block device stats because they often get broken or
are inaccurate. Indeed, there were recent virtio block device
changes in 3.14-rc1 (or was it 3.15?) that broke the device stats
and that took several weeks to get fixed properly.

Hence I don't think using the block device stats for something as
critical as determining whether we should check a filesystem or not.

Reducing runtime of the test harness is a good idea, but we should
not do that if the cost is that it reduces the reliability of error
detection.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>