btrfs: check for subvolume before deleting squota qgroup

The invariant that we want to maintain with subvolume qgroups is that
the qgroup can only be deleted if there is no root. With squotas, we
thought that it was sufficient to just check the usage, because we
assumed that deleting a subvolume will drive it's qgroups usage to 0,
and thus 0 usage implies no subvolume.

However, this is false, for two reasons:

- A subvol whose extents are all from before squotas was enabled.
- A subvol that was created in this transaction and for which we have
  not yet run any delayed refs.

In both cases, deleting the qgroup breaks the desired invariant and we
are left with a subvolume with no qgroup but squotas are enabled.

Fix this by unifying the deletion check logic between full qgroups and
squotas. Squotas do all the same checks *and* the additional usage == 0
check, which is the one extra rule peculiar to squotas.

Link: https://lore.kernel.org/linux-btrfs/adnBhWfJQ1n3hZC8@merlins.org/
Fixes: a8df35619948 ("btrfs: forbid deleting live subvol qgroup")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index cdf736d3a4e5b353bfb94a71833d2f8fe981a2c8..86c036a089f6d90300a6b1208c5237752f93e779 100644 (file)
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1715,32 +1715,24 @@ out:
        return ret;
 }
 
-static bool can_delete_parent_qgroup(struct btrfs_qgroup *qgroup)
-
+static bool can_delete_parent_qgroup(struct btrfs_fs_info *fs_info, struct btrfs_qgroup *qgroup)
 {
        ASSERT(btrfs_qgroup_level(qgroup->qgroupid));
+       if (btrfs_qgroup_mode(fs_info) == BTRFS_QGROUP_MODE_SIMPLE)
+               squota_check_parent_usage(fs_info, qgroup);
        return list_empty(&qgroup->members);
 }
 
 /*
- * Return true if we can delete the squota qgroup and false otherwise.
- *
- * Rules for whether we can delete:
- *
- * A subvolume qgroup can be removed iff the subvolume is fully deleted, which
- * is iff there is 0 usage in the qgroup.
- *
- * A higher level qgroup can be removed iff it has no members.
- * Note: We audit its usage to warn on inconsitencies without blocking deletion.
+ * Because a shared extent can outlive its owning subvolume, we cannot delete a
+ * subvol squota qgroup until all of the extents it owns are gone, even if the
+ * subvolume itself has been deleted.
  */
-static bool can_delete_squota_qgroup(struct btrfs_fs_info *fs_info, struct btrfs_qgroup *qgroup)
+static bool can_delete_squota_subvol_qgroup(struct btrfs_fs_info *fs_info,
+                                           struct btrfs_qgroup *qgroup)
 {
        ASSERT(btrfs_qgroup_mode(fs_info) == BTRFS_QGROUP_MODE_SIMPLE);
-
-       if (btrfs_qgroup_level(qgroup->qgroupid) > 0) {
-               squota_check_parent_usage(fs_info, qgroup);
-               return can_delete_parent_qgroup(qgroup);
-       }
+       ASSERT(btrfs_qgroup_level(qgroup->qgroupid) == 0);
 
        return !(qgroup->rfer || qgroup->excl || qgroup->rfer_cmpr || qgroup->excl_cmpr);
 }
@@ -1754,14 +1746,11 @@ static int can_delete_qgroup(struct btrfs_fs_info *fs_info, struct btrfs_qgroup
 {
        struct btrfs_key key;
        BTRFS_PATH_AUTO_FREE(path);
-
-       /* Since squotas cannot be inconsistent, they have special rules for deletion. */
-       if (btrfs_qgroup_mode(fs_info) == BTRFS_QGROUP_MODE_SIMPLE)
-               return can_delete_squota_qgroup(fs_info, qgroup);
+       int ret;
 
        /* For higher level qgroup, we can only delete it if it has no child. */
        if (btrfs_qgroup_level(qgroup->qgroupid))
-               return can_delete_parent_qgroup(qgroup);
+               return can_delete_parent_qgroup(fs_info, qgroup);
 
        /*
         * For level-0 qgroups, we can only delete it if it has no subvolume
@@ -1777,10 +1766,21 @@ static int can_delete_qgroup(struct btrfs_fs_info *fs_info, struct btrfs_qgroup
                return -ENOMEM;
 
        /*
-        * The @ret from btrfs_find_root() exactly matches our definition for
-        * the return value, thus can be returned directly.
+        * Any subvol qgroup, regardless of mode, cannot be deleted if the
+        * subvol still exists.
+        */
+       ret = btrfs_find_root(fs_info->tree_root, &key, path, NULL, NULL);
+       /*
+        * btrfs_find_root returns <0 on error, 0 if found, and >0 if not,
+        * so the "found" and "error" cases match our desired return values.
         */
-       return btrfs_find_root(fs_info->tree_root, &key, path, NULL, NULL);
+       if (ret <= 0)
+               return ret;
+
+       /* Squotas require additional checks, even if the subvol is deleted. */
+       if (btrfs_qgroup_mode(fs_info) == BTRFS_QGROUP_MODE_SIMPLE)
+               return can_delete_squota_subvol_qgroup(fs_info, qgroup);
+       return 1;
 }
 
 int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)