linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] btrfs qgroup cleanup
@ 2019-11-26  0:58 Marcos Paulo de Souza
  2019-11-26  0:58 ` [PATCH 1/2] btrfs: qgroup: Cleanup quota_root checks Marcos Paulo de Souza
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Marcos Paulo de Souza @ 2019-11-26  0:58 UTC (permalink / raw)
  To: linux-kernel; +Cc: dsterba, linux-btrfs, anand.jain, Marcos Paulo de Souza

From: Marcos Paulo de Souza <mpdesouza@suse.com>

While reading the qgroup code and doing some tests, I found some two things
that I would like to change:
* Remove some useless code that was being set only to check if
  fs_info->quota_root was not NULL
* Check why creating a qgroup was returning EINVAL

The answer to the second point was: EINVAL is returned when a user tries to
create/delete/list/assign a qgroup to a subvolume, but this subvolume does
not have quota enabled. Talking with David, he suggested to change it to
ENOTCONN, following what is currently being used in balance and scrub.

So here are the changes. Please take a look!

Marcos Paulo de Souza (2):
  btrfs: qgroup: Cleanup quota_root checks
  btrfs: qgroup: Return -ENOTCONN instead of -EINVAL

 fs/btrfs/qgroup.c | 44 +++++++++++++++-----------------------------
 1 file changed, 15 insertions(+), 29 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] btrfs: qgroup: Cleanup quota_root checks
  2019-11-26  0:58 [PATCH 0/2] btrfs qgroup cleanup Marcos Paulo de Souza
@ 2019-11-26  0:58 ` Marcos Paulo de Souza
  2019-11-26  2:47   ` Qu Wenruo
  2019-11-26  0:58 ` [PATCH 2/2] btrfs: qgroup: Return -ENOTCONN instead of -EINVAL Marcos Paulo de Souza
  2019-11-26 17:00 ` [PATCH 0/2] btrfs qgroup cleanup David Sterba
  2 siblings, 1 reply; 6+ messages in thread
From: Marcos Paulo de Souza @ 2019-11-26  0:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: dsterba, linux-btrfs, anand.jain, Marcos Paulo de Souza,
	Chris Mason, Josef Bacik

From: Marcos Paulo de Souza <mpdesouza@suse.com>

Remove some variables that are set only to be checked later, and never
used.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/qgroup.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index d4282e12f2a6..417fafb4b4f6 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1243,7 +1243,6 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
 			      u64 dst)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_root *quota_root;
 	struct btrfs_qgroup *parent;
 	struct btrfs_qgroup *member;
 	struct btrfs_qgroup_list *list;
@@ -1259,8 +1258,7 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
 		return -ENOMEM;
 
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
-	quota_root = fs_info->quota_root;
-	if (!quota_root) {
+	if (!fs_info->quota_root) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -1307,7 +1305,6 @@ static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
 				 u64 dst)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_root *quota_root;
 	struct btrfs_qgroup *parent;
 	struct btrfs_qgroup *member;
 	struct btrfs_qgroup_list *list;
@@ -1320,8 +1317,7 @@ static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
 	if (!tmp)
 		return -ENOMEM;
 
-	quota_root = fs_info->quota_root;
-	if (!quota_root) {
+	if (!fs_info->quota_root) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -1387,11 +1383,11 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 	int ret = 0;
 
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
-	quota_root = fs_info->quota_root;
-	if (!quota_root) {
+	if (!fs_info->quota_root) {
 		ret = -EINVAL;
 		goto out;
 	}
+	quota_root = fs_info->quota_root;
 	qgroup = find_qgroup_rb(fs_info, qgroupid);
 	if (qgroup) {
 		ret = -EEXIST;
@@ -1416,14 +1412,12 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_root *quota_root;
 	struct btrfs_qgroup *qgroup;
 	struct btrfs_qgroup_list *list;
 	int ret = 0;
 
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
-	quota_root = fs_info->quota_root;
-	if (!quota_root) {
+	if (!fs_info->quota_root) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -1465,7 +1459,6 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
 		       struct btrfs_qgroup_limit *limit)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_root *quota_root;
 	struct btrfs_qgroup *qgroup;
 	int ret = 0;
 	/* Sometimes we would want to clear the limit on this qgroup.
@@ -1475,8 +1468,7 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
 	const u64 CLEAR_VALUE = -1;
 
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
-	quota_root = fs_info->quota_root;
-	if (!quota_root) {
+	if (!fs_info->quota_root) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -2578,10 +2570,9 @@ int btrfs_qgroup_account_extents(struct btrfs_trans_handle *trans)
 int btrfs_run_qgroups(struct btrfs_trans_handle *trans)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_root *quota_root = fs_info->quota_root;
 	int ret = 0;
 
-	if (!quota_root)
+	if (!fs_info->quota_root)
 		return ret;
 
 	spin_lock(&fs_info->qgroup_lock);
@@ -2875,7 +2866,6 @@ static bool qgroup_check_limits(struct btrfs_fs_info *fs_info,
 static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce,
 			  enum btrfs_qgroup_rsv_type type)
 {
-	struct btrfs_root *quota_root;
 	struct btrfs_qgroup *qgroup;
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	u64 ref_root = root->root_key.objectid;
@@ -2894,8 +2884,7 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce,
 		enforce = false;
 
 	spin_lock(&fs_info->qgroup_lock);
-	quota_root = fs_info->quota_root;
-	if (!quota_root)
+	if (!fs_info->quota_root)
 		goto out;
 
 	qgroup = find_qgroup_rb(fs_info, ref_root);
@@ -2962,7 +2951,6 @@ void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
 			       u64 ref_root, u64 num_bytes,
 			       enum btrfs_qgroup_rsv_type type)
 {
-	struct btrfs_root *quota_root;
 	struct btrfs_qgroup *qgroup;
 	struct ulist_node *unode;
 	struct ulist_iterator uiter;
@@ -2980,8 +2968,7 @@ void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
 	}
 	spin_lock(&fs_info->qgroup_lock);
 
-	quota_root = fs_info->quota_root;
-	if (!quota_root)
+	if (!fs_info->quota_root)
 		goto out;
 
 	qgroup = find_qgroup_rb(fs_info, ref_root);
@@ -3681,7 +3668,6 @@ void __btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes,
 static void qgroup_convert_meta(struct btrfs_fs_info *fs_info, u64 ref_root,
 				int num_bytes)
 {
-	struct btrfs_root *quota_root = fs_info->quota_root;
 	struct btrfs_qgroup *qgroup;
 	struct ulist_node *unode;
 	struct ulist_iterator uiter;
@@ -3689,7 +3675,7 @@ static void qgroup_convert_meta(struct btrfs_fs_info *fs_info, u64 ref_root,
 
 	if (num_bytes == 0)
 		return;
-	if (!quota_root)
+	if (!fs_info->quota_root)
 		return;
 
 	spin_lock(&fs_info->qgroup_lock);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] btrfs: qgroup: Return -ENOTCONN instead of -EINVAL
  2019-11-26  0:58 [PATCH 0/2] btrfs qgroup cleanup Marcos Paulo de Souza
  2019-11-26  0:58 ` [PATCH 1/2] btrfs: qgroup: Cleanup quota_root checks Marcos Paulo de Souza
@ 2019-11-26  0:58 ` Marcos Paulo de Souza
  2019-11-26  2:50   ` Qu Wenruo
  2019-11-26 17:00 ` [PATCH 0/2] btrfs qgroup cleanup David Sterba
  2 siblings, 1 reply; 6+ messages in thread
From: Marcos Paulo de Souza @ 2019-11-26  0:58 UTC (permalink / raw)
  To: linux-kernel
  Cc: dsterba, linux-btrfs, anand.jain, Marcos Paulo de Souza,
	Chris Mason, Josef Bacik

From: Marcos Paulo de Souza <mpdesouza@suse.com>

[PROBLEM]
qgroup create/remove code is currently returning EINVAL when the user
tries to create a qgroup on a subvolume without quota enabled. EINVAL is
already being used for too many error scenarios so that is hard to depict
what is the problem.

[FIX]
Currently scrub and balance code return -ENOTCONN when the user tries to
cancel/pause and no scrub or balance is currently running for the desired
subvolume. Do the same here by returning -ENOTCONN  when a user
tries to create/delete/assing/list a qgroup on a subvolume without quota
enabled.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/qgroup.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 417fafb4b4f6..b046b04d7cce 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1259,7 +1259,7 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
 
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
 	if (!fs_info->quota_root) {
-		ret = -EINVAL;
+		ret = -ENOTCONN;
 		goto out;
 	}
 	member = find_qgroup_rb(fs_info, src);
@@ -1318,7 +1318,7 @@ static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
 		return -ENOMEM;
 
 	if (!fs_info->quota_root) {
-		ret = -EINVAL;
+		ret = -ENOTCONN;
 		goto out;
 	}
 
@@ -1384,7 +1384,7 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
 	if (!fs_info->quota_root) {
-		ret = -EINVAL;
+		ret = -ENOTCONN;
 		goto out;
 	}
 	quota_root = fs_info->quota_root;
@@ -1418,7 +1418,7 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
 
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
 	if (!fs_info->quota_root) {
-		ret = -EINVAL;
+		ret = -ENOTCONN;
 		goto out;
 	}
 
@@ -1469,7 +1469,7 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
 
 	mutex_lock(&fs_info->qgroup_ioctl_lock);
 	if (!fs_info->quota_root) {
-		ret = -EINVAL;
+		ret = -ENOTCONN;
 		goto out;
 	}
 
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] btrfs: qgroup: Cleanup quota_root checks
  2019-11-26  0:58 ` [PATCH 1/2] btrfs: qgroup: Cleanup quota_root checks Marcos Paulo de Souza
@ 2019-11-26  2:47   ` Qu Wenruo
  0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2019-11-26  2:47 UTC (permalink / raw)
  To: Marcos Paulo de Souza, linux-kernel
  Cc: dsterba, linux-btrfs, anand.jain, Marcos Paulo de Souza,
	Chris Mason, Josef Bacik


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



On 2019/11/26 上午8:58, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> Remove some variables that are set only to be checked later, and never
> used.

Indeed.

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

Thanks,
Qu
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>  fs/btrfs/qgroup.c | 34 ++++++++++------------------------
>  1 file changed, 10 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index d4282e12f2a6..417fafb4b4f6 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1243,7 +1243,6 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
>  			      u64 dst)
>  {
>  	struct btrfs_fs_info *fs_info = trans->fs_info;
> -	struct btrfs_root *quota_root;
>  	struct btrfs_qgroup *parent;
>  	struct btrfs_qgroup *member;
>  	struct btrfs_qgroup_list *list;
> @@ -1259,8 +1258,7 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
>  		return -ENOMEM;
>  
>  	mutex_lock(&fs_info->qgroup_ioctl_lock);
> -	quota_root = fs_info->quota_root;
> -	if (!quota_root) {
> +	if (!fs_info->quota_root) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> @@ -1307,7 +1305,6 @@ static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
>  				 u64 dst)
>  {
>  	struct btrfs_fs_info *fs_info = trans->fs_info;
> -	struct btrfs_root *quota_root;
>  	struct btrfs_qgroup *parent;
>  	struct btrfs_qgroup *member;
>  	struct btrfs_qgroup_list *list;
> @@ -1320,8 +1317,7 @@ static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
>  	if (!tmp)
>  		return -ENOMEM;
>  
> -	quota_root = fs_info->quota_root;
> -	if (!quota_root) {
> +	if (!fs_info->quota_root) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> @@ -1387,11 +1383,11 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
>  	int ret = 0;
>  
>  	mutex_lock(&fs_info->qgroup_ioctl_lock);
> -	quota_root = fs_info->quota_root;
> -	if (!quota_root) {
> +	if (!fs_info->quota_root) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> +	quota_root = fs_info->quota_root;
>  	qgroup = find_qgroup_rb(fs_info, qgroupid);
>  	if (qgroup) {
>  		ret = -EEXIST;
> @@ -1416,14 +1412,12 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
>  int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
>  {
>  	struct btrfs_fs_info *fs_info = trans->fs_info;
> -	struct btrfs_root *quota_root;
>  	struct btrfs_qgroup *qgroup;
>  	struct btrfs_qgroup_list *list;
>  	int ret = 0;
>  
>  	mutex_lock(&fs_info->qgroup_ioctl_lock);
> -	quota_root = fs_info->quota_root;
> -	if (!quota_root) {
> +	if (!fs_info->quota_root) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> @@ -1465,7 +1459,6 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
>  		       struct btrfs_qgroup_limit *limit)
>  {
>  	struct btrfs_fs_info *fs_info = trans->fs_info;
> -	struct btrfs_root *quota_root;
>  	struct btrfs_qgroup *qgroup;
>  	int ret = 0;
>  	/* Sometimes we would want to clear the limit on this qgroup.
> @@ -1475,8 +1468,7 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
>  	const u64 CLEAR_VALUE = -1;
>  
>  	mutex_lock(&fs_info->qgroup_ioctl_lock);
> -	quota_root = fs_info->quota_root;
> -	if (!quota_root) {
> +	if (!fs_info->quota_root) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
> @@ -2578,10 +2570,9 @@ int btrfs_qgroup_account_extents(struct btrfs_trans_handle *trans)
>  int btrfs_run_qgroups(struct btrfs_trans_handle *trans)
>  {
>  	struct btrfs_fs_info *fs_info = trans->fs_info;
> -	struct btrfs_root *quota_root = fs_info->quota_root;
>  	int ret = 0;
>  
> -	if (!quota_root)
> +	if (!fs_info->quota_root)
>  		return ret;
>  
>  	spin_lock(&fs_info->qgroup_lock);
> @@ -2875,7 +2866,6 @@ static bool qgroup_check_limits(struct btrfs_fs_info *fs_info,
>  static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce,
>  			  enum btrfs_qgroup_rsv_type type)
>  {
> -	struct btrfs_root *quota_root;
>  	struct btrfs_qgroup *qgroup;
>  	struct btrfs_fs_info *fs_info = root->fs_info;
>  	u64 ref_root = root->root_key.objectid;
> @@ -2894,8 +2884,7 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes, bool enforce,
>  		enforce = false;
>  
>  	spin_lock(&fs_info->qgroup_lock);
> -	quota_root = fs_info->quota_root;
> -	if (!quota_root)
> +	if (!fs_info->quota_root)
>  		goto out;
>  
>  	qgroup = find_qgroup_rb(fs_info, ref_root);
> @@ -2962,7 +2951,6 @@ void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
>  			       u64 ref_root, u64 num_bytes,
>  			       enum btrfs_qgroup_rsv_type type)
>  {
> -	struct btrfs_root *quota_root;
>  	struct btrfs_qgroup *qgroup;
>  	struct ulist_node *unode;
>  	struct ulist_iterator uiter;
> @@ -2980,8 +2968,7 @@ void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
>  	}
>  	spin_lock(&fs_info->qgroup_lock);
>  
> -	quota_root = fs_info->quota_root;
> -	if (!quota_root)
> +	if (!fs_info->quota_root)
>  		goto out;
>  
>  	qgroup = find_qgroup_rb(fs_info, ref_root);
> @@ -3681,7 +3668,6 @@ void __btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes,
>  static void qgroup_convert_meta(struct btrfs_fs_info *fs_info, u64 ref_root,
>  				int num_bytes)
>  {
> -	struct btrfs_root *quota_root = fs_info->quota_root;
>  	struct btrfs_qgroup *qgroup;
>  	struct ulist_node *unode;
>  	struct ulist_iterator uiter;
> @@ -3689,7 +3675,7 @@ static void qgroup_convert_meta(struct btrfs_fs_info *fs_info, u64 ref_root,
>  
>  	if (num_bytes == 0)
>  		return;
> -	if (!quota_root)
> +	if (!fs_info->quota_root)
>  		return;
>  
>  	spin_lock(&fs_info->qgroup_lock);
> 


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] btrfs: qgroup: Return -ENOTCONN instead of -EINVAL
  2019-11-26  0:58 ` [PATCH 2/2] btrfs: qgroup: Return -ENOTCONN instead of -EINVAL Marcos Paulo de Souza
@ 2019-11-26  2:50   ` Qu Wenruo
  0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2019-11-26  2:50 UTC (permalink / raw)
  To: Marcos Paulo de Souza, linux-kernel
  Cc: dsterba, linux-btrfs, anand.jain, Marcos Paulo de Souza,
	Chris Mason, Josef Bacik


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



On 2019/11/26 上午8:58, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> [PROBLEM]
> qgroup create/remove code is currently returning EINVAL when the user
> tries to create a qgroup on a subvolume without quota enabled. EINVAL is
> already being used for too many error scenarios so that is hard to depict
> what is the problem.
> 
> [FIX]
> Currently scrub and balance code return -ENOTCONN when the user tries to
> cancel/pause and no scrub or balance is currently running for the desired
> subvolume. Do the same here by returning -ENOTCONN  when a user
> tries to create/delete/assing/list a qgroup on a subvolume without quota
> enabled.

The generic error string for ENOTCONN is "Transport endpoint is not
connected", not something user can directly know.

So don't forget to modify btrfs-progs to interprete the error number to
"qgroup not enabled" error string.

Despite that, I think it looks good to me.

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

Thanks,
Qu
> 
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>  fs/btrfs/qgroup.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 417fafb4b4f6..b046b04d7cce 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1259,7 +1259,7 @@ int btrfs_add_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
>  
>  	mutex_lock(&fs_info->qgroup_ioctl_lock);
>  	if (!fs_info->quota_root) {
> -		ret = -EINVAL;
> +		ret = -ENOTCONN;
>  		goto out;
>  	}
>  	member = find_qgroup_rb(fs_info, src);
> @@ -1318,7 +1318,7 @@ static int __del_qgroup_relation(struct btrfs_trans_handle *trans, u64 src,
>  		return -ENOMEM;
>  
>  	if (!fs_info->quota_root) {
> -		ret = -EINVAL;
> +		ret = -ENOTCONN;
>  		goto out;
>  	}
>  
> @@ -1384,7 +1384,7 @@ int btrfs_create_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
>  
>  	mutex_lock(&fs_info->qgroup_ioctl_lock);
>  	if (!fs_info->quota_root) {
> -		ret = -EINVAL;
> +		ret = -ENOTCONN;
>  		goto out;
>  	}
>  	quota_root = fs_info->quota_root;
> @@ -1418,7 +1418,7 @@ int btrfs_remove_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid)
>  
>  	mutex_lock(&fs_info->qgroup_ioctl_lock);
>  	if (!fs_info->quota_root) {
> -		ret = -EINVAL;
> +		ret = -ENOTCONN;
>  		goto out;
>  	}
>  
> @@ -1469,7 +1469,7 @@ int btrfs_limit_qgroup(struct btrfs_trans_handle *trans, u64 qgroupid,
>  
>  	mutex_lock(&fs_info->qgroup_ioctl_lock);
>  	if (!fs_info->quota_root) {
> -		ret = -EINVAL;
> +		ret = -ENOTCONN;
>  		goto out;
>  	}
>  
> 


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] btrfs qgroup cleanup
  2019-11-26  0:58 [PATCH 0/2] btrfs qgroup cleanup Marcos Paulo de Souza
  2019-11-26  0:58 ` [PATCH 1/2] btrfs: qgroup: Cleanup quota_root checks Marcos Paulo de Souza
  2019-11-26  0:58 ` [PATCH 2/2] btrfs: qgroup: Return -ENOTCONN instead of -EINVAL Marcos Paulo de Souza
@ 2019-11-26 17:00 ` David Sterba
  2 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2019-11-26 17:00 UTC (permalink / raw)
  To: Marcos Paulo de Souza
  Cc: linux-kernel, dsterba, linux-btrfs, anand.jain, Marcos Paulo de Souza

On Mon, Nov 25, 2019 at 09:58:49PM -0300, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> While reading the qgroup code and doing some tests, I found some two things
> that I would like to change:
> * Remove some useless code that was being set only to check if
>   fs_info->quota_root was not NULL
> * Check why creating a qgroup was returning EINVAL
> 
> The answer to the second point was: EINVAL is returned when a user tries to
> create/delete/list/assign a qgroup to a subvolume, but this subvolume does
> not have quota enabled. Talking with David, he suggested to change it to
> ENOTCONN, following what is currently being used in balance and scrub.
> 
> So here are the changes. Please take a look!

Added to misc-next with minor subject edits, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-11-26 17:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-26  0:58 [PATCH 0/2] btrfs qgroup cleanup Marcos Paulo de Souza
2019-11-26  0:58 ` [PATCH 1/2] btrfs: qgroup: Cleanup quota_root checks Marcos Paulo de Souza
2019-11-26  2:47   ` Qu Wenruo
2019-11-26  0:58 ` [PATCH 2/2] btrfs: qgroup: Return -ENOTCONN instead of -EINVAL Marcos Paulo de Souza
2019-11-26  2:50   ` Qu Wenruo
2019-11-26 17:00 ` [PATCH 0/2] btrfs qgroup cleanup David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).