* [PATCH] btrfs/029: fix wrong usage of name filter
@ 2017-08-30 7:38 Misono, Tomohiro
2017-08-30 11:09 ` Eryu Guan
0 siblings, 1 reply; 8+ messages in thread
From: Misono, Tomohiro @ 2017-08-30 7:38 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs
btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
$SCRATCH_MNT directory.
In this function, it calls both _filter_test_dir and _filter_scratch
concatenated by pipe. Therefore if $TEST_DIR is a prefix of
$SCRATCH_MNT, this filter function gives wrong filtered name for
$SCRATCH_MNT and the test fails.
Fix this by calling _filter_test_dir and _filter_scratch directly.
Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
tests/btrfs/029 | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/tests/btrfs/029 b/tests/btrfs/029
index c390c95..74fded7 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_scratch
+ md5sum $2 | _filter_test_dir
# 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_test_dir
}
echo "test reflinks across different devices"
--
2.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs/029: fix wrong usage of name filter
2017-08-30 7:38 [PATCH] btrfs/029: fix wrong usage of name filter Misono, Tomohiro
@ 2017-08-30 11:09 ` Eryu Guan
2017-08-30 23:53 ` Misono, Tomohiro
0 siblings, 1 reply; 8+ messages in thread
From: Eryu Guan @ 2017-08-30 11:09 UTC (permalink / raw)
To: Misono, Tomohiro; +Cc: fstests, linux-btrfs
On Wed, Aug 30, 2017 at 04:38:16PM +0900, Misono, Tomohiro wrote:
> btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
> $SCRATCH_MNT directory.
>
> In this function, it calls both _filter_test_dir and _filter_scratch
> concatenated by pipe. Therefore if $TEST_DIR is a prefix of
> $SCRATCH_MNT, this filter function gives wrong filtered name for
> $SCRATCH_MNT and the test fails.
Sorry, I'm a bit confused, how could $TEST_DIR be a prefix of
$SCRATCH_MNT? Won't that fail the test at setup time?
Thanks,
Eryu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs/029: fix wrong usage of name filter
2017-08-30 11:09 ` Eryu Guan
@ 2017-08-30 23:53 ` Misono, Tomohiro
2017-08-31 7:33 ` Eryu Guan
0 siblings, 1 reply; 8+ messages in thread
From: Misono, Tomohiro @ 2017-08-30 23:53 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests, linux-btrfs
On 2017/08/30 20:09, Eryu Guan wrote:
> On Wed, Aug 30, 2017 at 04:38:16PM +0900, Misono, Tomohiro wrote:
>> btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
>> $SCRATCH_MNT directory.
>>
>> In this function, it calls both _filter_test_dir and _filter_scratch
>> concatenapted by pipe. Therefore if $TEST_DIR is a prefix of
>> $SCRATCH_MNT, this filter function gives wrong filtered name for
>> $SCRATCH_MNT and the test fails.
>
> Sorry, I'm a bit confused, how could $TEST_DIR be a prefix of
> $SCRATCH_MNT? Won't that fail the test at setup time?
I used "/mnt" for $TEST_DIR and "/mnt_scratch" for $SCRATCH_MNT and hit
this problem because "/mnt_scratch" is filtered to "$TEST_DIR_scrach"
instead of "$SCRATCH_MNT".
I think these are valid directory names and other btrfs tests run correctly
with these names.
Thanks,
Tomohiro
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs/029: fix wrong usage of name filter
2017-08-30 23:53 ` Misono, Tomohiro
@ 2017-08-31 7:33 ` Eryu Guan
2017-09-01 0:44 ` Misono, Tomohiro
0 siblings, 1 reply; 8+ messages in thread
From: Eryu Guan @ 2017-08-31 7:33 UTC (permalink / raw)
To: Misono, Tomohiro; +Cc: fstests, linux-btrfs
On Thu, Aug 31, 2017 at 08:53:09AM +0900, Misono, Tomohiro wrote:
> On 2017/08/30 20:09, Eryu Guan wrote:
> > On Wed, Aug 30, 2017 at 04:38:16PM +0900, Misono, Tomohiro wrote:
> >> btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
> >> $SCRATCH_MNT directory.
> >>
> >> In this function, it calls both _filter_test_dir and _filter_scratch
> >> concatenapted by pipe. Therefore if $TEST_DIR is a prefix of
> >> $SCRATCH_MNT, this filter function gives wrong filtered name for
> >> $SCRATCH_MNT and the test fails.
> >
> > Sorry, I'm a bit confused, how could $TEST_DIR be a prefix of
> > $SCRATCH_MNT? Won't that fail the test at setup time?
>
> I used "/mnt" for $TEST_DIR and "/mnt_scratch" for $SCRATCH_MNT and hit
> this problem because "/mnt_scratch" is filtered to "$TEST_DIR_scrach"
> instead of "$SCRATCH_MNT".
>
> I think these are valid directory names and other btrfs tests run correctly
> with these names.
Ah, yes, that's possible and a valid (though not usual) test setup. The
filter becomes complex when one string is prefix of another string, we
have similar problems when filtering TEST_DIR vs TEST_DEV and
SCRATCH_MNT vs SCRATCH_DEV in _filter_test_dir and _filter_scratch.
But the fix only works around btrfs/029, but there're other places we
use the two filters together, like in _filter_quota, TEST_DIR filter
results would be wrong if SCRATCH_MNT is a prefix of TEST_DIR, because
in _filter_quota it calls _filter_scratch first.
I think one solution is that we filter the longer string first, e.g.
move _filter_testdirs to common/filter and update it to something like:
# filter both test and scratch mount points and devices, but always
# filter the longer string if the other string is a substring of the
# first one.
_filter_testdirs()
{
if echo "$TEST_DIR" | grep -q "$SCRATCH_MNT"; then
_filter_test_dir | _filter_scratch
else
_filter_scratch | _filter_test_dir
fi
}
And use this new helper when needed. I found 4 places that need update,
they're btrfs/029, generic/409, generic/410, generic/411 and
_filter_quota().
Thanks,
Eryu
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs/029: fix wrong usage of name filter
2017-08-31 7:33 ` Eryu Guan
@ 2017-09-01 0:44 ` Misono, Tomohiro
2017-09-01 3:47 ` Eryu Guan
0 siblings, 1 reply; 8+ messages in thread
From: Misono, Tomohiro @ 2017-09-01 0:44 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests, linux-btrfs
On 2017/08/31 16:33, Eryu Guan wrote:
> On Thu, Aug 31, 2017 at 08:53:09AM +0900, Misono, Tomohiro wrote:
>> On 2017/08/30 20:09, Eryu Guan wrote:
>>> On Wed, Aug 30, 2017 at 04:38:16PM +0900, Misono, Tomohiro wrote:
>>>> btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
>>>> $SCRATCH_MNT directory.
>>>>
>>>> In this function, it calls both _filter_test_dir and _filter_scratch
>>>> concatenapted by pipe. Therefore if $TEST_DIR is a prefix of
>>>> $SCRATCH_MNT, this filter function gives wrong filtered name for
>>>> $SCRATCH_MNT and the test fails.
>>>
>>> Sorry, I'm a bit confused, how could $TEST_DIR be a prefix of
>>> $SCRATCH_MNT? Won't that fail the test at setup time?
>>
>> I used "/mnt" for $TEST_DIR and "/mnt_scratch" for $SCRATCH_MNT and hit
>> this problem because "/mnt_scratch" is filtered to "$TEST_DIR_scrach"
>> instead of "$SCRATCH_MNT".
>>
>> I think these are valid directory names and other btrfs tests run correctly
>> with these names.
>
> Ah, yes, that's possible and a valid (though not usual) test setup. The
> filter becomes complex when one string is prefix of another string, we
> have similar problems when filtering TEST_DIR vs TEST_DEV and
> SCRATCH_MNT vs SCRATCH_DEV in _filter_test_dir and _filter_scratch.
>
> But the fix only works around btrfs/029, but there're other places we
> use the two filters together, like in _filter_quota, TEST_DIR filter
> results would be wrong if SCRATCH_MNT is a prefix of TEST_DIR, because
> in _filter_quota it calls _filter_scratch first.
>
> I think one solution is that we filter the longer string first, e.g.
> move _filter_testdirs to common/filter and update it to something like:
>
> # filter both test and scratch mount points and devices, but always
> # filter the longer string if the other string is a substring of the
> # first one.
> _filter_testdirs()
> {
> if echo "$TEST_DIR" | grep -q "$SCRATCH_MNT"; then
> _filter_test_dir | _filter_scratch
> else
> _filter_scratch | _filter_test_dir
> fi
> }
>
> And use this new helper when needed. I found 4 places that need update,
> they're btrfs/029, generic/409, generic/410, generic/411 and
> _filter_quota().
>
Ok. I will do that if you won't, though I'm not sure other combination of
filters would pose the similar problem.
Thanks,
Tomohiro
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] btrfs/029: fix wrong usage of name filter
@ 2017-08-30 2:44 Misono, Tomohiro
2017-08-30 6:56 ` Misono, Tomohiro
0 siblings, 1 reply; 8+ messages in thread
From: Misono, Tomohiro @ 2017-08-30 2:44 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs
btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
$SCRATCH_MNT directory.
In this function, it calls both _filter_test_dir and _filter_scratch
concatenated by pipe. Therefore if $TEST_DIR is a prefix of
$SCRATCH_MNT, this filter function gives wrong filtered name for
$SCRATCH_MNT and the test fails.
Fix this by calling _filter_test_dir and _filter_scratch directly.
Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujits.com>
---
tests/btrfs/029 | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/tests/btrfs/029 b/tests/btrfs/029
index c390c95..74fded7 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_scratch
+ md5sum $2 | _filter_test_dir
# 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_test_dir
}
echo "test reflinks across different devices"
--
2.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] btrfs/029: fix wrong usage of name filter
2017-08-30 2:44 Misono, Tomohiro
@ 2017-08-30 6:56 ` Misono, Tomohiro
0 siblings, 0 replies; 8+ messages in thread
From: Misono, Tomohiro @ 2017-08-30 6:56 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs
Sorry, this patch contains leading spaces, I will resend this soon.
On 2017/08/30 11:44, Misono, Tomohiro wrote:
> btrfs/029 uses _filter_testdirs() to filter the name of $TEST_DIR and
> $SCRATCH_MNT directory.
>
> In this function, it calls both _filter_test_dir and _filter_scratch
> concatenated by pipe. Therefore if $TEST_DIR is a prefix of
> $SCRATCH_MNT, this filter function gives wrong filtered name for
> $SCRATCH_MNT and the test fails.
>
> Fix this by calling _filter_test_dir and _filter_scratch directly.
>
> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujits.com>
> ---
> tests/btrfs/029 | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/tests/btrfs/029 b/tests/btrfs/029
> index c390c95..74fded7 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_scratch
> + md5sum $2 | _filter_test_dir
>
> # 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_test_dir
> }
>
> echo "test reflinks across different devices"
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-09-01 3:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-30 7:38 [PATCH] btrfs/029: fix wrong usage of name filter Misono, Tomohiro
2017-08-30 11:09 ` Eryu Guan
2017-08-30 23:53 ` Misono, Tomohiro
2017-08-31 7:33 ` Eryu Guan
2017-09-01 0:44 ` Misono, Tomohiro
2017-09-01 3:47 ` Eryu Guan
-- strict thread matches above, loose matches on Subject: below --
2017-08-30 2:44 Misono, Tomohiro
2017-08-30 6:56 ` Misono, Tomohiro
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.