* [PATCH] xfstests: add function _require_fssum()
@ 2014-02-24 11:56 ` Filipe David Borba Manana
0 siblings, 0 replies; 14+ messages in thread
From: Filipe David Borba Manana @ 2014-02-24 11:56 UTC (permalink / raw)
To: xfs; +Cc: Filipe David Borba Manana, linux-btrfs
To avoid repeating detection of fssum presence in many btrfs tests, as
suggested by Dave Chinner.
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
common/rc | 7 +++++++
tests/btrfs/007 | 5 +----
tests/btrfs/016 | 5 +----
tests/btrfs/030 | 5 +----
tests/btrfs/038 | 5 +----
tests/btrfs/039 | 5 +----
tests/btrfs/040 | 5 +----
tests/btrfs/041 | 5 +----
tests/btrfs/042 | 5 +----
9 files changed, 15 insertions(+), 32 deletions(-)
mode change 100644 => 100755 tests/btrfs/016
diff --git a/common/rc b/common/rc
index 5df504c..cce05cc 100644
--- a/common/rc
+++ b/common/rc
@@ -2144,6 +2144,13 @@ _require_cp_reflink()
_notrun "This test requires a cp with --reflink support."
}
+_require_fssum()
+{
+ HERE=`pwd`
+ FSSUM_PROG=$HERE/src/fssum
+ [ -x $FSSUM_PROG ] || _notrun "fssum not built"
+}
+
# Given 2 files, verify that they have the same mapping but different
# inodes - i.e. an undisturbed reflink
# Silent if so, make noise if not
diff --git a/tests/btrfs/007 b/tests/btrfs/007
index 5df9ccb..5430613 100755
--- a/tests/btrfs/007
+++ b/tests/btrfs/007
@@ -31,7 +31,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1
@@ -52,11 +51,9 @@ _need_to_be_root
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_require_seek_data_hole
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
workout()
diff --git a/tests/btrfs/016 b/tests/btrfs/016
old mode 100644
new mode 100755
index 6faead1..d04c21a
--- a/tests/btrfs/016
+++ b/tests/btrfs/016
@@ -26,7 +26,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
tmp_dir=send_temp_$seq
@@ -51,9 +50,7 @@ trap "_cleanup ; exit \$status" 0 1 2 3 15
_supported_fs btrfs
_supported_os Linux
_require_scratch
-
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
+_require_fssum
_scratch_mkfs > /dev/null 2>&1
diff --git a/tests/btrfs/030 b/tests/btrfs/030
index a9f5fb4..a76a410 100755
--- a/tests/btrfs/030
+++ b/tests/btrfs/030
@@ -36,7 +36,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -54,11 +53,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/038 b/tests/btrfs/038
index 8893696..4941d3e 100755
--- a/tests/btrfs/038
+++ b/tests/btrfs/038
@@ -32,7 +32,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -50,11 +49,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/039 b/tests/btrfs/039
index 41e09be..758b23c 100755
--- a/tests/btrfs/039
+++ b/tests/btrfs/039
@@ -35,7 +35,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -53,11 +52,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/040 b/tests/btrfs/040
index 77c4a84..dfd495c 100755
--- a/tests/btrfs/040
+++ b/tests/btrfs/040
@@ -35,7 +35,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -53,11 +52,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/041 b/tests/btrfs/041
index bfc0e4b..a960001 100755
--- a/tests/btrfs/041
+++ b/tests/btrfs/041
@@ -31,7 +31,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -49,11 +48,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/042 b/tests/btrfs/042
index 2bd5147..9aa8967 100755
--- a/tests/btrfs/042
+++ b/tests/btrfs/042
@@ -36,7 +36,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -54,11 +53,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
--
1.7.9.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH] xfstests: add function _require_fssum()
2014-02-24 11:56 ` Filipe David Borba Manana
@ 2014-02-24 12:23 ` Dave Chinner
-1 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2014-02-24 12:23 UTC (permalink / raw)
To: Filipe David Borba Manana; +Cc: xfs, linux-btrfs
On Mon, Feb 24, 2014 at 11:56:23AM +0000, Filipe David Borba Manana wrote:
> To avoid repeating detection of fssum presence in many btrfs tests, as
> suggested by Dave Chinner.
>
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
> common/rc | 7 +++++++
> tests/btrfs/007 | 5 +----
> tests/btrfs/016 | 5 +----
> tests/btrfs/030 | 5 +----
> tests/btrfs/038 | 5 +----
> tests/btrfs/039 | 5 +----
> tests/btrfs/040 | 5 +----
> tests/btrfs/041 | 5 +----
> tests/btrfs/042 | 5 +----
> 9 files changed, 15 insertions(+), 32 deletions(-)
> mode change 100644 => 100755 tests/btrfs/016
>
> diff --git a/common/rc b/common/rc
> index 5df504c..cce05cc 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
> _notrun "This test requires a cp with --reflink support."
> }
>
> +_require_fssum()
> +{
> + HERE=`pwd`
> + FSSUM_PROG=$HERE/src/fssum
> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> +}
$here is defined by check to be the root of the xfstests instance
that is running. There's 60+ tests that already us it. Hence:
_require_fssum()
{
FSSUM_PROG=$here/src/fssum
[ -x $FSSUM_PROG ] || _notrun "fssum not built"
}
Is all you need here.
> +
> # Given 2 files, verify that they have the same mapping but different
> # inodes - i.e. an undisturbed reflink
> # Silent if so, make noise if not
> diff --git a/tests/btrfs/007 b/tests/btrfs/007
> index 5df9ccb..5430613 100755
> --- a/tests/btrfs/007
> +++ b/tests/btrfs/007
> @@ -31,7 +31,6 @@ seq=`basename $0`
> seqres=$RESULT_DIR/$seq
> echo "QA output created by $seq"
>
> -here=`pwd`
> tmp=`mktemp -d`
> status=1
Yeah, redefining $here is a bad thing to do :/
And I'd missed that this was being done in all the new btrfs tests,
otherwise I would have pulled it up earlier.
It also points out that the btrfs tests are using a non-standard
$tmp directory - one that is in the xfstests source directory.
That's a bad thing, too - tests should be using:
tmp=/tmp/$$
to store small temporary files.
If /tmp is too small for what a test needs, then the test should be
using $TEST_DIR as the store for the temporary files to exercise
the filesystem under test as much as possible. e.g. send image
files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
not in $tmp; filesystem image files that are mounted by loopback
should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
i.e. the idea is that you direct as much of the IO to the test_DIR
and SCRATCH_MNT as possible, not to the filesystem that is hosting
$tmp or the xfstests source directory....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] xfstests: add function _require_fssum()
@ 2014-02-24 12:23 ` Dave Chinner
0 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2014-02-24 12:23 UTC (permalink / raw)
To: Filipe David Borba Manana; +Cc: linux-btrfs, xfs
On Mon, Feb 24, 2014 at 11:56:23AM +0000, Filipe David Borba Manana wrote:
> To avoid repeating detection of fssum presence in many btrfs tests, as
> suggested by Dave Chinner.
>
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
> common/rc | 7 +++++++
> tests/btrfs/007 | 5 +----
> tests/btrfs/016 | 5 +----
> tests/btrfs/030 | 5 +----
> tests/btrfs/038 | 5 +----
> tests/btrfs/039 | 5 +----
> tests/btrfs/040 | 5 +----
> tests/btrfs/041 | 5 +----
> tests/btrfs/042 | 5 +----
> 9 files changed, 15 insertions(+), 32 deletions(-)
> mode change 100644 => 100755 tests/btrfs/016
>
> diff --git a/common/rc b/common/rc
> index 5df504c..cce05cc 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
> _notrun "This test requires a cp with --reflink support."
> }
>
> +_require_fssum()
> +{
> + HERE=`pwd`
> + FSSUM_PROG=$HERE/src/fssum
> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> +}
$here is defined by check to be the root of the xfstests instance
that is running. There's 60+ tests that already us it. Hence:
_require_fssum()
{
FSSUM_PROG=$here/src/fssum
[ -x $FSSUM_PROG ] || _notrun "fssum not built"
}
Is all you need here.
> +
> # Given 2 files, verify that they have the same mapping but different
> # inodes - i.e. an undisturbed reflink
> # Silent if so, make noise if not
> diff --git a/tests/btrfs/007 b/tests/btrfs/007
> index 5df9ccb..5430613 100755
> --- a/tests/btrfs/007
> +++ b/tests/btrfs/007
> @@ -31,7 +31,6 @@ seq=`basename $0`
> seqres=$RESULT_DIR/$seq
> echo "QA output created by $seq"
>
> -here=`pwd`
> tmp=`mktemp -d`
> status=1
Yeah, redefining $here is a bad thing to do :/
And I'd missed that this was being done in all the new btrfs tests,
otherwise I would have pulled it up earlier.
It also points out that the btrfs tests are using a non-standard
$tmp directory - one that is in the xfstests source directory.
That's a bad thing, too - tests should be using:
tmp=/tmp/$$
to store small temporary files.
If /tmp is too small for what a test needs, then the test should be
using $TEST_DIR as the store for the temporary files to exercise
the filesystem under test as much as possible. e.g. send image
files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
not in $tmp; filesystem image files that are mounted by loopback
should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
i.e. the idea is that you direct as much of the IO to the test_DIR
and SCRATCH_MNT as possible, not to the filesystem that is hosting
$tmp or the xfstests source directory....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] xfstests: add function _require_fssum()
2014-02-24 12:23 ` Dave Chinner
@ 2014-02-24 13:22 ` Filipe David Manana
-1 siblings, 0 replies; 14+ messages in thread
From: Filipe David Manana @ 2014-02-24 13:22 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs@oss.sgi.com, linux-btrfs@vger.kernel.org
On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Mon, Feb 24, 2014 at 11:56:23AM +0000, Filipe David Borba Manana wrote:
>> To avoid repeating detection of fssum presence in many btrfs tests, as
>> suggested by Dave Chinner.
>>
>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> ---
>> common/rc | 7 +++++++
>> tests/btrfs/007 | 5 +----
>> tests/btrfs/016 | 5 +----
>> tests/btrfs/030 | 5 +----
>> tests/btrfs/038 | 5 +----
>> tests/btrfs/039 | 5 +----
>> tests/btrfs/040 | 5 +----
>> tests/btrfs/041 | 5 +----
>> tests/btrfs/042 | 5 +----
>> 9 files changed, 15 insertions(+), 32 deletions(-)
>> mode change 100644 => 100755 tests/btrfs/016
>>
>> diff --git a/common/rc b/common/rc
>> index 5df504c..cce05cc 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
>> _notrun "This test requires a cp with --reflink support."
>> }
>>
>> +_require_fssum()
>> +{
>> + HERE=`pwd`
>> + FSSUM_PROG=$HERE/src/fssum
>> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
>> +}
>
> $here is defined by check to be the root of the xfstests instance
> that is running. There's 60+ tests that already us it. Hence:
>
> _require_fssum()
> {
> FSSUM_PROG=$here/src/fssum
> [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> }
>
> Is all you need here.
Hum, doesn't work unless the test file defines $here. At least when
running a single only (.e.g. ./check btrfs/041).
>
>> +
>> # Given 2 files, verify that they have the same mapping but different
>> # inodes - i.e. an undisturbed reflink
>> # Silent if so, make noise if not
>> diff --git a/tests/btrfs/007 b/tests/btrfs/007
>> index 5df9ccb..5430613 100755
>> --- a/tests/btrfs/007
>> +++ b/tests/btrfs/007
>> @@ -31,7 +31,6 @@ seq=`basename $0`
>> seqres=$RESULT_DIR/$seq
>> echo "QA output created by $seq"
>>
>> -here=`pwd`
>> tmp=`mktemp -d`
>> status=1
>
> Yeah, redefining $here is a bad thing to do :/
See above :)
>
> And I'd missed that this was being done in all the new btrfs tests,
> otherwise I would have pulled it up earlier.
>
> It also points out that the btrfs tests are using a non-standard
> $tmp directory - one that is in the xfstests source directory.
> That's a bad thing, too - tests should be using:
>
> tmp=/tmp/$$
>
> to store small temporary files.
>
> If /tmp is too small for what a test needs, then the test should be
> using $TEST_DIR as the store for the temporary files to exercise
> the filesystem under test as much as possible. e.g. send image
> files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
> not in $tmp; filesystem image files that are mounted by loopback
> should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
>
> i.e. the idea is that you direct as much of the IO to the test_DIR
> and SCRATCH_MNT as possible, not to the filesystem that is hosting
> $tmp or the xfstests source directory....
Right. Sounds like a separate patch (to use TEST_DIR/$$ for e.g. as a
place to store temporary test data).
Thanks Dave
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] xfstests: add function _require_fssum()
@ 2014-02-24 13:22 ` Filipe David Manana
0 siblings, 0 replies; 14+ messages in thread
From: Filipe David Manana @ 2014-02-24 13:22 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-btrfs@vger.kernel.org, xfs@oss.sgi.com
On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Mon, Feb 24, 2014 at 11:56:23AM +0000, Filipe David Borba Manana wrote:
>> To avoid repeating detection of fssum presence in many btrfs tests, as
>> suggested by Dave Chinner.
>>
>> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> ---
>> common/rc | 7 +++++++
>> tests/btrfs/007 | 5 +----
>> tests/btrfs/016 | 5 +----
>> tests/btrfs/030 | 5 +----
>> tests/btrfs/038 | 5 +----
>> tests/btrfs/039 | 5 +----
>> tests/btrfs/040 | 5 +----
>> tests/btrfs/041 | 5 +----
>> tests/btrfs/042 | 5 +----
>> 9 files changed, 15 insertions(+), 32 deletions(-)
>> mode change 100644 => 100755 tests/btrfs/016
>>
>> diff --git a/common/rc b/common/rc
>> index 5df504c..cce05cc 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
>> _notrun "This test requires a cp with --reflink support."
>> }
>>
>> +_require_fssum()
>> +{
>> + HERE=`pwd`
>> + FSSUM_PROG=$HERE/src/fssum
>> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
>> +}
>
> $here is defined by check to be the root of the xfstests instance
> that is running. There's 60+ tests that already us it. Hence:
>
> _require_fssum()
> {
> FSSUM_PROG=$here/src/fssum
> [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> }
>
> Is all you need here.
Hum, doesn't work unless the test file defines $here. At least when
running a single only (.e.g. ./check btrfs/041).
>
>> +
>> # Given 2 files, verify that they have the same mapping but different
>> # inodes - i.e. an undisturbed reflink
>> # Silent if so, make noise if not
>> diff --git a/tests/btrfs/007 b/tests/btrfs/007
>> index 5df9ccb..5430613 100755
>> --- a/tests/btrfs/007
>> +++ b/tests/btrfs/007
>> @@ -31,7 +31,6 @@ seq=`basename $0`
>> seqres=$RESULT_DIR/$seq
>> echo "QA output created by $seq"
>>
>> -here=`pwd`
>> tmp=`mktemp -d`
>> status=1
>
> Yeah, redefining $here is a bad thing to do :/
See above :)
>
> And I'd missed that this was being done in all the new btrfs tests,
> otherwise I would have pulled it up earlier.
>
> It also points out that the btrfs tests are using a non-standard
> $tmp directory - one that is in the xfstests source directory.
> That's a bad thing, too - tests should be using:
>
> tmp=/tmp/$$
>
> to store small temporary files.
>
> If /tmp is too small for what a test needs, then the test should be
> using $TEST_DIR as the store for the temporary files to exercise
> the filesystem under test as much as possible. e.g. send image
> files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
> not in $tmp; filesystem image files that are mounted by loopback
> should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
>
> i.e. the idea is that you direct as much of the IO to the test_DIR
> and SCRATCH_MNT as possible, not to the filesystem that is hosting
> $tmp or the xfstests source directory....
Right. Sounds like a separate patch (to use TEST_DIR/$$ for e.g. as a
place to store temporary test data).
Thanks Dave
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] xfstests: add function _require_fssum()
2014-02-24 13:22 ` Filipe David Manana
@ 2014-02-24 22:22 ` Dave Chinner
-1 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2014-02-24 22:22 UTC (permalink / raw)
To: Filipe David Manana; +Cc: xfs@oss.sgi.com, linux-btrfs@vger.kernel.org
On Mon, Feb 24, 2014 at 01:22:36PM +0000, Filipe David Manana wrote:
> On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Mon, Feb 24, 2014 at 11:56:23AM +0000, Filipe David Borba Manana wrote:
> >> To avoid repeating detection of fssum presence in many btrfs tests, as
> >> suggested by Dave Chinner.
> >>
> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> >> ---
> >> common/rc | 7 +++++++
> >> tests/btrfs/007 | 5 +----
> >> tests/btrfs/016 | 5 +----
> >> tests/btrfs/030 | 5 +----
> >> tests/btrfs/038 | 5 +----
> >> tests/btrfs/039 | 5 +----
> >> tests/btrfs/040 | 5 +----
> >> tests/btrfs/041 | 5 +----
> >> tests/btrfs/042 | 5 +----
> >> 9 files changed, 15 insertions(+), 32 deletions(-)
> >> mode change 100644 => 100755 tests/btrfs/016
> >>
> >> diff --git a/common/rc b/common/rc
> >> index 5df504c..cce05cc 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
> >> _notrun "This test requires a cp with --reflink support."
> >> }
> >>
> >> +_require_fssum()
> >> +{
> >> + HERE=`pwd`
> >> + FSSUM_PROG=$HERE/src/fssum
> >> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> >> +}
> >
> > $here is defined by check to be the root of the xfstests instance
> > that is running. There's 60+ tests that already us it. Hence:
> >
> > _require_fssum()
> > {
> > FSSUM_PROG=$here/src/fssum
> > [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> > }
> >
> > Is all you need here.
>
> Hum, doesn't work unless the test file defines $here. At least when
> running a single only (.e.g. ./check btrfs/041).
Right, I realised that it isn't exported from check, and the template
for new tests defines it as:
here=\`pwd\`
which is essentially the same thing. Almost every test does it.
$ git grep here=\`pwd\` |wc -l
364
And it works just fine when running a test as you describe above.
Many of the _requires* functions just use $here directly, and does a
lot of the common/* infrastructure. It is assumed that $here is set
and valid and so if it wasn't set, lots of different things would
break.
Oh, I note that btrfs/034 and btrfs/037 don't set it, so they need
fixing, too.
Looking at it, though, we shoul djust export the value from check
and remove the assignment that occurs in every test as they end up
being the same value:
$ sudo ./check generic/001
check_here=/home/dave/src/xfstests-dev
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 test2 3.14.0-rc3-dgc+
MKFS_OPTIONS -- -f -bsize=4096 /dev/vdb
MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
generic/001 5s ... [failed, exit status 1] - output mismatch (see /home/dave/src/xfstests-dev/results//generic/001.out.bad)
.....
$ cat /home/dave/src/xfstests-dev/results//generic/001.out.bad
QA output created by 001
here=/home/dave/src/xfstests-dev
$
> >> diff --git a/tests/btrfs/007 b/tests/btrfs/007
> >> index 5df9ccb..5430613 100755
> >> --- a/tests/btrfs/007
> >> +++ b/tests/btrfs/007
> >> @@ -31,7 +31,6 @@ seq=`basename $0`
> >> seqres=$RESULT_DIR/$seq
> >> echo "QA output created by $seq"
> >>
> >> -here=`pwd`
> >> tmp=`mktemp -d`
> >> status=1
> >
> > Yeah, redefining $here is a bad thing to do :/
Right, my mistake. It needs to be defined for the entire test, not
in a requires function. Hence I think we should just export it from
check....
> > It also points out that the btrfs tests are using a non-standard
> > $tmp directory - one that is in the xfstests source directory.
> > That's a bad thing, too - tests should be using:
> >
> > tmp=/tmp/$$
> >
> > to store small temporary files.
> >
> > If /tmp is too small for what a test needs, then the test should be
> > using $TEST_DIR as the store for the temporary files to exercise
> > the filesystem under test as much as possible. e.g. send image
> > files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
> > not in $tmp; filesystem image files that are mounted by loopback
> > should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
> >
> > i.e. the idea is that you direct as much of the IO to the test_DIR
> > and SCRATCH_MNT as possible, not to the filesystem that is hosting
> > $tmp or the xfstests source directory....
>
> Right. Sounds like a separate patch (to use TEST_DIR/$$ for e.g. as a
> place to store temporary test data).
tmp should be set to /tmp/$$. That's where the *test harness* will
direct temporary log files, mkfs output, etc. If you have temporary
data for the *test*, and it's large, use TEST_DIR, not $tmp.
$tmp is generally only for things like log files, time stamps,
output files that need later processing because they can't be
filtered directly, etc.
Why? Because if the test corrupts $TEST_DIR and you've got all the
test harness tmp files on TEST_DIR, how is the test harness going
to work out what went wrong during the test?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] xfstests: add function _require_fssum()
@ 2014-02-24 22:22 ` Dave Chinner
0 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2014-02-24 22:22 UTC (permalink / raw)
To: Filipe David Manana; +Cc: linux-btrfs@vger.kernel.org, xfs@oss.sgi.com
On Mon, Feb 24, 2014 at 01:22:36PM +0000, Filipe David Manana wrote:
> On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Mon, Feb 24, 2014 at 11:56:23AM +0000, Filipe David Borba Manana wrote:
> >> To avoid repeating detection of fssum presence in many btrfs tests, as
> >> suggested by Dave Chinner.
> >>
> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> >> ---
> >> common/rc | 7 +++++++
> >> tests/btrfs/007 | 5 +----
> >> tests/btrfs/016 | 5 +----
> >> tests/btrfs/030 | 5 +----
> >> tests/btrfs/038 | 5 +----
> >> tests/btrfs/039 | 5 +----
> >> tests/btrfs/040 | 5 +----
> >> tests/btrfs/041 | 5 +----
> >> tests/btrfs/042 | 5 +----
> >> 9 files changed, 15 insertions(+), 32 deletions(-)
> >> mode change 100644 => 100755 tests/btrfs/016
> >>
> >> diff --git a/common/rc b/common/rc
> >> index 5df504c..cce05cc 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
> >> _notrun "This test requires a cp with --reflink support."
> >> }
> >>
> >> +_require_fssum()
> >> +{
> >> + HERE=`pwd`
> >> + FSSUM_PROG=$HERE/src/fssum
> >> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> >> +}
> >
> > $here is defined by check to be the root of the xfstests instance
> > that is running. There's 60+ tests that already us it. Hence:
> >
> > _require_fssum()
> > {
> > FSSUM_PROG=$here/src/fssum
> > [ -x $FSSUM_PROG ] || _notrun "fssum not built"
> > }
> >
> > Is all you need here.
>
> Hum, doesn't work unless the test file defines $here. At least when
> running a single only (.e.g. ./check btrfs/041).
Right, I realised that it isn't exported from check, and the template
for new tests defines it as:
here=\`pwd\`
which is essentially the same thing. Almost every test does it.
$ git grep here=\`pwd\` |wc -l
364
And it works just fine when running a test as you describe above.
Many of the _requires* functions just use $here directly, and does a
lot of the common/* infrastructure. It is assumed that $here is set
and valid and so if it wasn't set, lots of different things would
break.
Oh, I note that btrfs/034 and btrfs/037 don't set it, so they need
fixing, too.
Looking at it, though, we shoul djust export the value from check
and remove the assignment that occurs in every test as they end up
being the same value:
$ sudo ./check generic/001
check_here=/home/dave/src/xfstests-dev
FSTYP -- xfs (debug)
PLATFORM -- Linux/x86_64 test2 3.14.0-rc3-dgc+
MKFS_OPTIONS -- -f -bsize=4096 /dev/vdb
MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
generic/001 5s ... [failed, exit status 1] - output mismatch (see /home/dave/src/xfstests-dev/results//generic/001.out.bad)
.....
$ cat /home/dave/src/xfstests-dev/results//generic/001.out.bad
QA output created by 001
here=/home/dave/src/xfstests-dev
$
> >> diff --git a/tests/btrfs/007 b/tests/btrfs/007
> >> index 5df9ccb..5430613 100755
> >> --- a/tests/btrfs/007
> >> +++ b/tests/btrfs/007
> >> @@ -31,7 +31,6 @@ seq=`basename $0`
> >> seqres=$RESULT_DIR/$seq
> >> echo "QA output created by $seq"
> >>
> >> -here=`pwd`
> >> tmp=`mktemp -d`
> >> status=1
> >
> > Yeah, redefining $here is a bad thing to do :/
Right, my mistake. It needs to be defined for the entire test, not
in a requires function. Hence I think we should just export it from
check....
> > It also points out that the btrfs tests are using a non-standard
> > $tmp directory - one that is in the xfstests source directory.
> > That's a bad thing, too - tests should be using:
> >
> > tmp=/tmp/$$
> >
> > to store small temporary files.
> >
> > If /tmp is too small for what a test needs, then the test should be
> > using $TEST_DIR as the store for the temporary files to exercise
> > the filesystem under test as much as possible. e.g. send image
> > files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
> > not in $tmp; filesystem image files that are mounted by loopback
> > should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
> >
> > i.e. the idea is that you direct as much of the IO to the test_DIR
> > and SCRATCH_MNT as possible, not to the filesystem that is hosting
> > $tmp or the xfstests source directory....
>
> Right. Sounds like a separate patch (to use TEST_DIR/$$ for e.g. as a
> place to store temporary test data).
tmp should be set to /tmp/$$. That's where the *test harness* will
direct temporary log files, mkfs output, etc. If you have temporary
data for the *test*, and it's large, use TEST_DIR, not $tmp.
$tmp is generally only for things like log files, time stamps,
output files that need later processing because they can't be
filtered directly, etc.
Why? Because if the test corrupts $TEST_DIR and you've got all the
test harness tmp files on TEST_DIR, how is the test harness going
to work out what went wrong during the test?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] xfstests: add function _require_fssum()
2014-02-24 22:22 ` Dave Chinner
@ 2014-02-24 23:08 ` Filipe David Manana
-1 siblings, 0 replies; 14+ messages in thread
From: Filipe David Manana @ 2014-02-24 23:08 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs@oss.sgi.com, linux-btrfs@vger.kernel.org
On Mon, Feb 24, 2014 at 10:22 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Mon, Feb 24, 2014 at 01:22:36PM +0000, Filipe David Manana wrote:
>> On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner <david@fromorbit.com> wrote:
>> > On Mon, Feb 24, 2014 at 11:56:23AM +0000, Filipe David Borba Manana wrote:
>> >> To avoid repeating detection of fssum presence in many btrfs tests, as
>> >> suggested by Dave Chinner.
>> >>
>> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> >> ---
>> >> common/rc | 7 +++++++
>> >> tests/btrfs/007 | 5 +----
>> >> tests/btrfs/016 | 5 +----
>> >> tests/btrfs/030 | 5 +----
>> >> tests/btrfs/038 | 5 +----
>> >> tests/btrfs/039 | 5 +----
>> >> tests/btrfs/040 | 5 +----
>> >> tests/btrfs/041 | 5 +----
>> >> tests/btrfs/042 | 5 +----
>> >> 9 files changed, 15 insertions(+), 32 deletions(-)
>> >> mode change 100644 => 100755 tests/btrfs/016
>> >>
>> >> diff --git a/common/rc b/common/rc
>> >> index 5df504c..cce05cc 100644
>> >> --- a/common/rc
>> >> +++ b/common/rc
>> >> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
>> >> _notrun "This test requires a cp with --reflink support."
>> >> }
>> >>
>> >> +_require_fssum()
>> >> +{
>> >> + HERE=`pwd`
>> >> + FSSUM_PROG=$HERE/src/fssum
>> >> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
>> >> +}
>> >
>> > $here is defined by check to be the root of the xfstests instance
>> > that is running. There's 60+ tests that already us it. Hence:
>> >
>> > _require_fssum()
>> > {
>> > FSSUM_PROG=$here/src/fssum
>> > [ -x $FSSUM_PROG ] || _notrun "fssum not built"
>> > }
>> >
>> > Is all you need here.
>>
>> Hum, doesn't work unless the test file defines $here. At least when
>> running a single only (.e.g. ./check btrfs/041).
>
> Right, I realised that it isn't exported from check, and the template
> for new tests defines it as:
>
> here=\`pwd\`
>
> which is essentially the same thing. Almost every test does it.
>
> $ git grep here=\`pwd\` |wc -l
> 364
>
> And it works just fine when running a test as you describe above.
> Many of the _requires* functions just use $here directly, and does a
> lot of the common/* infrastructure. It is assumed that $here is set
> and valid and so if it wasn't set, lots of different things would
> break.
>
> Oh, I note that btrfs/034 and btrfs/037 don't set it, so they need
> fixing, too.
>
> Looking at it, though, we shoul djust export the value from check
> and remove the assignment that occurs in every test as they end up
> being the same value:
>
> $ sudo ./check generic/001
> check_here=/home/dave/src/xfstests-dev
> FSTYP -- xfs (debug)
> PLATFORM -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> MKFS_OPTIONS -- -f -bsize=4096 /dev/vdb
> MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
>
> generic/001 5s ... [failed, exit status 1] - output mismatch (see /home/dave/src/xfstests-dev/results//generic/001.out.bad)
> .....
> $ cat /home/dave/src/xfstests-dev/results//generic/001.out.bad
> QA output created by 001
> here=/home/dave/src/xfstests-dev
> $
>
>> >> diff --git a/tests/btrfs/007 b/tests/btrfs/007
>> >> index 5df9ccb..5430613 100755
>> >> --- a/tests/btrfs/007
>> >> +++ b/tests/btrfs/007
>> >> @@ -31,7 +31,6 @@ seq=`basename $0`
>> >> seqres=$RESULT_DIR/$seq
>> >> echo "QA output created by $seq"
>> >>
>> >> -here=`pwd`
>> >> tmp=`mktemp -d`
>> >> status=1
>> >
>> > Yeah, redefining $here is a bad thing to do :/
>
> Right, my mistake. It needs to be defined for the entire test, not
> in a requires function. Hence I think we should just export it from
> check....
Hum, ok. So the decision is to let the tests explicitly define the
variable "here", and not export "here" from the main check script.
>
>> > It also points out that the btrfs tests are using a non-standard
>> > $tmp directory - one that is in the xfstests source directory.
>> > That's a bad thing, too - tests should be using:
>> >
>> > tmp=/tmp/$$
>> >
>> > to store small temporary files.
>> >
>> > If /tmp is too small for what a test needs, then the test should be
>> > using $TEST_DIR as the store for the temporary files to exercise
>> > the filesystem under test as much as possible. e.g. send image
>> > files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
>> > not in $tmp; filesystem image files that are mounted by loopback
>> > should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
>> >
>> > i.e. the idea is that you direct as much of the IO to the test_DIR
>> > and SCRATCH_MNT as possible, not to the filesystem that is hosting
>> > $tmp or the xfstests source directory....
>>
>> Right. Sounds like a separate patch (to use TEST_DIR/$$ for e.g. as a
>> place to store temporary test data).
>
> tmp should be set to /tmp/$$. That's where the *test harness* will
> direct temporary log files, mkfs output, etc. If you have temporary
> data for the *test*, and it's large, use TEST_DIR, not $tmp.
> $tmp is generally only for things like log files, time stamps,
> output files that need later processing because they can't be
> filtered directly, etc.
>
> Why? Because if the test corrupts $TEST_DIR and you've got all the
> test harness tmp files on TEST_DIR, how is the test harness going
> to work out what went wrong during the test?
Makes sense. All these btrfs tests are adding small files to /tmp,
less than ~100Kb or so (certainly not megabytes), so that seems fine.
Thanks Dave
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] xfstests: add function _require_fssum()
@ 2014-02-24 23:08 ` Filipe David Manana
0 siblings, 0 replies; 14+ messages in thread
From: Filipe David Manana @ 2014-02-24 23:08 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-btrfs@vger.kernel.org, xfs@oss.sgi.com
On Mon, Feb 24, 2014 at 10:22 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Mon, Feb 24, 2014 at 01:22:36PM +0000, Filipe David Manana wrote:
>> On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner <david@fromorbit.com> wrote:
>> > On Mon, Feb 24, 2014 at 11:56:23AM +0000, Filipe David Borba Manana wrote:
>> >> To avoid repeating detection of fssum presence in many btrfs tests, as
>> >> suggested by Dave Chinner.
>> >>
>> >> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
>> >> ---
>> >> common/rc | 7 +++++++
>> >> tests/btrfs/007 | 5 +----
>> >> tests/btrfs/016 | 5 +----
>> >> tests/btrfs/030 | 5 +----
>> >> tests/btrfs/038 | 5 +----
>> >> tests/btrfs/039 | 5 +----
>> >> tests/btrfs/040 | 5 +----
>> >> tests/btrfs/041 | 5 +----
>> >> tests/btrfs/042 | 5 +----
>> >> 9 files changed, 15 insertions(+), 32 deletions(-)
>> >> mode change 100644 => 100755 tests/btrfs/016
>> >>
>> >> diff --git a/common/rc b/common/rc
>> >> index 5df504c..cce05cc 100644
>> >> --- a/common/rc
>> >> +++ b/common/rc
>> >> @@ -2144,6 +2144,13 @@ _require_cp_reflink()
>> >> _notrun "This test requires a cp with --reflink support."
>> >> }
>> >>
>> >> +_require_fssum()
>> >> +{
>> >> + HERE=`pwd`
>> >> + FSSUM_PROG=$HERE/src/fssum
>> >> + [ -x $FSSUM_PROG ] || _notrun "fssum not built"
>> >> +}
>> >
>> > $here is defined by check to be the root of the xfstests instance
>> > that is running. There's 60+ tests that already us it. Hence:
>> >
>> > _require_fssum()
>> > {
>> > FSSUM_PROG=$here/src/fssum
>> > [ -x $FSSUM_PROG ] || _notrun "fssum not built"
>> > }
>> >
>> > Is all you need here.
>>
>> Hum, doesn't work unless the test file defines $here. At least when
>> running a single only (.e.g. ./check btrfs/041).
>
> Right, I realised that it isn't exported from check, and the template
> for new tests defines it as:
>
> here=\`pwd\`
>
> which is essentially the same thing. Almost every test does it.
>
> $ git grep here=\`pwd\` |wc -l
> 364
>
> And it works just fine when running a test as you describe above.
> Many of the _requires* functions just use $here directly, and does a
> lot of the common/* infrastructure. It is assumed that $here is set
> and valid and so if it wasn't set, lots of different things would
> break.
>
> Oh, I note that btrfs/034 and btrfs/037 don't set it, so they need
> fixing, too.
>
> Looking at it, though, we shoul djust export the value from check
> and remove the assignment that occurs in every test as they end up
> being the same value:
>
> $ sudo ./check generic/001
> check_here=/home/dave/src/xfstests-dev
> FSTYP -- xfs (debug)
> PLATFORM -- Linux/x86_64 test2 3.14.0-rc3-dgc+
> MKFS_OPTIONS -- -f -bsize=4096 /dev/vdb
> MOUNT_OPTIONS -- /dev/vdb /mnt/scratch
>
> generic/001 5s ... [failed, exit status 1] - output mismatch (see /home/dave/src/xfstests-dev/results//generic/001.out.bad)
> .....
> $ cat /home/dave/src/xfstests-dev/results//generic/001.out.bad
> QA output created by 001
> here=/home/dave/src/xfstests-dev
> $
>
>> >> diff --git a/tests/btrfs/007 b/tests/btrfs/007
>> >> index 5df9ccb..5430613 100755
>> >> --- a/tests/btrfs/007
>> >> +++ b/tests/btrfs/007
>> >> @@ -31,7 +31,6 @@ seq=`basename $0`
>> >> seqres=$RESULT_DIR/$seq
>> >> echo "QA output created by $seq"
>> >>
>> >> -here=`pwd`
>> >> tmp=`mktemp -d`
>> >> status=1
>> >
>> > Yeah, redefining $here is a bad thing to do :/
>
> Right, my mistake. It needs to be defined for the entire test, not
> in a requires function. Hence I think we should just export it from
> check....
Hum, ok. So the decision is to let the tests explicitly define the
variable "here", and not export "here" from the main check script.
>
>> > It also points out that the btrfs tests are using a non-standard
>> > $tmp directory - one that is in the xfstests source directory.
>> > That's a bad thing, too - tests should be using:
>> >
>> > tmp=/tmp/$$
>> >
>> > to store small temporary files.
>> >
>> > If /tmp is too small for what a test needs, then the test should be
>> > using $TEST_DIR as the store for the temporary files to exercise
>> > the filesystem under test as much as possible. e.g. send image
>> > files build form snapshots of SCRATCH_DEV should be stored on TEST_DIR,
>> > not in $tmp; filesystem image files that are mounted by loopback
>> > should be stored on TEST_DIR or SCRATCH_MNT, not $tmp. And so on.
>> >
>> > i.e. the idea is that you direct as much of the IO to the test_DIR
>> > and SCRATCH_MNT as possible, not to the filesystem that is hosting
>> > $tmp or the xfstests source directory....
>>
>> Right. Sounds like a separate patch (to use TEST_DIR/$$ for e.g. as a
>> place to store temporary test data).
>
> tmp should be set to /tmp/$$. That's where the *test harness* will
> direct temporary log files, mkfs output, etc. If you have temporary
> data for the *test*, and it's large, use TEST_DIR, not $tmp.
> $tmp is generally only for things like log files, time stamps,
> output files that need later processing because they can't be
> filtered directly, etc.
>
> Why? Because if the test corrupts $TEST_DIR and you've got all the
> test harness tmp files on TEST_DIR, how is the test harness going
> to work out what went wrong during the test?
Makes sense. All these btrfs tests are adding small files to /tmp,
less than ~100Kb or so (certainly not megabytes), so that seems fine.
Thanks Dave
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
--
Filipe David Manana,
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] xfstests: add function _require_fssum()
2014-02-24 23:08 ` Filipe David Manana
@ 2014-02-24 23:35 ` Dave Chinner
-1 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2014-02-24 23:35 UTC (permalink / raw)
To: Filipe David Manana; +Cc: xfs@oss.sgi.com, linux-btrfs@vger.kernel.org
On Mon, Feb 24, 2014 at 11:08:27PM +0000, Filipe David Manana wrote:
> On Mon, Feb 24, 2014 at 10:22 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Mon, Feb 24, 2014 at 01:22:36PM +0000, Filipe David Manana wrote:
> >> On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner <david@fromorbit.com> wrote:
> >> >> diff --git a/tests/btrfs/007 b/tests/btrfs/007
> >> >> index 5df9ccb..5430613 100755
> >> >> --- a/tests/btrfs/007
> >> >> +++ b/tests/btrfs/007
> >> >> @@ -31,7 +31,6 @@ seq=`basename $0`
> >> >> seqres=$RESULT_DIR/$seq
> >> >> echo "QA output created by $seq"
> >> >>
> >> >> -here=`pwd`
> >> >> tmp=`mktemp -d`
> >> >> status=1
> >> >
> >> > Yeah, redefining $here is a bad thing to do :/
> >
> > Right, my mistake. It needs to be defined for the entire test, not
> > in a requires function. Hence I think we should just export it from
> > check....
>
> Hum, ok. So the decision is to let the tests explicitly define the
> variable "here", and not export "here" from the main check script.
Well, that's the status quo right now.
What I'm suggesting is that we should just export it from check and
get rid of it from each test as a separate cleanup. i.e. It always
needs to be set to the same value (i.e. the root of the xfstests
tree) so there is no reason why it should not be set up once in a
central location.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] xfstests: add function _require_fssum()
@ 2014-02-24 23:35 ` Dave Chinner
0 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2014-02-24 23:35 UTC (permalink / raw)
To: Filipe David Manana; +Cc: linux-btrfs@vger.kernel.org, xfs@oss.sgi.com
On Mon, Feb 24, 2014 at 11:08:27PM +0000, Filipe David Manana wrote:
> On Mon, Feb 24, 2014 at 10:22 PM, Dave Chinner <david@fromorbit.com> wrote:
> > On Mon, Feb 24, 2014 at 01:22:36PM +0000, Filipe David Manana wrote:
> >> On Mon, Feb 24, 2014 at 12:23 PM, Dave Chinner <david@fromorbit.com> wrote:
> >> >> diff --git a/tests/btrfs/007 b/tests/btrfs/007
> >> >> index 5df9ccb..5430613 100755
> >> >> --- a/tests/btrfs/007
> >> >> +++ b/tests/btrfs/007
> >> >> @@ -31,7 +31,6 @@ seq=`basename $0`
> >> >> seqres=$RESULT_DIR/$seq
> >> >> echo "QA output created by $seq"
> >> >>
> >> >> -here=`pwd`
> >> >> tmp=`mktemp -d`
> >> >> status=1
> >> >
> >> > Yeah, redefining $here is a bad thing to do :/
> >
> > Right, my mistake. It needs to be defined for the entire test, not
> > in a requires function. Hence I think we should just export it from
> > check....
>
> Hum, ok. So the decision is to let the tests explicitly define the
> variable "here", and not export "here" from the main check script.
Well, that's the status quo right now.
What I'm suggesting is that we should just export it from check and
get rid of it from each test as a separate cleanup. i.e. It always
needs to be set to the same value (i.e. the root of the xfstests
tree) so there is no reason why it should not be set up once in a
central location.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2] xfstests: add function _require_fssum()
2014-02-24 11:56 ` Filipe David Borba Manana
@ 2014-02-25 13:46 ` Filipe David Borba Manana
-1 siblings, 0 replies; 14+ messages in thread
From: Filipe David Borba Manana @ 2014-02-25 13:46 UTC (permalink / raw)
To: xfs; +Cc: david, linux-btrfs, Filipe David Borba Manana
To avoid repeating detection of fssum presence in many btrfs tests, as
suggested by Dave Chinner.
Also exported the variable "here" from the main control script, to avoid
repeating its declaration in every single testcase file. Also removed the
declaration of "here" from btrfs test cases that require the fssum program
only. Removing it from all other test cases will be a separate change.
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: Exported the variable "here" from the main control file, as suggested by
Dave Chinner. Like this we avoid declaring it again in the new function
and in many other places.
check | 2 +-
common/rc | 6 ++++++
tests/btrfs/007 | 5 +----
tests/btrfs/016 | 5 +----
tests/btrfs/030 | 5 +----
tests/btrfs/038 | 5 +----
tests/btrfs/039 | 5 +----
tests/btrfs/040 | 5 +----
tests/btrfs/041 | 5 +----
tests/btrfs/042 | 5 +----
10 files changed, 15 insertions(+), 33 deletions(-)
mode change 100644 => 100755 tests/btrfs/016
diff --git a/check b/check
index 268417c..b619f6d 100755
--- a/check
+++ b/check
@@ -32,7 +32,7 @@ diff="diff -u"
showme=false
have_test_arg=false
randomize=false
-here=`pwd`
+export here=`pwd`
xfile=""
# start the initialisation work now
diff --git a/common/rc b/common/rc
index 5df504c..0f50f49 100644
--- a/common/rc
+++ b/common/rc
@@ -2144,6 +2144,12 @@ _require_cp_reflink()
_notrun "This test requires a cp with --reflink support."
}
+_require_fssum()
+{
+ FSSUM_PROG=$here/src/fssum
+ [ -x $FSSUM_PROG ] || _notrun "fssum not built"
+}
+
# Given 2 files, verify that they have the same mapping but different
# inodes - i.e. an undisturbed reflink
# Silent if so, make noise if not
diff --git a/tests/btrfs/007 b/tests/btrfs/007
index 5df9ccb..5430613 100755
--- a/tests/btrfs/007
+++ b/tests/btrfs/007
@@ -31,7 +31,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1
@@ -52,11 +51,9 @@ _need_to_be_root
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_require_seek_data_hole
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
workout()
diff --git a/tests/btrfs/016 b/tests/btrfs/016
old mode 100644
new mode 100755
index 6faead1..d04c21a
--- a/tests/btrfs/016
+++ b/tests/btrfs/016
@@ -26,7 +26,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
tmp_dir=send_temp_$seq
@@ -51,9 +50,7 @@ trap "_cleanup ; exit \$status" 0 1 2 3 15
_supported_fs btrfs
_supported_os Linux
_require_scratch
-
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
+_require_fssum
_scratch_mkfs > /dev/null 2>&1
diff --git a/tests/btrfs/030 b/tests/btrfs/030
index a9f5fb4..a76a410 100755
--- a/tests/btrfs/030
+++ b/tests/btrfs/030
@@ -36,7 +36,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -54,11 +53,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/038 b/tests/btrfs/038
index 8893696..4941d3e 100755
--- a/tests/btrfs/038
+++ b/tests/btrfs/038
@@ -32,7 +32,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -50,11 +49,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/039 b/tests/btrfs/039
index 41e09be..758b23c 100755
--- a/tests/btrfs/039
+++ b/tests/btrfs/039
@@ -35,7 +35,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -53,11 +52,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/040 b/tests/btrfs/040
index 77c4a84..dfd495c 100755
--- a/tests/btrfs/040
+++ b/tests/btrfs/040
@@ -35,7 +35,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -53,11 +52,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/041 b/tests/btrfs/041
index bfc0e4b..a960001 100755
--- a/tests/btrfs/041
+++ b/tests/btrfs/041
@@ -31,7 +31,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -49,11 +48,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/042 b/tests/btrfs/042
index 2bd5147..9aa8967 100755
--- a/tests/btrfs/042
+++ b/tests/btrfs/042
@@ -36,7 +36,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -54,11 +53,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
--
1.7.9.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2] xfstests: add function _require_fssum()
@ 2014-02-25 13:46 ` Filipe David Borba Manana
0 siblings, 0 replies; 14+ messages in thread
From: Filipe David Borba Manana @ 2014-02-25 13:46 UTC (permalink / raw)
To: xfs; +Cc: Filipe David Borba Manana, linux-btrfs
To avoid repeating detection of fssum presence in many btrfs tests, as
suggested by Dave Chinner.
Also exported the variable "here" from the main control script, to avoid
repeating its declaration in every single testcase file. Also removed the
declaration of "here" from btrfs test cases that require the fssum program
only. Removing it from all other test cases will be a separate change.
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
V2: Exported the variable "here" from the main control file, as suggested by
Dave Chinner. Like this we avoid declaring it again in the new function
and in many other places.
check | 2 +-
common/rc | 6 ++++++
tests/btrfs/007 | 5 +----
tests/btrfs/016 | 5 +----
tests/btrfs/030 | 5 +----
tests/btrfs/038 | 5 +----
tests/btrfs/039 | 5 +----
tests/btrfs/040 | 5 +----
tests/btrfs/041 | 5 +----
tests/btrfs/042 | 5 +----
10 files changed, 15 insertions(+), 33 deletions(-)
mode change 100644 => 100755 tests/btrfs/016
diff --git a/check b/check
index 268417c..b619f6d 100755
--- a/check
+++ b/check
@@ -32,7 +32,7 @@ diff="diff -u"
showme=false
have_test_arg=false
randomize=false
-here=`pwd`
+export here=`pwd`
xfile=""
# start the initialisation work now
diff --git a/common/rc b/common/rc
index 5df504c..0f50f49 100644
--- a/common/rc
+++ b/common/rc
@@ -2144,6 +2144,12 @@ _require_cp_reflink()
_notrun "This test requires a cp with --reflink support."
}
+_require_fssum()
+{
+ FSSUM_PROG=$here/src/fssum
+ [ -x $FSSUM_PROG ] || _notrun "fssum not built"
+}
+
# Given 2 files, verify that they have the same mapping but different
# inodes - i.e. an undisturbed reflink
# Silent if so, make noise if not
diff --git a/tests/btrfs/007 b/tests/btrfs/007
index 5df9ccb..5430613 100755
--- a/tests/btrfs/007
+++ b/tests/btrfs/007
@@ -31,7 +31,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1
@@ -52,11 +51,9 @@ _need_to_be_root
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_require_seek_data_hole
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
workout()
diff --git a/tests/btrfs/016 b/tests/btrfs/016
old mode 100644
new mode 100755
index 6faead1..d04c21a
--- a/tests/btrfs/016
+++ b/tests/btrfs/016
@@ -26,7 +26,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
tmp_dir=send_temp_$seq
@@ -51,9 +50,7 @@ trap "_cleanup ; exit \$status" 0 1 2 3 15
_supported_fs btrfs
_supported_os Linux
_require_scratch
-
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
+_require_fssum
_scratch_mkfs > /dev/null 2>&1
diff --git a/tests/btrfs/030 b/tests/btrfs/030
index a9f5fb4..a76a410 100755
--- a/tests/btrfs/030
+++ b/tests/btrfs/030
@@ -36,7 +36,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -54,11 +53,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/038 b/tests/btrfs/038
index 8893696..4941d3e 100755
--- a/tests/btrfs/038
+++ b/tests/btrfs/038
@@ -32,7 +32,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -50,11 +49,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/039 b/tests/btrfs/039
index 41e09be..758b23c 100755
--- a/tests/btrfs/039
+++ b/tests/btrfs/039
@@ -35,7 +35,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -53,11 +52,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/040 b/tests/btrfs/040
index 77c4a84..dfd495c 100755
--- a/tests/btrfs/040
+++ b/tests/btrfs/040
@@ -35,7 +35,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -53,11 +52,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/041 b/tests/btrfs/041
index bfc0e4b..a960001 100755
--- a/tests/btrfs/041
+++ b/tests/btrfs/041
@@ -31,7 +31,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -49,11 +48,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
diff --git a/tests/btrfs/042 b/tests/btrfs/042
index 2bd5147..9aa8967 100755
--- a/tests/btrfs/042
+++ b/tests/btrfs/042
@@ -36,7 +36,6 @@ seq=`basename $0`
seqres=$RESULT_DIR/$seq
echo "QA output created by $seq"
-here=`pwd`
tmp=`mktemp -d`
status=1 # failure is the default!
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -54,11 +53,9 @@ _cleanup()
_supported_fs btrfs
_supported_os Linux
_require_scratch
+_require_fssum
_need_to_be_root
-FSSUM_PROG=$here/src/fssum
-[ -x $FSSUM_PROG ] || _notrun "fssum not built"
-
rm -f $seqres.full
_scratch_mkfs >/dev/null 2>&1
--
1.7.9.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread