On Wed, Nov 10, 2021 at 06:52:17PM +0800, Qu Wenruo wrote:
>
>
> On 2021/11/10 18:48, Eryu Guan wrote:
> >On Wed, Nov 10, 2021 at 05:34:17PM +0800, Qu Wenruo wrote:
> >>In the coming btrfs-progs v5.15 release, mkfs.btrfs will change to use
> >>v2 cache by default.
> >>
> >>However nospace_cache mount option will not work with v2 cache, as it
> >>would make v2 cache out of sync with on-disk used space.
> >>
> >>So mounting a btrfs with v2 cache using "nospace_cache" will make btrfs
> >>to reject the mount.
> >>
> >>There are quite some test cases relying on nospace_cache to prevent v1
> >>cache to take up data space.
> >>
> >>For those test cases, we no longer need the "nospace_cache" mount option
> >>if the filesystem is already using v2 cache.
> >>Since v2 cache is using metadata space, it will no longer take up data
> >>space, thus no extra mount options for those test cases.
> >>
> >>By this, we can keep those existing tests to run without problem for
> >>both v1 and v2 cache.
> >>
> >>Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> >>---
> >>Changelog:
> >>v2:
> >>- Add _scratch_no_v1_cache_opt() function
> >>v3:
> >>- Add _require_btrfs_command for _scratch_no_v1_cache_opt()
> >>---
> >> common/btrfs | 11 +++++++++++
> >> tests/btrfs/102 | 2 +-
> >> tests/btrfs/140 | 5 ++---
> >> tests/btrfs/141 | 5 ++---
> >> tests/btrfs/142 | 5 ++---
> >> tests/btrfs/143 | 5 ++---
> >> tests/btrfs/151 | 4 ++--
> >> tests/btrfs/157 | 7 +++----
> >> tests/btrfs/158 | 7 +++----
> >> tests/btrfs/170 | 4 ++--
> >> tests/btrfs/199 | 4 ++--
> >> tests/btrfs/215 | 2 +-
> >> 12 files changed, 33 insertions(+), 28 deletions(-)
> >>
> >>diff --git a/common/btrfs b/common/btrfs
> >>index ac880bdd..e21c452c 100644
> >>--- a/common/btrfs
> >>+++ b/common/btrfs
> >>@@ -445,3 +445,14 @@ _scratch_btrfs_is_zoned()
> >> [ `_zone_type ${SCRATCH_DEV}` != "none" ] && return 0
> >> return 1
> >> }
> >>+
> >>+_scratch_no_v1_cache_opt()
> >
> >This name indicates it's a general helper, but it's btrfs-specific, how
> >about _scratch_btrfs_no_v1_cache_opt ?
> >
> >>+{
> >>+ _require_btrfs_command inspect-internal dump-tree
> >
> >This will call _notrun if btrfs command doesn't have inspect-internal
> >dump-tree sub-command, and _notrun will call exit, but ...
> >
> >>+
> >>+ if $BTRFS_UTIL_PROG inspect-internal dump-tree $SCRATCH_DEV |\
> >>+ grep -q "FREE_SPACE_TREE"; then
> >>+ return
> >>+ fi
> >>+ echo -n "-onospace_cache"
> >>+}
> >>diff --git a/tests/btrfs/102 b/tests/btrfs/102
> >>index e5a1b068..c1678b5d 100755
> >>--- a/tests/btrfs/102
> >>+++ b/tests/btrfs/102
> >>@@ -22,7 +22,7 @@ _scratch_mkfs >>$seqres.full 2>&1
> >> # Mount our filesystem without space caches enabled so that we do not get any
> >> # space used from the initial data block group that mkfs creates (space caches
> >> # used space from data block groups).
> >>-_scratch_mount "-o nospace_cache"
> >>+_scratch_mount $(_scratch_no_v1_cache_opt)
> >
> >_scratch_no_v1_cache_opt is called in a sub-shell, so the _notrun will
> >just exit the sub-shell, not the test itself. Should call the _require
> >rule in test.
>
> That means we will have a hard dependency on binding
> _scratch_btrfs_no_v1_cache_opt() with _require rule then.
>
> Then a sudden "_require_btrfs_command inspect-internal dump-tree"
> without context could be sometimes confusing AFAIK.
That's true.
>
> Considering "inspect-internal" should be in btrfs-progs for a very long
> time, any non-EOF distro should have them already, can we just remove
> the _require rule?
It seems like that dump-tree sub-command was added in 2016 in v4.5, so I
guess I'm fine with it.
Thanks,
Eryu
