Re: [PATCH 07/10] btrfs-progs: Add freespace tree as compat_ro supported feature

From: Nikolay Borisov <nborisov@suse.com>
To: Omar Sandoval <osandov@osandov.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 07/10] btrfs-progs: Add freespace tree as compat_ro supported feature
Date: Thu, 4 Oct 2018 21:36:18 +0300	[thread overview]
Message-ID: <80e859c3-fb8d-c9a5-3f14-359519816106@suse.com> (raw)
In-Reply-To: <20181004183008.GF25437@vader>

On  4.10.2018 21:30, Omar Sandoval wrote:
> On Mon, Oct 01, 2018 at 05:46:18PM +0300, Nikolay Borisov wrote:
>> The RO_FREE_SPACE_TREE(_VALID) flags are required in order to be able
>> to open an FST filesystem in repair mode. Add them to
>> BTRFS_FEATURE_COMPAT_RO_SUPP.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  ctree.h | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/ctree.h b/ctree.h
>> index a6d6c3decd87..3c396e7d293d 100644
>> --- a/ctree.h
>> +++ b/ctree.h
>> @@ -497,7 +497,9 @@ struct btrfs_super_block {
>>   * added here until read-write support for the free space tree is implemented in
>>   * btrfs-progs.
>>   */
> 
> This comment should go away.
> 
>> -#define BTRFS_FEATURE_COMPAT_RO_SUPP		0ULL
>> +#define BTRFS_FEATURE_COMPAT_RO_SUPP			\
>> +	(BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE |	\
>> +	 BTRFS_FEATURE_COMPAT_RO_FREE_SPACE_TREE_VALID)
>>  
>>  #define BTRFS_FEATURE_INCOMPAT_SUPP			\
>>  	(BTRFS_FEATURE_INCOMPAT_MIXED_BACKREF |		\
> 
> To repeat my question from before, did you test whether we can properly
> change the filesystem with, e.g., btrfstune or btrfs fi label? Given
> that some critical code was missing in the free space tree code, I'd be
> surprised if it worked correctly.
> 

Yes, it works. I manually tried changing the label of the filesystem
(though now I'm thinking whether I had v2 space cache enabled) and also
run all tests for progs. Will revise this however with explicit v2 fst.