[PATCH] btrfs-progs: check: allow force v1 space cache cleanup even the fs has v2 space cache enabled

[PATCH] btrfs-progs: check: allow force v1 space cache cleanup even the fs has v2 space cache enabled

[Qu Wenruo]

There are cases where v1 free space cache is still left while user has
already enabled v2 cache.

In that case, we still want to force v1 space cache cleanup in
btrfs-check.

This patch will remove the v2 check if we're cleaning up v1 cache,
allowing us to cleanup the leftover.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/check/main.c b/check/main.c
index 8ad7f5886f06..f4755d260bfe 100644
--- a/check/main.c
+++ b/check/main.c
@@ -9917,12 +9917,6 @@ static int do_clear_free_space_cache(int clear_version)
 	int ret = 0;
 
 	if (clear_version == 1) {
-		if (btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE)) {
-			error(
-		"free space cache v2 detected, use --clear-space-cache v2");
-			ret = 1;
-			goto close_out;
-		}
 		ret = clear_free_space_cache();
 		if (ret) {
 			error("failed to clear free space cache");
-- 
2.29.2

Re: [PATCH] btrfs-progs: check: allow force v1 space cache cleanup even the fs has v2 space cache enabled

[Stéphane Lesimple]

December 29, 2020 1:32 AM, "Qu Wenruo" <wqu@suse.com> wrote:

> There are cases where v1 free space cache is still left while user has
> already enabled v2 cache.
> 
> In that case, we still want to force v1 space cache cleanup in
> btrfs-check.
> 
> This patch will remove the v2 check if we're cleaning up v1 cache,
> allowing us to cleanup the leftover.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> check/main.c | 6 ------
> 1 file changed, 6 deletions(-)
> 
> diff --git a/check/main.c b/check/main.c
> index 8ad7f5886f06..f4755d260bfe 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -9917,12 +9917,6 @@ static int do_clear_free_space_cache(int clear_version)
> int ret = 0;
> 
> if (clear_version == 1) {
> - if (btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE)) {
> - error(
> - "free space cache v2 detected, use --clear-space-cache v2");
> - ret = 1;
> - goto close_out;
> - }
> ret = clear_free_space_cache();
> if (ret) {
> error("failed to clear free space cache");
> -- 
> 2.29.2


Maybe we should keep the message but make it not fatal?
Something like "free space cache v2 detected, use --clear-space-cache v2 to clear it. Proceeding in clearing potential v1 leftovers as asked..."?

Regards,

Stéphane.

Re: [PATCH] btrfs-progs: check: allow force v1 space cache cleanup even the fs has v2 space cache enabled

[David Sterba]

On Tue, Dec 29, 2020 at 09:34:51AM +0000, Stéphane Lesimple wrote:
> December 29, 2020 1:32 AM, "Qu Wenruo" <wqu@suse.com> wrote:
> 
> > There are cases where v1 free space cache is still left while user has
> > already enabled v2 cache.
> > 
> > In that case, we still want to force v1 space cache cleanup in
> > btrfs-check.
> > 
> > This patch will remove the v2 check if we're cleaning up v1 cache,
> > allowing us to cleanup the leftover.
> > 
> > Signed-off-by: Qu Wenruo <wqu@suse.com>
> > ---
> > check/main.c | 6 ------
> > 1 file changed, 6 deletions(-)
> > 
> > diff --git a/check/main.c b/check/main.c
> > index 8ad7f5886f06..f4755d260bfe 100644
> > --- a/check/main.c
> > +++ b/check/main.c
> > @@ -9917,12 +9917,6 @@ static int do_clear_free_space_cache(int clear_version)
> > int ret = 0;
> > 
> > if (clear_version == 1) {
> > - if (btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE)) {
> > - error(
> > - "free space cache v2 detected, use --clear-space-cache v2");
> > - ret = 1;
> > - goto close_out;
> > - }
> > ret = clear_free_space_cache();
> > if (ret) {
> > error("failed to clear free space cache");
> > -- 
> > 2.29.2
> 
> 
> Maybe we should keep the message but make it not fatal?
> Something like "free space cache v2 detected, use --clear-space-cache
> v2 to clear it. Proceeding in clearing potential v1 leftovers as
> asked..."?

This sounds like a good option, thanks. I'll update the patch when I
commit it.