Re: [PATCH] btrfs-progs: do not allow setting seed flag on fs with dirty log

[Anand Jain <anand.jain@oracle.com>]

On 4/15/22 19:37, Qu Wenruo wrote:
> [BUG]
> The following sequence operation can lead to a seed fs rejected by
> kernel:
> 
>   # Generate a fs with dirty log
>   mkfs.btrfs -f $file
>   mount $dev $mnt
>   xfs_io -f -c "pwrite 0 16k" -c fsync $mnt/file
>   cp $file $file.backup
>   umount $mnt
>   mv $file.backup $file
> 
>   # now $file has dirty log, set seed flag on it
>   btrfstune -S1 $file
> 
>   # mount will fail
>   mount $file $mnt
> 
> The mount failure with the following dmesg:
> 
> [  980.363667] loop0: detected capacity change from 0 to 262144
> [  980.371177] BTRFS info (device loop0): flagging fs with big metadata feature
> [  980.372229] BTRFS info (device loop0): using free space tree
> [  980.372639] BTRFS info (device loop0): has skinny extents
> [  980.375075] BTRFS info (device loop0): start tree-log replay
> [  980.375513] BTRFS warning (device loop0): log replay required on RO media
> [  980.381652] BTRFS error (device loop0): open_ctree failed
> 
> [CAUSE]
> Although btrfs will replay its dirty log even with RO mount, but kernel
> will treat seed device as RO device, and dirty log can not be replayed
> on RO device.
> 
> This rejection is already the better end, just imagine if we don't treat
> seed device as RO, and replayed the dirty log.
> The filesystem relying on the seed device will be completely screwed up.
> 
> [FIX]
> Just add extra check on log tree in btrfstune to reject setting seed
> flag on filesystems with dirty log.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

LGTM.

Reviewed-by: Anand Jain <anand.jain@oracle.com>

a small nit below.

> ---
>   btrfstune.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/btrfstune.c b/btrfstune.c
> index 33c83bf16291..7e4ad30a1cbd 100644
> --- a/btrfstune.c
> +++ b/btrfstune.c
> @@ -59,6 +59,10 @@ static int update_seeding_flag(struct btrfs_root *root, int set_flag)
>   						device);
>   			return 1;
>   		}
> +		if (btrfs_super_log_root(disk_super)) {
> +			error("this filesystem has dirty log, can not set seed flag");

Also, add a note on how to overcome dirty log. Mount / unmount?

Thanks, Anand

> +			return 1;
> +		}
>   		super_flags |= BTRFS_SUPER_FLAG_SEEDING;
>   	} else {
>   		if (!(super_flags & BTRFS_SUPER_FLAG_SEEDING)) {