public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfstests: replace xfs_check with xfs_repair -n
@ 2013-04-17 16:38 Chandra Seetharaman
  2013-04-17 16:58 ` Eric Sandeen
  0 siblings, 1 reply; 5+ messages in thread
From: Chandra Seetharaman @ 2013-04-17 16:38 UTC (permalink / raw)
  To: XFS mailing list

Replace the usage of "xfs_check" with "xfs_repair -n" as xfs_check
is planned to be depracated.

Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---

diff --git a/common/config b/common/config
index bf62996..dfbb5c2 100644
--- a/common/config
+++ b/common/config
@@ -154,7 +154,6 @@ export DF_PROG="`set_prog_path df`"
 
 export XFS_LOGPRINT_PROG="`set_prog_path xfs_logprint`"
 export XFS_REPAIR_PROG="`set_prog_path xfs_repair`"
-export XFS_CHECK_PROG="`set_prog_path xfs_check`"
 export XFS_DB_PROG="`set_prog_path xfs_db`"
 export XFS_GROWFS_PROG=`set_prog_path xfs_growfs`
 export XFS_IO_PROG="`set_prog_path xfs_io`"
diff --git a/common/rc b/common/rc
index 09fb83f..32a852f 100644
--- a/common/rc
+++ b/common/rc
@@ -166,7 +166,6 @@ case "$FSTYP" in
     xfs)
 	 [ "$XFS_LOGPRINT_PROG" = "" ] && _fatal "xfs_logprint not found"
 	 [ "$XFS_REPAIR_PROG" = "" ] && _fatal "xfs_repair not found"
-	 [ "$XFS_CHECK_PROG" = "" ] && _fatal "xfs_check not found"
 	 [ "$XFS_DB_PROG" = "" ] && _fatal "xfs_db not found"
 	 [ "$MKFS_XFS_PROG" = "" ] && _fatal "mkfs_xfs not found"
 	 ;;
@@ -589,7 +588,7 @@ _scratch_xfs_check()
         SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
     [ "$LARGE_SCRATCH_DEV" = yes ] && \
         SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
-    $XFS_CHECK_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
+    $XFS_REPAIR_PROG -n $SCRATCH_OPTIONS $* $SCRATCH_DEV &>/dev/null
 }
 
 _scratch_xfs_repair()
@@ -1422,25 +1421,6 @@ _check_xfs_filesystem()
         ok=0
     fi
 
-    # xfs_check runs out of memory on large files, so even providing the test
-    # option (-t) to avoid indexing the free space trees doesn't make it pass on
-    # large filesystems. Avoid it.
-    if [ "$LARGE_SCRATCH_DEV" != yes ]; then
-	    $XFS_CHECK_PROG $extra_log_options $device 2>&1 |\
-		 _fix_malloc >$tmp.fs_check
-    fi
-    if [ -s $tmp.fs_check ]
-    then
-        echo "_check_xfs_filesystem: filesystem on $device is inconsistent (c) (see $seqres.full)"
-
-        echo "_check_xfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
-        echo "*** xfs_check output ***"		>>$seqres.full
-        cat $tmp.fs_check			>>$seqres.full
-        echo "*** end xfs_check output"		>>$seqres.full
-
-        ok=0
-    fi
-
     $XFS_REPAIR_PROG -n $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
     if [ $? -ne 0 ]
     then
diff --git a/crash/xfscrash b/crash/xfscrash
index 7831d7e..6437d54 100755
--- a/crash/xfscrash
+++ b/crash/xfscrash
@@ -119,12 +119,11 @@ _check()
     
     if [ $expect -eq 0 ]
     then
-        _echo "      *** xfs_check ($LOG/check_clean.out)"   
-        xfs_check $TEST_DEV &> $LOG/check_clean.out || fail=1
-        [ -s /tmp/xfs_check_clean.out ] && fail=1
+        _echo "      *** Running xfs_repair -n ($LOG/check_clean.out)"   
+        xfs_repair -n $TEST_DEV &> $LOG/check_clean.out && fail=1
     else
-        _echo "      *** xfs_check ($LOG/check_dirty.out)"   
-        xfs_check $TEST_DEV &> $LOG/check_dirty.out || fail=1
+        _echo "      *** Running xfs_repair -n ($LOG/check_dirty.out)"   
+        xfs_repair -n $TEST_DEV &> $LOG/check_dirty.out || fail=1
     fi
     
     if [ $fail -eq 0 -a $expect -eq 0 ]
diff --git a/tests/xfs/017 b/tests/xfs/017
index 9fc16c2..03f907e 100755
--- a/tests/xfs/017
+++ b/tests/xfs/017
@@ -83,7 +83,7 @@ do
         echo "*** XFS_CHECK ***"            >>$seqres.full
         echo ""                             >>$seqres.full
         _scratch_xfs_check                  >>$seqres.full 2>&1 \
-            || _fail "xfs_check failed"
+            || _fail "xfs check failed"
         _scratch_mount -o remount,rw \
             || _fail "remount rw failed"
 done
diff --git a/tests/xfs/085 b/tests/xfs/085
index 27f29a3..02c6703 100755
--- a/tests/xfs/085
+++ b/tests/xfs/085
@@ -70,7 +70,7 @@ _print_logstate
 
 # curious if FS consistent at start
 if false; then
-    if $XFS_CHECK_PROG $SCRATCH_DEV; then
+    if $XFS_REPAIR_PROG -n $SCRATCH_DEV &>/dev/null; then
        echo "*** checked ok ***"
     fi
 fi
diff --git a/tests/xfs/291 b/tests/xfs/291
old mode 100644
new mode 100755
index f842679..dc3c342
--- a/tests/xfs/291
+++ b/tests/xfs/291
@@ -111,9 +111,9 @@ for I in `seq 1 2 5000`; do
 done
 
 _scratch_unmount
-# Can xfs_repair and xfs_check cope with this monster?
+# Can xfs_repair and check cope with this monster?
 _scratch_xfs_repair >> $seqres.full 2>&1 || _fail "xfs_repair failed"
-xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_check failed"
+xfs_repair -n $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_repair -n failed"
 
 # Yes they can!  Now...
 # Can xfs_metadump cope with this monster?


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

* Re: [PATCH] xfstests: replace xfs_check with xfs_repair -n
  2013-04-17 16:38 [PATCH] xfstests: replace xfs_check with xfs_repair -n Chandra Seetharaman
@ 2013-04-17 16:58 ` Eric Sandeen
  2013-04-17 18:03   ` Chandra Seetharaman
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Sandeen @ 2013-04-17 16:58 UTC (permalink / raw)
  To: sekharan; +Cc: XFS mailing list

On 4/17/13 9:38 AM, Chandra Seetharaman wrote:
> Replace the usage of "xfs_check" with "xfs_repair -n" as xfs_check
> is planned to be depracated.

Hm, I thought the plan was to keep xfs_check around for xfstests
use, for now; as Dave said in the earlier thread:

> xfstests also still needs to run xfs_check. That means we also need
> either an override flag an make $XFS_CHECK_PROG have it set
> appropriately or add an internal xfs_db wrapper that runs the
> xfs_check functionality appropriately. The second is probably the
> better option...

but that's not what this patch does...

-Eric

> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> ---
> 
> diff --git a/common/config b/common/config
> index bf62996..dfbb5c2 100644
> --- a/common/config
> +++ b/common/config
> @@ -154,7 +154,6 @@ export DF_PROG="`set_prog_path df`"
>  
>  export XFS_LOGPRINT_PROG="`set_prog_path xfs_logprint`"
>  export XFS_REPAIR_PROG="`set_prog_path xfs_repair`"
> -export XFS_CHECK_PROG="`set_prog_path xfs_check`"
>  export XFS_DB_PROG="`set_prog_path xfs_db`"
>  export XFS_GROWFS_PROG=`set_prog_path xfs_growfs`
>  export XFS_IO_PROG="`set_prog_path xfs_io`"
> diff --git a/common/rc b/common/rc
> index 09fb83f..32a852f 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -166,7 +166,6 @@ case "$FSTYP" in
>      xfs)
>  	 [ "$XFS_LOGPRINT_PROG" = "" ] && _fatal "xfs_logprint not found"
>  	 [ "$XFS_REPAIR_PROG" = "" ] && _fatal "xfs_repair not found"
> -	 [ "$XFS_CHECK_PROG" = "" ] && _fatal "xfs_check not found"
>  	 [ "$XFS_DB_PROG" = "" ] && _fatal "xfs_db not found"
>  	 [ "$MKFS_XFS_PROG" = "" ] && _fatal "mkfs_xfs not found"
>  	 ;;
> @@ -589,7 +588,7 @@ _scratch_xfs_check()
>          SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
>      [ "$LARGE_SCRATCH_DEV" = yes ] && \
>          SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
> -    $XFS_CHECK_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
> +    $XFS_REPAIR_PROG -n $SCRATCH_OPTIONS $* $SCRATCH_DEV &>/dev/null
>  }
>  
>  _scratch_xfs_repair()
> @@ -1422,25 +1421,6 @@ _check_xfs_filesystem()
>          ok=0
>      fi
>  
> -    # xfs_check runs out of memory on large files, so even providing the test
> -    # option (-t) to avoid indexing the free space trees doesn't make it pass on
> -    # large filesystems. Avoid it.
> -    if [ "$LARGE_SCRATCH_DEV" != yes ]; then
> -	    $XFS_CHECK_PROG $extra_log_options $device 2>&1 |\
> -		 _fix_malloc >$tmp.fs_check
> -    fi
> -    if [ -s $tmp.fs_check ]
> -    then
> -        echo "_check_xfs_filesystem: filesystem on $device is inconsistent (c) (see $seqres.full)"
> -
> -        echo "_check_xfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
> -        echo "*** xfs_check output ***"		>>$seqres.full
> -        cat $tmp.fs_check			>>$seqres.full
> -        echo "*** end xfs_check output"		>>$seqres.full
> -
> -        ok=0
> -    fi
> -
>      $XFS_REPAIR_PROG -n $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
>      if [ $? -ne 0 ]
>      then
> diff --git a/crash/xfscrash b/crash/xfscrash
> index 7831d7e..6437d54 100755
> --- a/crash/xfscrash
> +++ b/crash/xfscrash
> @@ -119,12 +119,11 @@ _check()
>      
>      if [ $expect -eq 0 ]
>      then
> -        _echo "      *** xfs_check ($LOG/check_clean.out)"   
> -        xfs_check $TEST_DEV &> $LOG/check_clean.out || fail=1
> -        [ -s /tmp/xfs_check_clean.out ] && fail=1
> +        _echo "      *** Running xfs_repair -n ($LOG/check_clean.out)"   
> +        xfs_repair -n $TEST_DEV &> $LOG/check_clean.out && fail=1
>      else
> -        _echo "      *** xfs_check ($LOG/check_dirty.out)"   
> -        xfs_check $TEST_DEV &> $LOG/check_dirty.out || fail=1
> +        _echo "      *** Running xfs_repair -n ($LOG/check_dirty.out)"   
> +        xfs_repair -n $TEST_DEV &> $LOG/check_dirty.out || fail=1
>      fi
>      
>      if [ $fail -eq 0 -a $expect -eq 0 ]
> diff --git a/tests/xfs/017 b/tests/xfs/017
> index 9fc16c2..03f907e 100755
> --- a/tests/xfs/017
> +++ b/tests/xfs/017
> @@ -83,7 +83,7 @@ do
>          echo "*** XFS_CHECK ***"            >>$seqres.full
>          echo ""                             >>$seqres.full
>          _scratch_xfs_check                  >>$seqres.full 2>&1 \
> -            || _fail "xfs_check failed"
> +            || _fail "xfs check failed"
>          _scratch_mount -o remount,rw \
>              || _fail "remount rw failed"
>  done
> diff --git a/tests/xfs/085 b/tests/xfs/085
> index 27f29a3..02c6703 100755
> --- a/tests/xfs/085
> +++ b/tests/xfs/085
> @@ -70,7 +70,7 @@ _print_logstate
>  
>  # curious if FS consistent at start
>  if false; then
> -    if $XFS_CHECK_PROG $SCRATCH_DEV; then
> +    if $XFS_REPAIR_PROG -n $SCRATCH_DEV &>/dev/null; then
>         echo "*** checked ok ***"
>      fi
>  fi
> diff --git a/tests/xfs/291 b/tests/xfs/291
> old mode 100644
> new mode 100755
> index f842679..dc3c342
> --- a/tests/xfs/291
> +++ b/tests/xfs/291
> @@ -111,9 +111,9 @@ for I in `seq 1 2 5000`; do
>  done
>  
>  _scratch_unmount
> -# Can xfs_repair and xfs_check cope with this monster?
> +# Can xfs_repair and check cope with this monster?
>  _scratch_xfs_repair >> $seqres.full 2>&1 || _fail "xfs_repair failed"
> -xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_check failed"
> +xfs_repair -n $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_repair -n failed"
>  
>  # Yes they can!  Now...
>  # Can xfs_metadump cope with this monster?
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

* Re: [PATCH] xfstests: replace xfs_check with xfs_repair -n
  2013-04-17 16:58 ` Eric Sandeen
@ 2013-04-17 18:03   ` Chandra Seetharaman
  2013-04-17 18:23     ` Eric Sandeen
  0 siblings, 1 reply; 5+ messages in thread
From: Chandra Seetharaman @ 2013-04-17 18:03 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: XFS mailing list

Hi Eric,

Thanks for the quick feedback.

On Wed, 2013-04-17 at 09:58 -0700, Eric Sandeen wrote:
> On 4/17/13 9:38 AM, Chandra Seetharaman wrote:
> > Replace the usage of "xfs_check" with "xfs_repair -n" as xfs_check
> > is planned to be depracated.
> 
> Hm, I thought the plan was to keep xfs_check around for xfstests

I didn't think the plan was to keep xfs_check, may be I misunderstood.
My understanding was that we wanted to deprecate xfs_check, but first we
have to make xfstests not use xfs_check.

> use, for now; as Dave said in the earlier thread:
> 
> > xfstests also still needs to run xfs_check. That means we also need
> > either an override flag an make $XFS_CHECK_PROG have it set
> > appropriately or add an internal xfs_db wrapper that runs the
> > xfs_check functionality appropriately. The second is probably the
> > better option...
> 
> but that's not what this patch does...

The usages of xfs_check in xfstests looked simple and straight forward.
Besides, I thought we should do what we suggest our users to do :),
hence replaced xfs_check with "xfs_repair -n".

Does this patch break something or technically incorrect ?

Do you think I should instead use 
    xfs_db -F -i -p xfs_check -c "check" <dev>

Please advise.

Chandra
> -Eric
> 
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > ---
> > 
> > diff --git a/common/config b/common/config
> > index bf62996..dfbb5c2 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -154,7 +154,6 @@ export DF_PROG="`set_prog_path df`"
> >  
> >  export XFS_LOGPRINT_PROG="`set_prog_path xfs_logprint`"
> >  export XFS_REPAIR_PROG="`set_prog_path xfs_repair`"
> > -export XFS_CHECK_PROG="`set_prog_path xfs_check`"
> >  export XFS_DB_PROG="`set_prog_path xfs_db`"
> >  export XFS_GROWFS_PROG=`set_prog_path xfs_growfs`
> >  export XFS_IO_PROG="`set_prog_path xfs_io`"
> > diff --git a/common/rc b/common/rc
> > index 09fb83f..32a852f 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -166,7 +166,6 @@ case "$FSTYP" in
> >      xfs)
> >  	 [ "$XFS_LOGPRINT_PROG" = "" ] && _fatal "xfs_logprint not found"
> >  	 [ "$XFS_REPAIR_PROG" = "" ] && _fatal "xfs_repair not found"
> > -	 [ "$XFS_CHECK_PROG" = "" ] && _fatal "xfs_check not found"
> >  	 [ "$XFS_DB_PROG" = "" ] && _fatal "xfs_db not found"
> >  	 [ "$MKFS_XFS_PROG" = "" ] && _fatal "mkfs_xfs not found"
> >  	 ;;
> > @@ -589,7 +588,7 @@ _scratch_xfs_check()
> >          SCRATCH_OPTIONS="-l $SCRATCH_LOGDEV"
> >      [ "$LARGE_SCRATCH_DEV" = yes ] && \
> >          SCRATCH_OPTIONS=$SCRATCH_OPTIONS" -t"
> > -    $XFS_CHECK_PROG $SCRATCH_OPTIONS $* $SCRATCH_DEV
> > +    $XFS_REPAIR_PROG -n $SCRATCH_OPTIONS $* $SCRATCH_DEV &>/dev/null
> >  }
> >  
> >  _scratch_xfs_repair()
> > @@ -1422,25 +1421,6 @@ _check_xfs_filesystem()
> >          ok=0
> >      fi
> >  
> > -    # xfs_check runs out of memory on large files, so even providing the test
> > -    # option (-t) to avoid indexing the free space trees doesn't make it pass on
> > -    # large filesystems. Avoid it.
> > -    if [ "$LARGE_SCRATCH_DEV" != yes ]; then
> > -	    $XFS_CHECK_PROG $extra_log_options $device 2>&1 |\
> > -		 _fix_malloc >$tmp.fs_check
> > -    fi
> > -    if [ -s $tmp.fs_check ]
> > -    then
> > -        echo "_check_xfs_filesystem: filesystem on $device is inconsistent (c) (see $seqres.full)"
> > -
> > -        echo "_check_xfs_filesystem: filesystem on $device is inconsistent" >>$seqres.full
> > -        echo "*** xfs_check output ***"		>>$seqres.full
> > -        cat $tmp.fs_check			>>$seqres.full
> > -        echo "*** end xfs_check output"		>>$seqres.full
> > -
> > -        ok=0
> > -    fi
> > -
> >      $XFS_REPAIR_PROG -n $extra_log_options $extra_rt_options $device >$tmp.repair 2>&1
> >      if [ $? -ne 0 ]
> >      then
> > diff --git a/crash/xfscrash b/crash/xfscrash
> > index 7831d7e..6437d54 100755
> > --- a/crash/xfscrash
> > +++ b/crash/xfscrash
> > @@ -119,12 +119,11 @@ _check()
> >      
> >      if [ $expect -eq 0 ]
> >      then
> > -        _echo "      *** xfs_check ($LOG/check_clean.out)"   
> > -        xfs_check $TEST_DEV &> $LOG/check_clean.out || fail=1
> > -        [ -s /tmp/xfs_check_clean.out ] && fail=1
> > +        _echo "      *** Running xfs_repair -n ($LOG/check_clean.out)"   
> > +        xfs_repair -n $TEST_DEV &> $LOG/check_clean.out && fail=1
> >      else
> > -        _echo "      *** xfs_check ($LOG/check_dirty.out)"   
> > -        xfs_check $TEST_DEV &> $LOG/check_dirty.out || fail=1
> > +        _echo "      *** Running xfs_repair -n ($LOG/check_dirty.out)"   
> > +        xfs_repair -n $TEST_DEV &> $LOG/check_dirty.out || fail=1
> >      fi
> >      
> >      if [ $fail -eq 0 -a $expect -eq 0 ]
> > diff --git a/tests/xfs/017 b/tests/xfs/017
> > index 9fc16c2..03f907e 100755
> > --- a/tests/xfs/017
> > +++ b/tests/xfs/017
> > @@ -83,7 +83,7 @@ do
> >          echo "*** XFS_CHECK ***"            >>$seqres.full
> >          echo ""                             >>$seqres.full
> >          _scratch_xfs_check                  >>$seqres.full 2>&1 \
> > -            || _fail "xfs_check failed"
> > +            || _fail "xfs check failed"
> >          _scratch_mount -o remount,rw \
> >              || _fail "remount rw failed"
> >  done
> > diff --git a/tests/xfs/085 b/tests/xfs/085
> > index 27f29a3..02c6703 100755
> > --- a/tests/xfs/085
> > +++ b/tests/xfs/085
> > @@ -70,7 +70,7 @@ _print_logstate
> >  
> >  # curious if FS consistent at start
> >  if false; then
> > -    if $XFS_CHECK_PROG $SCRATCH_DEV; then
> > +    if $XFS_REPAIR_PROG -n $SCRATCH_DEV &>/dev/null; then
> >         echo "*** checked ok ***"
> >      fi
> >  fi
> > diff --git a/tests/xfs/291 b/tests/xfs/291
> > old mode 100644
> > new mode 100755
> > index f842679..dc3c342
> > --- a/tests/xfs/291
> > +++ b/tests/xfs/291
> > @@ -111,9 +111,9 @@ for I in `seq 1 2 5000`; do
> >  done
> >  
> >  _scratch_unmount
> > -# Can xfs_repair and xfs_check cope with this monster?
> > +# Can xfs_repair and check cope with this monster?
> >  _scratch_xfs_repair >> $seqres.full 2>&1 || _fail "xfs_repair failed"
> > -xfs_check $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_check failed"
> > +xfs_repair -n $SCRATCH_DEV >> $seqres.full 2>&1 || _fail "xfs_repair -n failed"
> >  
> >  # Yes they can!  Now...
> >  # Can xfs_metadump cope with this monster?
> > 
> > 
> > _______________________________________________
> > xfs mailing list
> > xfs@oss.sgi.com
> > http://oss.sgi.com/mailman/listinfo/xfs
> > 
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

* Re: [PATCH] xfstests: replace xfs_check with xfs_repair -n
  2013-04-17 18:03   ` Chandra Seetharaman
@ 2013-04-17 18:23     ` Eric Sandeen
  2013-04-17 18:32       ` Chandra Seetharaman
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Sandeen @ 2013-04-17 18:23 UTC (permalink / raw)
  To: sekharan; +Cc: XFS mailing list

On 4/17/13 11:03 AM, Chandra Seetharaman wrote:
> Hi Eric,
> 
> Thanks for the quick feedback.
> 
> On Wed, 2013-04-17 at 09:58 -0700, Eric Sandeen wrote:
>> On 4/17/13 9:38 AM, Chandra Seetharaman wrote:
>>> Replace the usage of "xfs_check" with "xfs_repair -n" as xfs_check
>>> is planned to be depracated.
>>
>> Hm, I thought the plan was to keep xfs_check around for xfstests
> 
> I didn't think the plan was to keep xfs_check, may be I misunderstood.
> My understanding was that we wanted to deprecate xfs_check, but first we
> have to make xfstests not use xfs_check.
> 
>> use, for now; as Dave said in the earlier thread:
>>
>>> xfstests also still needs to run xfs_check. That means we also need
>>> either an override flag an make $XFS_CHECK_PROG have it set
>>> appropriately or add an internal xfs_db wrapper that runs the
>>> xfs_check functionality appropriately. The second is probably the
>>> better option...
>>
>> but that's not what this patch does...
> 
> The usages of xfs_check in xfstests looked simple and straight forward.
> Besides, I thought we should do what we suggest our users to do :),
> hence replaced xfs_check with "xfs_repair -n".

Dave or others can chime in too, but I think we still want xfs_check
(xfs_db) as a verifier against xfs_repair.

> Does this patch break something or technically incorrect ?

We used to explicitly run both xfs_repair and xfs_check to get two
distinct verification passes; the patch removes part of that, so I'd
say yes, it does "break" things a little.

> Do you think I should instead use 
>     xfs_db -F -i -p xfs_check -c "check" <dev>

Right, if the xfs_check script itself is going away, I think we still
want to invoke "xfs_check" behavior one way or another in xfstests to
keep current xfs verification levels for now.

Thanks,
-Eric

> Please advise.




_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

* Re: [PATCH] xfstests: replace xfs_check with xfs_repair -n
  2013-04-17 18:23     ` Eric Sandeen
@ 2013-04-17 18:32       ` Chandra Seetharaman
  0 siblings, 0 replies; 5+ messages in thread
From: Chandra Seetharaman @ 2013-04-17 18:32 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: XFS mailing list

On Wed, 2013-04-17 at 11:23 -0700, Eric Sandeen wrote:
> On 4/17/13 11:03 AM, Chandra Seetharaman wrote:
> > Hi Eric,
> > 
> > Thanks for the quick feedback.
> > 
> > On Wed, 2013-04-17 at 09:58 -0700, Eric Sandeen wrote:
> >> On 4/17/13 9:38 AM, Chandra Seetharaman wrote:
> >>> Replace the usage of "xfs_check" with "xfs_repair -n" as xfs_check
> >>> is planned to be depracated.
> >>
> >> Hm, I thought the plan was to keep xfs_check around for xfstests
> > 
> > I didn't think the plan was to keep xfs_check, may be I misunderstood.
> > My understanding was that we wanted to deprecate xfs_check, but first we
> > have to make xfstests not use xfs_check.
> > 
> >> use, for now; as Dave said in the earlier thread:
> >>
> >>> xfstests also still needs to run xfs_check. That means we also need
> >>> either an override flag an make $XFS_CHECK_PROG have it set
> >>> appropriately or add an internal xfs_db wrapper that runs the
> >>> xfs_check functionality appropriately. The second is probably the
> >>> better option...
> >>
> >> but that's not what this patch does...
> > 
> > The usages of xfs_check in xfstests looked simple and straight forward.
> > Besides, I thought we should do what we suggest our users to do :),
> > hence replaced xfs_check with "xfs_repair -n".
> 
> Dave or others can chime in too, but I think we still want xfs_check
> (xfs_db) as a verifier against xfs_repair.
> 
> > Does this patch break something or technically incorrect ?
> 
> We used to explicitly run both xfs_repair and xfs_check to get two
> distinct verification passes; the patch removes part of that, so I'd
> say yes, it does "break" things a little.
> 
> > Do you think I should instead use 
> >     xfs_db -F -i -p xfs_check -c "check" <dev>
> 
> Right, if the xfs_check script itself is going away, I think we still
> want to invoke "xfs_check" behavior one way or another in xfstests to
> keep current xfs verification levels for now.

It is clear now. will make appropriate changes and resubmit.

> 
> Thanks,
> -Eric
> 
> > Please advise.
> 
> 
> 
> 


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

end of thread, other threads:[~2013-04-17 18:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-17 16:38 [PATCH] xfstests: replace xfs_check with xfs_repair -n Chandra Seetharaman
2013-04-17 16:58 ` Eric Sandeen
2013-04-17 18:03   ` Chandra Seetharaman
2013-04-17 18:23     ` Eric Sandeen
2013-04-17 18:32       ` Chandra Seetharaman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox