thr3ads.net - Btrfs devel - [PATCH] xfstests btrfs/284: shorten duration, fix output [Apr 2013]

If this information is useful, please help other people find it:
Share via:

Eric Sandeen

2013-Apr-26 18:45 UTC

[PATCH] xfstests btrfs/284: shorten duration, fix output

test 284 had... some issues.

First, it took so long nobody ran it; so shorten the extent
count by a factor of about 100.

Having fixed that, we see failures in 2 cases; when start or
len is -1, but the golden output file didn''t have error
output, as if they should pass.

I''m going to argue that these *should* both fail; start = -1
has no real meaning.  length = -1 might mean "the rest
of the file" but if that''s what you really want, just
don''t specify -l.

So add failure output for those cases.

Send all command output to $seq.full, in case that changes
in the future; just capture the return value.

Then remove the return value echo on failure (50?) because
who knows when that might change to some other magic value.

Ok, then when defrag actually works, old defrag returned
"20" (because?) but a recent commit changed it to 0.
So accommodate that too.

And remove a stray "HAVE_DEFRAG=1" while we''re at it.
That variable is never used.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/tests/btrfs/284 b/tests/btrfs/284
old mode 100644
new mode 100755
index d952977..67161a3
--- a/tests/btrfs/284
+++ b/tests/btrfs/284
@@ -26,7 +26,7 @@ seqres=$RESULT_DIR/$seq
 echo "QA output created by $seq"
 here="`pwd`"
 tmp=/tmp/$$
-cnt=11999
+cnt=119
 filesize=48000
 
 status=1	# failure is the default!
@@ -58,11 +58,12 @@ _create_file()
 _btrfs_online_defrag()
 {
 	str=""
+	# start = -1 is invalid, should fail
 	if [ "$2" = "2" ];then
 		str="$str -s -1 -l $((filesize / 2)) "
 	elif [ "$2" = "3" ];then
 		str="$str -s $((filesize + 1)) -l $((filesize / 2)) "
-		HAVE_DEFRAG=1
+	# len = -1 is invalid, should fail
 	elif [ "$2" = "4" ];then
 		str="$str -l -1 "
 	elif [ "$2" = "5" ];then
@@ -76,20 +77,22 @@ _btrfs_online_defrag()
 	fi
 
 	if [ "$str" != "" ]; then
-		$BTRFS_UTIL_PROG filesystem defragment $str $SCRATCH_MNT/tmp_file
+		$BTRFS_UTIL_PROG filesystem defragment $str $SCRATCH_MNT/tmp_file >>
$seq.full 2>&1
 	else
 		if [ "$1" = "1" ];then
-			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT/tmp_file
+			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT/tmp_file >>
$seq.full 2>&1
 		elif [ "$1" = "2" ];then
-			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT/tmp_dir
+			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT/tmp_dir >>
$seq.full 2>&1
 		elif [ "$1" = "3" ];then
-			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT
+			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT >> $seq.full
2>&1
 		fi
 	fi
 	ret_val=$?
 	_scratch_remount
-	if [ $ret_val -ne 20 ];then
-		echo "btrfs filesystem defragment failed! err is $ret_val"
+	# Older defrag returned "20" for success
+	# e9393c2 btrfs-progs: defrag return zero on success
+	if [ $ret_val -ne 0 -a $ret_val -ne 20 ]; then
+		echo "btrfs filesystem defragment failed!"
 	fi
 }
 
@@ -140,19 +143,19 @@ _scratch_mount
 _require_defrag
 
 echo "defrag object | defragment range | defragment compress"
-echo "a single file |  default | off"
+echo "a single file | default | off"
 _rundefrag 1 1 1
 
 echo "a single file | default |  on"
 _rundefrag 1 1 2
 
-echo "a single file | start < 0 && 0 < len < file size |
off"
+echo "a single file | start < 0 && 0 < len < file size |
off (should fail)"
 _rundefrag 1 2 1
 
 echo "a single file | start > file size && 0 < len < file
size | off"
 _rundefrag 1 3 1
 
-echo "a single file | start = 0 && len < 0 | off"
+echo "a single file | start = 0 && len < 0 | off (should
fail)"
 _rundefrag 1 4 1
 
 echo "a single file | start = 0 && len > file size | off"
diff --git a/tests/btrfs/284.out b/tests/btrfs/284.out
index 4a69f82..c942271 100644
--- a/tests/btrfs/284.out
+++ b/tests/btrfs/284.out
@@ -1,10 +1,12 @@
 QA output created by 284
 defrag object | defragment range | defragment compress
-a single file |  default | off
+a single file | default | off
 a single file | default |  on
-a single file | start < 0 && 0 < len < file size | off
+a single file | start < 0 && 0 < len < file size | off (should
fail)
+btrfs filesystem defragment failed!
 a single file | start > file size && 0 < len < file size | off
-a single file | start = 0 && len < 0 | off
+a single file | start = 0 && len < 0 | off (should fail)
+btrfs filesystem defragment failed!
 a single file | start = 0 && len > file size | off
 a single file | start = 0 && 0 < len < file size | off
 a directory | default | off


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

Rich Johnston

2013-May-14 15:19 UTC

head link

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output

On 04/26/2013 01:45 PM, Eric Sandeen wrote:> test 284 had... some issues.
>
> First, it took so long nobody ran it; so shorten the extent
> count by a factor of about 100.
>
> Having fixed that, we see failures in 2 cases; when start or
> len is -1, but the golden output file didn''t have error
> output, as if they should pass.
>
> I''m going to argue that these *should* both fail; start = -1
> has no real meaning.  length = -1 might mean "the rest
> of the file" but if that''s what you really want, just
> don''t specify -l.
>
> So add failure output for those cases.
>
> Send all command output to $seq.full, in case that changes
> in the future; just capture the return value.
>
> Then remove the return value echo on failure (50?) because
> who knows when that might change to some other magic value.
>
> Ok, then when defrag actually works, old defrag returned
> "20" (because?) but a recent commit changed it to 0.
> So accommodate that too.
>
> And remove a stray "HAVE_DEFRAG=1" while we''re at it.
> That variable is never used.
>
So should I be seeing failures with
btrfs-progs-0.20-0.2.git91d9eec.el6.x86_64 installed?

./check btrfs/284
FSTYP         -- btrfs
PLATFORM      -- Linux/x86_64 cxfsxe4 3.9.0+
MKFS_OPTIONS  -- /dev/sdk2
MOUNT_OPTIONS -- /dev/sdk2 /mnt/scratch

btrfs/284	 - output mismatch (see 
/usr/src/rcj/xfstests/results/btrfs/284.out.bad)
     --- tests/btrfs/284.out	2013-05-14 09:31:35.000000000 -0500
     +++ /usr/src/rcj/xfstests/results/btrfs/284.out.bad	2013-05-14 
10:10:45.000000000 -0500
     @@ -6,7 +6,6 @@
      btrfs filesystem defragment failed!
      a single file | start > file size && 0 < len < file size
| off
      a single file | start = 0 && len < 0 | off (should fail)
     -btrfs filesystem defragment failed!
      a single file | start = 0 && len > file size | off
      a single file | start = 0 && 0 < len < file size | off
      a directory | default | off
      ...
      (Run ''diff -u tests/btrfs/284.out 
/usr/src/rcj/xfstests/results/btrfs/284.out.bad'' to see the entire
diff)
Ran: btrfs/284
Failures: btrfs/284
Failed 1 of 1 tests

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

Eric Sandeen

2013-May-14 17:38 UTC

head link

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output

On 5/14/13 10:19 AM, Rich Johnston wrote:> On 04/26/2013 01:45 PM, Eric Sandeen wrote:
>> test 284 had... some issues.
>>
>> First, it took so long nobody ran it; so shorten the extent
>> count by a factor of about 100.
>>
>> Having fixed that, we see failures in 2 cases; when start or
>> len is -1, but the golden output file didn''t have error
>> output, as if they should pass.
>>
>> I''m going to argue that these *should* both fail; start = -1
>> has no real meaning.  length = -1 might mean "the rest
>> of the file" but if that''s what you really want, just
>> don''t specify -l.
>>
>> So add failure output for those cases.
>>
>> Send all command output to $seq.full, in case that changes
>> in the future; just capture the return value.
>>
>> Then remove the return value echo on failure (50?) because
>> who knows when that might change to some other magic value.
>>
>> Ok, then when defrag actually works, old defrag returned
>> "20" (because?) but a recent commit changed it to 0.
>> So accommodate that too.
>>
>> And remove a stray "HAVE_DEFRAG=1" while we''re at
it.
>> That variable is never used.
>>
> 
> So should I be seeing failures with
> btrfs-progs-0.20-0.2.git91d9eec.el6.x86_64 installed?
Maybe? ...if that''s an old version in rhel6.

If you really want to investigate this, you could grab i.e.
http://kojipkgs.fedoraproject.org/packages/btrfs-progs/0.20.rc1.20130501git7854c8b/3.fc20/src/btrfs-progs-0.20.rc1.20130501git7854c8b-3.fc20.src.rpm
and rebuild it for something newer.  (Or I could double check . . . )

But honestly as the sgi xfstests maintainer I think you are going well above and
beyond your duties here.

Ideally, someone from the btrfs community could help out here, and test/review
the change...

-Eric
> ./check btrfs/284
> FSTYP         -- btrfs
> PLATFORM      -- Linux/x86_64 cxfsxe4 3.9.0+
> MKFS_OPTIONS  -- /dev/sdk2
> MOUNT_OPTIONS -- /dev/sdk2 /mnt/scratch
> 
> btrfs/284     - output mismatch (see
/usr/src/rcj/xfstests/results/btrfs/284.out.bad)
>     --- tests/btrfs/284.out    2013-05-14 09:31:35.000000000 -0500
>     +++ /usr/src/rcj/xfstests/results/btrfs/284.out.bad    2013-05-14
10:10:45.000000000 -0500
>     @@ -6,7 +6,6 @@
>      btrfs filesystem defragment failed!
>      a single file | start > file size && 0 < len < file
size | off
>      a single file | start = 0 && len < 0 | off (should fail)
>     -btrfs filesystem defragment failed!
>      a single file | start = 0 && len > file size | off
>      a single file | start = 0 && 0 < len < file size | off
>      a directory | default | off
>      ...
>      (Run ''diff -u tests/btrfs/284.out
/usr/src/rcj/xfstests/results/btrfs/284.out.bad'' to see the entire
diff)
> Ran: btrfs/284
> Failures: btrfs/284
> Failed 1 of 1 tests
> 
--
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

Josef Bacik

2013-May-14 20:42 UTC

head link

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output

On Tue, May 14, 2013 at 09:19:46AM -0600, Rich Johnston
wrote:> On 04/26/2013 01:45 PM, Eric Sandeen wrote:
> > test 284 had... some issues.
> >
> > First, it took so long nobody ran it; so shorten the extent
> > count by a factor of about 100.
> >
> > Having fixed that, we see failures in 2 cases; when start or
> > len is -1, but the golden output file didn''t have error
> > output, as if they should pass.
> >
> > I''m going to argue that these *should* both fail; start = -1
> > has no real meaning.  length = -1 might mean "the rest
> > of the file" but if that''s what you really want, just
> > don''t specify -l.
> >
> > So add failure output for those cases.
> >
> > Send all command output to $seq.full, in case that changes
> > in the future; just capture the return value.
> >
> > Then remove the return value echo on failure (50?) because
> > who knows when that might change to some other magic value.
> >
> > Ok, then when defrag actually works, old defrag returned
> > "20" (because?) but a recent commit changed it to 0.
> > So accommodate that too.
> >
> > And remove a stray "HAVE_DEFRAG=1" while we''re at
it.
> > That variable is never used.
> >
> 
> So should I be seeing failures with
> btrfs-progs-0.20-0.2.git91d9eec.el6.x86_64 installed?
> 
> ./check btrfs/284
> FSTYP         -- btrfs
> PLATFORM      -- Linux/x86_64 cxfsxe4 3.9.0+
> MKFS_OPTIONS  -- /dev/sdk2
> MOUNT_OPTIONS -- /dev/sdk2 /mnt/scratch
> 
> btrfs/284	 - output mismatch (see 
> /usr/src/rcj/xfstests/results/btrfs/284.out.bad)
>      --- tests/btrfs/284.out	2013-05-14 09:31:35.000000000 -0500
>      +++ /usr/src/rcj/xfstests/results/btrfs/284.out.bad	2013-05-14 
> 10:10:45.000000000 -0500
>      @@ -6,7 +6,6 @@
>       btrfs filesystem defragment failed!
>       a single file | start > file size && 0 < len < file
size | off
>       a single file | start = 0 && len < 0 | off (should fail)
>      -btrfs filesystem defragment failed!
>       a single file | start = 0 && len > file size | off
>       a single file | start = 0 && 0 < len < file size | off
>       a directory | default | off
>       ...
>       (Run ''diff -u tests/btrfs/284.out 
> /usr/src/rcj/xfstests/results/btrfs/284.out.bad'' to see the entire
diff)
> Ran: btrfs/284
> Failures: btrfs/284
> Failed 1 of 1 tests
> 
Yeah defrag used to spit out a return value on success, that has been fixed
recently.  This patch looks good, I just ran it on my box and it ran fine, you
can add

Reviewed-by: Josef Bacik <jbacik@fusionio.com>

Thanks,

Josef

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

Liu Bo

2013-May-15 01:42 UTC

head link

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output

On Fri, Apr 26, 2013 at 01:45:21PM -0500, Eric Sandeen
wrote:> test 284 had... some issues.
> 
> First, it took so long nobody ran it; so shorten the extent
> count by a factor of about 100.
> 
> Having fixed that, we see failures in 2 cases; when start or
> len is -1, but the golden output file didn''t have error
> output, as if they should pass.
> 
> I''m going to argue that these *should* both fail; start = -1
> has no real meaning.  length = -1 might mean "the rest
> of the file" but if that''s what you really want, just
> don''t specify -l.
> 
> So add failure output for those cases.
> 
> Send all command output to $seq.full, in case that changes
> in the future; just capture the return value.
> 
> Then remove the return value echo on failure (50?) because
> who knows when that might change to some other magic value.
> 
> Ok, then when defrag actually works, old defrag returned
> "20" (because?) but a recent commit changed it to 0.
> So accommodate that too.
> 
> And remove a stray "HAVE_DEFRAG=1" while we''re at it.
> That variable is never used.
Thanks for working on this, Eric!

You can add

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

thanks,
liubo
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/tests/btrfs/284 b/tests/btrfs/284
> old mode 100644
> new mode 100755
> index d952977..67161a3
> --- a/tests/btrfs/284
> +++ b/tests/btrfs/284
> @@ -26,7 +26,7 @@ seqres=$RESULT_DIR/$seq
>  echo "QA output created by $seq"
>  here="`pwd`"
>  tmp=/tmp/$$
> -cnt=11999
> +cnt=119
>  filesize=48000
>  
>  status=1	# failure is the default!
> @@ -58,11 +58,12 @@ _create_file()
>  _btrfs_online_defrag()
>  {
>  	str=""
> +	# start = -1 is invalid, should fail
>  	if [ "$2" = "2" ];then
>  		str="$str -s -1 -l $((filesize / 2)) "
>  	elif [ "$2" = "3" ];then
>  		str="$str -s $((filesize + 1)) -l $((filesize / 2)) "
> -		HAVE_DEFRAG=1
> +	# len = -1 is invalid, should fail
>  	elif [ "$2" = "4" ];then
>  		str="$str -l -1 "
>  	elif [ "$2" = "5" ];then
> @@ -76,20 +77,22 @@ _btrfs_online_defrag()
>  	fi
>  
>  	if [ "$str" != "" ]; then
> -		$BTRFS_UTIL_PROG filesystem defragment $str $SCRATCH_MNT/tmp_file
> +		$BTRFS_UTIL_PROG filesystem defragment $str $SCRATCH_MNT/tmp_file
>> $seq.full 2>&1
>  	else
>  		if [ "$1" = "1" ];then
> -			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT/tmp_file
> +			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT/tmp_file >>
$seq.full 2>&1
>  		elif [ "$1" = "2" ];then
> -			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT/tmp_dir
> +			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT/tmp_dir >>
$seq.full 2>&1
>  		elif [ "$1" = "3" ];then
> -			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT
> +			$BTRFS_UTIL_PROG filesystem defragment $SCRATCH_MNT >> $seq.full
2>&1
>  		fi
>  	fi
>  	ret_val=$?
>  	_scratch_remount
> -	if [ $ret_val -ne 20 ];then
> -		echo "btrfs filesystem defragment failed! err is $ret_val"
> +	# Older defrag returned "20" for success
> +	# e9393c2 btrfs-progs: defrag return zero on success
> +	if [ $ret_val -ne 0 -a $ret_val -ne 20 ]; then
> +		echo "btrfs filesystem defragment failed!"
>  	fi
>  }
>  
> @@ -140,19 +143,19 @@ _scratch_mount
>  _require_defrag
>  
>  echo "defrag object | defragment range | defragment compress"
> -echo "a single file |  default | off"
> +echo "a single file | default | off"
>  _rundefrag 1 1 1
>  
>  echo "a single file | default |  on"
>  _rundefrag 1 1 2
>  
> -echo "a single file | start < 0 && 0 < len < file
size | off"
> +echo "a single file | start < 0 && 0 < len < file
size | off (should fail)"
>  _rundefrag 1 2 1
>  
>  echo "a single file | start > file size && 0 < len <
file size | off"
>  _rundefrag 1 3 1
>  
> -echo "a single file | start = 0 && len < 0 | off"
> +echo "a single file | start = 0 && len < 0 | off (should
fail)"
>  _rundefrag 1 4 1
>  
>  echo "a single file | start = 0 && len > file size |
off"
> diff --git a/tests/btrfs/284.out b/tests/btrfs/284.out
> index 4a69f82..c942271 100644
> --- a/tests/btrfs/284.out
> +++ b/tests/btrfs/284.out
> @@ -1,10 +1,12 @@
>  QA output created by 284
>  defrag object | defragment range | defragment compress
> -a single file |  default | off
> +a single file | default | off
>  a single file | default |  on
> -a single file | start < 0 && 0 < len < file size | off
> +a single file | start < 0 && 0 < len < file size | off
(should fail)
> +btrfs filesystem defragment failed!
>  a single file | start > file size && 0 < len < file size
| off
> -a single file | start = 0 && len < 0 | off
> +a single file | start = 0 && len < 0 | off (should fail)
> +btrfs filesystem defragment failed!
>  a single file | start = 0 && len > file size | off
>  a single file | start = 0 && 0 < len < file size | off
>  a directory | default | off
> 
> 
> --
> 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
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Rich Johnston

2013-May-15 12:30 UTC

head link

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output

On 04/26/2013 01:45 PM, Eric Sandeen wrote:> test 284 had... some issues.
>
> diff --git a/tests/btrfs/284 b/tests/btrfs/284
> old mode 100644
> new mode 100755
> index d952977..67161a3
> --- a/tests/btrfs/284
> +++ b/tests/btrfs/284
>This patch has been committed:

commit 91f87e3b89c0f7350a56d397ba7255d0e3d6b486
Author: Eric Sandeen <sandeen@redhat.com>
Date:   Wed May 15 07:18:07 2013 -0500

     xfstests btrfs/284: shorten duration, fix output

     Signed-off-by: Eric Sandeen <sandeen@redhat.com>
     Reviewed-by: Josef Bacik <jbacik@fusionio.com>
     Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

Thanks
--Rich

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

Btrfs devel - Apr 2013 - [PATCH] xfstests btrfs/284: shorten duration, fix output

[PATCH] xfstests btrfs/284: shorten duration, fix output

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output

Re: [PATCH] xfstests btrfs/284: shorten duration, fix output