[PATCH 00/20] fix magic return value in btrfs-progs

[PATCH 00/20] fix magic return value in btrfs-progs

[Wang Shilong]

This patchset tries to fix all the magic return value in btrfs-progs.
Most commands will have three kinds of return value:

0 success
1 usage of syntax errors

Exceptions come from balance/scrub/replace. For example, replace cancel
will return 2 if there is no operations in progress.

Some tools including(btrfsck,chunk-recover) since they are still under
development. Also we should update man page if these magic return values
have been corrected.

Any comments are welcome.

Notice: this patchset is based on David integration-20130902

Wang Shilong (20):
  Btrfs-progs: return 1 rather than 129 in usage()
  Btrfs-progs: fix magic return value in cmds-subvolume.c
  Btrfs-progs: fix magic return value in cmds-chunk.c
  Btrfs-progs: fix magic return value in cmds-dedup.c
  Btrfs-progs: fix magic return value in cmds-device.c
  Btrfs-progs: fix magic return value in cmds-filesystem.c
  Btrfs-progs: fix magic return value in cmds-inspect.c
  Btrfs-progs: fix magic return value in cmds-qgroup.c
  Btrfs-progs: fix magic return value in cmds-quota.c
  Btrfs-progs: fix magic return value in cmds-receive.c
  Btrfs-progs: fix magic return value in cmds-restore.c
  Btrfs-progs: fix magic return value in cmds-send.c
  Btrfs-progs: fix magic return value in btrfs-imgae.c
  Btrfs-progs: fix magic return value in btrfs-zero-log.c
  Btrfs-progs: fix magic return value in send-test.c
  Btrfs-progs: fix magic return value in dir-test.c
  Btrfs-progs: fix magic return value in random-test.c
  Btrfs-progs: fix magic return value in cmds-balance.c
  Btrfs-progs: fix magic return value in cmds-replace.c
  Btrfs-progs: fix magic return value in cmds-scrub.c

 btrfs-image.c     |  2 +-
 btrfs-zero-log.c  |  8 +++--
 cmds-balance.c    | 93 ++++++++++++++++++++++++++++++++++---------------------
 cmds-chunk.c      |  9 ++++--
 cmds-dedup.c      |  6 ++--
 cmds-device.c     | 24 ++++++--------
 cmds-filesystem.c | 28 ++++++++---------
 cmds-inspect.c    | 10 +++---
 cmds-qgroup.c     | 26 +++++++---------
 cmds-quota.c      | 10 +++---
 cmds-receive.c    |  4 +--
 cmds-replace.c    | 16 ++++++----
 cmds-restore.c    | 18 +++++------
 cmds-scrub.c      | 20 ++++++------
 cmds-send.c       |  2 +-
 cmds-subvolume.c  | 45 ++++++++++++---------------
 dir-test.c        | 16 +++++-----
 help.c            |  2 +-
 random-test.c     | 18 +++++------
 send-test.c       |  2 +-
 20 files changed, 189 insertions(+), 170 deletions(-)

-- 
1.7.11.7

[PATCH 01/20] Btrfs-progs: return 1 rather than 129 in usage()

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

if usage or syntax error happens, we return 1.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 help.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/help.c b/help.c
index d429a6b..09dc706 100644
--- a/help.c
+++ b/help.c
@@ -121,7 +121,7 @@ void usage_command(const struct cmd_struct *cmd, int full, int err)
 void usage(const char * const *usagestr)
 {
 	usage_command_usagestr(usagestr, NULL, 1, 1);
-	exit(129);
+	exit(1);
 }
 
 static void usage_command_group_internal(const struct cmd_group *grp, int full,
-- 
1.7.11.7

[PATCH 02/20] Btrfs-progs: fix magic return value in cmds-subvolume.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

The patch also fixes some coding styles problems.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-subvolume.c | 45 +++++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index e1fa81a..2c62492 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -218,14 +218,14 @@ again:
 	path = argv[cnt];
 
 	res = test_issubvolume(path);
-	if(res<0){
+	if (res < 0) {
 		fprintf(stderr, "ERROR: error accessing '%s'\n", path);
-		ret = 12;
+		ret = 1;
 		goto out;
 	}
-	if(!res){
+	if (!res) {
 		fprintf(stderr, "ERROR: '%s' is not a subvolume\n", path);
-		ret = 13;
+		ret = 1;
 		goto out;
 	}
 
@@ -236,11 +236,11 @@ again:
 	vname = basename(vname);
 	free(cpath);
 
-	if( !strcmp(vname,".") || !strcmp(vname,"..") ||
-	     strchr(vname, '/') ){
+	if (!strcmp(vname, ".") || !strcmp(vname, "..") ||
+	     strchr(vname, '/')) {
 		fprintf(stderr, "ERROR: incorrect subvolume name ('%s')\n",
 			vname);
-		ret = 14;
+		ret = 1;
 		goto out;
 	}
 
@@ -248,14 +248,14 @@ again:
 	if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
 		fprintf(stderr, "ERROR: snapshot name too long ('%s)\n",
 			vname);
-		ret = 14;
+		ret = 1;
 		goto out;
 	}
 
 	fd = open_file_or_dir(dname, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", dname);
-		ret = 12;
+		ret = 1;
 		goto out;
 	}
 
@@ -269,7 +269,7 @@ again:
 	if(res < 0 ){
 		fprintf( stderr, "ERROR: cannot delete '%s/%s' - %s\n",
 			dname, vname, strerror(e));
-		ret = 11;
+		ret = 1;
 		goto out;
 	}
 
@@ -471,8 +471,7 @@ out:
 		btrfs_list_free_comparer_set(comparer_set);
 	if (uerr)
 		usage(cmd_subvol_list_usage);
-
-	return ret;
+	return !!ret;
 }
 
 static const char * const cmd_snapshot_usage[] = {
@@ -694,9 +693,7 @@ static int cmd_subvol_get_default(int argc, char **argv)
 		btrfs_list_free_filter_set(filter_set);
 out:
 	close_file_or_dir(fd, dirstream);
-	if (ret)
-		return 1;
-	return 0;
+	return !!ret;
 }
 
 static const char * const cmd_subvol_set_default_usage[] = {
@@ -765,23 +762,21 @@ static int cmd_find_new(int argc, char **argv)
 	ret = test_issubvolume(subvol);
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: error accessing '%s'\n", subvol);
-		return 12;
+		return 1;
 	}
 	if (!ret) {
 		fprintf(stderr, "ERROR: '%s' is not a subvolume\n", subvol);
-		return 13;
+		return 1;
 	}
 
 	fd = open_file_or_dir(subvol, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", subvol);
-		return 12;
+		return 1;
 	}
 	ret = btrfs_list_find_updated_files(fd, 0, last_gen);
 	close_file_or_dir(fd, dirstream);
-	if (ret)
-		return 19;
-	return 0;
+	return !!ret;
 }
 
 static const char * const cmd_subvol_show_usage[] = {
@@ -800,7 +795,7 @@ static int cmd_subvol_show(int argc, char **argv)
 	char raw_prefix[] = "\t\t\t\t";
 	u64 sv_id, mntid;
 	int fd = -1, mntfd = -1;
-	int ret = -1;
+	int ret = 1;
 	DIR *dirstream1 = NULL, *dirstream2 = NULL;
 
 	if (check_argc_exact(argc, 2))
@@ -820,7 +815,6 @@ static int cmd_subvol_show(int argc, char **argv)
 	}
 	if (!ret) {
 		fprintf(stderr, "ERROR: '%s' is not a subvolume\n", fullpath);
-		ret = -1;
 		goto out;
 	}
 
@@ -830,7 +824,7 @@ static int cmd_subvol_show(int argc, char **argv)
 				"%s\n", fullpath, strerror(-ret));
 		goto out;
 	}
-	ret = -1;
+	ret = 1;
 	svpath = get_subvol_name(mnt, fullpath);
 
 	fd = open_file_or_dir(fullpath, &dirstream1);
@@ -935,8 +929,7 @@ out:
 		free(mnt);
 	if (fullpath)
 		free(fullpath);
-
-	return ret;
+	return !!ret;
 }
 
 const struct cmd_group subvolume_cmd_group = {
-- 
1.7.11.7

[PATCH 03/20] Btrfs-progs: fix magic return value in cmds-chunk.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-chunk.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/cmds-chunk.c b/cmds-chunk.c
index 54f0573..115db61 100644
--- a/cmds-chunk.c
+++ b/cmds-chunk.c
@@ -794,13 +794,15 @@ static int scan_devices(struct recover_control *rc)
 	int ret = 0;
 	int fd;
 	struct btrfs_device *dev;
+	int e;
 
 	list_for_each_entry(dev, &rc->fs_devices->devices, dev_list) {
 		fd = open(dev->name, O_RDONLY);
 		if (fd < 0) {
+			e = errno;
 			fprintf(stderr, "Failed to open device %s\n",
 				dev->name);
-			return -1;
+			return -e;
 		}
 		ret = scan_one_device(rc, fd, dev);
 		close(fd);
@@ -1785,7 +1787,7 @@ int cmd_chunk_recover(int argc, char *argv[])
 	ret = check_mounted(file);
 	if (ret) {
 		fprintf(stderr, "the device is busy\n");
-		return ret;
+		goto out;
 	}
 
 	ret = btrfs_recover_chunk_tree(file, verbose, yes);
@@ -1797,5 +1799,6 @@ int cmd_chunk_recover(int argc, char *argv[])
 	} else {
 		fprintf(stdout, "Fail to recover the chunk tree.\n");
 	}
-	return ret;
+out:
+	return !!ret;
 }
-- 
1.7.11.7

[PATCH 04/20] Btrfs-progs: fix magic return value in cmds-dedup.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-dedup.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cmds-dedup.c b/cmds-dedup.c
index eb43414..674952a 100644
--- a/cmds-dedup.c
+++ b/cmds-dedup.c
@@ -39,7 +39,7 @@ int dedup_ctl(int cmd, int argc, char **argv)
 	DIR *dirstream;
 
 	if (check_argc_exact(argc, 2))
-		return -1;
+		return -EINVAL;
 
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
@@ -71,7 +71,7 @@ static int cmd_dedup_enable(int argc, char **argv)
 	int ret = dedup_ctl(BTRFS_DEDUP_CTL_REG, argc, argv);
 	if (ret < 0)
 		usage(cmd_dedup_enable_usage);
-	return ret;
+	return !!ret;
 }
 
 static const char * const cmd_dedup_disable_usage[] = {
@@ -85,7 +85,7 @@ static int cmd_dedup_disable(int argc, char **argv)
 	int ret = dedup_ctl(BTRFS_DEDUP_CTL_UNREG, argc, argv);
 	if (ret < 0)
 		usage(cmd_dedup_disable_usage);
-	return ret;
+	return !!ret;
 }
 
 const struct cmd_group dedup_cmd_group = {
-- 
1.7.11.7

[PATCH 05/20] Btrfs-progs: fix magic return value in cmds-device.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-device.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 7524d08..800a050 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -55,7 +55,7 @@ static int cmd_add_dev(int argc, char **argv)
 	fdmnt = open_file_or_dir(mntpnt, &dirstream);
 	if (fdmnt < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", mntpnt);
-		return 12;
+		return 1;
 	}
 
 	for (i = 1; i < argc - 1; i++ ){
@@ -120,10 +120,7 @@ static int cmd_add_dev(int argc, char **argv)
 	}
 
 	close_file_or_dir(fdmnt, dirstream);
-	if (ret)
-		return ret+20;
-	else
-		return 0;
+	return !!ret;
 }
 
 static const char * const cmd_rm_dev_usage[] = {
@@ -146,7 +143,7 @@ static int cmd_rm_dev(int argc, char **argv)
 	fdmnt = open_file_or_dir(mntpnt, &dirstream);
 	if (fdmnt < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", mntpnt);
-		return 12;
+		return 1;
 	}
 
 	for(i=1 ; i < argc - 1; i++ ){
@@ -170,10 +167,7 @@ static int cmd_rm_dev(int argc, char **argv)
 	}
 
 	close_file_or_dir(fdmnt, dirstream);
-	if( ret)
-		return ret+20;
-	else
-		return 0;
+	return !!ret;
 }
 
 static const char * const cmd_scan_dev_usage[] = {
@@ -202,7 +196,7 @@ static int cmd_scan_dev(int argc, char **argv)
 		ret = scan_for_btrfs(where, 1);
 		if (ret){
 			fprintf(stderr, "ERROR: error %d while scanning\n", ret);
-			return 18;
+			return 1;
 		}
 		return 0;
 	}
@@ -210,7 +204,7 @@ static int cmd_scan_dev(int argc, char **argv)
 	fd = open("/dev/btrfs-control", O_RDWR);
 	if (fd < 0) {
 		perror("failed to open /dev/btrfs-control");
-		return 10;
+		return 1;
 	}
 
 	for( i = devstart ; i < argc ; i++ ){
@@ -232,7 +226,7 @@ static int cmd_scan_dev(int argc, char **argv)
 			close(fd);
 			fprintf(stderr, "ERROR: unable to scan the device '%s' - %s\n",
 				argv[i], strerror(e));
-			return 11;
+			return 1;
 		}
 	}
 
@@ -258,7 +252,7 @@ static int cmd_ready_dev(int argc, char **argv)
 	fd = open("/dev/btrfs-control", O_RDWR);
 	if (fd < 0) {
 		perror("failed to open /dev/btrfs-control");
-		return 10;
+		return 1;
 	}
 
 	strncpy(args.name, argv[argc - 1], BTRFS_PATH_NAME_MAX);
@@ -320,7 +314,7 @@ static int cmd_dev_stats(int argc, char **argv)
 
 	if (fdmnt < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", dev_path);
-		return 12;
+		return 1;
 	}
 
 	ret = get_fs_info(dev_path, &fi_args, &di_args);
-- 
1.7.11.7

[PATCH 06/20] Btrfs-progs: fix magic return value in cmds-filesystem.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-filesystem.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index ca0855b..815d59a 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -62,12 +62,14 @@ static int cmd_df(int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	sargs_orig = sargs = malloc(sizeof(struct btrfs_ioctl_space_args));
-	if (!sargs)
-		return -ENOMEM;
+	if (!sargs) {
+		ret = -ENOMEM;
+		goto out;
+	}
 
 	sargs->space_slots = 0;
 	sargs->total_spaces = 0;
@@ -157,7 +159,7 @@ out:
 	close_file_or_dir(fd, dirstream);
 	free(sargs);
 
-	return 0;
+	return !!ret;
 }
 
 static int uuid_search(struct btrfs_fs_devices *fs_devices, char *search)
@@ -248,7 +250,7 @@ static int cmd_show(int argc, char **argv)
 
 	if (ret){
 		fprintf(stderr, "ERROR: error %d while scanning\n", ret);
-		return 18;
+		return 1;
 	}
 	
 	if(searchstart < argc)
@@ -286,7 +288,7 @@ static int cmd_sync(int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	printf("FSSync '%s'\n", path);
@@ -296,7 +298,7 @@ static int cmd_sync(int argc, char **argv)
 	if( res < 0 ){
 		fprintf(stderr, "ERROR: unable to fs-syncing '%s' - %s\n", 
 			path, strerror(e));
-		return 16;
+		return 1;
 	}
 
 	return 0;
@@ -429,12 +431,10 @@ static int cmd_defrag(int argc, char **argv)
 	}
 	if (verbose)
 		printf("%s\n", BTRFS_BUILD_VERSION);
-	if (errors) {
+	if (errors)
 		fprintf(stderr, "total %d failures\n", errors);
-		exit(1);
-	}
 
-	return errors;
+	return !!errors;
 }
 
 static const char * const cmd_resize_usage[] = {
@@ -462,13 +462,13 @@ static int cmd_resize(int argc, char **argv)
 	if (len == 0 || len >= BTRFS_VOL_NAME_MAX) {
 		fprintf(stderr, "ERROR: size value too long ('%s)\n",
 			amount);
-		return 14;
+		return 1;
 	}
 
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	printf("Resize '%s' of '%s'\n", path, amount);
@@ -479,7 +479,7 @@ static int cmd_resize(int argc, char **argv)
 	if( res < 0 ){
 		fprintf(stderr, "ERROR: unable to resize '%s' - %s\n", 
 			path, strerror(e));
-		return 30;
+		return 1;
 	}
 	return 0;
 }
-- 
1.7.11.7

[PATCH 07/20] Btrfs-progs: fix magic return value in cmds-inspect.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-inspect.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/cmds-inspect.c b/cmds-inspect.c
index 9101470..bdebf7d 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -44,7 +44,7 @@ static int __ino_to_path_fd(u64 inum, int fd, int verbose, const char *prepend)
 
 	fspath = malloc(4096);
 	if (!fspath)
-		return 1;
+		return -ENOMEM;
 
 	memset(fspath, 0, sizeof(*fspath));
 	ipa.inum = inum;
@@ -78,7 +78,7 @@ static int __ino_to_path_fd(u64 inum, int fd, int verbose, const char *prepend)
 
 out:
 	free(fspath);
-	return ret;
+	return !!ret;
 }
 
 static const char * const cmd_inode_resolve_usage[] = {
@@ -117,13 +117,13 @@ static int cmd_inode_resolve(int argc, char **argv)
 	fd = open_file_or_dir(argv[optind+1], &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", argv[optind+1]);
-		return 12;
+		return 1;
 	}
 
 	ret = __ino_to_path_fd(atoll(argv[optind]), fd, verbose,
 			       argv[optind+1]);
 	close_file_or_dir(fd, dirstream);
-	return ret;
+	return !!ret;
 
 }
 
@@ -256,7 +256,7 @@ static int cmd_logical_resolve(int argc, char **argv)
 out:
 	close_file_or_dir(fd, dirstream);
 	free(inodes);
-	return ret;
+	return !!ret;
 }
 
 static const char * const cmd_subvolid_resolve_usage[] = {
-- 
1.7.11.7

[PATCH 08/20] Btrfs-progs: fix magic return value in cmds-qgroup.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-qgroup.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 6fa4c17..ff2a1fa 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -54,12 +54,12 @@ static int qgroup_assign(int assign, int argc, char **argv)
 	 */
 	if ((args.src >> 48) >= (args.dst >> 48)) {
 		fprintf(stderr, "ERROR: bad relation requested '%s'\n", path);
-		return 12;
+		return 1;
 	}
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = ioctl(fd, BTRFS_IOC_QGROUP_ASSIGN, &args);
@@ -68,7 +68,7 @@ static int qgroup_assign(int assign, int argc, char **argv)
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: unable to assign quota group: %s\n",
 			strerror(e));
-		return 30;
+		return 1;
 	}
 	return 0;
 }
@@ -92,7 +92,7 @@ static int qgroup_create(int create, int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = ioctl(fd, BTRFS_IOC_QGROUP_CREATE, &args);
@@ -101,7 +101,7 @@ static int qgroup_create(int create, int argc, char **argv)
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: unable to create quota group: %s\n",
 			strerror(e));
-		return 30;
+		return 1;
 	}
 	return 0;
 }
@@ -310,19 +310,17 @@ static int cmd_qgroup_show(int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = list_qgroups(fd);
 	e = errno;
 	close_file_or_dir(fd, dirstream);
-	if (ret < 0) {
+	if (ret < 0)
 		fprintf(stderr, "ERROR: can't list qgroups: %s\n",
 				strerror(e));
-		return 30;
-	}
 
-	return ret;
+	return !!ret;
 }
 
 static const char * const cmd_qgroup_limit_usage[] = {
@@ -392,12 +390,12 @@ static int cmd_qgroup_limit(int argc, char **argv)
 		ret = test_issubvolume(path);
 		if (ret < 0) {
 			fprintf(stderr, "ERROR: error accessing '%s'\n", path);
-			return 12;
+			return 1;
 		}
 		if (!ret) {
 			fprintf(stderr, "ERROR: '%s' is not a subvolume\n",
 				path);
-			return 13;
+			return 1;
 		}
 		/*
 		 * keep qgroupid at 0, this indicates that the subvolume the
@@ -412,7 +410,7 @@ static int cmd_qgroup_limit(int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = ioctl(fd, BTRFS_IOC_QGROUP_LIMIT, &args);
@@ -421,7 +419,7 @@ static int cmd_qgroup_limit(int argc, char **argv)
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: unable to limit requested quota group: "
 			"%s\n", strerror(e));
-		return 30;
+		return 1;
 	}
 	return 0;
 }
-- 
1.7.11.7

[PATCH 09/20] Btrfs-progs: fix magic return value in cmds-quota.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-quota.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/cmds-quota.c b/cmds-quota.c
index 94e3a5c..89cc89c 100644
--- a/cmds-quota.c
+++ b/cmds-quota.c
@@ -48,7 +48,7 @@ static int quota_ctl(int cmd, int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = ioctl(fd, BTRFS_IOC_QUOTA_CTL, &args);
@@ -57,7 +57,7 @@ static int quota_ctl(int cmd, int argc, char **argv)
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: quota command failed: %s\n",
 			strerror(e));
-		return 30;
+		return 1;
 	}
 	return 0;
 }
@@ -132,7 +132,7 @@ static int cmd_quota_rescan(int argc, char **argv)
 
 	if (ioctlnum != BTRFS_IOC_QUOTA_RESCAN && wait_for_completion) {
 		fprintf(stderr, "ERROR: -w cannot be used with -s\n");
-		return 12;
+		return 1;
 	}
 
 	if (check_argc_exact(argc - optind, 1))
@@ -144,7 +144,7 @@ static int cmd_quota_rescan(int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = ioctl(fd, ioctlnum, &args);
@@ -160,7 +160,7 @@ static int cmd_quota_rescan(int argc, char **argv)
 		if (ret < 0) {
 			fprintf(stderr, "ERROR: quota rescan failed: "
 				"%s\n", strerror(e));
-			return 30;
+			return 1;
 		}  else {
 			printf("quota rescan started\n");
 		}
-- 
1.7.11.7

[PATCH 10/20] Btrfs-progs: fix magic return value in cmds-receive.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-receive.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmds-receive.c b/cmds-receive.c
index 1630f64..2b880c0 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -951,13 +951,13 @@ int cmd_receive(int argc, char **argv)
 		receive_fd = open(fromfile, O_RDONLY | O_NOATIME);
 		if (receive_fd < 0) {
 			fprintf(stderr, "ERROR: failed to open %s\n", fromfile);
-			return -errno;
+			return 1;
 		}
 	}
 
 	ret = do_receive(&r, tomnt, receive_fd);
 
-	return ret;
+	return !!ret;
 }
 
 const char * const cmd_receive_usage[] = {
-- 
1.7.11.7

[PATCH 11/20] Btrfs-progs: fix magic return value in cmds-restore.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-restore.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 4c87483..04a8382 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -247,7 +247,7 @@ static int copy_one_inline(int fd, struct btrfs_path *path, u64 pos)
 	outbuf = malloc(ram_size);
 	if (!outbuf) {
 		fprintf(stderr, "No memory\n");
-		return -1;
+		return -ENOMEM;
 	}
 
 	ret = decompress(buf, outbuf, len, &ram_size, compress);
@@ -305,7 +305,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 	inbuf = malloc(disk_size);
 	if (!inbuf) {
 		fprintf(stderr, "No memory\n");
-		return -1;
+		return -ENOMEM;
 	}
 
 	if (compress != BTRFS_COMPRESS_NONE) {
@@ -313,7 +313,7 @@ static int copy_one_extent(struct btrfs_root *root, int fd,
 		if (!outbuf) {
 			fprintf(stderr, "No memory\n");
 			free(inbuf);
-			return -1;
+			return -ENOMEM;
 		}
 	}
 again:
@@ -543,7 +543,7 @@ static int copy_file(struct btrfs_root *root, int fd, struct btrfs_key *key,
 	path = btrfs_alloc_path();
 	if (!path) {
 		fprintf(stderr, "Ran out of memory\n");
-		return -1;
+		return -ENOMEM;
 	}
 	path->skip_locking = 1;
 
@@ -676,7 +676,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 	path = btrfs_alloc_path();
 	if (!path) {
 		fprintf(stderr, "Ran out of memory\n");
-		return -1;
+		return -ENOMEM;
 	}
 	path->skip_locking = 1;
 
@@ -823,7 +823,7 @@ static int search_dir(struct btrfs_root *root, struct btrfs_key *key,
 			if (!dir) {
 				fprintf(stderr, "Ran out of memory\n");
 				btrfs_free_path(path);
-				return -1;
+				return -ENOMEM;
 			}
 
 			if (location.type == BTRFS_ROOT_ITEM_KEY) {
@@ -917,7 +917,7 @@ static int do_list_roots(struct btrfs_root *root)
 	path = btrfs_alloc_path();
 	if (!path) {
 		fprintf(stderr, "Failed to alloc path\n");
-		return -1;
+		return -ENOMEM;
 	}
 
 	key.offset = 0;
@@ -1210,7 +1210,7 @@ int cmd_restore(int argc, char **argv)
 	if ((ret = check_mounted(argv[optind])) < 0) {
 		fprintf(stderr, "Could not check mount status: %s\n",
 			strerror(-ret));
-		return ret;
+		return 1;
 	} else if (ret) {
 		fprintf(stderr, "%s is currently mounted.  Aborting.\n", argv[optind]);
 		return 1;
@@ -1284,5 +1284,5 @@ out:
 	if (mreg)
 		regfree(mreg);
 	close_ctree(root);
-	return ret;
+	return !!ret;
 }
-- 
1.7.11.7

[PATCH 12/20] Btrfs-progs: fix magic return value in cmds-send.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

If btrfs send return failure, we return 1，otherwise 0 will be returned.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-send.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-send.c b/cmds-send.c
index f7f98bc..374d040 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -715,7 +715,7 @@ out:
 		close(send.mnt_fd);
 	free(send.root_path);
 	subvol_uuid_search_finit(&send.sus);
-	return ret;
+	return !!ret;
 }
 
 const char * const cmd_send_usage[] = {
-- 
1.7.11.7

[PATCH 13/20] Btrfs-progs: fix magic return value in btrfs-imgae.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 btrfs-image.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/btrfs-image.c b/btrfs-image.c
index 3ea3730..ab229f3 100644
--- a/btrfs-image.c
+++ b/btrfs-image.c
@@ -2588,5 +2588,5 @@ out:
 	else
 		fclose(out);
 
-	return ret;
+	return !!ret;
 }
-- 
1.7.11.7

[PATCH 14/20] Btrfs-progs: fix magic return value in btrfs-zero-log.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 btrfs-zero-log.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/btrfs-zero-log.c b/btrfs-zero-log.c
index f249aec..31ec215 100644
--- a/btrfs-zero-log.c
+++ b/btrfs-zero-log.c
@@ -52,10 +52,11 @@ int main(int ac, char **av)
 
 	if((ret = check_mounted(av[1])) < 0) {
 		fprintf(stderr, "Could not check mount status: %s\n", strerror(-ret));
-		return ret;
+		goto out;
 	} else if(ret) {
 		fprintf(stderr, "%s is currently mounted. Aborting.\n", av[1]);
-		return -EBUSY;
+		ret = -EBUSY;
+		goto out;
 	}
 
 	root = open_ctree(av[1], 0, 1);
@@ -68,5 +69,6 @@ int main(int ac, char **av)
 	btrfs_set_super_log_root_level(root->fs_info->super_copy, 0);
 	btrfs_commit_transaction(trans, root);
 	close_ctree(root);
-	return ret;
+out:
+	return !!ret;
 }
-- 
1.7.11.7

[PATCH 15/20] Btrfs-progs: fix magic return value in send-test.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 send-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/send-test.c b/send-test.c
index cb1f57d..3775f5f 100644
--- a/send-test.c
+++ b/send-test.c
@@ -454,5 +454,5 @@ int main(int argc, char **argv)
 
 	pthread_attr_destroy(&t_attr);
 out:
-	return ret;
+	return !!ret;
 }
-- 
1.7.11.7

[PATCH 16/20] Btrfs-progs: fix magic return value in dir-test.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 dir-test.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/dir-test.c b/dir-test.c
index d95219a..a54b777 100644
--- a/dir-test.c
+++ b/dir-test.c
@@ -140,7 +140,7 @@ fatal_release:
 	btrfs_release_path(&path);
 fatal:
 	printf("failed to insert %lu ret %d\n", oid, ret);
-	return -1;
+	return ret;
 }
 
 static int insert_dup(struct btrfs_trans_handle *trans, struct btrfs_root
@@ -213,7 +213,7 @@ out_release:
 	btrfs_release_path(path);
 out:
 	printf("failed to delete %lu %d\n", radix_index, ret);
-	return -1;
+	return ret;
 }
 
 static int del_one(struct btrfs_trans_handle *trans, struct btrfs_root *root,
@@ -241,7 +241,7 @@ static int del_one(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 out_release:
 	btrfs_release_path(&path);
 	printf("failed to delete %lu %d\n", oid, ret);
-	return -1;
+	return ret;
 }
 
 static int lookup_item(struct btrfs_trans_handle *trans, struct btrfs_root
@@ -269,7 +269,7 @@ static int lookup_item(struct btrfs_trans_handle *trans, struct btrfs_root
 	btrfs_release_path(&path);
 	if (ret) {
 		printf("unable to find key %lu\n", oid);
-		return -1;
+		return ret;
 	}
 	return 0;
 }
@@ -292,7 +292,7 @@ static int lookup_enoent(struct btrfs_trans_handle *trans, struct btrfs_root
 	btrfs_release_path(&path);
 	if (!ret) {
 		printf("able to find key that should not exist %lu\n", oid);
-		return -1;
+		return ret;
 	}
 	return 0;
 }
@@ -342,14 +342,14 @@ static int empty_tree(struct btrfs_trans_handle *trans, struct btrfs_root
 			fprintf(stderr,
 				"failed to remove %lu from tree\n",
 				found);
-			return -1;
+			return ret;
 		}
 		if (!keep_running)
 			break;
 	}
 	return 0;
 	fprintf(stderr, "failed to delete from the radix %lu\n", found);
-	return -1;
+	return ret;
 }
 
 static int fill_tree(struct btrfs_trans_handle *trans, struct btrfs_root *root,
@@ -512,6 +512,6 @@ int main(int ac, char **av)
 	}
 out:
 	close_ctree(root, &super);
-	return err;
+	return !!err;
 }
 
-- 
1.7.11.7

[PATCH 17/20] Btrfs-progs: fix magic return value in random-test.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 random-test.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/random-test.c b/random-test.c
index 2f44593..b7c6cdb 100644
--- a/random-test.c
+++ b/random-test.c
@@ -43,7 +43,7 @@ again:
 	ret = radix_tree_gang_lookup(root, (void **)res, num, 2);
 	if (exists) {
 		if (ret == 0)
-			return -1;
+			return -EEXIST;
 		num = res[0];
 	} else if (ret != 0 && num == res[0]) {
 		num++;
@@ -79,7 +79,7 @@ static int ins_one(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	return ret;
 error:
 	printf("failed to insert %llu\n", (unsigned long long)key.objectid);
-	return -1;
+	return ret;
 }
 
 static int insert_dup(struct btrfs_trans_handle *trans, struct btrfs_root
@@ -98,7 +98,7 @@ static int insert_dup(struct btrfs_trans_handle *trans, struct btrfs_root
 	if (ret != -EEXIST) {
 		printf("insert on %llu gave us %d\n",
 		       (unsigned long long)key.objectid, ret);
-		return 1;
+		return ret;
 	}
 	return 0;
 }
@@ -127,7 +127,7 @@ static int del_one(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 	return 0;
 error:
 	printf("failed to delete %llu\n", (unsigned long long)key.objectid);
-	return -1;
+	return ret;
 }
 
 static int lookup_item(struct btrfs_trans_handle *trans, struct btrfs_root
@@ -147,7 +147,7 @@ static int lookup_item(struct btrfs_trans_handle *trans, struct btrfs_root
 	return 0;
 error:
 	printf("unable to find key %llu\n", (unsigned long long)key.objectid);
-	return -1;
+	return ret;
 }
 
 static int lookup_enoent(struct btrfs_trans_handle *trans, struct btrfs_root
@@ -168,7 +168,7 @@ static int lookup_enoent(struct btrfs_trans_handle *trans, struct btrfs_root
 error:
 	printf("able to find key that should not exist %llu\n",
 	       (unsigned long long)key.objectid);
-	return -1;
+	return -EEXIST;
 }
 
 static int empty_tree(struct btrfs_trans_handle *trans, struct btrfs_root
@@ -209,7 +209,7 @@ static int empty_tree(struct btrfs_trans_handle *trans, struct btrfs_root
 			fprintf(stderr,
 				"failed to remove %lu from tree\n",
 				found);
-			return -1;
+			return ret;
 		}
 		btrfs_release_path(&path);
 		ptr = radix_tree_delete(radix, found);
@@ -221,7 +221,7 @@ static int empty_tree(struct btrfs_trans_handle *trans, struct btrfs_root
 	return 0;
 error:
 	fprintf(stderr, "failed to delete from the radix %lu\n", found);
-	return -1;
+	return -ENOENT;
 }
 
 static int fill_tree(struct btrfs_trans_handle *trans, struct btrfs_root *root,
@@ -428,6 +428,6 @@ int main(int ac, char **av)
 	}
 out:
 	close_ctree(root, &super);
-	return err;
+	return !!err;
 }
 
-- 
1.7.11.7

[PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

If there is no balance in progress.resume/pause/cancel will
return 2. For usage or syntal errors will return 1. 0 means
operations return successfully.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-balance.c | 93 ++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 58 insertions(+), 35 deletions(-)

diff --git a/cmds-balance.c b/cmds-balance.c
index b7382ef..fd68051 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -58,7 +58,7 @@ static int parse_one_profile(const char *profile, u64 *flags)
 		*flags |= BTRFS_AVAIL_ALLOC_BIT_SINGLE;
 	} else {
 		fprintf(stderr, "Unknown profile '%s'\n", profile);
-		return 1;
+		return -ENOENT;
 	}
 
 	return 0;
@@ -68,12 +68,14 @@ static int parse_profiles(char *profiles, u64 *flags)
 {
 	char *this_char;
 	char *save_ptr = NULL; /* Satisfy static checkers */
+	int ret;
 
 	for (this_char = strtok_r(profiles, "|", &save_ptr);
 	     this_char != NULL;
 	     this_char = strtok_r(NULL, "|", &save_ptr)) {
-		if (parse_one_profile(this_char, flags))
-			return 1;
+		ret = parse_one_profile(this_char, flags);
+		if (ret)
+			return ret;
 	}
 
 	return 0;
@@ -86,7 +88,7 @@ static int parse_u64(const char *str, u64 *result)
 
 	val = strtoull(str, &endptr, 10);
 	if (*endptr)
-		return 1;
+		return -EINVAL;
 
 	*result = val;
 	return 0;
@@ -95,6 +97,7 @@ static int parse_u64(const char *str, u64 *result)
 static int parse_range(const char *range, u64 *start, u64 *end)
 {
 	char *dots;
+	int ret;
 
 	dots = strstr(range, "..");
 	if (dots) {
@@ -107,29 +110,31 @@ static int parse_range(const char *range, u64 *start, u64 *end)
 			*end = (u64)-1;
 			skipped++;
 		} else {
-			if (parse_u64(rest, end))
-				return 1;
+			ret = parse_u64(rest, end);
+			if (ret)
+				return ret;
 		}
 		if (dots == range) {
 			*start = 0;
 			skipped++;
 		} else {
+			ret = parse_u64(rest, end);
 			if (parse_u64(range, start))
-				return 1;
+				return ret;
 		}
 
 		if (*start >= *end) {
 			fprintf(stderr, "Range %llu..%llu doesn't make "
 				"sense\n", (unsigned long long)*start,
 				(unsigned long long)*end);
-			return 1;
+			return -EINVAL;
 		}
 
 		if (skipped <= 1)
 			return 0;
 	}
 
-	return 1;
+	return -EINVAL;
 }
 
 static int parse_filters(char *filters, struct btrfs_balance_args *args)
@@ -137,6 +142,7 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
 	char *this_char;
 	char *value;
 	char *save_ptr = NULL; /* Satisfy static checkers */
+	int ret = 0;
 
 	if (!filters)
 		return 0;
@@ -150,70 +156,72 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
 			if (!value || !*value) {
 				fprintf(stderr, "the profiles filter requires "
 				       "an argument\n");
-				return 1;
+				return -EINVAL;
 			}
 			if (parse_profiles(value, &args->profiles)) {
 				fprintf(stderr, "Invalid profiles argument\n");
-				return 1;
+				return -EINVAL;
 			}
 			args->flags |= BTRFS_BALANCE_ARGS_PROFILES;
 		} else if (!strcmp(this_char, "usage")) {
 			if (!value || !*value) {
 				fprintf(stderr, "the usage filter requires "
 				       "an argument\n");
-				return 1;
+				return -EINVAL;
 			}
 			if (parse_u64(value, &args->usage) ||
 			    args->usage > 100) {
 				fprintf(stderr, "Invalid usage argument: %s\n",
 				       value);
-				return 1;
+				return -EINVAL;
 			}
 			args->flags |= BTRFS_BALANCE_ARGS_USAGE;
 		} else if (!strcmp(this_char, "devid")) {
 			if (!value || !*value) {
 				fprintf(stderr, "the devid filter requires "
 				       "an argument\n");
-				return 1;
+				return -EINVAL;
 			}
 			if (parse_u64(value, &args->devid) ||
 			    args->devid == 0) {
 				fprintf(stderr, "Invalid devid argument: %s\n",
 				       value);
-				return 1;
+				return -EINVAL;
 			}
 			args->flags |= BTRFS_BALANCE_ARGS_DEVID;
 		} else if (!strcmp(this_char, "drange")) {
 			if (!value || !*value) {
 				fprintf(stderr, "the drange filter requires "
 				       "an argument\n");
-				return 1;
+				return -EINVAL;
 			}
-			if (parse_range(value, &args->pstart, &args->pend)) {
+			ret = parse_range(value, &args->pstart, &args->pend);
+			if (ret) {
 				fprintf(stderr, "Invalid drange argument\n");
-				return 1;
+				return ret;
 			}
 			args->flags |= BTRFS_BALANCE_ARGS_DRANGE;
 		} else if (!strcmp(this_char, "vrange")) {
 			if (!value || !*value) {
 				fprintf(stderr, "the vrange filter requires "
 				       "an argument\n");
-				return 1;
+				return -EINVAL;
 			}
-			if (parse_range(value, &args->vstart, &args->vend)) {
+			ret = parse_range(value, &args->vstart, &args->vend);
+			if (ret) {
 				fprintf(stderr, "Invalid vrange argument\n");
-				return 1;
+				return ret;
 			}
 			args->flags |= BTRFS_BALANCE_ARGS_VRANGE;
 		} else if (!strcmp(this_char, "convert")) {
 			if (!value || !*value) {
 				fprintf(stderr, "the convert option requires "
 				       "an argument\n");
-				return 1;
+				return -EINVAL;
 			}
 			if (parse_one_profile(value, &args->target)) {
 				fprintf(stderr, "Invalid convert argument\n");
-				return 1;
+				return -EINVAL;
 			}
 			args->flags |= BTRFS_BALANCE_ARGS_CONVERT;
 		} else if (!strcmp(this_char, "soft")) {
@@ -221,7 +229,7 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
 		} else {
 			fprintf(stderr, "Unrecognized balance option '%s'\n",
 				this_char);
-			return 1;
+			return -EINVAL;
 		}
 	}
 
@@ -297,9 +305,10 @@ static int do_balance(const char *path, struct btrfs_ioctl_balance_args *args,
 	DIR *dirstream = NULL;
 
 	fd = open_file_or_dir(path, &dirstream);
+	e = errno;
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
+		return e;
 	}
 
 	ret = ioctl(fd, BTRFS_IOC_BALANCE_V2, args);
@@ -330,7 +339,7 @@ static int do_balance(const char *path, struct btrfs_ioctl_balance_args *args,
 			if (e != EINPROGRESS)
 				fprintf(stderr, "There may be more info in "
 					"syslog - try dmesg | tail\n");
-			ret = 19;
+			ret = 1;
 		}
 	} else {
 		printf("Done, had to relocate %llu out of %llu chunks\n",
@@ -370,6 +379,7 @@ static int cmd_balance_start(int argc, char **argv)
 	int verbose = 0;
 	int nofilters = 1;
 	int i;
+	int ret = 0;
 
 	memset(&args, 0, sizeof(args));
 
@@ -473,7 +483,8 @@ static int cmd_balance_start(int argc, char **argv)
 	if (verbose)
 		dump_ioctl_balance_args(&args);
 
-	return do_balance(argv[optind], &args, nofilters);
+	ret = do_balance(argv[optind], &args, nofilters);
+	return !!ret;
 }
 
 static const char * const cmd_balance_pause_usage[] = {
@@ -498,7 +509,7 @@ static int cmd_balance_pause(int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = ioctl(fd, BTRFS_IOC_BALANCE_CTL, BTRFS_BALANCE_CTL_PAUSE);
@@ -508,7 +519,10 @@ static int cmd_balance_pause(int argc, char **argv)
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: balance pause on '%s' failed - %s\n",
 			path, (e == ENOTCONN) ? "Not running" : strerror(e));
-		return 19;
+		if (e == ENOTCONN)
+			return 2;
+		else
+			return 1;
 	}
 
 	return 0;
@@ -536,7 +550,7 @@ static int cmd_balance_cancel(int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = ioctl(fd, BTRFS_IOC_BALANCE_CTL, BTRFS_BALANCE_CTL_CANCEL);
@@ -546,7 +560,10 @@ static int cmd_balance_cancel(int argc, char **argv)
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: balance cancel on '%s' failed - %s\n",
 			path, (e == ENOTCONN) ? "Not in progress" : strerror(e));
-		return 19;
+		if (e == ENOTCONN)
+			return 2;
+		else
+			return 1;
 	}
 
 	return 0;
@@ -575,7 +592,7 @@ static int cmd_balance_resume(int argc, char **argv)
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	memset(&args, 0, sizeof(args));
@@ -596,12 +613,15 @@ static int cmd_balance_resume(int argc, char **argv)
 				"failed - %s\n", path,
 				(e == ENOTCONN) ? "Not in progress" :
 						  "Already running");
-			return 19;
+			if (e == ENOTCONN)
+				return 2;
+			else
+				return 1;
 		} else {
 			fprintf(stderr,
 "ERROR: error during balancing '%s' - %s\n"
 "There may be more info in syslog - try dmesg | tail\n", path, strerror(e));
-			return 19;
+			return 1;
 		}
 	} else {
 		printf("Done, had to relocate %llu out of %llu chunks\n",
@@ -719,6 +739,8 @@ const struct cmd_group balance_cmd_group = {
 
 int cmd_balance(int argc, char **argv)
 {
+	int ret = 0;
+
 	if (argc == 2) {
 		/* old 'btrfs filesystem balance <path>' syntax */
 		struct btrfs_ioctl_balance_args args;
@@ -726,7 +748,8 @@ int cmd_balance(int argc, char **argv)
 		memset(&args, 0, sizeof(args));
 		args.flags |= BTRFS_BALANCE_TYPE_MASK;
 
-		return do_balance(argv[1], &args, 1);
+		ret = do_balance(argv[1], &args, 1);
+		return !!ret;
 	}
 
 	return handle_command_group(&balance_cmd_group, argc, argv);
-- 
1.7.11.7

[PATCH 19/20] Btrfs-progs: fix magic return value in cmds-replace.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

There are 3 kinds of return values in replace cancel:

0: cancel successfully.
1: usage or syntal errors
2: cancel a not started or finished replacing operations.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-replace.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/cmds-replace.c b/cmds-replace.c
index 1df719b..d9b0940 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -324,7 +324,7 @@ leave_with_error:
 		close(fdsrcdev);
 	if (fddstdev != -1)
 		close(fddstdev);
-	return -1;
+	return 1;
 }
 
 static const char *const cmd_status_replace_usage[] = {
@@ -367,12 +367,12 @@ static int cmd_status_replace(int argc, char **argv)
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access \"%s\": %s\n",
 			path, strerror(e));
-		return -1;
+		return 1;
 	}
 
 	ret = print_replace_status(fd, path, once);
 	close_file_or_dir(fd, dirstream);
-	return ret;
+	return !!ret;
 }
 
 static int print_replace_status(int fd, const char *path, int once)
@@ -530,7 +530,7 @@ static int cmd_cancel_replace(int argc, char **argv)
 	if (fd < 0) {
 		fprintf(stderr, "ERROR: can't access \"%s\": %s\n",
 			path, strerror(errno));
-		return -1;
+		return 1;
 	}
 
 	args.cmd = BTRFS_IOCTL_DEV_REPLACE_CMD_CANCEL;
@@ -541,9 +541,13 @@ static int cmd_cancel_replace(int argc, char **argv)
 		fprintf(stderr, "ERROR: ioctl(DEV_REPLACE_CANCEL) failed on \"%s\": %s, %s\n",
 			path, strerror(e),
 			replace_dev_result2string(args.result));
-		return ret;
+		return 1;
+	}
+	if (args.result == BTRFS_IOCTL_DEV_REPLACE_RESULT_NOT_STARTED) {
+		printf("INFO: ioctl(DEV_REPLACE_CANCEL)\"%s\": %s\n",
+			path, replace_dev_result2string(args.result));
+		return 2;
 	}
-
 	return 0;
 }
 
-- 
1.7.11.7

[PATCH 20/20] Btrfs-progs: fix magic return value in cmds-scrub.c

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

There will be four kinds of return value for command "scrub start":

0: scrub dosen't find errors and return success.
1: usage or syntax errors.
3: scrub finds errors and correct all of them.
4: scrub finds errors and some of them are not correctable.

Three kinds of return values for scrub cancel/resume:

0: cancel successfully.
1: usage or syntax errors.
2: cancel a not started or finished scrub.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
 cmds-scrub.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/cmds-scrub.c b/cmds-scrub.c
index 55da405..605af45 100644
--- a/cmds-scrub.c
+++ b/cmds-scrub.c
@@ -1018,7 +1018,7 @@ static int mkdir_p(char *path)
 		path[i] = '\0';
 		ret = mkdir(path, 0777);
 		if (ret && errno != EEXIST)
-			return 1;
+			return -errno;
 		path[i] = '/';
 	}
 
@@ -1155,7 +1155,7 @@ static int scrub_start(int argc, char **argv, int resume)
 
 	if (fdmnt < 0) {
 		ERR(!do_quiet, "ERROR: can't access '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = get_fs_info(path, &fi_args, &di_args);
@@ -1261,8 +1261,7 @@ static int scrub_start(int argc, char **argv, int resume)
 		if (!do_quiet)
 			printf("scrub: nothing to resume for %s, fsid %s\n",
 			       path, fsid);
-		err = 0;
-		goto out;
+		return 2;
 	}
 
 	ret = prg_fd = socket(AF_UNIX, SOCK_STREAM, 0);
@@ -1501,9 +1500,9 @@ out:
 	if (err)
 		return 1;
 	if (e_correctable)
-		return 7;
+		return 3;
 	if (e_uncorrectable)
-		return 8;
+		return 4;
 	return 0;
 }
 
@@ -1557,7 +1556,10 @@ static int cmd_scrub_cancel(int argc, char **argv)
 	if (ret < 0) {
 		fprintf(stderr, "ERROR: scrub cancel failed on %s: %s\n", path,
 			errno == ENOTCONN ? "not running" : strerror(errno));
-		ret = 1;
+		if (errno == ENOTCONN)
+			ret = 2;
+		else
+			ret = 1;
 		goto out;
 	}
 
@@ -1642,7 +1644,7 @@ static int cmd_scrub_status(int argc, char **argv)
 
 	if (fdmnt < 0) {
 		fprintf(stderr, "ERROR: can't access to '%s'\n", path);
-		return 12;
+		return 1;
 	}
 
 	ret = get_fs_info(path, &fi_args, &di_args);
@@ -1727,7 +1729,7 @@ out:
 		close(fdres);
 	close_file_or_dir(fdmnt, dirstream);
 
-	return err;
+	return !!err;
 }
 
 const struct cmd_group scrub_cmd_group = {
-- 
1.7.11.7

Re: [PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

[Ilya Dryomov]

Hi Wang,

Thank you for doing the grunt work, it's been a long standing todo.
See the comments below.

On Wed, Sep 4, 2013 at 6:22 PM, Wang Shilong <wangshilong1991@gmail.com> wrote:
> From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>
> If there is no balance in progress.resume/pause/cancel will
> return 2. For usage or syntal errors will return 1. 0 means
> operations return successfully.

This needs to be reworded (spelling, punctuation).

>
> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
> ---
>  cmds-balance.c | 93 ++++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 58 insertions(+), 35 deletions(-)
>
> diff --git a/cmds-balance.c b/cmds-balance.c
> index b7382ef..fd68051 100644
> --- a/cmds-balance.c
> +++ b/cmds-balance.c
> @@ -58,7 +58,7 @@ static int parse_one_profile(const char *profile, u64 *flags)
>                 *flags |= BTRFS_AVAIL_ALLOC_BIT_SINGLE;
>         } else {
>                 fprintf(stderr, "Unknown profile '%s'\n", profile);
> -               return 1;
> +               return -ENOENT;
>         }
>
>         return 0;
> @@ -68,12 +68,14 @@ static int parse_profiles(char *profiles, u64 *flags)
>  {
>         char *this_char;
>         char *save_ptr = NULL; /* Satisfy static checkers */
> +       int ret;
>
>         for (this_char = strtok_r(profiles, "|", &save_ptr);
>              this_char != NULL;
>              this_char = strtok_r(NULL, "|", &save_ptr)) {
> -               if (parse_one_profile(this_char, flags))
> -                       return 1;
> +               ret = parse_one_profile(this_char, flags);
> +               if (ret)
> +                       return ret;
>         }
>
>         return 0;
> @@ -86,7 +88,7 @@ static int parse_u64(const char *str, u64 *result)
>
>         val = strtoull(str, &endptr, 10);
>         if (*endptr)
> -               return 1;
> +               return -EINVAL;
>
>         *result = val;
>         return 0;
> @@ -95,6 +97,7 @@ static int parse_u64(const char *str, u64 *result)
>  static int parse_range(const char *range, u64 *start, u64 *end)
>  {
>         char *dots;
> +       int ret;
>
>         dots = strstr(range, "..");
>         if (dots) {
> @@ -107,29 +110,31 @@ static int parse_range(const char *range, u64 *start, u64 *end)
>                         *end = (u64)-1;
>                         skipped++;
>                 } else {
> -                       if (parse_u64(rest, end))
> -                               return 1;
> +                       ret = parse_u64(rest, end);
> +                       if (ret)
> +                               return ret;
>                 }
>                 if (dots == range) {
>                         *start = 0;
>                         skipped++;
>                 } else {
> +                       ret = parse_u64(rest, end);
>                         if (parse_u64(range, start))
> -                               return 1;
> +                               return ret;
>                 }
>
>                 if (*start >= *end) {
>                         fprintf(stderr, "Range %llu..%llu doesn't make "
>                                 "sense\n", (unsigned long long)*start,
>                                 (unsigned long long)*end);
> -                       return 1;
> +                       return -EINVAL;
>                 }
>
>                 if (skipped <= 1)
>                         return 0;
>         }
>
> -       return 1;
> +       return -EINVAL;
>  }
>
>  static int parse_filters(char *filters, struct btrfs_balance_args *args)
> @@ -137,6 +142,7 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
>         char *this_char;
>         char *value;
>         char *save_ptr = NULL; /* Satisfy static checkers */
> +       int ret = 0;
>
>         if (!filters)
>                 return 0;
> @@ -150,70 +156,72 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
>                         if (!value || !*value) {
>                                 fprintf(stderr, "the profiles filter requires "
>                                        "an argument\n");
> -                               return 1;
> +                               return -EINVAL;
>                         }
>                         if (parse_profiles(value, &args->profiles)) {
>                                 fprintf(stderr, "Invalid profiles argument\n");
> -                               return 1;
> +                               return -EINVAL;
>                         }
>                         args->flags |= BTRFS_BALANCE_ARGS_PROFILES;
>                 } else if (!strcmp(this_char, "usage")) {
>                         if (!value || !*value) {
>                                 fprintf(stderr, "the usage filter requires "
>                                        "an argument\n");
> -                               return 1;
> +                               return -EINVAL;
>                         }
>                         if (parse_u64(value, &args->usage) ||
>                             args->usage > 100) {
>                                 fprintf(stderr, "Invalid usage argument: %s\n",
>                                        value);
> -                               return 1;
> +                               return -EINVAL;
>                         }
>                         args->flags |= BTRFS_BALANCE_ARGS_USAGE;
>                 } else if (!strcmp(this_char, "devid")) {
>                         if (!value || !*value) {
>                                 fprintf(stderr, "the devid filter requires "
>                                        "an argument\n");
> -                               return 1;
> +                               return -EINVAL;
>                         }
>                         if (parse_u64(value, &args->devid) ||
>                             args->devid == 0) {
>                                 fprintf(stderr, "Invalid devid argument: %s\n",
>                                        value);
> -                               return 1;
> +                               return -EINVAL;
>                         }
>                         args->flags |= BTRFS_BALANCE_ARGS_DEVID;
>                 } else if (!strcmp(this_char, "drange")) {
>                         if (!value || !*value) {
>                                 fprintf(stderr, "the drange filter requires "
>                                        "an argument\n");
> -                               return 1;
> +                               return -EINVAL;
>                         }
> -                       if (parse_range(value, &args->pstart, &args->pend)) {
> +                       ret = parse_range(value, &args->pstart, &args->pend);
> +                       if (ret) {
>                                 fprintf(stderr, "Invalid drange argument\n");
> -                               return 1;
> +                               return ret;
>                         }
>                         args->flags |= BTRFS_BALANCE_ARGS_DRANGE;
>                 } else if (!strcmp(this_char, "vrange")) {
>                         if (!value || !*value) {
>                                 fprintf(stderr, "the vrange filter requires "
>                                        "an argument\n");
> -                               return 1;
> +                               return -EINVAL;
>                         }
> -                       if (parse_range(value, &args->vstart, &args->vend)) {
> +                       ret = parse_range(value, &args->vstart, &args->vend);
> +                       if (ret) {
>                                 fprintf(stderr, "Invalid vrange argument\n");
> -                               return 1;
> +                               return ret;
>                         }
>                         args->flags |= BTRFS_BALANCE_ARGS_VRANGE;
>                 } else if (!strcmp(this_char, "convert")) {
>                         if (!value || !*value) {
>                                 fprintf(stderr, "the convert option requires "
>                                        "an argument\n");
> -                               return 1;
> +                               return -EINVAL;
>                         }
>                         if (parse_one_profile(value, &args->target)) {
>                                 fprintf(stderr, "Invalid convert argument\n");
> -                               return 1;
> +                               return -EINVAL;
>                         }
>                         args->flags |= BTRFS_BALANCE_ARGS_CONVERT;
>                 } else if (!strcmp(this_char, "soft")) {
> @@ -221,7 +229,7 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
>                 } else {
>                         fprintf(stderr, "Unrecognized balance option '%s'\n",
>                                 this_char);
> -                       return 1;
> +                       return -EINVAL;
>                 }
>         }
>

All of the above hunks are unnecessary and need to be dropped.
Returning 0/1 is not magic, and all of those are just (static) helpers.

> @@ -297,9 +305,10 @@ static int do_balance(const char *path, struct btrfs_ioctl_balance_args *args,
>         DIR *dirstream = NULL;
>
>         fd = open_file_or_dir(path, &dirstream);
> +       e = errno;
>         if (fd < 0) {
>                 fprintf(stderr, "ERROR: can't access to '%s'\n", path);
> -               return 12;
> +               return e;
>         }
>
>         ret = ioctl(fd, BTRFS_IOC_BALANCE_V2, args);

Unnecessary, do a simple 'return 1' here.  You are explicitly
discarding the return value below in your patch anyway.

> @@ -330,7 +339,7 @@ static int do_balance(const char *path, struct btrfs_ioctl_balance_args *args,
>                         if (e != EINPROGRESS)
>                                 fprintf(stderr, "There may be more info in "
>                                         "syslog - try dmesg | tail\n");
> -                       ret = 19;
> +                       ret = 1;
>                 }
>         } else {
>                 printf("Done, had to relocate %llu out of %llu chunks\n",

Ack.

> @@ -370,6 +379,7 @@ static int cmd_balance_start(int argc, char **argv)
>         int verbose = 0;
>         int nofilters = 1;
>         int i;
> +       int ret = 0;
>
>         memset(&args, 0, sizeof(args));
>
> @@ -473,7 +483,8 @@ static int cmd_balance_start(int argc, char **argv)
>         if (verbose)
>                 dump_ioctl_balance_args(&args);
>
> -       return do_balance(argv[optind], &args, nofilters);
> +       ret = do_balance(argv[optind], &args, nofilters);
> +       return !!ret;
>  }
>
>  static const char * const cmd_balance_pause_usage[] = {

Unnecessary, needs to be dropped.  The above two hunks (with the
suggested change) already make sure do_balance() returns 0/1.

> @@ -498,7 +509,7 @@ static int cmd_balance_pause(int argc, char **argv)
>         fd = open_file_or_dir(path, &dirstream);
>         if (fd < 0) {
>                 fprintf(stderr, "ERROR: can't access to '%s'\n", path);
> -               return 12;
> +               return 1;
>         }
>
>         ret = ioctl(fd, BTRFS_IOC_BALANCE_CTL, BTRFS_BALANCE_CTL_PAUSE);
> @@ -508,7 +519,10 @@ static int cmd_balance_pause(int argc, char **argv)
>         if (ret < 0) {
>                 fprintf(stderr, "ERROR: balance pause on '%s' failed - %s\n",
>                         path, (e == ENOTCONN) ? "Not running" : strerror(e));
> -               return 19;
> +               if (e == ENOTCONN)
> +                       return 2;
> +               else
> +                       return 1;
>         }
>
>         return 0;
> @@ -536,7 +550,7 @@ static int cmd_balance_cancel(int argc, char **argv)
>         fd = open_file_or_dir(path, &dirstream);
>         if (fd < 0) {
>                 fprintf(stderr, "ERROR: can't access to '%s'\n", path);
> -               return 12;
> +               return 1;
>         }
>
>         ret = ioctl(fd, BTRFS_IOC_BALANCE_CTL, BTRFS_BALANCE_CTL_CANCEL);
> @@ -546,7 +560,10 @@ static int cmd_balance_cancel(int argc, char **argv)
>         if (ret < 0) {
>                 fprintf(stderr, "ERROR: balance cancel on '%s' failed - %s\n",
>                         path, (e == ENOTCONN) ? "Not in progress" : strerror(e));
> -               return 19;
> +               if (e == ENOTCONN)
> +                       return 2;
> +               else
> +                       return 1;
>         }
>
>         return 0;
> @@ -575,7 +592,7 @@ static int cmd_balance_resume(int argc, char **argv)
>         fd = open_file_or_dir(path, &dirstream);
>         if (fd < 0) {
>                 fprintf(stderr, "ERROR: can't access to '%s'\n", path);
> -               return 12;
> +               return 1;
>         }
>
>         memset(&args, 0, sizeof(args));
> @@ -596,12 +613,15 @@ static int cmd_balance_resume(int argc, char **argv)
>                                 "failed - %s\n", path,
>                                 (e == ENOTCONN) ? "Not in progress" :
>                                                   "Already running");
> -                       return 19;
> +                       if (e == ENOTCONN)
> +                               return 2;
> +                       else
> +                               return 1;
>                 } else {
>                         fprintf(stderr,
>  "ERROR: error during balancing '%s' - %s\n"
>  "There may be more info in syslog - try dmesg | tail\n", path, strerror(e));
> -                       return 19;
> +                       return 1;
>                 }
>         } else {
>                 printf("Done, had to relocate %llu out of %llu chunks\n",

Ack up to this point.

> @@ -719,6 +739,8 @@ const struct cmd_group balance_cmd_group = {
>
>  int cmd_balance(int argc, char **argv)
>  {
> +       int ret = 0;
> +
>         if (argc == 2) {
>                 /* old 'btrfs filesystem balance <path>' syntax */
>                 struct btrfs_ioctl_balance_args args;
> @@ -726,7 +748,8 @@ int cmd_balance(int argc, char **argv)
>                 memset(&args, 0, sizeof(args));
>                 args.flags |= BTRFS_BALANCE_TYPE_MASK;
>
> -               return do_balance(argv[1], &args, 1);
> +               ret = do_balance(argv[1], &args, 1);
> +               return !!ret;
>         }
>
>         return handle_command_group(&balance_cmd_group, argc, argv);

Unnecessary, needs to be dropped.  See the first do_balance() call
site.

Thanks,

                Ilya

Re: [PATCH 00/20] fix magic return value in btrfs-progs

[Anand Jain]

On 09/04/2013 11:22 PM, Wang Shilong wrote:
> This patchset tries to fix all the magic return value in btrfs-progs.
> Most commands will have three kinds of return value:
>
> 0 success
> 1 usage of syntax errors
>
> Exceptions come from balance/scrub/replace. For example, replace cancel
> will return 2 if there is no operations in progress.

  Thanks for writing this much needed.
  Its better to have these return error codes defined
  in a header. So that it would guide the future developments.

Anand

Re: [PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

[Wang Shilong]

Hi Illya,

On 09/05/2013 12:26 AM, Ilya Dryomov wrote:
> Hi Wang,
>
> Thank you for doing the grunt work, it's been a long standing todo.
> See the comments below.
>
> On Wed, Sep 4, 2013 at 6:22 PM, Wang Shilong <wangshilong1991@gmail.com> wrote:
>> From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>>
>> If there is no balance in progress.resume/pause/cancel will
>> return 2. For usage or syntal errors will return 1. 0 means
>> operations return successfully.
> This needs to be reworded (spelling, punctuation).
will fix this.
>
>> Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
>> ---
>>   cmds-balance.c | 93 ++++++++++++++++++++++++++++++++++++----------------------
>>   1 file changed, 58 insertions(+), 35 deletions(-)
>>
>> diff --git a/cmds-balance.c b/cmds-balance.c
>> index b7382ef..fd68051 100644
>> --- a/cmds-balance.c
>> +++ b/cmds-balance.c
>> @@ -58,7 +58,7 @@ static int parse_one_profile(const char *profile, u64 *flags)
>>                  *flags |= BTRFS_AVAIL_ALLOC_BIT_SINGLE;
>>          } else {
>>                  fprintf(stderr, "Unknown profile '%s'\n", profile);
>> -               return 1;
>> +               return -ENOENT;
>>          }
>>
>>          return 0;
>> @@ -68,12 +68,14 @@ static int parse_profiles(char *profiles, u64 *flags)
>>   {
>>          char *this_char;
>>          char *save_ptr = NULL; /* Satisfy static checkers */
>> +       int ret;
>>
>>          for (this_char = strtok_r(profiles, "|", &save_ptr);
>>               this_char != NULL;
>>               this_char = strtok_r(NULL, "|", &save_ptr)) {
>> -               if (parse_one_profile(this_char, flags))
>> -                       return 1;
>> +               ret = parse_one_profile(this_char, flags);
>> +               if (ret)
>> +                       return ret;
>>          }
>>
>>          return 0;
>> @@ -86,7 +88,7 @@ static int parse_u64(const char *str, u64 *result)
>>
>>          val = strtoull(str, &endptr, 10);
>>          if (*endptr)
>> -               return 1;
>> +               return -EINVAL;
>>
>>          *result = val;
>>          return 0;
>> @@ -95,6 +97,7 @@ static int parse_u64(const char *str, u64 *result)
>>   static int parse_range(const char *range, u64 *start, u64 *end)
>>   {
>>          char *dots;
>> +       int ret;
>>
>>          dots = strstr(range, "..");
>>          if (dots) {
>> @@ -107,29 +110,31 @@ static int parse_range(const char *range, u64 *start, u64 *end)
>>                          *end = (u64)-1;
>>                          skipped++;
>>                  } else {
>> -                       if (parse_u64(rest, end))
>> -                               return 1;
>> +                       ret = parse_u64(rest, end);
>> +                       if (ret)
>> +                               return ret;
>>                  }
>>                  if (dots == range) {
>>                          *start = 0;
>>                          skipped++;
>>                  } else {
>> +                       ret = parse_u64(rest, end);
>>                          if (parse_u64(range, start))
>> -                               return 1;
>> +                               return ret;
>>                  }
>>
>>                  if (*start >= *end) {
>>                          fprintf(stderr, "Range %llu..%llu doesn't make "
>>                                  "sense\n", (unsigned long long)*start,
>>                                  (unsigned long long)*end);
>> -                       return 1;
>> +                       return -EINVAL;
>>                  }
>>
>>                  if (skipped <= 1)
>>                          return 0;
>>          }
>>
>> -       return 1;
>> +       return -EINVAL;
>>   }
>>
>>   static int parse_filters(char *filters, struct btrfs_balance_args *args)
>> @@ -137,6 +142,7 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
>>          char *this_char;
>>          char *value;
>>          char *save_ptr = NULL; /* Satisfy static checkers */
>> +       int ret = 0;
>>
>>          if (!filters)
>>                  return 0;
>> @@ -150,70 +156,72 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
>>                          if (!value || !*value) {
>>                                  fprintf(stderr, "the profiles filter requires "
>>                                         "an argument\n");
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>>                          if (parse_profiles(value, &args->profiles)) {
>>                                  fprintf(stderr, "Invalid profiles argument\n");
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>>                          args->flags |= BTRFS_BALANCE_ARGS_PROFILES;
>>                  } else if (!strcmp(this_char, "usage")) {
>>                          if (!value || !*value) {
>>                                  fprintf(stderr, "the usage filter requires "
>>                                         "an argument\n");
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>>                          if (parse_u64(value, &args->usage) ||
>>                              args->usage > 100) {
>>                                  fprintf(stderr, "Invalid usage argument: %s\n",
>>                                         value);
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>>                          args->flags |= BTRFS_BALANCE_ARGS_USAGE;
>>                  } else if (!strcmp(this_char, "devid")) {
>>                          if (!value || !*value) {
>>                                  fprintf(stderr, "the devid filter requires "
>>                                         "an argument\n");
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>>                          if (parse_u64(value, &args->devid) ||
>>                              args->devid == 0) {
>>                                  fprintf(stderr, "Invalid devid argument: %s\n",
>>                                         value);
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>>                          args->flags |= BTRFS_BALANCE_ARGS_DEVID;
>>                  } else if (!strcmp(this_char, "drange")) {
>>                          if (!value || !*value) {
>>                                  fprintf(stderr, "the drange filter requires "
>>                                         "an argument\n");
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>> -                       if (parse_range(value, &args->pstart, &args->pend)) {
>> +                       ret = parse_range(value, &args->pstart, &args->pend);
>> +                       if (ret) {
>>                                  fprintf(stderr, "Invalid drange argument\n");
>> -                               return 1;
>> +                               return ret;
>>                          }
>>                          args->flags |= BTRFS_BALANCE_ARGS_DRANGE;
>>                  } else if (!strcmp(this_char, "vrange")) {
>>                          if (!value || !*value) {
>>                                  fprintf(stderr, "the vrange filter requires "
>>                                         "an argument\n");
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>> -                       if (parse_range(value, &args->vstart, &args->vend)) {
>> +                       ret = parse_range(value, &args->vstart, &args->vend);
>> +                       if (ret) {
>>                                  fprintf(stderr, "Invalid vrange argument\n");
>> -                               return 1;
>> +                               return ret;
>>                          }
>>                          args->flags |= BTRFS_BALANCE_ARGS_VRANGE;
>>                  } else if (!strcmp(this_char, "convert")) {
>>                          if (!value || !*value) {
>>                                  fprintf(stderr, "the convert option requires "
>>                                         "an argument\n");
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>>                          if (parse_one_profile(value, &args->target)) {
>>                                  fprintf(stderr, "Invalid convert argument\n");
>> -                               return 1;
>> +                               return -EINVAL;
>>                          }
>>                          args->flags |= BTRFS_BALANCE_ARGS_CONVERT;
>>                  } else if (!strcmp(this_char, "soft")) {
>> @@ -221,7 +229,7 @@ static int parse_filters(char *filters, struct btrfs_balance_args *args)
>>                  } else {
>>                          fprintf(stderr, "Unrecognized balance option '%s'\n",
>>                                  this_char);
>> -                       return 1;
>> +                       return -EINVAL;
>>                  }
>>          }
>>
> All of the above hunks are unnecessary and need to be dropped.
> Returning 0/1 is not magic, and all of those are just (static) helpers
The reason is that i want to make functions return value more meaningful.
We should do this as much as possible. And at the last, we chose to 
return 1.

Thanks,
Wang
>
>> @@ -297,9 +305,10 @@ static int do_balance(const char *path, struct btrfs_ioctl_balance_args *args,
>>          DIR *dirstream = NULL;
>>
>>          fd = open_file_or_dir(path, &dirstream);
>> +       e = errno;
>>          if (fd < 0) {
>>                  fprintf(stderr, "ERROR: can't access to '%s'\n", path);
>> -               return 12;
>> +               return e;
>>          }
>>
>>          ret = ioctl(fd, BTRFS_IOC_BALANCE_V2, args);
> Unnecessary, do a simple 'return 1' here.  You are explicitly
> discarding the return value below in your patch anyway.
>
>> @@ -330,7 +339,7 @@ static int do_balance(const char *path, struct btrfs_ioctl_balance_args *args,
>>                          if (e != EINPROGRESS)
>>                                  fprintf(stderr, "There may be more info in "
>>                                          "syslog - try dmesg | tail\n");
>> -                       ret = 19;
>> +                       ret = 1;
>>                  }
>>          } else {
>>                  printf("Done, had to relocate %llu out of %llu chunks\n",
> Ack.
>
>> @@ -370,6 +379,7 @@ static int cmd_balance_start(int argc, char **argv)
>>          int verbose = 0;
>>          int nofilters = 1;
>>          int i;
>> +       int ret = 0;
>>
>>          memset(&args, 0, sizeof(args));
>>
>> @@ -473,7 +483,8 @@ static int cmd_balance_start(int argc, char **argv)
>>          if (verbose)
>>                  dump_ioctl_balance_args(&args);
>>
>> -       return do_balance(argv[optind], &args, nofilters);
>> +       ret = do_balance(argv[optind], &args, nofilters);
>> +       return !!ret;
>>   }
>>
>>   static const char * const cmd_balance_pause_usage[] = {
> Unnecessary, needs to be dropped.  The above two hunks (with the
> suggested change) already make sure do_balance() returns 0/1.
>
>> @@ -498,7 +509,7 @@ static int cmd_balance_pause(int argc, char **argv)
>>          fd = open_file_or_dir(path, &dirstream);
>>          if (fd < 0) {
>>                  fprintf(stderr, "ERROR: can't access to '%s'\n", path);
>> -               return 12;
>> +               return 1;
>>          }
>>
>>          ret = ioctl(fd, BTRFS_IOC_BALANCE_CTL, BTRFS_BALANCE_CTL_PAUSE);
>> @@ -508,7 +519,10 @@ static int cmd_balance_pause(int argc, char **argv)
>>          if (ret < 0) {
>>                  fprintf(stderr, "ERROR: balance pause on '%s' failed - %s\n",
>>                          path, (e == ENOTCONN) ? "Not running" : strerror(e));
>> -               return 19;
>> +               if (e == ENOTCONN)
>> +                       return 2;
>> +               else
>> +                       return 1;
>>          }
>>
>>          return 0;
>> @@ -536,7 +550,7 @@ static int cmd_balance_cancel(int argc, char **argv)
>>          fd = open_file_or_dir(path, &dirstream);
>>          if (fd < 0) {
>>                  fprintf(stderr, "ERROR: can't access to '%s'\n", path);
>> -               return 12;
>> +               return 1;
>>          }
>>
>>          ret = ioctl(fd, BTRFS_IOC_BALANCE_CTL, BTRFS_BALANCE_CTL_CANCEL);
>> @@ -546,7 +560,10 @@ static int cmd_balance_cancel(int argc, char **argv)
>>          if (ret < 0) {
>>                  fprintf(stderr, "ERROR: balance cancel on '%s' failed - %s\n",
>>                          path, (e == ENOTCONN) ? "Not in progress" : strerror(e));
>> -               return 19;
>> +               if (e == ENOTCONN)
>> +                       return 2;
>> +               else
>> +                       return 1;
>>          }
>>
>>          return 0;
>> @@ -575,7 +592,7 @@ static int cmd_balance_resume(int argc, char **argv)
>>          fd = open_file_or_dir(path, &dirstream);
>>          if (fd < 0) {
>>                  fprintf(stderr, "ERROR: can't access to '%s'\n", path);
>> -               return 12;
>> +               return 1;
>>          }
>>
>>          memset(&args, 0, sizeof(args));
>> @@ -596,12 +613,15 @@ static int cmd_balance_resume(int argc, char **argv)
>>                                  "failed - %s\n", path,
>>                                  (e == ENOTCONN) ? "Not in progress" :
>>                                                    "Already running");
>> -                       return 19;
>> +                       if (e == ENOTCONN)
>> +                               return 2;
>> +                       else
>> +                               return 1;
>>                  } else {
>>                          fprintf(stderr,
>>   "ERROR: error during balancing '%s' - %s\n"
>>   "There may be more info in syslog - try dmesg | tail\n", path, strerror(e));
>> -                       return 19;
>> +                       return 1;
>>                  }
>>          } else {
>>                  printf("Done, had to relocate %llu out of %llu chunks\n",
> Ack up to this point.
>
>> @@ -719,6 +739,8 @@ const struct cmd_group balance_cmd_group = {
>>
>>   int cmd_balance(int argc, char **argv)
>>   {
>> +       int ret = 0;
>> +
>>          if (argc == 2) {
>>                  /* old 'btrfs filesystem balance <path>' syntax */
>>                  struct btrfs_ioctl_balance_args args;
>> @@ -726,7 +748,8 @@ int cmd_balance(int argc, char **argv)
>>                  memset(&args, 0, sizeof(args));
>>                  args.flags |= BTRFS_BALANCE_TYPE_MASK;
>>
>> -               return do_balance(argv[1], &args, 1);
>> +               ret = do_balance(argv[1], &args, 1);
>> +               return !!ret;
>>          }
>>
>>          return handle_command_group(&balance_cmd_group, argc, argv);
> Unnecessary, needs to be dropped.  See the first do_balance() call
> site.
>
> Thanks,
>
>                  Ilya
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

[Stefan Behrens]

On Thu, 05 Sep 2013 15:44:11 +0800, Wang Shilong wrote:
[..]
>>> @@ -297,9 +305,10 @@ static int do_balance(const char *path, struct
>>> btrfs_ioctl_balance_args *args,
>>>          DIR *dirstream = NULL;
>>>
>>>          fd = open_file_or_dir(path, &dirstream);
>>> +       e = errno;
>>>          if (fd < 0) {
>>>                  fprintf(stderr, "ERROR: can't access to '%s'\n", path);
>>> -               return 12;
>>> +               return e;

Since I didn't understand whether you rejected or acknowledged Ilya's
comments, if you don't do so, please change the above line to "return
-e" like it is everywhere else: errno is returned as a negative value, 0
means no error, a positive value means the function failed but the
return value cannot be interpreted as an errno.

Re: [PATCH 18/20] Btrfs-progs: fix magic return value in cmds-balance.c

[Wang Shilong]

On 09/05/2013 04:21 PM, Stefan Behrens wrote:
> On Thu, 05 Sep 2013 15:44:11 +0800, Wang Shilong wrote:
> [..]
>>>> @@ -297,9 +305,10 @@ static int do_balance(const char *path, struct
>>>> btrfs_ioctl_balance_args *args,
>>>>           DIR *dirstream = NULL;
>>>>
>>>>           fd = open_file_or_dir(path, &dirstream);
>>>> +       e = errno;
>>>>           if (fd < 0) {
>>>>                   fprintf(stderr, "ERROR: can't access to '%s'\n", path);
>>>> -               return 12;
>>>> +               return e;
> Since I didn't understand whether you rejected or acknowledged Ilya's
My answer: acknowledged.

Will send a V2 later, just wait more comments.

Thanks,
Wang
> comments, if you don't do so, please change the above line to "return
> -e" like it is everywhere else: errno is returned as a negative value, 0
> means no error, a positive value means the function failed but the
> return value cannot be interpreted as an errno.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 00/20] fix magic return value in btrfs-progs

[Wang Shilong]

On 09/05/2013 10:14 AM, Anand Jain wrote:
>
> On 09/04/2013 11:22 PM, Wang Shilong wrote:
>> This patchset tries to fix all the magic return value in btrfs-progs.
>> Most commands will have three kinds of return value:
>>
>> 0 success
>> 1 usage of syntax errors
>>
>> Exceptions come from balance/scrub/replace. For example, replace cancel
>> will return 2 if there is no operations in progress.
>
>  Thanks for writing this much needed.
>  Its better to have these return error codes defined
>  in a header. So that it would guide the future developments.
Agree.

We also need update man page.^_^

Thanks,
Wang
>
> Anand
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 00/20] fix magic return value in btrfs-progs

[David Sterba]

Frist, thank you very much for the patches! I'm going to merge all
except 18/20, expecting V2.

On Thu, Sep 05, 2013 at 10:14:06AM +0800, Anand Jain wrote:
> 
> On 09/04/2013 11:22 PM, Wang Shilong wrote:
> >This patchset tries to fix all the magic return value in btrfs-progs.
> >Most commands will have three kinds of return value:
> >
> >0 success
> >1 usage of syntax errors
> >
> >Exceptions come from balance/scrub/replace. For example, replace cancel
> >will return 2 if there is no operations in progress.
> 
>  Thanks for writing this much needed.
>  Its better to have these return error codes defined
>  in a header. So that it would guide the future developments.

I agree. This means to add symbolic names to the scrub returns codes in
20/20, as an example of a command-specific return codes. I haven't found
a good place where to document the generic return values
(0/1/-errno etc). Something suites commands.h, others utils.h. If you
find more appropriate locations, feel free to use them.

david