On 2021/5/19 上午12:06, Sidong Yang wrote:

On Sun, May 16, 2021 at 07:55:25AM +0800, Qu Wenruo wrote:

quoted

On 2021/5/15 下午9:35, David Sterba wrote:

quoted

On Sat, May 15, 2021 at 10:42:15AM +0800, Qu Wenruo wrote:

quoted

On 2021/5/15 上午10:36, Sidong Yang wrote:

quoted

This patch adds the options --delete-qgroup and --no-delete-qgroup. When
the option is enabled, delete subvolume command destroies associated
qgroup together. This patch make it as default option. Even though quota
is disabled, it enables quota temporary and restore it after.

No, this is not a good idea at all.

First thing first, if quota is disabled, all qgroup info including the
level 0 qgroups will also be deleted, thus no need to enable in the
first place.

Secondly, there is already a patch in the past to delete level 0 qgroups
in kernel space, which should be a much better solution.

I've filed the issue to do it in the userspace because it gives user
more control whether to do the deletion or not and to also cover all
kernels that won't get the patch (ie. old stable kernels).

Changing the default behaviour is always risky and has a potential to
break user scripts. IMO adding the option to progs and changing the
default there is safer in this case.

Then shouldn't it still go through ioctl options?

Doing it completely in user space doesn't seem correct to me.

Yes, It still use ioctl calls for destroying qgroup.

What I mean is to add new ioctl flags for the existing destory subvolume.

By that newer btrfs-progs can try to delete using the new flags,
If fails, go back to regular subovlume delete (without deleting qgroup)
and gives user a warning.

This still allows user to determine whether to delete qgroup along the
ioctl, and still keeps compatibility.

THanks,
Qu

I think this code has pros and cons.
IMHO, as david said, It also has some benefits when doing it in userspace for
old kernel versions. and give a chance to change softly their codes
which use btrfs-progs with options.

But when kernel supports this operation, maybe this code will always failed.
because the qgroup was already destroyed with it's subvolume in kernel.
and the code will be meaningless.

So in long run, I think it's better way to doing this in kernel.

quoted

Thanks,
Qu