[PATCH] btrfs: qgroup: Always remove all qgroup relation in btrfs_remove_qgroup()

[PATCH] btrfs: qgroup: Always remove all qgroup relation in btrfs_remove_qgroup()

[Misono Tomohiro]

In btrfs_remove_qgroup(), each qgroup relation is removed by calling
__del_qgroup_relation(). However, __del_qgroup_relation() returns 1
if deletion of qgroup relation causes inconsistency and current code
exits immediately  in that case.

Therefore if there are several qgroup relations and removing first
relation causes inconsistency, remaining items will not be removed.

Fix this by continuing to remove items if return value of
__del_qgroup_relation() is 1.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 fs/btrfs/qgroup.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 2ba29f0609d9..f18284253e77 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1428,12 +1428,21 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 		goto out;
 
 	while (!list_empty(&qgroup->groups)) {
+		int ret2;
+
 		list = list_first_entry(&qgroup->groups,
 					struct btrfs_qgroup_list, next_group);
-		ret = __del_qgroup_relation(trans, qgroupid,
+		ret2 = __del_qgroup_relation(trans, qgroupid,
 					    list->group->qgroupid);
-		if (ret)
-			goto out;
+		if (ret2) {
+			ret = ret2;
+			/*
+			 * __del_qgroup_relation() returns 1 if qgroup becomes
+			 * inconsistent. Continue to remove items in that case.
+			 */
+			if (ret != 1)
+				goto out;
+		}
 	}
 
 	spin_lock(&fs_info->qgroup_lock);
-- 
2.14.4

Re: [PATCH] btrfs: qgroup: Always remove all qgroup relation in btrfs_remove_qgroup()

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1766 bytes --]



On 8/9/18 12:10 PM, Misono Tomohiro wrote:
> In btrfs_remove_qgroup(), each qgroup relation is removed by calling
> __del_qgroup_relation(). However, __del_qgroup_relation() returns 1
> if deletion of qgroup relation causes inconsistency and current code
> exits immediately  in that case.
> 
> Therefore if there are several qgroup relations and removing first
> relation causes inconsistency, remaining items will not be removed.
> 
> Fix this by continuing to remove items if return value of
> __del_qgroup_relation() is 1.
> 
> Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>

Indeed we still need to remove all qgroup relationship even we will make
qgroup inconsistent.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  fs/btrfs/qgroup.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 2ba29f0609d9..f18284253e77 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1428,12 +1428,21 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
>  		goto out;
>  
>  	while (!list_empty(&qgroup->groups)) {
> +		int ret2;
> +
>  		list = list_first_entry(&qgroup->groups,
>  					struct btrfs_qgroup_list, next_group);
> -		ret = __del_qgroup_relation(trans, qgroupid,
> +		ret2 = __del_qgroup_relation(trans, qgroupid,
>  					    list->group->qgroupid);
> -		if (ret)
> -			goto out;
> +		if (ret2) {
> +			ret = ret2;
> +			/*
> +			 * __del_qgroup_relation() returns 1 if qgroup becomes
> +			 * inconsistent. Continue to remove items in that case.
> +			 */
> +			if (ret != 1)
> +				goto out;
> +		}
>  	}
>  
>  	spin_lock(&fs_info->qgroup_lock);
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]