diff mbox series

btrfs/284: list a couple btrfs-progs git commits

Message ID 7be1169e950b807f24e4b2ac33177e44fc13e434.1678189053.git.fdmanana@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs/284: list a couple btrfs-progs git commits | expand

Commit Message

Filipe Manana March 7, 2023, 11:38 a.m. UTC
From: Filipe Manana <fdmanana@suse.com>

This test may often fail when running with btrfs-progs versions not very
recent. The corresponding git commits in btrfs-progs that fix issues
uncovered by this test are:

1) 6f4a51886b37 ("btrfs-progs: receive: fix silent data loss after fall back from encoded write")
   Introduced in btrfs-progs v6.0.2;

2) e3209f8792f4 ("btrfs-progs: receive: fix a corruption when decompressing zstd extents"")
   Introduced in btrfs-progs v6.2.

So add the corresponding _fixed_by_git_commit calls to the test.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 tests/btrfs/284 | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Anand Jain March 8, 2023, 8:03 a.m. UTC | #1
On 07/03/2023 19:38, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> This test may often fail when running with btrfs-progs versions not very
> recent. The corresponding git commits in btrfs-progs that fix issues
> uncovered by this test are:
> 
> 1) 6f4a51886b37 ("btrfs-progs: receive: fix silent data loss after fall back from encoded write")
>     Introduced in btrfs-progs v6.0.2;
> 
> 2) e3209f8792f4 ("btrfs-progs: receive: fix a corruption when decompressing zstd extents"")
>     Introduced in btrfs-progs v6.2.
> 
> So add the corresponding _fixed_by_git_commit calls to the test.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>   tests/btrfs/284 | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/tests/btrfs/284 b/tests/btrfs/284
> index 0d31e5d9..c6692668 100755
> --- a/tests/btrfs/284
> +++ b/tests/btrfs/284
> @@ -20,6 +20,11 @@ _require_test
>   _require_scratch_size $(($LOAD_FACTOR * 1 * 1024 * 1024))
>   _require_fssum
>   
> +_fixed_by_git_commit btrfs-progs e3209f8792f4 \
> +	"btrfs-progs: receive: fix a corruption when decompressing zstd extents"
> +_fixed_by_git_commit btrfs-progs 6f4a51886b37 \
> +	"btrfs-progs: receive: fix silent data loss after fall back from encoded write"
> +
>   send_files_dir=$TEST_DIR/btrfs-test-$seq
>   
>   rm -fr $send_files_dir


Along with this, why not check the btrfs-progs version using
'btrfs --version' and call _not_run()?

Thanks, Anand
Zorro Lang March 8, 2023, 8:47 a.m. UTC | #2
On Wed, Mar 08, 2023 at 04:03:57PM +0800, Anand Jain wrote:
> On 07/03/2023 19:38, fdmanana@kernel.org wrote:
> > From: Filipe Manana <fdmanana@suse.com>
> > 
> > This test may often fail when running with btrfs-progs versions not very
> > recent. The corresponding git commits in btrfs-progs that fix issues
> > uncovered by this test are:
> > 
> > 1) 6f4a51886b37 ("btrfs-progs: receive: fix silent data loss after fall back from encoded write")
> >     Introduced in btrfs-progs v6.0.2;
> > 
> > 2) e3209f8792f4 ("btrfs-progs: receive: fix a corruption when decompressing zstd extents"")
> >     Introduced in btrfs-progs v6.2.
> > 
> > So add the corresponding _fixed_by_git_commit calls to the test.
> > 
> > Signed-off-by: Filipe Manana <fdmanana@suse.com>
> > ---
> >   tests/btrfs/284 | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/tests/btrfs/284 b/tests/btrfs/284
> > index 0d31e5d9..c6692668 100755
> > --- a/tests/btrfs/284
> > +++ b/tests/btrfs/284
> > @@ -20,6 +20,11 @@ _require_test
> >   _require_scratch_size $(($LOAD_FACTOR * 1 * 1024 * 1024))
> >   _require_fssum
> > +_fixed_by_git_commit btrfs-progs e3209f8792f4 \
> > +	"btrfs-progs: receive: fix a corruption when decompressing zstd extents"
> > +_fixed_by_git_commit btrfs-progs 6f4a51886b37 \
> > +	"btrfs-progs: receive: fix silent data loss after fall back from encoded write"
> > +
> >   send_files_dir=$TEST_DIR/btrfs-test-$seq
> >   rm -fr $send_files_dir
> 
> 
> Along with this, why not check the btrfs-progs version using
> 'btrfs --version' and call _not_run()?

Does this case expose some known bugs, right? Or the failures due to some
features missing?

If this case uncovers some known issues, then the failure is expected on unfixed
version. We should let the failure exposing, not hide it by _notrun.

And the package version is not a good way to jundge if a issue is fixed or a
feature is merged. Due to many downstream packages might merge some upstream
patches independently.

Thanks,
Zorro

> 
> Thanks, Anand
>
Anand Jain March 8, 2023, 9:16 a.m. UTC | #3
On 3/8/23 16:47, Zorro Lang wrote:
> On Wed, Mar 08, 2023 at 04:03:57PM +0800, Anand Jain wrote:
>> On 07/03/2023 19:38, fdmanana@kernel.org wrote:
>>> From: Filipe Manana <fdmanana@suse.com>
>>>
>>> This test may often fail when running with btrfs-progs versions not very
>>> recent. The corresponding git commits in btrfs-progs that fix issues
>>> uncovered by this test are:
>>>
>>> 1) 6f4a51886b37 ("btrfs-progs: receive: fix silent data loss after fall back from encoded write")
>>>      Introduced in btrfs-progs v6.0.2;
>>>
>>> 2) e3209f8792f4 ("btrfs-progs: receive: fix a corruption when decompressing zstd extents"")
>>>      Introduced in btrfs-progs v6.2.
>>>
>>> So add the corresponding _fixed_by_git_commit calls to the test.
>>>
>>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
>>> ---
>>>    tests/btrfs/284 | 5 +++++
>>>    1 file changed, 5 insertions(+)
>>>
>>> diff --git a/tests/btrfs/284 b/tests/btrfs/284
>>> index 0d31e5d9..c6692668 100755
>>> --- a/tests/btrfs/284
>>> +++ b/tests/btrfs/284
>>> @@ -20,6 +20,11 @@ _require_test
>>>    _require_scratch_size $(($LOAD_FACTOR * 1 * 1024 * 1024))
>>>    _require_fssum
>>> +_fixed_by_git_commit btrfs-progs e3209f8792f4 \
>>> +	"btrfs-progs: receive: fix a corruption when decompressing zstd extents"
>>> +_fixed_by_git_commit btrfs-progs 6f4a51886b37 \
>>> +	"btrfs-progs: receive: fix silent data loss after fall back from encoded write"
>>> +
>>>    send_files_dir=$TEST_DIR/btrfs-test-$seq
>>>    rm -fr $send_files_dir
>>
>>
>> Along with this, why not check the btrfs-progs version using
>> 'btrfs --version' and call _not_run()?
> 
> Does this case expose some known bugs, right? Or the failures due to some
> features missing?
> 

It tests for a bug.

> If this case uncovers some known issues, then the failure is expected on unfixed
> version. We should let the failure exposing, not hide it by _notrun.

Makes sense.

> And the package version is not a good way to jundge if a issue is fixed or a
> feature is merged. Due to many downstream packages might merge some upstream
> patches independently.
> 

Yeah, I agree. You can guarantee that if btrfs-progs is plain vanilla.

Looks good as it is.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

Thanks, Anand

> Thanks,
> Zorro
> 
>>
>> Thanks, Anand
>>
>
Filipe Manana March 8, 2023, 10:09 a.m. UTC | #4
On Wed, Mar 8, 2023 at 9:17 AM Anand Jain <anand.jain@oracle.com> wrote:
>
>
>
> On 3/8/23 16:47, Zorro Lang wrote:
> > On Wed, Mar 08, 2023 at 04:03:57PM +0800, Anand Jain wrote:
> >> On 07/03/2023 19:38, fdmanana@kernel.org wrote:
> >>> From: Filipe Manana <fdmanana@suse.com>
> >>>
> >>> This test may often fail when running with btrfs-progs versions not very
> >>> recent. The corresponding git commits in btrfs-progs that fix issues
> >>> uncovered by this test are:
> >>>
> >>> 1) 6f4a51886b37 ("btrfs-progs: receive: fix silent data loss after fall back from encoded write")
> >>>      Introduced in btrfs-progs v6.0.2;
> >>>
> >>> 2) e3209f8792f4 ("btrfs-progs: receive: fix a corruption when decompressing zstd extents"")
> >>>      Introduced in btrfs-progs v6.2.
> >>>
> >>> So add the corresponding _fixed_by_git_commit calls to the test.
> >>>
> >>> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> >>> ---
> >>>    tests/btrfs/284 | 5 +++++
> >>>    1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/tests/btrfs/284 b/tests/btrfs/284
> >>> index 0d31e5d9..c6692668 100755
> >>> --- a/tests/btrfs/284
> >>> +++ b/tests/btrfs/284
> >>> @@ -20,6 +20,11 @@ _require_test
> >>>    _require_scratch_size $(($LOAD_FACTOR * 1 * 1024 * 1024))
> >>>    _require_fssum
> >>> +_fixed_by_git_commit btrfs-progs e3209f8792f4 \
> >>> +   "btrfs-progs: receive: fix a corruption when decompressing zstd extents"
> >>> +_fixed_by_git_commit btrfs-progs 6f4a51886b37 \
> >>> +   "btrfs-progs: receive: fix silent data loss after fall back from encoded write"
> >>> +
> >>>    send_files_dir=$TEST_DIR/btrfs-test-$seq
> >>>    rm -fr $send_files_dir
> >>
> >>
> >> Along with this, why not check the btrfs-progs version using
> >> 'btrfs --version' and call _not_run()?
> >
> > Does this case expose some known bugs, right? Or the failures due to some
> > features missing?
> >
>
> It tests for a bug.

The test is meant to be a generic stress test for send v2 streams.

It happens to have uncovered 2 bugs (so far). And if it finds out more
bugs in the future, I'll surely list more commits in it.

So I don't get where you got the idea to skip running a test based on
the btrfs-progs version.
We don't do that anywhere in fstests, neither for btrfs-progs nor
kernel or anything else. The reason was already pointed out to you:
distros, vendors, often backport commits to older versions - working
for a company with a distro, I would expect you to be familiar with
that :)

Thanks.

>
> > If this case uncovers some known issues, then the failure is expected on unfixed
> > version. We should let the failure exposing, not hide it by _notrun.
>
> Makes sense.
>
> > And the package version is not a good way to jundge if a issue is fixed or a
> > feature is merged. Due to many downstream packages might merge some upstream
> > patches independently.
> >
>
> Yeah, I agree. You can guarantee that if btrfs-progs is plain vanilla.
>
> Looks good as it is.
>
> Reviewed-by: Anand Jain <anand.jain@oracle.com>
>
> Thanks, Anand
>
> > Thanks,
> > Zorro
> >
> >>
> >> Thanks, Anand
> >>
> >
diff mbox series

Patch

diff --git a/tests/btrfs/284 b/tests/btrfs/284
index 0d31e5d9..c6692668 100755
--- a/tests/btrfs/284
+++ b/tests/btrfs/284
@@ -20,6 +20,11 @@  _require_test
 _require_scratch_size $(($LOAD_FACTOR * 1 * 1024 * 1024))
 _require_fssum
 
+_fixed_by_git_commit btrfs-progs e3209f8792f4 \
+	"btrfs-progs: receive: fix a corruption when decompressing zstd extents"
+_fixed_by_git_commit btrfs-progs 6f4a51886b37 \
+	"btrfs-progs: receive: fix silent data loss after fall back from encoded write"
+
 send_files_dir=$TEST_DIR/btrfs-test-$seq
 
 rm -fr $send_files_dir