All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Add new common filter function
@ 2017-09-01  5:39 Misono, Tomohiro
  2017-09-01  5:41 ` [PATCH 2/2] Use common filter function in several tests Misono, Tomohiro
  2017-09-01  7:04 ` [PATCH 1/2] Add new common filter function Eryu Guan
  0 siblings, 2 replies; 5+ messages in thread
From: Misono, Tomohiro @ 2017-09-01  5:39 UTC (permalink / raw)
  To: fstests, Eryu Guan; +Cc: linux-btrfs

Several tests uses both _filter_test_dir and _filter_scratch
concatenated by pipe to filter $TEST_DIR and $SCRATCH_MNT. However, this
would fail if the shorter string is a substring of the other (like
"/mnt" and "/mnt2").

This patch introduces new common filter function to safely call both
_filter_test_dir and _filter_scratch.

I chedked this with btrfs/029, generic/409,410,411, and generic/381,383,
xfs/106,108 (which calls _filter_quota). Thanks Eryu for advice.

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
 common/filter | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/common/filter b/common/filter
index 1ef342b..75570f9 100644
--- a/common/filter
+++ b/common/filter
@@ -295,6 +295,17 @@ _filter_scratch()
 	    -e "/.use_space/d"
 }
 
+_filter_testdir_and_scratch()
+{
+	# filter both $TEST_DIR and $SCRATCH_MNT, but always filter the longer
+	# string first if the other string is a substring of the first one
+	if echo "$TEST_DIR" | grep -q "$SCRATCH_MNT"; then
+	  _filter_test_dir | _filter_scratch
+	else
+	  _filter_scratch | _filter_test_dir
+	fi
+}
+
 # Turn any device in the scratch pool into SCRATCH_DEV
 _filter_scratch_pool()
 {
-- 
2.9.5


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] Use common filter function in several tests
  2017-09-01  5:39 [PATCH 1/2] Add new common filter function Misono, Tomohiro
@ 2017-09-01  5:41 ` Misono, Tomohiro
  2017-09-01  7:04 ` [PATCH 1/2] Add new common filter function Eryu Guan
  1 sibling, 0 replies; 5+ messages in thread
From: Misono, Tomohiro @ 2017-09-01  5:41 UTC (permalink / raw)
  To: fstests, Eryu Guan; +Cc: linux-btrfs

Use newly introduced common function to filter both $TEST_DIR and
$SCRATCH_MNT.

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
 common/filter     |  2 +-
 tests/btrfs/029   | 11 +++--------
 tests/generic/409 |  3 +--
 tests/generic/410 |  3 +--
 tests/generic/411 |  3 +--
 5 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/common/filter b/common/filter
index 75570f9..5d50f61 100644
--- a/common/filter
+++ b/common/filter
@@ -322,7 +322,7 @@ _filter_quota()
 {
 	# Long dev name might be split onto its own line; last
 	# seds remove that newline if present
-	_filter_scratch | _filter_test_dir | _filter_spaces | \
+	_filter_testdir_and_scratch | _filter_spaces | \
 	sed -e 'N;s/SCRATCH_DEV\n/SCRATCH_DEV/g' | \
 	sed -e 'N;s/TEST_DEV\n/TEST_DEV/g'
 }
diff --git a/tests/btrfs/029 b/tests/btrfs/029
index c390c95..b6e2dbb 100755
--- a/tests/btrfs/029
+++ b/tests/btrfs/029
@@ -66,19 +66,14 @@ _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 $XFS_IO_PROG -f -c 'pwrite -S 0x61 0 9000' $SCRATCH_MNT/original >> $seqres.full
 
-_filter_testdirs()
-{
-    _filter_test_dir | _filter_scratch
-}
-
 _create_reflinks()
 {
     # auto reflink, should fall back to non-reflink
     rm -rf $2
     echo "reflink=auto:"
     cp --reflink=auto $1 $2
-    md5sum $1 | _filter_testdirs
-    md5sum $2 | _filter_testdirs
+    md5sum $1 | _filter_testdir_and_scratch
+    md5sum $2 | _filter_testdir_and_scratch
 
     # always reflink, should fail outright
     rm -rf $2
@@ -86,7 +81,7 @@ _create_reflinks()
     cp --reflink=always $1 $2 >> $seqres.full 2>&1 || echo "cp reflink failed"
 
     # The failed target actually gets created by cp:
-    ls $2 | _filter_testdirs
+    ls $2 | _filter_testdir_and_scratch
 }
 
 echo "test reflinks across different devices"
diff --git a/tests/generic/409 b/tests/generic/409
index 22af44c..3ad65c9 100755
--- a/tests/generic/409
+++ b/tests/generic/409
@@ -104,8 +104,7 @@ find_mnt()
 		    -e "s;$mpB;mpB;g" \
 		    -e "s;$mpC;mpC;g" \
 		    -e "s;$mpD;mpD;g" | \
-		_filter_spaces | _filter_scratch | \
-		_filter_test_dir | sort
+		_filter_spaces | _filter_testdir_and_scratch | sort
 	echo "======"
 }
 
diff --git a/tests/generic/410 b/tests/generic/410
index 18cb0c1..63ab716 100755
--- a/tests/generic/410
+++ b/tests/generic/410
@@ -110,8 +110,7 @@ find_mnt()
 		sed -e "s;$mpA;mpA;g" \
 		    -e "s;$mpB;mpB;g" \
 		    -e "s;$mpC;mpC;g" | \
-		_filter_spaces | _filter_scratch | \
-		_filter_test_dir | sort
+		_filter_spaces | _filter_testdir_and_scratch | sort
 	echo "======"
 }
 
diff --git a/tests/generic/411 b/tests/generic/411
index 5db49a4..83f6d26 100755
--- a/tests/generic/411
+++ b/tests/generic/411
@@ -93,8 +93,7 @@ find_mnt()
 		sed -e "s;$mpA;mpA;g" \
 		    -e "s;$mpB;mpB;g" \
 		    -e "s;$mpC;mpC;g" | \
-		_filter_spaces | _filter_scratch | \
-		_filter_test_dir | sort
+		_filter_spaces | _filter_testdir_and_scratch | sort
 	echo "======"
 }
 
-- 
2.9.5


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Add new common filter function
  2017-09-01  5:39 [PATCH 1/2] Add new common filter function Misono, Tomohiro
  2017-09-01  5:41 ` [PATCH 2/2] Use common filter function in several tests Misono, Tomohiro
@ 2017-09-01  7:04 ` Eryu Guan
  2017-09-01  7:14   ` Amir Goldstein
  1 sibling, 1 reply; 5+ messages in thread
From: Eryu Guan @ 2017-09-01  7:04 UTC (permalink / raw)
  To: Misono, Tomohiro; +Cc: fstests, linux-btrfs

On Fri, Sep 01, 2017 at 02:39:44PM +0900, Misono, Tomohiro wrote:
> Several tests uses both _filter_test_dir and _filter_scratch
> concatenated by pipe to filter $TEST_DIR and $SCRATCH_MNT. However, this
> would fail if the shorter string is a substring of the other (like
> "/mnt" and "/mnt2").
> 
> This patch introduces new common filter function to safely call both
> _filter_test_dir and _filter_scratch.
> 
> I chedked this with btrfs/029, generic/409,410,411, and generic/381,383,
> xfs/106,108 (which calls _filter_quota). Thanks Eryu for advice.
> 
> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>

Thanks! Though I don't think we need two separate patches, so I merged
the two patches together at commit time.

> ---
>  common/filter | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/common/filter b/common/filter
> index 1ef342b..75570f9 100644
> --- a/common/filter
> +++ b/common/filter
> @@ -295,6 +295,17 @@ _filter_scratch()
>  	    -e "/.use_space/d"
>  }
>  
> +_filter_testdir_and_scratch()
> +{
> +	# filter both $TEST_DIR and $SCRATCH_MNT, but always filter the longer
> +	# string first if the other string is a substring of the first one
> +	if echo "$TEST_DIR" | grep -q "$SCRATCH_MNT"; then
> +	  _filter_test_dir | _filter_scratch
> +	else
> +	  _filter_scratch | _filter_test_dir

And fixed the indention here, used tab :)

Thanks,
Eryu

> +	fi
> +}
> +
>  # Turn any device in the scratch pool into SCRATCH_DEV
>  _filter_scratch_pool()
>  {
> -- 
> 2.9.5
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Add new common filter function
  2017-09-01  7:04 ` [PATCH 1/2] Add new common filter function Eryu Guan
@ 2017-09-01  7:14   ` Amir Goldstein
  2017-09-01  7:43     ` Eryu Guan
  0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2017-09-01  7:14 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Misono, Tomohiro, fstests, Linux Btrfs

On Fri, Sep 1, 2017 at 10:04 AM, Eryu Guan <eguan@redhat.com> wrote:
> On Fri, Sep 01, 2017 at 02:39:44PM +0900, Misono, Tomohiro wrote:
>> Several tests uses both _filter_test_dir and _filter_scratch
>> concatenated by pipe to filter $TEST_DIR and $SCRATCH_MNT. However, this
>> would fail if the shorter string is a substring of the other (like
>> "/mnt" and "/mnt2").
>>
>> This patch introduces new common filter function to safely call both
>> _filter_test_dir and _filter_scratch.
>>
>> I chedked this with btrfs/029, generic/409,410,411, and generic/381,383,
>> xfs/106,108 (which calls _filter_quota). Thanks Eryu for advice.
>>
>> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
>
> Thanks! Though I don't think we need two separate patches, so I merged
> the two patches together at commit time.
>

FYI, there is still a way for a creative user to mess this up:

TEST_DEV=/test
TEST_DIR=/test/ovl-mnt
SCRATCH_DEV=/ovl
SCRATCH_MNT=/ovl/ovl-mnt

$SCRATCH_DEV is a substring of $TEST_DIR
and _filter_scratch is run first.

It's not that crazy to get to this config with the 'new'
-overlay command, e.g.:
TEST_DEV=/dev/sda
TEST_DIR=/test
SCRATCH_DEV=/dev/sdb
SCRATCH_MNT=/ovl

Will be auto converted to the values above.

This patch didn't break this use case.

>> ---
>>  common/filter | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/common/filter b/common/filter
>> index 1ef342b..75570f9 100644
>> --- a/common/filter
>> +++ b/common/filter
>> @@ -295,6 +295,17 @@ _filter_scratch()
>>           -e "/.use_space/d"
>>  }
>>
>> +_filter_testdir_and_scratch()
>> +{
>> +     # filter both $TEST_DIR and $SCRATCH_MNT, but always filter the longer
>> +     # string first if the other string is a substring of the first one
>> +     if echo "$TEST_DIR" | grep -q "$SCRATCH_MNT"; then
>> +       _filter_test_dir | _filter_scratch
>> +     else
>> +       _filter_scratch | _filter_test_dir
>
> And fixed the indention here, used tab :)
>
> Thanks,
> Eryu
>
>> +     fi
>> +}
>> +
>>  # Turn any device in the scratch pool into SCRATCH_DEV
>>  _filter_scratch_pool()
>>  {
>> --
>> 2.9.5
>>
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Add new common filter function
  2017-09-01  7:14   ` Amir Goldstein
@ 2017-09-01  7:43     ` Eryu Guan
  0 siblings, 0 replies; 5+ messages in thread
From: Eryu Guan @ 2017-09-01  7:43 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Misono, Tomohiro, fstests, Linux Btrfs

On Fri, Sep 01, 2017 at 10:14:41AM +0300, Amir Goldstein wrote:
> On Fri, Sep 1, 2017 at 10:04 AM, Eryu Guan <eguan@redhat.com> wrote:
> > On Fri, Sep 01, 2017 at 02:39:44PM +0900, Misono, Tomohiro wrote:
> >> Several tests uses both _filter_test_dir and _filter_scratch
> >> concatenated by pipe to filter $TEST_DIR and $SCRATCH_MNT. However, this
> >> would fail if the shorter string is a substring of the other (like
> >> "/mnt" and "/mnt2").
> >>
> >> This patch introduces new common filter function to safely call both
> >> _filter_test_dir and _filter_scratch.
> >>
> >> I chedked this with btrfs/029, generic/409,410,411, and generic/381,383,
> >> xfs/106,108 (which calls _filter_quota). Thanks Eryu for advice.
> >>
> >> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
> >
> > Thanks! Though I don't think we need two separate patches, so I merged
> > the two patches together at commit time.
> >
> 
> FYI, there is still a way for a creative user to mess this up:
> 
> TEST_DEV=/test
> TEST_DIR=/test/ovl-mnt
> SCRATCH_DEV=/ovl
> SCRATCH_MNT=/ovl/ovl-mnt
> 
> $SCRATCH_DEV is a substring of $TEST_DIR
> and _filter_scratch is run first.
> 
> It's not that crazy to get to this config with the 'new'
> -overlay command, e.g.:
> TEST_DEV=/dev/sda
> TEST_DIR=/test
> SCRATCH_DEV=/dev/sdb
> SCRATCH_MNT=/ovl
> 
> Will be auto converted to the values above.

Yeah, overlay makes it more complicated.

> 
> This patch didn't break this use case.

Good to know! I just tested this config with a quick overlay tests run,
it did pass.

Seems like we need some tests for fstests itself, like
./check selftest ? :)

Thanks,
Eryu

> 
> >> ---
> >>  common/filter | 11 +++++++++++
> >>  1 file changed, 11 insertions(+)
> >>
> >> diff --git a/common/filter b/common/filter
> >> index 1ef342b..75570f9 100644
> >> --- a/common/filter
> >> +++ b/common/filter
> >> @@ -295,6 +295,17 @@ _filter_scratch()
> >>           -e "/.use_space/d"
> >>  }
> >>
> >> +_filter_testdir_and_scratch()
> >> +{
> >> +     # filter both $TEST_DIR and $SCRATCH_MNT, but always filter the longer
> >> +     # string first if the other string is a substring of the first one
> >> +     if echo "$TEST_DIR" | grep -q "$SCRATCH_MNT"; then
> >> +       _filter_test_dir | _filter_scratch
> >> +     else
> >> +       _filter_scratch | _filter_test_dir
> >
> > And fixed the indention here, used tab :)
> >
> > Thanks,
> > Eryu
> >
> >> +     fi
> >> +}
> >> +
> >>  # Turn any device in the scratch pool into SCRATCH_DEV
> >>  _filter_scratch_pool()
> >>  {
> >> --
> >> 2.9.5
> >>
> > --
> > To unsubscribe from this list: send the line "unsubscribe fstests" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-01  7:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-01  5:39 [PATCH 1/2] Add new common filter function Misono, Tomohiro
2017-09-01  5:41 ` [PATCH 2/2] Use common filter function in several tests Misono, Tomohiro
2017-09-01  7:04 ` [PATCH 1/2] Add new common filter function Eryu Guan
2017-09-01  7:14   ` Amir Goldstein
2017-09-01  7:43     ` Eryu Guan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.