Re: [PATCH v2 7/8] btrfs-progs: check: Implement removing received data for RW subvols

[Nikolay Borisov <nborisov@suse.com>]

On 14.09.21 г. 12:25, Qu Wenruo wrote:
> 
> 
> On 2021/9/14 下午5:05, Nikolay Borisov wrote:
>> Making a received subvolume RO should preclude doing an incremental send
>> of said subvolume since it can't be guaranteed that nothing changed in
>> the structure and this in turn has correctness implications for send.
>>
>> There is a pending kernel change that implements this behavior and
>> ensures that in the future RO volumes which are switched to RW can't be
>> used for incremental send. However, old kernels won't have that patch
>> backported. To ensure there is a supported way for users to put their
>> subvolumes in sane state let's implement the same functionality in
>> progs.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>   check/main.c        | 16 +++++++++++++++-
>>   check/mode-lowmem.c | 11 ++++++++++-
>>   2 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/check/main.c b/check/main.c
>> index 6369bdd90656..9d3822a2ebae 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -3544,6 +3544,7 @@ static int check_fs_root(struct btrfs_root *root,
>>       int ret = 0;
>>       int err = 0;
>>       bool generation_err = false;
>> +    bool rw_received_err = false;
>>       int wret;
>>       int level;
>>       u64 super_generation;
>> @@ -3658,6 +3659,19 @@ static int check_fs_root(struct btrfs_root *root,
>>                       sizeof(found_key)));
>>       }
>>
>> +    if (!((btrfs_root_flags(root_item) & BTRFS_ROOT_SUBVOL_RDONLY) ||
>> +            btrfs_is_empty_uuid(root_item->received_uuid))) {
>> +        error("Subvolume id: %llu is RW and has a received uuid",
>> +                root->root_key.objectid);
>> +        rw_received_err = true;
>> +        if (repair) {
>> +            ret = repair_received_subvol(root);
>> +            if (ret)
>> +                return ret;
>> +            rw_received_err = false;
>> +        }
>> +    }
>> +
>>       while (1) {
>>           ctx.item_count++;
>>           wret = walk_down_tree(root, &path, wc, &level, &nrefs);
>> @@ -3722,7 +3736,7 @@ static int check_fs_root(struct btrfs_root *root,
>>
>>       free_corrupt_blocks_tree(&corrupt_blocks);
>>       gfs_info->corrupt_blocks = NULL;
>> -    if (!ret && generation_err)
>> +    if (!ret && (generation_err ||  rw_received_err))
>>           ret = -1;
>>       return ret;
>>   }
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 323e66bc4cb1..d8f783bea424 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -5197,8 +5197,17 @@ static int check_btrfs_root(struct btrfs_root
>> *root, int check_all)
>>           ret = check_fs_first_inode(root);
>>           if (ret < 0)
>>               return FATAL_ERROR;
>> -    }
>>
>> +        if (!((btrfs_root_flags(root_item) &
>> BTRFS_ROOT_SUBVOL_RDONLY) ||
>> +                    btrfs_is_empty_uuid(root_item->received_uuid))) {
>> +            error("Subvolume id: %llu is RW and has a received uuid",
>> +                  root->root_key.objectid);
>> +            if (repair)
>> +                ret = repair_received_subvol(root);
>> +            if (ret < 0)
>> +                return FATAL_ERROR;
>> +        }
> 
> Not sure if we need to error out completely.
> 
> I guess continue the check would be better?

There's that, but we still need to find a way to signify there is an
error. So for lowmem mode the possibility is to introduce yet another
bit (from 26 to 27) to signify this particular problem. Ok, will fix it
in the next iteration but I'm still waiting for more feedback,
especially from David.
> 
> Despite that, everything looks good to me.
> 
> Thanks,
> Qu
>> +    }
>>
>>       level = btrfs_header_level(root->node);
>>       btrfs_init_path(&path);
>>
>