[PATCH 1/2] btrfs-progs: qgroup: move btrfs_show_qgroups's error handler to __qgroup_search

[PATCH 1/2] btrfs-progs: qgroup: move btrfs_show_qgroups's error handler to __qgroup_search

[Lu Fengqi]

We have to process the return value of BTRFS_IOC_TREE_SEARCH ioctl in
advance, so that we can distinguish between the two case where quota
is not enabled (ioctl return -ENOENT) and either parent qgroup or child
qgroup does not exist (update_qgroup_relation return -ENOENT). Besides
this, any error in this routine has been reported, so we don't need to
report again in cmd_qgroup_show.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 cmds-qgroup.c |  4 ----
 qgroup.c      | 14 ++++++++++++--
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 38382ea9..d07bb0c0 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -399,10 +399,6 @@ static int cmd_qgroup_show(int argc, char **argv)
 					qgroupid);
 	}
 	ret = btrfs_show_qgroups(fd, filter_set, comparer_set);
-	if (ret == -ENOENT)
-		error("can't list qgroups: quotas not enabled");
-	else if (ret < 0)
-		error("can't list qgroups: %s", strerror(-ret));
 	close_file_or_dir(fd, dirstream);
 
 out:
diff --git a/qgroup.c b/qgroup.c
index 156825fd..23463d8a 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -1065,8 +1065,18 @@ static int __qgroups_search(int fd, struct qgroup_lookup *qgroup_lookup)
 
 	while (1) {
 		ret = ioctl(fd, BTRFS_IOC_TREE_SEARCH, &args);
-		if (ret < 0)
-			return -errno;
+		if (ret < 0) {
+			if (errno == ENOENT) {
+				error("can't list qgroups: quotas not enabled");
+				ret = -ENOTTY;
+			} else {
+				error("can't list qgroups: %s",
+				       strerror(errno));
+				ret = -errno;
+			}
+
+			break;
+		}
 
 		/* the ioctl returns the number of item it found in nr_items */
 		if (sk->nr_items == 0)
-- 
2.15.0

[PATCH v2 2/2] btrfs-progs: qgroup: cleanup __qgroup_search

[Lu Fengqi]

Replace the if statement with the switch statement, and return the
appropriate value for the future use rather than directly exit.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---

Changelog:
v2:
1. revoke incorrect goto pattern
2. split the error handler movement to another patch

 qgroup.c | 48 ++++++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/qgroup.c b/qgroup.c
index 23463d8a..b5b893f4 100644
--- a/qgroup.c
+++ b/qgroup.c
@@ -1046,8 +1046,10 @@ static int __qgroups_search(int fd, struct qgroup_lookup *qgroup_lookup)
 	struct btrfs_ioctl_search_header *sh;
 	unsigned long off = 0;
 	unsigned int i;
+	struct btrfs_qgroup_status_item *si;
 	struct btrfs_qgroup_info_item *info;
 	struct btrfs_qgroup_limit_item *limit;
+	u64 flags;
 	u64 qgroupid;
 	u64 qgroupid1;
 
@@ -1092,44 +1094,47 @@ static int __qgroups_search(int fd, struct qgroup_lookup *qgroup_lookup)
 								  off);
 			off += sizeof(*sh);
 
-			if (btrfs_search_header_type(sh)
-			    == BTRFS_QGROUP_STATUS_KEY) {
-				struct btrfs_qgroup_status_item *si;
-				u64 flags;
-
+			switch (btrfs_search_header_type(sh)) {
+			case BTRFS_QGROUP_STATUS_KEY:
 				si = (struct btrfs_qgroup_status_item *)
 				     (args.buf + off);
 				flags = btrfs_stack_qgroup_status_flags(si);
+
 				print_status_flag_warning(flags);
-			} else if (btrfs_search_header_type(sh)
-				   == BTRFS_QGROUP_INFO_KEY) {
+				break;
+			case BTRFS_QGROUP_INFO_KEY:
 				qgroupid = btrfs_search_header_offset(sh);
 				info = (struct btrfs_qgroup_info_item *)
 				       (args.buf + off);
 
-				update_qgroup_info(qgroup_lookup, qgroupid,
-						   info);
-			} else if (btrfs_search_header_type(sh)
-				   == BTRFS_QGROUP_LIMIT_KEY) {
+				ret = update_qgroup_info(qgroup_lookup,
+							 qgroupid, info);
+				break;
+			case BTRFS_QGROUP_LIMIT_KEY:
 				qgroupid = btrfs_search_header_offset(sh);
 				limit = (struct btrfs_qgroup_limit_item *)
 					(args.buf + off);
 
-				update_qgroup_limit(qgroup_lookup, qgroupid,
-						    limit);
-			} else if (btrfs_search_header_type(sh)
-				   == BTRFS_QGROUP_RELATION_KEY) {
+				ret = update_qgroup_limit(qgroup_lookup,
+							  qgroupid, limit);
+				break;
+			case BTRFS_QGROUP_RELATION_KEY:
 				qgroupid = btrfs_search_header_offset(sh);
 				qgroupid1 = btrfs_search_header_objectid(sh);
 
 				if (qgroupid < qgroupid1)
-					goto skip;
+					break;
+
+				ret = update_qgroup_relation(qgroup_lookup,
+							qgroupid, qgroupid1);
+				break;
+			default:
+				return ret;
+			}
+
+			if (ret)
+				return ret;
 
-				update_qgroup_relation(qgroup_lookup, qgroupid,
-						       qgroupid1);
-			} else
-				goto done;
-skip:
 			off += btrfs_search_header_len(sh);
 
 			/*
@@ -1151,7 +1156,6 @@ skip:
 			break;
 	}
 
-done:
 	return ret;
 }
 
-- 
2.15.0

Re: [PATCH 1/2] btrfs-progs: qgroup: move btrfs_show_qgroups's error handler to __qgroup_search

[David Sterba]

On Mon, Nov 13, 2017 at 01:33:15PM +0800, Lu Fengqi wrote:
> We have to process the return value of BTRFS_IOC_TREE_SEARCH ioctl in
> advance, so that we can distinguish between the two case where quota
> is not enabled (ioctl return -ENOENT) and either parent qgroup or child
> qgroup does not exist (update_qgroup_relation return -ENOENT). Besides
> this, any error in this routine has been reported, so we don't need to
> report again in cmd_qgroup_show.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

1 and 2 applied, thanks.