Re: [PATCH v2] fstests: btrfs/246: add test case to make sure btrfs can create compressed inline extent

[Eryu Guan <guan@eryu.me>]

On Sun, Aug 29, 2021 at 09:49:05PM +0800, Qu Wenruo wrote:
> 
> 
> On 2021/8/29 下午9:42, Eryu Guan wrote:
> > On Thu, Aug 26, 2021 at 01:34:32PM +0800, Qu Wenruo wrote:
> > > Btrfs has the ability to inline small file extents into its metadata,
> > > and such inlined extents can be further compressed if needed.
> > > 
> > > The new test case is for a regression caused by commit f2165627319f
> > > ("btrfs: compression: don't try to compress if we don't have enough
> > > pages").
> > > 
> > > That commit prevents btrfs from creating compressed inline extents, even
> > > "-o compress,max_inline=2048" is specified, only uncompressed inline
> > > extents can be created.
> > > 
> > > The test case will make sure that the content of the small file is
> > > consistent between cycle mount, then use "btrfs inspect dump-tree" to
> > > verify the created extent is both inlined and compressed.
> > > 
> > > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > 
> > Is there a proposed fix available that could be referenced in the commit
> > log?
> 
> The upstream commit is 4e9655763b82 ("Revert "btrfs: compression: don't try
> to compress if we don't have enough pages""), which is merged after I
> submitted the patch.

Ok. It also could be helpful to just include the proposed patch title if
it's not merged yet.

> 
> > 
> > > ---
> > > Changelog:
> > > v2:
> > > - Also output the sha256sum to make sure the content is consistent
> > > ---
> > >   tests/btrfs/246     | 53 +++++++++++++++++++++++++++++++++++++++++++++
> > >   tests/btrfs/246.out |  5 +++++
> > >   2 files changed, 58 insertions(+)
> > >   create mode 100755 tests/btrfs/246
> > >   create mode 100644 tests/btrfs/246.out
> > > 
> > > diff --git a/tests/btrfs/246 b/tests/btrfs/246
> > > new file mode 100755
> > > index 00000000..e0d8016f
> > > --- /dev/null
> > > +++ b/tests/btrfs/246
> > > @@ -0,0 +1,53 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2021 SUSE Linux Products GmbH.  All Rights Reserved.
> > > +#
> > > +# FS QA Test 246
> > > +#
> > > +# Make sure btrfs can create compressed inline extents
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto quick compress
> > > +
> > > +# Override the default cleanup function.
> > > +_cleanup()
> > > +{
> > > +	cd /
> > > +	rm -r -f $tmp.*
> > > +}
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +# For __populate_find_inode()
> > > +. ./common/populate
> > 
> > This function starts with double underscore, I take it as a 'private'
> > function in common/populate. But all it does is returning the inode
> > number of the given file, I think we could just open-code it in this
> > test as
> > 
> > ino=$(stat -c %i $SCRATCH_MNT/foobar)
> > 
> > Otherwise test looks fine to me.
> 
> Mind me to send an update to include the fix in commit message and use the
> local ino helper?

You're responding quickly, I haven't finalized this week's update yet,
so I can add the fix commit info and remove __populate_find_inode on
commit :)

Thanks,
Eryu