Re: [PATCH v2 05/18] btrfs: do not allow free space tree rebuild on extent tree v2

[David Sterba <dsterba@suse.cz>]

On Wed, Nov 15, 2023 at 05:49:48PM +0800, Anand Jain wrote:
> On 11/9/23 03:08, Josef Bacik wrote:
> > We currently don't allow these options to be set if we're extent tree v2
> > via the mount option parsing.  However when we switch to the new mount
> > API we'll no longer have the super block loaded, so won't be able to
> > make this distinction at mount option parsing time.  Address this by
> > checking for extent tree v2 at the point where we make the decision to
> > rebuild the free space tree.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> >   fs/btrfs/disk-io.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> > index b486cbec492b..072c45811c41 100644
> > --- a/fs/btrfs/disk-io.c
> > +++ b/fs/btrfs/disk-io.c
> > @@ -2951,7 +2951,8 @@ int btrfs_start_pre_rw_mount(struct btrfs_fs_info *fs_info)
> >   	bool rebuild_free_space_tree = false;
> >   
> >   	if (btrfs_test_opt(fs_info, CLEAR_CACHE) &&
> > -	    btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE)) {
> > +	    btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
> > +	    !btrfs_fs_incompat(fs_info, EXTENT_TREE_V2)) {
> >   		rebuild_free_space_tree = true;
> >   	} else if (btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE) &&
> >   		   !btrfs_fs_compat_ro(fs_info, FREE_SPACE_TREE_VALID)) {
> 
> If there is v3 you can consider to add a comment similar to that
> is in btrfs_parse_options().
> Also, IMO, it is a good idea to include a btrfs_info() statement
> to indicate that the clear_cache option is ignored.

Agreed, we have a lot of verbosity around the mount options, if some
option combination is invalid or not working as expected a message
should pe printed.