[PATCH RESEND 1/4] btrfs-progs: Check fstype in find_mount_root()

[PATCH RESEND 1/4] btrfs-progs: Check fstype in find_mount_root()

[Qu Wenruo]

When calling find_mount_root(), caller in fact wants to find the mount
point of *BTRFS*.

So also check ent->fstype in find_mount_root() and output proper error
messages if needed.
This will suppress a lot of "Inapproiate ioctl for device" error
message.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 utils.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/utils.c b/utils.c
index 993d085..507ec6c 100644
--- a/utils.c
+++ b/utils.c
@@ -2412,6 +2412,7 @@ int find_mount_root(const char *path, char **mount_root)
 	struct mntent *ent;
 	int len;
 	int ret;
+	int not_btrfs;
 	int longest_matchlen = 0;
 	char *longest_match = NULL;
 
@@ -2432,6 +2433,10 @@ int find_mount_root(const char *path, char **mount_root)
 				free(longest_match);
 				longest_matchlen = len;
 				longest_match = strdup(ent->mnt_dir);
+				if (strcmp(ent->mnt_type, "btrfs"))
+					not_btrfs = 1;
+				else
+					not_btrfs = 0;
 			}
 		}
 	}
@@ -2443,6 +2448,12 @@ int find_mount_root(const char *path, char **mount_root)
 			path);
 		return -ENOENT;
 	}
+	if (not_btrfs) {
+		fprintf(stderr,
+			"ERROR: %s does not belong to a btrfs mount points.\n",
+			path);
+		return -EINVAL;
+	}
 
 	ret = 0;
 	*mount_root = realpath(longest_match, NULL);
-- 
2.0.1

[PATCH 2/4] btrfs-progs: Integrate error message output into find_mount_root().

[Qu Wenruo]

Before this patch, find_mount_root() and the caller both output error
message, which sometimes make the output duplicated and hard to judge
what the problem is.

This pathh will integrate all the error messages output into
find_mount_root() to give more meaning error prompt and remove the
unneeded caller error messages.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-receive.c   |  2 --
 cmds-send.c      |  8 +-------
 cmds-subvolume.c |  5 +----
 utils.c          | 15 ++++++++++++---
 4 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/cmds-receive.c b/cmds-receive.c
index 48380a5..084d97d 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -981,8 +981,6 @@ static int do_receive(struct btrfs_receive *r, const char *tomnt, int r_fd,
 	ret = find_mount_root(dest_dir_full_path, &r->root_path);
 	if (ret < 0) {
 		ret = -EINVAL;
-		fprintf(stderr, "ERROR: failed to determine mount point "
-			"for %s\n", dest_dir_full_path);
 		goto out;
 	}
 	r->mnt_fd = open(r->root_path, O_RDONLY | O_NOATIME);
diff --git a/cmds-send.c b/cmds-send.c
index 9a73b32..091f32b 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -357,8 +357,6 @@ static int init_root_path(struct btrfs_send *s, const char *subvol)
 	ret = find_mount_root(subvol, &s->root_path);
 	if (ret < 0) {
 		ret = -EINVAL;
-		fprintf(stderr, "ERROR: failed to determine mount point "
-				"for %s\n", subvol);
 		goto out;
 	}
 
@@ -622,12 +620,8 @@ int cmd_send(int argc, char **argv)
 		}
 
 		ret = find_mount_root(subvol, &mount_root);
-		if (ret < 0) {
-			fprintf(stderr, "ERROR: find_mount_root failed on %s: "
-					"%s\n", subvol,
-				strerror(-ret));
+		if (ret < 0)
 			goto out;
-		}
 		if (strcmp(send.root_path, mount_root) != 0) {
 			ret = -EINVAL;
 			fprintf(stderr, "ERROR: all subvols must be from the "
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index 639fb10..b252eab 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -981,11 +981,8 @@ static int cmd_subvol_show(int argc, char **argv)
 	}
 
 	ret = find_mount_root(fullpath, &mnt);
-	if (ret < 0) {
-		fprintf(stderr, "ERROR: find_mount_root failed on %s: "
-				"%s\n", fullpath, strerror(-ret));
+	if (ret < 0)
 		goto out;
-	}
 	ret = 1;
 	svpath = get_subvol_name(mnt, fullpath);
 
diff --git a/utils.c b/utils.c
index 507ec6c..07173ee 100644
--- a/utils.c
+++ b/utils.c
@@ -2417,13 +2417,19 @@ int find_mount_root(const char *path, char **mount_root)
 	char *longest_match = NULL;
 
 	fd = open(path, O_RDONLY | O_NOATIME);
-	if (fd < 0)
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: Failed to open %s: %s\n",
+			path, strerror(errno));
 		return -errno;
+	}
 	close(fd);
 
 	mnttab = setmntent("/proc/self/mounts", "r");
-	if (!mnttab)
+	if (!mnttab) {
+		fprintf(stderr, "ERROR: Failed to setmntent: %s\n",
+			strerror(errno));
 		return -errno;
+	}
 
 	while ((ent = getmntent(mnttab))) {
 		len = strlen(ent->mnt_dir);
@@ -2457,8 +2463,11 @@ int find_mount_root(const char *path, char **mount_root)
 
 	ret = 0;
 	*mount_root = realpath(longest_match, NULL);
-	if (!*mount_root)
+	if (!*mount_root) {
+		fprintf(stderr, "Failed to resolve path %s: %s\n",
+			longest_match, strerror(errno));
 		ret = -errno;
+	}
 
 	free(longest_match);
 	return ret;
-- 
2.0.1

[PATCH 3/4] btrfs-progs: Fix wrong indent in btrfs-progs.

[Qu Wenruo]

When editing cmds-filesystem.c, I found cmd_filesystem_df() uses 7
spaces as indent instead of 1 tab (or 8 spaces). which makes indent
quite embarrassing.
Such problem is especillay hard to detect when reviewing patches,
since the leading '+' makes a tab only 7 spaces long, makeing 7 spaces
look the same with a tab.

This patch fixes all the 7 spaces indent.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 cmds-filesystem.c | 79 +++++++++++++++++++++++++++----------------------------
 ctree.h           | 15 ++++++-----
 utils.c           | 10 +++----
 3 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 4b2d27e..0a9b62a 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -114,23 +114,23 @@ static const char * const filesystem_cmd_group_usage[] = {
 };
 
 static const char * const cmd_filesystem_df_usage[] = {
-       "btrfs filesystem df <path>",
-       "Show space usage information for a mount point",
-       NULL
+	"btrfs filesystem df <path>",
+	"Show space usage information for a mount point",
+	NULL
 };
 
 static void print_df(struct btrfs_ioctl_space_args *sargs)
 {
-       u64 i;
-       struct btrfs_ioctl_space_info *sp = sargs->spaces;
-
-       for (i = 0; i < sargs->total_spaces; i++, sp++) {
-               printf("%s, %s: total=%s, used=%s\n",
-                       group_type_str(sp->flags),
-                       group_profile_str(sp->flags),
-                       pretty_size(sp->total_bytes),
-                       pretty_size(sp->used_bytes));
-       }
+	u64 i;
+	struct btrfs_ioctl_space_info *sp = sargs->spaces;
+
+	for (i = 0; i < sargs->total_spaces; i++, sp++) {
+		printf("%s, %s: total=%s, used=%s\n",
+		       group_type_str(sp->flags),
+		       group_profile_str(sp->flags),
+		       pretty_size(sp->total_bytes),
+		       pretty_size(sp->used_bytes));
+	}
 }
 
 static int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret)
@@ -183,33 +183,32 @@ static int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret)
 
 static int cmd_filesystem_df(int argc, char **argv)
 {
-       struct btrfs_ioctl_space_args *sargs = NULL;
-       int ret;
-       int fd;
-       char *path;
-       DIR *dirstream = NULL;
-
-       if (check_argc_exact(argc, 2))
-               usage(cmd_filesystem_df_usage);
-
-       path = argv[1];
-
-       fd = open_file_or_dir(path, &dirstream);
-       if (fd < 0) {
-               fprintf(stderr, "ERROR: can't access '%s'\n", path);
-               return 1;
-       }
-       ret = get_df(fd, &sargs);
-
-       if (!ret && sargs) {
-               print_df(sargs);
-               free(sargs);
-       } else {
-               fprintf(stderr, "ERROR: get_df failed %s\n", strerror(-ret));
-       }
-
-       close_file_or_dir(fd, dirstream);
-       return !!ret;
+	struct btrfs_ioctl_space_args *sargs = NULL;
+	int ret;
+	int fd;
+	char *path;
+	DIR *dirstream = NULL;
+
+	if (check_argc_exact(argc, 2))
+		usage(cmd_filesystem_df_usage);
+
+	path = argv[1];
+
+	fd = open_file_or_dir(path, &dirstream);
+	if (fd < 0) {
+		fprintf(stderr, "ERROR: can't access '%s'\n", path);
+		return 1;
+	}
+	ret = get_df(fd, &sargs);
+	if (!ret && sargs) {
+		print_df(sargs);
+		free(sargs);
+	} else {
+		fprintf(stderr, "ERROR: get_df failed %s\n", strerror(-ret));
+	}
+
+	close_file_or_dir(fd, dirstream);
+	return !!ret;
 }
 
 static int match_search_item_kernel(__u8 *fsid, char *mnt, char *label,
diff --git a/ctree.h b/ctree.h
index 35d3633..83d85b3 100644
--- a/ctree.h
+++ b/ctree.h
@@ -939,10 +939,10 @@ struct btrfs_block_group_cache {
 };
 
 struct btrfs_extent_ops {
-       int (*alloc_extent)(struct btrfs_root *root, u64 num_bytes,
-		           u64 hint_byte, struct btrfs_key *ins);
-       int (*free_extent)(struct btrfs_root *root, u64 bytenr,
-		          u64 num_bytes);
+	int (*alloc_extent)(struct btrfs_root *root, u64 num_bytes,
+			    u64 hint_byte, struct btrfs_key *ins);
+	int (*free_extent)(struct btrfs_root *root, u64 bytenr,
+			   u64 num_bytes);
 };
 
 struct btrfs_device;
@@ -2117,9 +2117,10 @@ BTRFS_SETGET_STACK_FUNCS(stack_qgroup_limit_rsv_exclusive,
 static inline u32 btrfs_file_extent_inline_item_len(struct extent_buffer *eb,
 						    struct btrfs_item *e)
 {
-       unsigned long offset;
-       offset = offsetof(struct btrfs_file_extent_item, disk_bytenr);
-       return btrfs_item_size(eb, e) - offset;
+	unsigned long offset;
+
+	offset = offsetof(struct btrfs_file_extent_item, disk_bytenr);
+	return btrfs_item_size(eb, e) - offset;
 }
 
 /* this returns the number of file bytes represented by the inline item.
diff --git a/utils.c b/utils.c
index 07173ee..356bdce 100644
--- a/utils.c
+++ b/utils.c
@@ -1485,15 +1485,15 @@ char *__strncpy__null(char *dest, const char *src, size_t n)
  */
 static int check_label(const char *input)
 {
-       int len = strlen(input);
+	int len = strlen(input);
 
-       if (len > BTRFS_LABEL_SIZE - 1) {
+	if (len > BTRFS_LABEL_SIZE - 1) {
 		fprintf(stderr, "ERROR: Label %s is too long (max %d)\n",
 			input, BTRFS_LABEL_SIZE - 1);
-               return -1;
-       }
+		return -1;
+	}
 
-       return 0;
+	return 0;
 }
 
 static int set_label_unmounted(const char *dev, const char *label)
-- 
2.0.1

[PATCH v2 RESEND 4/4] btrfs-progs: Add mount point output for 'btrfs fi df'

[Qu Wenruo]

Add mount point output for 'btrfs fi df'.
Also since the patch uses find_mount_root() to find mount point,
now 'btrfs fi df' can output more meaningful error message when given a
non-btrfs path.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
v2:
  Call realpath() before find_mount_root() to deal with relative path
---
 cmds-filesystem.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 0a9b62a..d76a1a5 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -187,12 +187,28 @@ static int cmd_filesystem_df(int argc, char **argv)
 	int ret;
 	int fd;
 	char *path;
+	char *real_path = NULL;
+	char *mount_point = NULL;
 	DIR *dirstream = NULL;
 
 	if (check_argc_exact(argc, 2))
 		usage(cmd_filesystem_df_usage);
 
 	path = argv[1];
+	real_path = realpath(path, NULL);
+	if (!real_path) {
+		fprintf(stderr, "ERROR: Failed to resolve real path for %s: %s\n",
+			path, strerror(errno));
+		return 1;
+	}
+	ret = find_mount_root(real_path, &mount_point);
+	if (ret < 0) {
+		free(real_path);
+		return 1;
+	}
+	printf("Mounted on: %s\n", mount_point);
+	free(real_path);
+	free(mount_point);
 
 	fd = open_file_or_dir(path, &dirstream);
 	if (fd < 0) {
-- 
2.0.1

Re: [PATCH 2/4] btrfs-progs: Integrate error message output into find_mount_root().

[Satoru Takeuchi]

Hi Qu,

(2014/07/10 12:05), Qu Wenruo wrote:
> Before this patch, find_mount_root() and the caller both output error
> message, which sometimes make the output duplicated and hard to judge
> what the problem is.
> 
> This pathh will integrate all the error messages output into
> find_mount_root() to give more meaning error prompt and remove the
> unneeded caller error messages.
> 
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>   cmds-receive.c   |  2 --
>   cmds-send.c      |  8 +-------
>   cmds-subvolume.c |  5 +----
>   utils.c          | 15 ++++++++++++---
>   4 files changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/cmds-receive.c b/cmds-receive.c
> index 48380a5..084d97d 100644
> --- a/cmds-receive.c
> +++ b/cmds-receive.c
> @@ -981,8 +981,6 @@ static int do_receive(struct btrfs_receive *r, const char *tomnt, int r_fd,
>   	ret = find_mount_root(dest_dir_full_path, &r->root_path);
>   	if (ret < 0) {
>   		ret = -EINVAL;
> -		fprintf(stderr, "ERROR: failed to determine mount point "
> -			"for %s\n", dest_dir_full_path);
>   		goto out;
>   	}
>   	r->mnt_fd = open(r->root_path, O_RDONLY | O_NOATIME);
> diff --git a/cmds-send.c b/cmds-send.c
> index 9a73b32..091f32b 100644
> --- a/cmds-send.c
> +++ b/cmds-send.c
> @@ -357,8 +357,6 @@ static int init_root_path(struct btrfs_send *s, const char *subvol)
>   	ret = find_mount_root(subvol, &s->root_path);
>   	if (ret < 0) {
>   		ret = -EINVAL;
> -		fprintf(stderr, "ERROR: failed to determine mount point "
> -				"for %s\n", subvol);
>   		goto out;
>   	}
>   
> @@ -622,12 +620,8 @@ int cmd_send(int argc, char **argv)
>   		}
>   
>   		ret = find_mount_root(subvol, &mount_root);
> -		if (ret < 0) {
> -			fprintf(stderr, "ERROR: find_mount_root failed on %s: "
> -					"%s\n", subvol,
> -				strerror(-ret));
> +		if (ret < 0)
>   			goto out;
> -		}
>   		if (strcmp(send.root_path, mount_root) != 0) {
>   			ret = -EINVAL;
>   			fprintf(stderr, "ERROR: all subvols must be from the "
> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
> index 639fb10..b252eab 100644
> --- a/cmds-subvolume.c
> +++ b/cmds-subvolume.c
> @@ -981,11 +981,8 @@ static int cmd_subvol_show(int argc, char **argv)
>   	}
>   
>   	ret = find_mount_root(fullpath, &mnt);
> -	if (ret < 0) {
> -		fprintf(stderr, "ERROR: find_mount_root failed on %s: "
> -				"%s\n", fullpath, strerror(-ret));
> +	if (ret < 0)
>   		goto out;
> -	}
>   	ret = 1;
>   	svpath = get_subvol_name(mnt, fullpath);
>   
> diff --git a/utils.c b/utils.c
> index 507ec6c..07173ee 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -2417,13 +2417,19 @@ int find_mount_root(const char *path, char **mount_root)
>   	char *longest_match = NULL;
>   
>   	fd = open(path, O_RDONLY | O_NOATIME);
> -	if (fd < 0)
> +	if (fd < 0) {
> +		fprintf(stderr, "ERROR: Failed to open %s: %s\n",
> +			path, strerror(errno));

It drops part of original messages. It doesn't show this error
is from find_mount_root(). I consider the original meaning keep as is.
How do you think?

Thanks,
Satoru

>   		return -errno;
> +	}
>   	close(fd);
>   
>   	mnttab = setmntent("/proc/self/mounts", "r");
> -	if (!mnttab)
> +	if (!mnttab) {
> +		fprintf(stderr, "ERROR: Failed to setmntent: %s\n",
> +			strerror(errno));
>   		return -errno;
> +	}
>   
>   	while ((ent = getmntent(mnttab))) {
>   		len = strlen(ent->mnt_dir);
> @@ -2457,8 +2463,11 @@ int find_mount_root(const char *path, char **mount_root)
>   
>   	ret = 0;
>   	*mount_root = realpath(longest_match, NULL);
> -	if (!*mount_root)
> +	if (!*mount_root) {
> +		fprintf(stderr, "Failed to resolve path %s: %s\n",
> +			longest_match, strerror(errno));
>   		ret = -errno;
> +	}
>   
>   	free(longest_match);
>   	return ret;
> 

Re: [PATCH 3/4] btrfs-progs: Fix wrong indent in btrfs-progs.

[Satoru Takeuchi]

(2014/07/10 12:05), Qu Wenruo wrote:
> When editing cmds-filesystem.c, I found cmd_filesystem_df() uses 7
> spaces as indent instead of 1 tab (or 8 spaces). which makes indent
> quite embarrassing.
> Such problem is especillay hard to detect when reviewing patches,
> since the leading '+' makes a tab only 7 spaces long, makeing 7 spaces
> look the same with a tab.
> 
> This patch fixes all the 7 spaces indent.
> 
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Reviewed-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>

> ---
>   cmds-filesystem.c | 79 +++++++++++++++++++++++++++----------------------------
>   ctree.h           | 15 ++++++-----
>   utils.c           | 10 +++----
>   3 files changed, 52 insertions(+), 52 deletions(-)
> 
> diff --git a/cmds-filesystem.c b/cmds-filesystem.c
> index 4b2d27e..0a9b62a 100644
> --- a/cmds-filesystem.c
> +++ b/cmds-filesystem.c
> @@ -114,23 +114,23 @@ static const char * const filesystem_cmd_group_usage[] = {
>   };
>   
>   static const char * const cmd_filesystem_df_usage[] = {
> -       "btrfs filesystem df <path>",
> -       "Show space usage information for a mount point",
> -       NULL
> +	"btrfs filesystem df <path>",
> +	"Show space usage information for a mount point",
> +	NULL
>   };
>   
>   static void print_df(struct btrfs_ioctl_space_args *sargs)
>   {
> -       u64 i;
> -       struct btrfs_ioctl_space_info *sp = sargs->spaces;
> -
> -       for (i = 0; i < sargs->total_spaces; i++, sp++) {
> -               printf("%s, %s: total=%s, used=%s\n",
> -                       group_type_str(sp->flags),
> -                       group_profile_str(sp->flags),
> -                       pretty_size(sp->total_bytes),
> -                       pretty_size(sp->used_bytes));
> -       }
> +	u64 i;
> +	struct btrfs_ioctl_space_info *sp = sargs->spaces;
> +
> +	for (i = 0; i < sargs->total_spaces; i++, sp++) {
> +		printf("%s, %s: total=%s, used=%s\n",
> +		       group_type_str(sp->flags),
> +		       group_profile_str(sp->flags),
> +		       pretty_size(sp->total_bytes),
> +		       pretty_size(sp->used_bytes));
> +	}
>   }
>   
>   static int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret)
> @@ -183,33 +183,32 @@ static int get_df(int fd, struct btrfs_ioctl_space_args **sargs_ret)
>   
>   static int cmd_filesystem_df(int argc, char **argv)
>   {
> -       struct btrfs_ioctl_space_args *sargs = NULL;
> -       int ret;
> -       int fd;
> -       char *path;
> -       DIR *dirstream = NULL;
> -
> -       if (check_argc_exact(argc, 2))
> -               usage(cmd_filesystem_df_usage);
> -
> -       path = argv[1];
> -
> -       fd = open_file_or_dir(path, &dirstream);
> -       if (fd < 0) {
> -               fprintf(stderr, "ERROR: can't access '%s'\n", path);
> -               return 1;
> -       }
> -       ret = get_df(fd, &sargs);
> -
> -       if (!ret && sargs) {
> -               print_df(sargs);
> -               free(sargs);
> -       } else {
> -               fprintf(stderr, "ERROR: get_df failed %s\n", strerror(-ret));
> -       }
> -
> -       close_file_or_dir(fd, dirstream);
> -       return !!ret;
> +	struct btrfs_ioctl_space_args *sargs = NULL;
> +	int ret;
> +	int fd;
> +	char *path;
> +	DIR *dirstream = NULL;
> +
> +	if (check_argc_exact(argc, 2))
> +		usage(cmd_filesystem_df_usage);
> +
> +	path = argv[1];
> +
> +	fd = open_file_or_dir(path, &dirstream);
> +	if (fd < 0) {
> +		fprintf(stderr, "ERROR: can't access '%s'\n", path);
> +		return 1;
> +	}
> +	ret = get_df(fd, &sargs);
> +	if (!ret && sargs) {
> +		print_df(sargs);
> +		free(sargs);
> +	} else {
> +		fprintf(stderr, "ERROR: get_df failed %s\n", strerror(-ret));
> +	}
> +
> +	close_file_or_dir(fd, dirstream);
> +	return !!ret;
>   }
>   
>   static int match_search_item_kernel(__u8 *fsid, char *mnt, char *label,
> diff --git a/ctree.h b/ctree.h
> index 35d3633..83d85b3 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -939,10 +939,10 @@ struct btrfs_block_group_cache {
>   };
>   
>   struct btrfs_extent_ops {
> -       int (*alloc_extent)(struct btrfs_root *root, u64 num_bytes,
> -		           u64 hint_byte, struct btrfs_key *ins);
> -       int (*free_extent)(struct btrfs_root *root, u64 bytenr,
> -		          u64 num_bytes);
> +	int (*alloc_extent)(struct btrfs_root *root, u64 num_bytes,
> +			    u64 hint_byte, struct btrfs_key *ins);
> +	int (*free_extent)(struct btrfs_root *root, u64 bytenr,
> +			   u64 num_bytes);
>   };
>   
>   struct btrfs_device;
> @@ -2117,9 +2117,10 @@ BTRFS_SETGET_STACK_FUNCS(stack_qgroup_limit_rsv_exclusive,
>   static inline u32 btrfs_file_extent_inline_item_len(struct extent_buffer *eb,
>   						    struct btrfs_item *e)
>   {
> -       unsigned long offset;
> -       offset = offsetof(struct btrfs_file_extent_item, disk_bytenr);
> -       return btrfs_item_size(eb, e) - offset;
> +	unsigned long offset;
> +
> +	offset = offsetof(struct btrfs_file_extent_item, disk_bytenr);
> +	return btrfs_item_size(eb, e) - offset;
>   }
>   
>   /* this returns the number of file bytes represented by the inline item.
> diff --git a/utils.c b/utils.c
> index 07173ee..356bdce 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1485,15 +1485,15 @@ char *__strncpy__null(char *dest, const char *src, size_t n)
>    */
>   static int check_label(const char *input)
>   {
> -       int len = strlen(input);
> +	int len = strlen(input);
>   
> -       if (len > BTRFS_LABEL_SIZE - 1) {
> +	if (len > BTRFS_LABEL_SIZE - 1) {
>   		fprintf(stderr, "ERROR: Label %s is too long (max %d)\n",
>   			input, BTRFS_LABEL_SIZE - 1);
> -               return -1;
> -       }
> +		return -1;
> +	}
>   
> -       return 0;
> +	return 0;
>   }
>   
>   static int set_label_unmounted(const char *dev, const char *label)
> 

Re: [PATCH 2/4] btrfs-progs: Integrate error message output into find_mount_root().

[Miao Xie]

Takeuchi-san

On Thu, 10 Jul 2014 16:33:23 +0900, Satoru Takeuchi wrote:
> (2014/07/10 12:05), Qu Wenruo wrote:
>> Before this patch, find_mount_root() and the caller both output error
>> message, which sometimes make the output duplicated and hard to judge
>> what the problem is.
>>
>> This pathh will integrate all the error messages output into
>> find_mount_root() to give more meaning error prompt and remove the
>> unneeded caller error messages.
>>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>>   cmds-receive.c   |  2 --
>>   cmds-send.c      |  8 +-------
>>   cmds-subvolume.c |  5 +----
>>   utils.c          | 15 ++++++++++++---
>>   4 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/cmds-receive.c b/cmds-receive.c
>> index 48380a5..084d97d 100644
>> --- a/cmds-receive.c
>> +++ b/cmds-receive.c
>> @@ -981,8 +981,6 @@ static int do_receive(struct btrfs_receive *r, const char *tomnt, int r_fd,
>>   	ret = find_mount_root(dest_dir_full_path, &r->root_path);
>>   	if (ret < 0) {
>>   		ret = -EINVAL;
>> -		fprintf(stderr, "ERROR: failed to determine mount point "
>> -			"for %s\n", dest_dir_full_path);
>>   		goto out;
>>   	}
>>   	r->mnt_fd = open(r->root_path, O_RDONLY | O_NOATIME);
>> diff --git a/cmds-send.c b/cmds-send.c
>> index 9a73b32..091f32b 100644
>> --- a/cmds-send.c
>> +++ b/cmds-send.c
>> @@ -357,8 +357,6 @@ static int init_root_path(struct btrfs_send *s, const char *subvol)
>>   	ret = find_mount_root(subvol, &s->root_path);
>>   	if (ret < 0) {
>>   		ret = -EINVAL;
>> -		fprintf(stderr, "ERROR: failed to determine mount point "
>> -				"for %s\n", subvol);
>>   		goto out;
>>   	}
>>   
>> @@ -622,12 +620,8 @@ int cmd_send(int argc, char **argv)
>>   		}
>>   
>>   		ret = find_mount_root(subvol, &mount_root);
>> -		if (ret < 0) {
>> -			fprintf(stderr, "ERROR: find_mount_root failed on %s: "
>> -					"%s\n", subvol,
>> -				strerror(-ret));
>> +		if (ret < 0)
>>   			goto out;
>> -		}
>>   		if (strcmp(send.root_path, mount_root) != 0) {
>>   			ret = -EINVAL;
>>   			fprintf(stderr, "ERROR: all subvols must be from the "
>> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
>> index 639fb10..b252eab 100644
>> --- a/cmds-subvolume.c
>> +++ b/cmds-subvolume.c
>> @@ -981,11 +981,8 @@ static int cmd_subvol_show(int argc, char **argv)
>>   	}
>>   
>>   	ret = find_mount_root(fullpath, &mnt);
>> -	if (ret < 0) {
>> -		fprintf(stderr, "ERROR: find_mount_root failed on %s: "
>> -				"%s\n", fullpath, strerror(-ret));
>> +	if (ret < 0)
>>   		goto out;
>> -	}
>>   	ret = 1;
>>   	svpath = get_subvol_name(mnt, fullpath);
>>   
>> diff --git a/utils.c b/utils.c
>> index 507ec6c..07173ee 100644
>> --- a/utils.c
>> +++ b/utils.c
>> @@ -2417,13 +2417,19 @@ int find_mount_root(const char *path, char **mount_root)
>>   	char *longest_match = NULL;
>>   
>>   	fd = open(path, O_RDONLY | O_NOATIME);
>> -	if (fd < 0)
>> +	if (fd < 0) {
>> +		fprintf(stderr, "ERROR: Failed to open %s: %s\n",
>> +			path, strerror(errno));
> 
> It drops part of original messages. It doesn't show this error
> is from find_mount_root(). I consider the original meaning keep as is.
> How do you think?

I think it is strange for the common users to show the name of a internal function.
Maybe we should introduce two kinds of the message, one is for the common users,
the other is for the developers to debug.

Thanks
Miao

> Thanks,
> Satoru
> 
>>   		return -errno;
>> +	}
>>   	close(fd);
>>   
>>   	mnttab = setmntent("/proc/self/mounts", "r");
>> -	if (!mnttab)
>> +	if (!mnttab) {
>> +		fprintf(stderr, "ERROR: Failed to setmntent: %s\n",
>> +			strerror(errno));
>>   		return -errno;
>> +	}
>>   
>>   	while ((ent = getmntent(mnttab))) {
>>   		len = strlen(ent->mnt_dir);
>> @@ -2457,8 +2463,11 @@ int find_mount_root(const char *path, char **mount_root)
>>   
>>   	ret = 0;
>>   	*mount_root = realpath(longest_match, NULL);
>> -	if (!*mount_root)
>> +	if (!*mount_root) {
>> +		fprintf(stderr, "Failed to resolve path %s: %s\n",
>> +			longest_match, strerror(errno));
>>   		ret = -errno;
>> +	}
>>   
>>   	free(longest_match);
>>   	return ret;
>>
> 
> --
> 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 2/4] btrfs-progs: Integrate error message output into find_mount_root().

[Qu Wenruo]

-------- Original Message --------
Subject: Re: [PATCH 2/4] btrfs-progs: Integrate error message output 
into find_mount_root().
From: Miao Xie <miaox@cn.fujitsu.com>
To: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>, Qu Wenruo 
<quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
Date: 2014年07月10日 16:10
> Takeuchi-san
>
> On Thu, 10 Jul 2014 16:33:23 +0900, Satoru Takeuchi wrote:
>> (2014/07/10 12:05), Qu Wenruo wrote:
>>> Before this patch, find_mount_root() and the caller both output error
>>> message, which sometimes make the output duplicated and hard to judge
>>> what the problem is.
>>>
>>> This pathh will integrate all the error messages output into
>>> find_mount_root() to give more meaning error prompt and remove the
>>> unneeded caller error messages.
>>>
>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>> ---
>>>    cmds-receive.c   |  2 --
>>>    cmds-send.c      |  8 +-------
>>>    cmds-subvolume.c |  5 +----
>>>    utils.c          | 15 ++++++++++++---
>>>    4 files changed, 14 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/cmds-receive.c b/cmds-receive.c
>>> index 48380a5..084d97d 100644
>>> --- a/cmds-receive.c
>>> +++ b/cmds-receive.c
>>> @@ -981,8 +981,6 @@ static int do_receive(struct btrfs_receive *r, const char *tomnt, int r_fd,
>>>    	ret = find_mount_root(dest_dir_full_path, &r->root_path);
>>>    	if (ret < 0) {
>>>    		ret = -EINVAL;
>>> -		fprintf(stderr, "ERROR: failed to determine mount point "
>>> -			"for %s\n", dest_dir_full_path);
>>>    		goto out;
>>>    	}
>>>    	r->mnt_fd = open(r->root_path, O_RDONLY | O_NOATIME);
>>> diff --git a/cmds-send.c b/cmds-send.c
>>> index 9a73b32..091f32b 100644
>>> --- a/cmds-send.c
>>> +++ b/cmds-send.c
>>> @@ -357,8 +357,6 @@ static int init_root_path(struct btrfs_send *s, const char *subvol)
>>>    	ret = find_mount_root(subvol, &s->root_path);
>>>    	if (ret < 0) {
>>>    		ret = -EINVAL;
>>> -		fprintf(stderr, "ERROR: failed to determine mount point "
>>> -				"for %s\n", subvol);
>>>    		goto out;
>>>    	}
>>>    
>>> @@ -622,12 +620,8 @@ int cmd_send(int argc, char **argv)
>>>    		}
>>>    
>>>    		ret = find_mount_root(subvol, &mount_root);
>>> -		if (ret < 0) {
>>> -			fprintf(stderr, "ERROR: find_mount_root failed on %s: "
>>> -					"%s\n", subvol,
>>> -				strerror(-ret));
>>> +		if (ret < 0)
>>>    			goto out;
>>> -		}
>>>    		if (strcmp(send.root_path, mount_root) != 0) {
>>>    			ret = -EINVAL;
>>>    			fprintf(stderr, "ERROR: all subvols must be from the "
>>> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
>>> index 639fb10..b252eab 100644
>>> --- a/cmds-subvolume.c
>>> +++ b/cmds-subvolume.c
>>> @@ -981,11 +981,8 @@ static int cmd_subvol_show(int argc, char **argv)
>>>    	}
>>>    
>>>    	ret = find_mount_root(fullpath, &mnt);
>>> -	if (ret < 0) {
>>> -		fprintf(stderr, "ERROR: find_mount_root failed on %s: "
>>> -				"%s\n", fullpath, strerror(-ret));
>>> +	if (ret < 0)
>>>    		goto out;
>>> -	}
>>>    	ret = 1;
>>>    	svpath = get_subvol_name(mnt, fullpath);
>>>    
>>> diff --git a/utils.c b/utils.c
>>> index 507ec6c..07173ee 100644
>>> --- a/utils.c
>>> +++ b/utils.c
>>> @@ -2417,13 +2417,19 @@ int find_mount_root(const char *path, char **mount_root)
>>>    	char *longest_match = NULL;
>>>    
>>>    	fd = open(path, O_RDONLY | O_NOATIME);
>>> -	if (fd < 0)
>>> +	if (fd < 0) {
>>> +		fprintf(stderr, "ERROR: Failed to open %s: %s\n",
>>> +			path, strerror(errno));
>> It drops part of original messages. It doesn't show this error
>> is from find_mount_root(). I consider the original meaning keep as is.
>> How do you think?
> I think it is strange for the common users to show the name of a internal function.
> Maybe we should introduce two kinds of the message, one is for the common users,
> the other is for the developers to debug.
>
> Thanks
> Miao
I agree with Miao's idea.
It's true that some developers needs to get info from the output,
but IMO the error messages are often used to indicate what *users* do wrong,
since most problem is caused by wrong parameter given by users.

For example, I always forget to run 'btrfs fi df /mnt' and the 
'Operation not permiited' message
makes me realize the permission problem. And the function name or other 
messages are less
important than that.

On the other hand, if developers encounter problems, they will gdb the 
program or grep the source
to find out the problem. So function name in error message seems not so 
demanding for me.

It would also be a greate idea for adding new frame work to show debug 
message,
but I'd prefer to make the frame some times later(Maybe when btrfs-progs 
become more comlicated than current?)

Thanks,
Qu


>
>> Thanks,
>> Satoru
>>
>>>    		return -errno;
>>> +	}
>>>    	close(fd);
>>>    
>>>    	mnttab = setmntent("/proc/self/mounts", "r");
>>> -	if (!mnttab)
>>> +	if (!mnttab) {
>>> +		fprintf(stderr, "ERROR: Failed to setmntent: %s\n",
>>> +			strerror(errno));
>>>    		return -errno;
>>> +	}
>>>    
>>>    	while ((ent = getmntent(mnttab))) {
>>>    		len = strlen(ent->mnt_dir);
>>> @@ -2457,8 +2463,11 @@ int find_mount_root(const char *path, char **mount_root)
>>>    
>>>    	ret = 0;
>>>    	*mount_root = realpath(longest_match, NULL);
>>> -	if (!*mount_root)
>>> +	if (!*mount_root) {
>>> +		fprintf(stderr, "Failed to resolve path %s: %s\n",
>>> +			longest_match, strerror(errno));
>>>    		ret = -errno;
>>> +	}
>>>    
>>>    	free(longest_match);
>>>    	return ret;
>>>
>> --
>> 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 RESEND 1/4] btrfs-progs: Check fstype in find_mount_root()

[Martin Steigerwald]

Am Donnerstag, 10. Juli 2014, 11:05:10 schrieb Qu Wenruo:
> When calling find_mount_root(), caller in fact wants to find the mount
> point of *BTRFS*.
> 
> So also check ent->fstype in find_mount_root() and output proper error
> messages if needed.
> This will suppress a lot of "Inapproiate ioctl for device" error
> message.
> 
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
>  utils.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/utils.c b/utils.c
> index 993d085..507ec6c 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -2412,6 +2412,7 @@ int find_mount_root(const char *path, char
> **mount_root) struct mntent *ent;
>  	int len;
>  	int ret;
> +	int not_btrfs;
>  	int longest_matchlen = 0;
>  	char *longest_match = NULL;
> 
> @@ -2432,6 +2433,10 @@ int find_mount_root(const char *path, char
> **mount_root) free(longest_match);
>  				longest_matchlen = len;
>  				longest_match = strdup(ent->mnt_dir);
> +				if (strcmp(ent->mnt_type, "btrfs"))
> +					not_btrfs = 1;
> +				else
> +					not_btrfs = 0;
>  			}
>  		}
>  	}
> @@ -2443,6 +2448,12 @@ int find_mount_root(const char *path, char
> **mount_root) path);
>  		return -ENOENT;
>  	}
> +	if (not_btrfs) {
> +		fprintf(stderr,
> +			"ERROR: %s does not belong to a btrfs mount points.\n",

Just a typo: mount point

> +			path);
> +		return -EINVAL;
> +	}
> 
>  	ret = 0;
>  	*mount_root = realpath(longest_match, NULL);

Thanks,
-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7

Re: [PATCH 2/4] btrfs-progs: Integrate error message output into find_mount_root().

[Satoru Takeuchi]

(2014/07/10 17:26), Qu Wenruo wrote:
>
> -------- Original Message --------
> Subject: Re: [PATCH 2/4] btrfs-progs: Integrate error message output into find_mount_root().
> From: Miao Xie <miaox@cn.fujitsu.com>
> To: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>, Qu Wenruo <quwenruo@cn.fujitsu.com>, linux-btrfs@vger.kernel.org
> Date: 2014年07月10日 16:10
>> Takeuchi-san
>>
>> On Thu, 10 Jul 2014 16:33:23 +0900, Satoru Takeuchi wrote:
>>> (2014/07/10 12:05), Qu Wenruo wrote:
>>>> Before this patch, find_mount_root() and the caller both output error
>>>> message, which sometimes make the output duplicated and hard to judge
>>>> what the problem is.
>>>>
>>>> This pathh will integrate all the error messages output into
>>>> find_mount_root() to give more meaning error prompt and remove the
>>>> unneeded caller error messages.
>>>>
>>>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>>>> ---
>>>>    cmds-receive.c   |  2 --
>>>>    cmds-send.c      |  8 +-------
>>>>    cmds-subvolume.c |  5 +----
>>>>    utils.c          | 15 ++++++++++++---
>>>>    4 files changed, 14 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/cmds-receive.c b/cmds-receive.c
>>>> index 48380a5..084d97d 100644
>>>> --- a/cmds-receive.c
>>>> +++ b/cmds-receive.c
>>>> @@ -981,8 +981,6 @@ static int do_receive(struct btrfs_receive *r, const char *tomnt, int r_fd,
>>>>        ret = find_mount_root(dest_dir_full_path, &r->root_path);
>>>>        if (ret < 0) {
>>>>            ret = -EINVAL;
>>>> -        fprintf(stderr, "ERROR: failed to determine mount point "
>>>> -            "for %s\n", dest_dir_full_path);
>>>>            goto out;
>>>>        }
>>>>        r->mnt_fd = open(r->root_path, O_RDONLY | O_NOATIME);
>>>> diff --git a/cmds-send.c b/cmds-send.c
>>>> index 9a73b32..091f32b 100644
>>>> --- a/cmds-send.c
>>>> +++ b/cmds-send.c
>>>> @@ -357,8 +357,6 @@ static int init_root_path(struct btrfs_send *s, const char *subvol)
>>>>        ret = find_mount_root(subvol, &s->root_path);
>>>>        if (ret < 0) {
>>>>            ret = -EINVAL;
>>>> -        fprintf(stderr, "ERROR: failed to determine mount point "
>>>> -                "for %s\n", subvol);
>>>>            goto out;
>>>>        }
>>>> @@ -622,12 +620,8 @@ int cmd_send(int argc, char **argv)
>>>>            }
>>>>            ret = find_mount_root(subvol, &mount_root);
>>>> -        if (ret < 0) {
>>>> -            fprintf(stderr, "ERROR: find_mount_root failed on %s: "
>>>> -                    "%s\n", subvol,
>>>> -                strerror(-ret));
>>>> +        if (ret < 0)
>>>>                goto out;
>>>> -        }
>>>>            if (strcmp(send.root_path, mount_root) != 0) {
>>>>                ret = -EINVAL;
>>>>                fprintf(stderr, "ERROR: all subvols must be from the "
>>>> diff --git a/cmds-subvolume.c b/cmds-subvolume.c
>>>> index 639fb10..b252eab 100644
>>>> --- a/cmds-subvolume.c
>>>> +++ b/cmds-subvolume.c
>>>> @@ -981,11 +981,8 @@ static int cmd_subvol_show(int argc, char **argv)
>>>>        }
>>>>        ret = find_mount_root(fullpath, &mnt);
>>>> -    if (ret < 0) {
>>>> -        fprintf(stderr, "ERROR: find_mount_root failed on %s: "
>>>> -                "%s\n", fullpath, strerror(-ret));
>>>> +    if (ret < 0)
>>>>            goto out;
>>>> -    }
>>>>        ret = 1;
>>>>        svpath = get_subvol_name(mnt, fullpath);
>>>> diff --git a/utils.c b/utils.c
>>>> index 507ec6c..07173ee 100644
>>>> --- a/utils.c
>>>> +++ b/utils.c
>>>> @@ -2417,13 +2417,19 @@ int find_mount_root(const char *path, char **mount_root)
>>>>        char *longest_match = NULL;
>>>>        fd = open(path, O_RDONLY | O_NOATIME);
>>>> -    if (fd < 0)
>>>> +    if (fd < 0) {
>>>> +        fprintf(stderr, "ERROR: Failed to open %s: %s\n",
>>>> +            path, strerror(errno));
>>> It drops part of original messages. It doesn't show this error
>>> is from find_mount_root(). I consider the original meaning keep as is.
>>> How do you think?
>> I think it is strange for the common users to show the name of a internal function.
>> Maybe we should introduce two kinds of the message, one is for the common users,
>> the other is for the developers to debug.
>>
>> Thanks
>> Miao
> I agree with Miao's idea.
> It's true that some developers needs to get info from the output,
> but IMO the error messages are often used to indicate what *users* do wrong,
> since most problem is caused by wrong parameter given by users.
>
> For example, I always forget to run 'btrfs fi df /mnt' and the 'Operation not permiited' message
> makes me realize the permission problem. And the function name or other messages are less
> important than that.
>
> On the other hand, if developers encounter problems, they will gdb the program or grep the source
> to find out the problem. So function name in error message seems not so demanding for me.

OK, I got it.

Reviewed-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>

>
> It would also be a greate idea for adding new frame work to show debug message,
> but I'd prefer to make the frame some times later(Maybe when btrfs-progs become more comlicated than current?)

It's nice. I consider the messages of btrfs-progs are a bit messy.

Satoru

>
> Thanks,
> Qu
>
>
>>
>>> Thanks,
>>> Satoru
>>>
>>>>            return -errno;
>>>> +    }
>>>>        close(fd);
>>>>        mnttab = setmntent("/proc/self/mounts", "r");
>>>> -    if (!mnttab)
>>>> +    if (!mnttab) {
>>>> +        fprintf(stderr, "ERROR: Failed to setmntent: %s\n",
>>>> +            strerror(errno));
>>>>            return -errno;
>>>> +    }
>>>>        while ((ent = getmntent(mnttab))) {
>>>>            len = strlen(ent->mnt_dir);
>>>> @@ -2457,8 +2463,11 @@ int find_mount_root(const char *path, char **mount_root)
>>>>        ret = 0;
>>>>        *mount_root = realpath(longest_match, NULL);
>>>> -    if (!*mount_root)
>>>> +    if (!*mount_root) {
>>>> +        fprintf(stderr, "Failed to resolve path %s: %s\n",
>>>> +            longest_match, strerror(errno));
>>>>            ret = -errno;
>>>> +    }
>>>>        free(longest_match);
>>>>        return ret;
>>>>
>>> --
>>> 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 RESEND 1/4] btrfs-progs: Check fstype in find_mount_root()

[David Sterba]

On Thu, Jul 10, 2014 at 11:05:10AM +0800, Qu Wenruo wrote:
> When calling find_mount_root(), caller in fact wants to find the mount
> point of *BTRFS*.
> 
> So also check ent->fstype in find_mount_root() and output proper error
> messages if needed.

The utils.c functions should be mostly silent about the errors as this
this the common code and it's up to the callers to print the messages.
The existing printf in find_mount_root had appeared before the function
was moved to utils.c.

> This will suppress a lot of "Inapproiate ioctl for device" error
> message.

Catching the error early is a good thing of course.

Re: [PATCH RESEND 1/4] btrfs-progs: Check fstype in find_mount_root()

[Qu Wenruo]

David, thanks for all the comments about the 'fi di' related patchset.

-------- Original Message --------
Subject: Re: [PATCH RESEND 1/4] btrfs-progs: Check fstype in 
find_mount_root()
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2014年07月23日 03:15
> On Thu, Jul 10, 2014 at 11:05:10AM +0800, Qu Wenruo wrote:
>> When calling find_mount_root(), caller in fact wants to find the mount
>> point of *BTRFS*.
>>
>> So also check ent->fstype in find_mount_root() and output proper error
>> messages if needed.
> The utils.c functions should be mostly silent about the errors as this
> this the common code and it's up to the callers to print the messages.
> The existing printf in find_mount_root had appeared before the function
> was moved to utils.c.
Thanks for the info about convention in utils.c
I'll update the patch and remove the printf from the original codes and 
my patches.

>
>> This will suppress a lot of "Inapproiate ioctl for device" error
>> message.
> Catching the error early is a good thing of course.
BTW, I did not see the patchset the latest integration branch, so after 
all the update about the patchset,
should I resend the patchset rebased on the latest integration branch?

Thanks,
Qu.

Re: [PATCH 3/4] btrfs-progs: Fix wrong indent in btrfs-progs.

[David Sterba]

On Thu, Jul 10, 2014 at 11:05:12AM +0800, Qu Wenruo wrote:
> When editing cmds-filesystem.c, I found cmd_filesystem_df() uses 7
> spaces as indent instead of 1 tab (or 8 spaces). which makes indent
> quite embarrassing.
> Such problem is especillay hard to detect when reviewing patches,
> since the leading '+' makes a tab only 7 spaces long, makeing 7 spaces
> look the same with a tab.
> 
> This patch fixes all the 7 spaces indent.

The whitespace changes cause patch conflicts and I'm inclined not to add
them or postpone after the patch queue is relatively calm.