Re: [PATCH v2 2/2] btrfs: automatically remove the subvolume qgroup

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/4/30 02:01, David Sterba 写道:
> On Mon, Apr 29, 2024 at 06:13:33AM -0700, Boris Burkov wrote:
>> I support the auto deletion in the kernel as you propose, I think it
>> just makes sense. Who wants stale, empty qgroups around that aren't
>> attached to any subvol? I suppose that with the drop_thresh thing, it is
>> possible some parent qgroup still reflects the usage until the next full
>> scan?
>
> The stale qgroups have been out for a long time so removing them after
> subvolume deletion is changing default behaviour, this always breaks
> somebody's scripts or tools.

If needed I can introduce a compat bit (the first one), to tell the
behavior difference.

And if we go the compat bit way, I believe it can be the first example
on how to do a kernel behavior change correctly without breaking any
user space assumption.

Thanks,
Qu
>
>> Thinking out loud -- for regular qgroups, we could avoid this all if we
>> do the reaping when usage goes to 0 and there is no subvol. So remove
>> the qgroup as a consequence of the rescan, not the subvol delete. I
>> imagine this is quite a bit messier, though :(
>>
>> We could also just not auto-reap if that condition occurs (inconsistent
>> qg with a parent), but I think that may be surprising for the same
>> reasons that got you working on this in the first place...
>>
>> Do we know of an explicit need to support --no-delete-qgroup? It feels
>> like it is perfectly normal for us to improve the default behavior of
>> the kernel or userspace tools without supporting the old behavior as a
>> flag forever (without a user).
>
> $ id=$(btrfs inspect rootid subvol)
> $ btrfs subvolume delete subvol
> $ btrfs qgroup remove 0/$id 1/1 .                   <---- fails
> $ btrfs qgroup destroy 0/$id .                      <---- fails
>
>> Put another way, I think it would be perfectly reasonable to term the
>> stale qgroups a leaked memory resource and this patch a bug fix, if we
>> are willing to get overly philosophical about it. We don't carry around
>> eternal flags for bug fixes, unless it's some rather exceptional case.
>
> The command line option does not do what I expected, if somebody would
> have to update the scripts to add it then we can do the kernel
> auto-remove and the document it. Eventually we can translate the -ENOENT
> error code to be ignored.
>
>> If we are on the hook for supporing that flag because we already added
>> it to progs and don't want to deprecate it, then maybe we can think of
>> something compatible with default auto-reap.