Re: [PATCH] btrfs: qgroup: Mark qgroup inconsistent if we're inherting snapshot to a new qgroup

[Josef Bacik <josef@toxicpanda.com>]

On 4/2/20 2:37 AM, Qu Wenruo wrote:
> [BUG]
> For the following operation, qgroup is guaranteed to be screwed up due
> to snapshot adding to a new qgroup:
> 
>    # mkfs.btrfs -f $dev
>    # mount $dev $mnt
>    # btrfs qgroup en $mnt
>    # btrfs subv create $mnt/src
>    # xfs_io -f -c "pwrite 0 1m" $mnt/src/file
>    # sync
>    # btrfs qgroup create 1/0 $mnt/src
>    # btrfs subv snapshot -i 1/0 $mnt/src $mnt/snapshot
>    # btrfs qgroup show -prce $mnt/src
>    qgroupid         rfer         excl     max_rfer     max_excl parent  child
>    --------         ----         ----     --------     -------- ------  -----
>    0/5          16.00KiB     16.00KiB         none         none ---     ---
>    0/257         1.02MiB     16.00KiB         none         none ---     ---
>    0/258         1.02MiB     16.00KiB         none         none 1/0     ---
>    1/0             0.00B        0.00B         none         none ---     0/258
> 	        ^^^^^^^^^^^^^^^^^^^^
> 

Can we get an xfstest for this?  When I go through qgroups in a few months I 
want to have as many regression tests as possible to rely on.

> [CAUSE]
> The problem is in btrfs_qgroup_inherit(), we don't have good enough
> check to determine if the new relation ship would break the existing
> accounting.
> 
> Unlike btrfs_add_qgroup_relation(), which has proper check to determine
> if we can do quick update without a rescan, in btrfs_qgroup_inherit() we
> can even assign a snapshot to multiple qgroups.
> 
> [FIX]
> Fix the problem by manually marking qgroup inconsistent for snapshot
> inheritance.
> 
> For subvolume creation, since all its extents are exclusively owned by
> itself, we don't need to rescan.
> 
> In theory, we should call relationship check like quick_update_accounting()
> when doing qgroup inheritance and inform user about qgroup inconsistent.
> 
> But we don't have good enough mechanism to inform user in the snapshot
> creation context, thus we can only silently mark the qgroup
> inconsistent.
> 
> Anyway, user shouldn't use qgroup inheritance during snapshot creation,
> and should add qgroup relationship after snapshot creation by 'btrfs
> qgroup assign', which has a much better UI to inform user about qgroup
> inconsistent and kick in rescan automatically.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef