On 2018/08/08 1:23, Lu Fengqi wrote:
> On Mon, Aug 06, 2018 at 01:53:28PM +0900, Misono Tomohiro wrote:
>> When qgroup is on, subvolume deletion does not remove qgroup items
>> of the subvolume (qgroup info, limit, relation) from quota tree and
>> they need to get removed manually by "btrfs qgroup destroy".
>>
>> Since level 0 qgroup cannot be used/inherited by any other subvolume,
>> let's remove them automatically when subvolume is deleted
>> (to be precise, when the subvolume root is dropped).
>>
>> Reviewed-by: Lu Fengqi <lufq.fnst@xxxxxxxxxxxxxx>
>> Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
>> Signed-off-by: Misono Tomohiro <misono.tomohiro@xxxxxxxxxxxxxx>
>> ---
>> v2 -> v3:
>> Use root->root_key.objectid instead of root->objectid
>> Add Reviewed-by tag
>>
>> v1 -> v2:
>> Move call of btrfs_remove_qgroup() from btrfs_delete_subvolume()
>> to btrfs_snapshot_destroy() so that it will be called after the
>> subvolume root is really dropped
>>
>> fs/btrfs/extent-tree.c | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 9e7b237b9547..48edf839ed2c 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -8871,12 +8871,13 @@ int btrfs_drop_snapshot(struct btrfs_root *root,
>> struct btrfs_root_item *root_item = &root->root_item;
>> struct walk_control *wc;
>> struct btrfs_key key;
>> + u64 objectid = root->root_key.objectid;
>> int err = 0;
>> int ret;
>> int level;
>> bool root_dropped = false;
>>
>> - btrfs_debug(fs_info, "Drop subvolume %llu", root->objectid);
>> + btrfs_debug(fs_info, "Drop subvolume %llu", objectid);
>>
>> path = btrfs_alloc_path();
>> if (!path) {
>> @@ -9030,7 +9031,7 @@ int btrfs_drop_snapshot(struct btrfs_root *root,
>> goto out_end_trans;
>> }
>>
>> - if (root->root_key.objectid != BTRFS_TREE_RELOC_OBJECTID) {
>> + if (objectid != BTRFS_TREE_RELOC_OBJECTID) {
>> ret = btrfs_find_root(tree_root, &root->root_key, path,
>> NULL, NULL);
>> if (ret < 0) {
>> @@ -9043,8 +9044,7 @@ int btrfs_drop_snapshot(struct btrfs_root *root,
>> *
>> * The most common failure here is just -ENOENT.
>> */
>> - btrfs_del_orphan_item(trans, tree_root,
>> - root->root_key.objectid);
>> + btrfs_del_orphan_item(trans, tree_root, objectid);
>> }
>> }
>>
>> @@ -9056,6 +9056,14 @@ int btrfs_drop_snapshot(struct btrfs_root *root,
>> btrfs_put_fs_root(root);
>> }
>> root_dropped = true;
>> +
>> + /* Remove level-0 qgroup items since no other subvolume can use them */
>> + ret = btrfs_remove_qgroup(trans, objectid);
>> + if (ret && ret != -EINVAL && ret != -ENOENT) {
>
> I'm sorry for missing the snapshot case. If it is a snapshot, then when
> we remove the relevant qgroup, we will not be able to perform
> quick_update_accounting(), and it will return 1. So we shouldn't abort
> the transaction when the return value = 1.
>
> btrfs_remove_qgroup
> -> __del_qgroup_relation
> -> quick_update_accounting << if qgroup->excl != qgroup->rfer; return 1
>
oh, thanks for pointing out, I missed that case...
I wonder if there is a way to keep qgroup consistency without enforcing rescan.
btw, I noticed that removing qgroup relation when excl != rfer causes
some problem without this patch:
(4.18-rc7)
$ mkfs.btrfs -fq $DEV
$ mount $DEV /mnt
$ btrfs quota enable /mnt
$ btrfs qgroup create 1/0 /mnt
$ btrfs sub create /mnt/sub
$ btrfs qgroup assign 0/257 1/0 /mnt
$ dd if=/dev/urandom of=/mnt/sub/file bs=1k count=1000
$ btrfs sub snap /mnt/sub /mnt/snap
$ dd if=/dev/urandom of=/mnt/sub/file2 bs=1k count=1000
$ btrfs fi sync /mnt
$ btrfs qgroup show -pcre /mnt
qgroupid rfer excl max_rfer max_excl parent child
-------- ---- ---- -------- -------- ------ -----
0/5 16.00KiB 16.00KiB none none --- ---
0/257 1.97MiB 1016.00KiB none none 1/0 ---
0/258 1016.00KiB 16.00KiB none none --- ---
1/0 1.97MiB 1.97MiB none none --- 0/257
^^^^^^^ should be 1016Kib
$ btrfs qgroup remove 0/257 1/0 /mnt
WARNING: quotas may be inconsistent, rescan needed
$ btrfs qgroup show -pcre /mnt
WARNING: qgroup data inconsistent, rescan recommended
qgroupid rfer excl max_rfer max_excl parent child
-------- ---- ---- -------- -------- ------ -----
0/5 16.00KiB 16.00KiB none none --- ---
0/258 1016.00KiB 1016.00KiB none none --- ---
1/0 1.97MiB 1.97MiB none none --- ---
$ btrfs quota rescan -w /mnt
$ btrfs qgroup show -pcre /mnt
qgroupid rfer excl max_rfer max_excl parent child
-------- ---- ---- -------- -------- ------ -----
0/5 16.00KiB 16.00KiB none none --- ---
0/258 1016.00KiB 1016.00KiB none none --- ---
1/0 1.97MiB 1.97MiB none none --- ---
^^^^^^ ^^^^^^^ not changed, although inconsistency flag is cleared
I will look at the case to better understand qgroup.
Thanks,
Misono
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
