[PATCH 00/11] btrfs-progs: Use btrfs_open_dir to avoid show error of ioctl or tree search

[PATCH 00/11] btrfs-progs: Use btrfs_open_dir to avoid show error of ioctl or tree search

[Zhao Lei]

Use btrfs_open_dir() instead of open_file_or_dir(), to show error before
real action(in ioctl or tree search), to make the error message exact
and unified.

It can also make code simple:
  85 insertions(+), 185 deletions(-)

Also include some small bug fix.

Before patch:
  # grep open_file_or_dir *.c
  btrfs-fragments.c:      fd = open_file_or_dir(path, &dirstream);
  cmds-balance.c: fd = open_file_or_dir(path, &dirstream);
  cmds-balance.c: fd = open_file_or_dir(path, &dirstream);
  cmds-balance.c: fd = open_file_or_dir(path, &dirstream);
  cmds-balance.c: fd = open_file_or_dir(path, &dirstream);
  cmds-balance.c: fd = open_file_or_dir(path, &dirstream);
  cmds-filesystem.c:      fd = open_file_or_dir(path, &dirstream);
  cmds-filesystem.c:      fd = open_file_or_dir(path, &dirstream);
  cmds-filesystem.c:              fd = open_file_or_dir(argv[i], &dirstream);
  cmds-filesystem.c:      fd = open_file_or_dir(path, &dirstream);
  cmds-fi-usage.c:                fd = open_file_or_dir(argv[i], &dirstream);
  cmds-inspect.c: fd = open_file_or_dir(argv[optind+1], &dirstream);
  cmds-inspect.c: fd = open_file_or_dir(argv[optind+1], &dirstream);
  cmds-inspect.c:                         path_fd = open_file_or_dir(full_path, &dirs);
  cmds-inspect.c: fd = open_file_or_dir(argv[2], &dirstream);
  cmds-inspect.c: fd = open_file_or_dir(argv[1], &dirstream);
  cmds-inspect.c: fd = open_file_or_dir(argv[optind], &dirstream);
  cmds-qgroup.c:  fd = open_file_or_dir(path, &dirstream);
  cmds-qgroup.c:  fd = open_file_or_dir(path, &dirstream);
  cmds-qgroup.c:  fd = open_file_or_dir(path, &dirstream);
  cmds-qgroup.c:  fd = open_file_or_dir(path, &dirstream);
  cmds-quota.c:   fd = open_file_or_dir(path, &dirstream);
  cmds-quota.c:   fd = open_file_or_dir(path, &dirstream);
  cmds-replace.c: fd = open_file_or_dir(path, &dirstream);
  cmds-replace.c: fd = open_file_or_dir(path, &dirstream);
  cmds-subvolume.c:       fddst = open_file_or_dir(dstdir, &dirstream);
  cmds-subvolume.c:       fd = open_file_or_dir(dname, &dirstream);
  cmds-subvolume.c:       fd = open_file_or_dir(subvol, &dirstream);
  cmds-subvolume.c:       fddst = open_file_or_dir(dstdir, &dirstream1);
  cmds-subvolume.c:       fd = open_file_or_dir(subvol, &dirstream2);
  cmds-subvolume.c:       fd = open_file_or_dir(subvol, &dirstream);
  cmds-subvolume.c:       fd = open_file_or_dir(path, &dirstream);
  cmds-subvolume.c:       fd = open_file_or_dir(subvol, &dirstream);
  cmds-subvolume.c:       fd = open_file_or_dir(fullpath, &dirstream1);
  cmds-subvolume.c:       mntfd = open_file_or_dir(mnt, &dirstream2);
  cmds-subvolume.c:       fd = open_file_or_dir(argv[optind], &dirstream);
  props.c:        fd = open_file_or_dir3(object, &dirstream, open_flags);
  utils.c:                fdmnt = open_file_or_dir(mp, dirstream);
  utils.c:                fdmnt = open_file_or_dir(path, dirstream);
  utils.c: * Do the following checks before calling open_file_or_dir():
  utils.c:        ret = open_file_or_dir(path, dirstream);
  utils.c:int open_file_or_dir3(const char *fname, DIR **dirstream, int open_flags)
  utils.c:int open_file_or_dir(const char *fname, DIR **dirstream)
  utils.c:        return open_file_or_dir3(fname, dirstream, O_RDWR);
  utils.c:        fd = open_file_or_dir(path, &dirstream);
  #

After patch:
  # grep open_file_or_dir *.c
  cmds-filesystem.c:              fd = open_file_or_dir(argv[i], &dirstream); *1
  props.c:        fd = open_file_or_dir3(object, &dirstream, open_flags);
  utils.c:                ret = open_file_or_dir(mp, dirstream);
  utils.c: * Do the following checks before calling open_file_or_dir():
  utils.c:        ret = open_file_or_dir(path, dirstream);
  utils.c:int open_file_or_dir3(const char *fname, DIR **dirstream, int open_flags)
  utils.c:int open_file_or_dir(const char *fname, DIR **dirstream)
  utils.c:        return open_file_or_dir3(fname, dirstream, O_RDWR);
  utils.c:        fd = open_file_or_dir(path, &dirstream);
  #
  *1: It is used to open dir or file, can not use btrfs_open_dir()
      instead.

Zhao Lei (11):
  btrfs-progs: subvolume: use btrfs_open_dir for btrfs subvolume command
  btrfs-progs: filesystem: use btrfs_open_dir for btrfs filesystem
    command
  btrfs-progs: balance: use btrfs_open_dir for btrfs balance command
  btrfs-progs: inspect: Bypass unnecessary clean function in open_error
  btrfs-progs: inspect: set return value of error case
  btrfs-progs: inspect: use btrfs_open_dir for btrfs inspect command
  btrfs-progs: qgroup: use btrfs_open_dir for btrfs qgroup command
  btrfs-progs: quota: use btrfs_open_dir for btrfs quota command
  btrfs-progs: use btrfs_open_dir in open_path_or_dev_mnt
  btrfs-progs: replace: use btrfs_open_dir for btrfs replace command
  btrfs-progs: fragments: use btrfs_open_dir for btrfs-fragments command

 btrfs-fragments.c |  6 ++----
 cmds-balance.c    | 30 ++++++++++-------------------
 cmds-device.c     | 13 ++-----------
 cmds-fi-usage.c   |  4 +---
 cmds-filesystem.c | 19 +++++++------------
 cmds-inspect.c    | 26 +++++++++-----------------
 cmds-qgroup.c     | 24 ++++++++----------------
 cmds-quota.c      | 12 ++++--------
 cmds-replace.c    | 29 ++++++----------------------
 cmds-scrub.c      | 28 +++++-----------------------
 cmds-subvolume.c  | 56 +++++++++++++++++++------------------------------------
 utils.c           | 21 +++++++++++----------
 utils.h           |  2 +-
 13 files changed, 85 insertions(+), 185 deletions(-)

-- 
1.8.5.1

[PATCH 01/11] btrfs-progs: subvolume: use btrfs_open_dir for btrfs subvolume command

[Zhao Lei]

We can use btrfs_open_dir() to check whether target dir is
in btrfs's mount point before open, instead of checking it in
kernel space of ioctl, and return fuzzy error message.

Before patch:
  # (/mnt/tmp is not btrfs mountpoint)
  #
  # btrfs subvolume create /mnt/tmp/123
  Create subvolume '/mnt/tmp/123'
  ERROR: cannot create subvolume - Inappropriate ioctl for device
  #

After patch:
  # btrfs subvolume create /mnt/tmp/123
  ERROR: not btrfs filesystem: /mnt/tmp
  #

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-subvolume.c | 56 +++++++++++++++++++-------------------------------------
 1 file changed, 19 insertions(+), 37 deletions(-)

diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index c40330a..be1a54a 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -181,11 +181,9 @@ static int cmd_subvol_create(int argc, char **argv)
 		goto out;
 	}
 
-	fddst = open_file_or_dir(dstdir, &dirstream);
-	if (fddst < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", dstdir);
+	fddst = btrfs_open_dir(dstdir, &dirstream, 1);
+	if (fddst < 0)
 		goto out;
-	}
 
 	printf("Create subvolume '%s/%s'\n", dstdir, newname);
 	if (inherit) {
@@ -348,9 +346,8 @@ again:
 	vname = basename(dupvname);
 	free(cpath);
 
-	fd = open_file_or_dir(dname, &dirstream);
+	fd = btrfs_open_dir(dname, &dirstream, 1);
 	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", dname);
 		ret = 1;
 		goto out;
 	}
@@ -564,7 +561,7 @@ static int cmd_subvol_list(int argc, char **argv)
 	}
 
 	subvol = argv[optind];
-	fd = open_file_or_dir(subvol, &dirstream);
+	fd = btrfs_open_dir(subvol, &dirstream, 1);
 	if (fd < 0) {
 		ret = -1;
 		fprintf(stderr, "ERROR: can't access '%s'\n", subvol);
@@ -723,17 +720,13 @@ static int cmd_subvol_snapshot(int argc, char **argv)
 		goto out;
 	}
 
-	fddst = open_file_or_dir(dstdir, &dirstream1);
-	if (fddst < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", dstdir);
+	fddst = btrfs_open_dir(dstdir, &dirstream1, 1);
+	if (fddst < 0)
 		goto out;
-	}
 
-	fd = open_file_or_dir(subvol, &dirstream2);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", dstdir);
+	fd = btrfs_open_dir(subvol, &dirstream2, 1);
+	if (fd < 0)
 		goto out;
-	}
 
 	if (readonly) {
 		args.flags |= BTRFS_SUBVOL_RDONLY;
@@ -791,11 +784,9 @@ static int cmd_subvol_get_default(int argc, char **argv)
 		usage(cmd_subvol_get_default_usage);
 
 	subvol = argv[1];
-	fd = open_file_or_dir(subvol, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", subvol);
+	fd = btrfs_open_dir(subvol, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = btrfs_list_get_default_subvolume(fd, &default_id);
 	if (ret) {
@@ -859,11 +850,9 @@ static int cmd_subvol_set_default(int argc, char **argv)
 
 	objectid = arg_strtou64(subvolid);
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_DEFAULT_SUBVOL, &objectid);
 	e = errno;
@@ -906,11 +895,9 @@ static int cmd_subvol_find_new(int argc, char **argv)
 		return 1;
 	}
 
-	fd = open_file_or_dir(subvol, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", subvol);
+	fd = btrfs_open_dir(subvol, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_SYNC);
 	if (ret < 0) {
@@ -980,11 +967,9 @@ static int cmd_subvol_show(int argc, char **argv)
 	ret = 1;
 	svpath = get_subvol_name(mnt, fullpath);
 
-	fd = open_file_or_dir(fullpath, &dirstream1);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", fullpath);
+	fd = btrfs_open_dir(fullpath, &dirstream1, 1);
+	if (fd < 0)
 		goto out;
-	}
 
 	ret = btrfs_list_get_path_rootid(fd, &sv_id);
 	if (ret) {
@@ -993,11 +978,9 @@ static int cmd_subvol_show(int argc, char **argv)
 		goto out;
 	}
 
-	mntfd = open_file_or_dir(mnt, &dirstream2);
-	if (mntfd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", mnt);
+	mntfd = btrfs_open_dir(mnt, &dirstream2, 1);
+	if (mntfd < 0)
 		goto out;
-	}
 
 	if (sv_id == BTRFS_FS_TREE_OBJECTID) {
 		printf("%s is btrfs root\n", fullpath);
@@ -1271,9 +1254,8 @@ static int cmd_subvol_sync(int argc, char **argv)
 	if (check_argc_min(argc - optind, 1))
 		usage(cmd_subvol_sync_usage);
 
-	fd = open_file_or_dir(argv[optind], &dirstream);
+	fd = btrfs_open_dir(argv[optind], &dirstream, 1);
 	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", argv[optind]);
 		ret = 1;
 		goto out;
 	}
-- 
1.8.5.1

[PATCH 02/11] btrfs-progs: filesystem: use btrfs_open_dir for btrfs filesystem command

[Zhao Lei]

We can use btrfs_open_dir() to check whether target dir is
in btrfs's mount point before open, instead of checking it in
kernel space of ioctl, and return fuzzy error message.

Before patch:
  # (/mnt/tmp is not btrfs mountpoint)
  #
  # btrfs filesystem df /mnt/tmp
  ERROR: couldn't get space info - Inappropriate ioctl for device
  ERROR: get_df failed Inappropriate ioctl for device
  #

After patch:
  # ./btrfs filesystem df /mnt/tmp
  ERROR: not btrfs filesystem: /mnt/tmp
  #

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-fi-usage.c   |  4 +---
 cmds-filesystem.c | 19 +++++++------------
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
index 50d6333..5fefed4 100644
--- a/cmds-fi-usage.c
+++ b/cmds-fi-usage.c
@@ -901,10 +901,8 @@ int cmd_filesystem_usage(int argc, char **argv)
 		int chunkcount = 0;
 		int devcount = 0;
 
-		fd = open_file_or_dir(argv[i], &dirstream);
+		fd = btrfs_open_dir(argv[i], &dirstream, 1);
 		if (fd < 0) {
-			fprintf(stderr, "ERROR: can't access '%s'\n",
-				argv[i]);
 			ret = 1;
 			goto out;
 		}
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 3663734..91bf1fa 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -205,11 +205,10 @@ static int cmd_filesystem_df(int argc, char **argv)
 
 	path = argv[1];
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
+
 	ret = get_df(fd, &sargs);
 
 	if (ret == 0) {
@@ -939,11 +938,9 @@ static int cmd_filesystem_sync(int argc, char **argv)
 
 	path = argv[1];
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	printf("FSSync '%s'\n", path);
 	res = ioctl(fd, BTRFS_IOC_SYNC);
@@ -1229,11 +1226,9 @@ static int cmd_filesystem_resize(int argc, char **argv)
 		return 1;
 	}
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	printf("Resize '%s' of '%s'\n", path, amount);
 	memset(&args, 0, sizeof(args));
-- 
1.8.5.1

[PATCH 03/11] btrfs-progs: balance: use btrfs_open_dir for btrfs balance command

[Zhao Lei]

We can use btrfs_open_dir() to check whether target dir is
in btrfs's mount point before open, instead of checking it in
kernel space of ioctl, and return fuzzy error message.

Before patch:
  # btrfs balance start /mnt/tmp
  ERROR: error during balancing '/mnt/tmp' - Inappropriate ioctl for device
  There may be more info in syslog - try dmesg | tail
  #

After patch:
  # btrfs balance start /mnt/tmp
  ERROR: not btrfs filesystem: /mnt/tmp
  #

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-balance.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/cmds-balance.c b/cmds-balance.c
index 9af218b..b02e40d 100644
--- a/cmds-balance.c
+++ b/cmds-balance.c
@@ -306,11 +306,9 @@ static int do_balance(const char *path, struct btrfs_ioctl_balance_args *args,
 	int e;
 	DIR *dirstream = NULL;
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_BALANCE_V2, args);
 	e = errno;
@@ -503,11 +501,9 @@ static int cmd_balance_pause(int argc, char **argv)
 
 	path = argv[1];
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_BALANCE_CTL, BTRFS_BALANCE_CTL_PAUSE);
 	e = errno;
@@ -544,11 +540,9 @@ static int cmd_balance_cancel(int argc, char **argv)
 
 	path = argv[1];
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_BALANCE_CTL, BTRFS_BALANCE_CTL_CANCEL);
 	e = errno;
@@ -586,11 +580,9 @@ static int cmd_balance_resume(int argc, char **argv)
 
 	path = argv[1];
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	memset(&args, 0, sizeof(args));
 	args.flags |= BTRFS_BALANCE_RESUME;
@@ -679,11 +671,9 @@ static int cmd_balance_status(int argc, char **argv)
 
 	path = argv[optind];
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 2;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_BALANCE_PROGRESS, &args);
 	e = errno;
-- 
1.8.5.1

[PATCH 04/11] btrfs-progs: inspect: Bypass unnecessary clean function in open_error

[Zhao Lei]

No need to cleanup fd in open_fail case, because it is not opened.

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-inspect.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/cmds-inspect.c b/cmds-inspect.c
index fc3db99..879fd43 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -626,9 +626,8 @@ static int cmd_inspect_min_dev_size(int argc, char **argv)
 	}
 
 	ret = print_min_dev_size(fd, devid);
-out:
 	close_file_or_dir(fd, dirstream);
-
+out:
 	return !!ret;
 }
 
-- 
1.8.5.1

[PATCH 05/11] btrfs-progs: inspect: set return value of error case

[Zhao Lei]

In case of open_file_or_dir() failed, ret is not set to right value,
and the function will return unwanted value(ret of sprintf).

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-inspect.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cmds-inspect.c b/cmds-inspect.c
index 879fd43..a13a170 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -243,6 +243,7 @@ static int cmd_inspect_logical_resolve(int argc, char **argv)
 				if (path_fd < 0) {
 					fprintf(stderr, "ERROR: can't access "
 						"'%s'\n", full_path);
+					ret = -ENOENT;
 					goto out;
 				}
 			}
-- 
1.8.5.1

[PATCH 06/11] btrfs-progs: inspect: use btrfs_open_dir for btrfs inspect command

[Zhao Lei]

We can use btrfs_open_dir() to check whether target dir is
in btrfs's mount point before open, instead of checking it in
kernel space of ioctl, and return fuzzy error message.

Before patch:
  # ./btrfs inspect-internal rootid /mnt/tmp1
  ERROR: Failed to lookup root id - Inappropriate ioctl for device
  btrfs inspect-internal rootid: rootid failed with ret=-1
  # ./btrfs inspect-internal inode-resolve 256 /mnt/tmp1
  ioctl ret=-1, error: Inappropriate ioctl for device
  # ./btrfs inspect-internal min-dev-size /mnt/tmp1
  Error invoking tree search ioctl: Inappropriate ioctl for device

After patch:
  # ./btrfs inspect-internal rootid /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  # ./btrfs inspect-internal inode-resolve 256 /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  # ./btrfs inspect-internal min-dev-size /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-inspect.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/cmds-inspect.c b/cmds-inspect.c
index a13a170..40ab49b 100644
--- a/cmds-inspect.c
+++ b/cmds-inspect.c
@@ -116,11 +116,9 @@ static int cmd_inspect_inode_resolve(int argc, char **argv)
 	if (check_argc_exact(argc - optind, 2))
 		usage(cmd_inspect_inode_resolve_usage);
 
-	fd = open_file_or_dir(argv[optind+1], &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", argv[optind+1]);
+	fd = btrfs_open_dir(argv[optind + 1], &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = __ino_to_path_fd(arg_strtou64(argv[optind]), fd, verbose,
 			       argv[optind+1]);
@@ -189,9 +187,8 @@ static int cmd_inspect_logical_resolve(int argc, char **argv)
 	loi.size = size;
 	loi.inodes = ptr_to_u64(inodes);
 
-	fd = open_file_or_dir(argv[optind+1], &dirstream);
+	fd = btrfs_open_dir(argv[optind + 1], &dirstream, 1);
 	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", argv[optind+1]);
 		ret = 12;
 		goto out;
 	}
@@ -239,10 +236,8 @@ static int cmd_inspect_logical_resolve(int argc, char **argv)
 						name);
 				BUG_ON(ret >= bytes_left);
 				free(name);
-				path_fd = open_file_or_dir(full_path, &dirs);
+				path_fd = btrfs_open_dir(full_path, &dirs, 1);
 				if (path_fd < 0) {
-					fprintf(stderr, "ERROR: can't access "
-						"'%s'\n", full_path);
 					ret = -ENOENT;
 					goto out;
 				}
@@ -279,9 +274,8 @@ static int cmd_inspect_subvolid_resolve(int argc, char **argv)
 	if (check_argc_exact(argc, 3))
 		usage(cmd_inspect_subvolid_resolve_usage);
 
-	fd = open_file_or_dir(argv[2], &dirstream);
+	fd = btrfs_open_dir(argv[2], &dirstream, 1);
 	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", argv[2]);
 		ret = -ENOENT;
 		goto out;
 	}
@@ -320,9 +314,8 @@ static int cmd_inspect_rootid(int argc, char **argv)
 	if (check_argc_exact(argc, 2))
 		usage(cmd_inspect_rootid_usage);
 
-	fd = open_file_or_dir(argv[1], &dirstream);
+	fd = btrfs_open_dir(argv[1], &dirstream, 1);
 	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", argv[1]);
 		ret = -ENOENT;
 		goto out;
 	}
@@ -619,9 +612,8 @@ static int cmd_inspect_min_dev_size(int argc, char **argv)
 	if (check_argc_exact(argc - optind, 1))
 		usage(cmd_inspect_min_dev_size_usage);
 
-	fd = open_file_or_dir(argv[optind], &dirstream);
+	fd = btrfs_open_dir(argv[optind], &dirstream, 1);
 	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", argv[optind]);
 		ret = -ENOENT;
 		goto out;
 	}
-- 
1.8.5.1

[PATCH 07/11] btrfs-progs: qgroup: use btrfs_open_dir for btrfs qgroup command

[Zhao Lei]

We can use btrfs_open_dir() to check whether target dir is
in btrfs's mount point before open, instead of checking it in
kernel space of ioctl, and return fuzzy error message.

Before patch:
  # ./btrfs qgroup create 1/5  /mnt/tmp1
  ERROR: unable to create quota group: Inappropriate ioctl for device
  #
  # ./btrfs qgroup assign 1/5 2/5 /mnt/tmp1
  ERROR: unable to assign quota group: Inappropriate ioctl for device
  #
  # ./btrfs qgroup show /mnt/tmp1
  ERROR: can't perform the search - Inappropriate ioctl for device
  ERROR: can't list qgroups: Inappropriate ioctl for device
  #
  # ./btrfs qgroup limit 1G 1/5  /mnt/tmp1
  ERROR: unable to limit requested quota group: Inappropriate ioctl for device

After patch:
  # ./btrfs qgroup create 1/5 /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  # ./btrfs qgroup assign 1/5 2/5 /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  # ./btrfs qgroup show /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  # ./btrfs qgroup limit 1G 1/5 /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-qgroup.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/cmds-qgroup.c b/cmds-qgroup.c
index 48c1733..0ad99f4 100644
--- a/cmds-qgroup.c
+++ b/cmds-qgroup.c
@@ -79,11 +79,9 @@ static int qgroup_assign(int assign, int argc, char **argv)
 		fprintf(stderr, "ERROR: bad relation requested '%s'\n", path);
 		return 1;
 	}
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_QGROUP_ASSIGN, &args);
 	e = errno;
@@ -137,11 +135,9 @@ static int qgroup_create(int create, int argc, char **argv)
 	args.create = create;
 	args.qgroupid = parse_qgroupid(argv[1]);
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_QGROUP_CREATE, &args);
 	e = errno;
@@ -351,11 +347,9 @@ static int cmd_qgroup_show(int argc, char **argv)
 		usage(cmd_qgroup_show_usage);
 
 	path = argv[optind];
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	if (filter_flag) {
 		qgroupid = btrfs_get_path_rootid(fd);
@@ -460,11 +454,9 @@ static int cmd_qgroup_limit(int argc, char **argv)
 	} else
 		usage(cmd_qgroup_limit_usage);
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_QGROUP_LIMIT, &args);
 	e = errno;
-- 
1.8.5.1

[PATCH 08/11] btrfs-progs: quota: use btrfs_open_dir for btrfs quota command

[Zhao Lei]

We can use btrfs_open_dir() to check whether target dir is
in btrfs's mount point before open, instead of checking it in
kernel space of ioctl, and return fuzzy error message.

Before patch:
  # ./btrfs quota enable /mnt/tmp1
  ERROR: quota command failed: Inappropriate ioctl for device
  # ./btrfs quota disable /mnt/tmp1
  ERROR: quota command failed: Inappropriate ioctl for device
  # ./btrfs quota rescan /mnt/tmp1
  ERROR: quota rescan failed: Inappropriate ioctl for device
  #

After patch:
  # ./btrfs quota enable /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  # ./btrfs quota disable /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  # ./btrfs quota rescan /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  #

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-quota.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/cmds-quota.c b/cmds-quota.c
index 8adc1bf..efbc3ef 100644
--- a/cmds-quota.c
+++ b/cmds-quota.c
@@ -45,11 +45,9 @@ static int quota_ctl(int cmd, int argc, char **argv)
 	memset(&args, 0, sizeof(args));
 	args.cmd = cmd;
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, BTRFS_IOC_QUOTA_CTL, &args);
 	e = errno;
@@ -141,11 +139,9 @@ static int cmd_quota_rescan(int argc, char **argv)
 	memset(&args, 0, sizeof(args));
 
 	path = argv[optind];
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = ioctl(fd, ioctlnum, &args);
 	e = errno;
-- 
1.8.5.1

[PATCH 09/11] btrfs-progs: use btrfs_open_dir in open_path_or_dev_mnt

[Zhao Lei]

Use btrfs_open_dir() in open_path_or_dev_mnt() to make the function
return error when target is neither block device nor btrfs mount point.

Also add "verbose" argument to let function output common error
message instead of putting duplicated lines in caller.

Before patch:
  # ./btrfs device stats /mnt/tmp1
  ERROR: getting dev info for devstats failed: Inappropriate ioctl for device
  # ./btrfs replace start /dev/vdd /dev/vde /mnt/tmp1
  ERROR: ioctl(DEV_REPLACE_STATUS) failed on "/mnt/tmp1": Inappropriate ioctl for device

After patch:
  # ./btrfs device stats /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  # ./btrfs replace start /dev/vdd /dev/vde /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-device.c  | 13 ++-----------
 cmds-replace.c | 13 ++-----------
 cmds-scrub.c   | 28 +++++-----------------------
 utils.c        | 21 +++++++++++----------
 utils.h        |  2 +-
 5 files changed, 21 insertions(+), 56 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 5f2b952..a9354f5 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -385,18 +385,9 @@ static int cmd_device_stats(int argc, char **argv)
 
 	dev_path = argv[optind];
 
-	fdmnt = open_path_or_dev_mnt(dev_path, &dirstream);
-
-	if (fdmnt < 0) {
-		if (errno == EINVAL)
-			fprintf(stderr,
-				"ERROR: '%s' is not a mounted btrfs device\n",
-				dev_path);
-		else
-			fprintf(stderr, "ERROR: can't access '%s': %s\n",
-				dev_path, strerror(errno));
+	fdmnt = open_path_or_dev_mnt(dev_path, &dirstream, 1);
+	if (fdmnt < 0)
 		return 1;
-	}
 
 	ret = get_fs_info(dev_path, &fi_args, &di_args);
 	if (ret) {
diff --git a/cmds-replace.c b/cmds-replace.c
index 9ab8438..385b764 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -170,18 +170,9 @@ static int cmd_replace_start(int argc, char **argv)
 		usage(cmd_replace_start_usage);
 	path = argv[optind + 2];
 
-	fdmnt = open_path_or_dev_mnt(path, &dirstream);
-
-	if (fdmnt < 0) {
-		if (errno == EINVAL)
-			fprintf(stderr,
-				"ERROR: '%s' is not a mounted btrfs device\n",
-				path);
-		else
-			fprintf(stderr, "ERROR: can't access '%s': %s\n",
-				path, strerror(errno));
+	fdmnt = open_path_or_dev_mnt(path, &dirstream, 1);
+	if (fdmnt < 0)
 		goto leave_with_error;
-	}
 
 	/* check for possible errors before backgrounding */
 	status_args.cmd = BTRFS_IOCTL_DEV_REPLACE_CMD_STATUS;
diff --git a/cmds-scrub.c b/cmds-scrub.c
index ea6ffc9..da614f2 100644
--- a/cmds-scrub.c
+++ b/cmds-scrub.c
@@ -1198,17 +1198,9 @@ static int scrub_start(int argc, char **argv, int resume)
 
 	path = argv[optind];
 
-	fdmnt = open_path_or_dev_mnt(path, &dirstream);
-
-	if (fdmnt < 0) {
-		if (errno == EINVAL)
-			error_on(!do_quiet, "'%s' is not a mounted btrfs device",
-				path);
-		else
-			error_on(!do_quiet, "can't access '%s': %s",
-				path, strerror(errno));
+	fdmnt = open_path_or_dev_mnt(path, &dirstream, !do_quiet);
+	if (fdmnt < 0)
 		return 1;
-	}
 
 	ret = get_fs_info(path, &fi_args, &di_args);
 	if (ret) {
@@ -1604,12 +1596,8 @@ static int cmd_scrub_cancel(int argc, char **argv)
 
 	path = argv[1];
 
-	fdmnt = open_path_or_dev_mnt(path, &dirstream);
+	fdmnt = open_path_or_dev_mnt(path, &dirstream, 1);
 	if (fdmnt < 0) {
-		if (errno == EINVAL)
-			error("'%s' is not a mounted btrfs device", path);
-		else
-			error("can't access '%s': %s", path, strerror(errno));
 		ret = 1;
 		goto out;
 	}
@@ -1705,15 +1693,9 @@ static int cmd_scrub_status(int argc, char **argv)
 
 	path = argv[optind];
 
-	fdmnt = open_path_or_dev_mnt(path, &dirstream);
-
-	if (fdmnt < 0) {
-		if (errno == EINVAL)
-			error("'%s' is not a mounted btrfs device", path);
-		else
-			error("can't access '%s': %s", path, strerror(errno));
+	fdmnt = open_path_or_dev_mnt(path, &dirstream, 1);
+	if (fdmnt < 0)
 		return 1;
-	}
 
 	ret = get_fs_info(path, &fi_args, &di_args);
 	if (ret) {
diff --git a/utils.c b/utils.c
index f1e3248..6f5df23 100644
--- a/utils.c
+++ b/utils.c
@@ -1081,27 +1081,28 @@ out:
  *
  * On error, return -1, errno should be set.
  */
-int open_path_or_dev_mnt(const char *path, DIR **dirstream)
+int open_path_or_dev_mnt(const char *path, DIR **dirstream, int verbose)
 {
 	char mp[PATH_MAX];
-	int fdmnt;
-
-	fdmnt = is_block_device(path);
-	if (fdmnt == 1) {
-		int ret;
+	int ret;
 
+	if (is_block_device(path)) {
 		ret = get_btrfs_mount(path, mp, sizeof(mp));
 		if (ret < 0) {
 			/* not a mounted btrfs dev */
+			error_on(verbose, "'%s' is not a mounted btrfs device",
+				 path);
 			errno = EINVAL;
 			return -1;
 		}
-		fdmnt = open_file_or_dir(mp, dirstream);
-	} else if (fdmnt == 0) {
-		fdmnt = open_file_or_dir(path, dirstream);
+		ret = open_file_or_dir(mp, dirstream);
+		error_on(verbose && ret < 0, "can't access '%s': %s",
+			 path, strerror(errno));
+	} else {
+		ret = btrfs_open_dir(path, dirstream, 1);
 	}
 
-	return fdmnt;
+	return ret;
 }
 
 /*
diff --git a/utils.h b/utils.h
index 044ea15..1dc12ec 100644
--- a/utils.h
+++ b/utils.h
@@ -158,7 +158,7 @@ char *__strncpy__null(char *dest, const char *src, size_t n);
 int is_block_device(const char *file);
 int is_mount_point(const char *file);
 int check_arg_type(const char *input);
-int open_path_or_dev_mnt(const char *path, DIR **dirstream);
+int open_path_or_dev_mnt(const char *path, DIR **dirstream, int verbose);
 int btrfs_open_dir(const char *path, DIR **dirstream, int verbose);
 u64 btrfs_device_size(int fd, struct stat *st);
 /* Helper to always get proper size of the destination string */
-- 
1.8.5.1

[PATCH 10/11] btrfs-progs: replace: use btrfs_open_dir for btrfs replace command

[Zhao Lei]

We can use btrfs_open_dir() to check whether target dir is
in btrfs's mount point before open, instead of checking it in
kernel space of ioctl, and return fuzzy error message.

Before patch:
  # ./btrfs replace cancel /mnt/tmp1
  ERROR: ioctl(DEV_REPLACE_CANCEL) failed on "/mnt/tmp1": Inappropriate ioctl for device
  # ./btrfs replace status /mnt/tmp1
  ERROR: ioctl(DEV_REPLACE_STATUS) failed on "/mnt/tmp1": Inappropriate ioctl for device

After patch:
  # ./btrfs replace cancel /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1
  # ./btrfs replace status /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 cmds-replace.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/cmds-replace.c b/cmds-replace.c
index 385b764..9596f2a 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -348,7 +348,6 @@ static const char *const cmd_replace_status_usage[] = {
 static int cmd_replace_status(int argc, char **argv)
 {
 	int fd;
-	int e;
 	int c;
 	char *path;
 	int once = 0;
@@ -370,13 +369,9 @@ static int cmd_replace_status(int argc, char **argv)
 		usage(cmd_replace_status_usage);
 
 	path = argv[optind];
-	fd = open_file_or_dir(path, &dirstream);
-	e = errno;
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access \"%s\": %s\n",
-			path, strerror(e));
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	ret = print_replace_status(fd, path, once);
 	close_file_or_dir(fd, dirstream);
@@ -541,12 +536,9 @@ static int cmd_replace_cancel(int argc, char **argv)
 		usage(cmd_replace_cancel_usage);
 
 	path = argv[optind];
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access \"%s\": %s\n",
-			path, strerror(errno));
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		return 1;
-	}
 
 	args.cmd = BTRFS_IOCTL_DEV_REPLACE_CMD_CANCEL;
 	args.result = BTRFS_IOCTL_DEV_REPLACE_RESULT_NO_RESULT;
-- 
1.8.5.1

[PATCH 11/11] btrfs-progs: fragments: use btrfs_open_dir for btrfs-fragments command

[Zhao Lei]

We can use btrfs_open_dir() to check whether target dir is
in btrfs's mount point before open, instead of checking it in
deeper code, and return fuzzy error message.

Before patch:
  ./btrfs-fragments -o 123 /mnt/tmp1
  ERROR: can't perform the search

After patch:
  # ./btrfs-fragments -o 123 /mnt/tmp1
  ERROR: not a btrfs filesystem: /mnt/tmp1

Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
 btrfs-fragments.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/btrfs-fragments.c b/btrfs-fragments.c
index d742f60..17768c3 100644
--- a/btrfs-fragments.c
+++ b/btrfs-fragments.c
@@ -436,11 +436,9 @@ int main(int argc, char **argv)
 
 	path = argv[optind++];
 
-	fd = open_file_or_dir(path, &dirstream);
-	if (fd < 0) {
-		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+	fd = btrfs_open_dir(path, &dirstream, 1);
+	if (fd < 0)
 		exit(1);
-	}
 
 	if (flags == 0)
 		flags = BTRFS_BLOCK_GROUP_DATA | BTRFS_BLOCK_GROUP_METADATA;
-- 
1.8.5.1

Re: [PATCH 00/11] btrfs-progs: Use btrfs_open_dir to avoid show error of ioctl or tree search

[David Sterba]

On Mon, Oct 12, 2015 at 09:22:53PM +0800, Zhao Lei wrote:
> Use btrfs_open_dir() instead of open_file_or_dir(), to show error before
> real action(in ioctl or tree search), to make the error message exact
> and unified.
> 
> Zhao Lei (11):
>   btrfs-progs: subvolume: use btrfs_open_dir for btrfs subvolume command
>   btrfs-progs: filesystem: use btrfs_open_dir for btrfs filesystem
>     command
>   btrfs-progs: balance: use btrfs_open_dir for btrfs balance command
>   btrfs-progs: inspect: Bypass unnecessary clean function in open_error
>   btrfs-progs: inspect: set return value of error case
>   btrfs-progs: inspect: use btrfs_open_dir for btrfs inspect command
>   btrfs-progs: qgroup: use btrfs_open_dir for btrfs qgroup command
>   btrfs-progs: quota: use btrfs_open_dir for btrfs quota command
>   btrfs-progs: use btrfs_open_dir in open_path_or_dev_mnt
>   btrfs-progs: replace: use btrfs_open_dir for btrfs replace command
>   btrfs-progs: fragments: use btrfs_open_dir for btrfs-fragments command

All merged, thanks! I appreciate you took the time to test all the
changes and the patch separation made review easy.