[PATCH v2 0/3] btrfs-progs: Some fix for fi usage

[PATCH v2 0/3] btrfs-progs: Some fix for fi usage

[Misono Tomohiro]

v1->v2
  These were sent several months ago, just rebased to current devel branch.

Patch 1 and 2 aims to fix the "fi du" to include the information of "fi df"
even when running without root privilege.

Patch 3 is an independent cleanup.

Tomohiro Misono (3):
  btrfs-progs: fi usage: change warning message more appropriately
  btrfs-progs: fi usage: change to output more info without root privilege
  btrfs-progs: fi usage: cleanup unnecessary permission error check

 cmds-fi-usage.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

-- 
2.14.3

[PATCH v2 1/3] btrfs-progs: fi usage: change warning message more appropriately

[Misono Tomohiro]

"fi usage" shows the warning "RAID5/6 numbers will be incorrect" when
running without root privilege even if raid5/6 is not used.  What
happens is it cannot get the per device profile usage info, so change
the message more appropriately.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
 cmds-fi-usage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
index de7ad668..f2307fe0 100644
--- a/cmds-fi-usage.c
+++ b/cmds-fi-usage.c
@@ -629,7 +629,7 @@ int load_chunk_and_device_info(int fd, struct chunk_info **chunkinfo,
 	ret = load_chunk_info(fd, chunkinfo, chunkcount);
 	if (ret == -EPERM) {
 		warning(
-"cannot read detailed chunk info, RAID5/6 numbers will be incorrect, run as root");
+"cannot read detailed chunk info. per device usage will not be shown, run as root");
 	} else if (ret) {
 		return ret;
 	}
-- 
2.14.3

[PATCH v2 2/3] btrfs-progs: fi usage: change to output more info without root privilege

[Misono Tomohiro]

Although per device usage cannot be shown without root privilege,
per profile usage can be shown.

To achieve this, we just basically need to remove the check of nullness
of chunkinfo in print_filesystem_usage_by_chunk(), because other
functions except print_unused() properly handles chunkinfo by
chunkcount, which is 0 if chunkinfo is null.

As a result, "fi usage" always includes the information of "fi df".

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
 cmds-fi-usage.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
index f2307fe0..2d45b3bb 100644
--- a/cmds-fi-usage.c
+++ b/cmds-fi-usage.c
@@ -923,9 +923,11 @@ static void _cmd_filesystem_usage_linear(unsigned unit_mode,
 		printf("\n");
 	}
 
-	printf("Unallocated:\n");
-	print_unused(info_ptr, info_count, device_info_ptr, device_info_count,
-			unit_mode | UNITS_NEGATIVE);
+	if (info_count) {
+		printf("Unallocated:\n");
+		print_unused(info_ptr, info_count, device_info_ptr,
+				device_info_count, unit_mode | UNITS_NEGATIVE);
+	}
 }
 
 static int print_filesystem_usage_by_chunk(int fd,
@@ -936,9 +938,6 @@ static int print_filesystem_usage_by_chunk(int fd,
 	struct btrfs_ioctl_space_args *sargs;
 	int ret = 0;
 
-	if (!chunkinfo)
-		return 0;
-
 	sargs = load_space_info(fd, path);
 	if (!sargs) {
 		ret = 1;
-- 
2.14.3

[PATCH v2 3/3] btrfs-progs: fi usage: cleanup unnecessary permission error check

[Misono Tomohiro]

Since BTRFS_IOC_FS_INFO does not require root privilege, there is no
need to check EPERM error.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
 cmds-fi-usage.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
index 2d45b3bb..5ce87a37 100644
--- a/cmds-fi-usage.c
+++ b/cmds-fi-usage.c
@@ -549,8 +549,6 @@ static int load_device_info(int fd, struct device_info **device_info_ptr,
 
 	ret = ioctl(fd, BTRFS_IOC_FS_INFO, &fi_args);
 	if (ret < 0) {
-		if (errno == EPERM)
-			return -errno;
 		error("cannot get filesystem info: %m");
 		return 1;
 	}
@@ -635,11 +633,6 @@ int load_chunk_and_device_info(int fd, struct chunk_info **chunkinfo,
 	}
 
 	ret = load_device_info(fd, devinfo, devcount);
-	if (ret == -EPERM) {
-		warning(
-		"cannot get filesystem info from ioctl(FS_INFO), run as root");
-		ret = 0;
-	}
 
 	return ret;
 }
-- 
2.14.3

Re: [PATCH v2 1/3] btrfs-progs: fi usage: change warning message more appropriately

[Nikolay Borisov]

On 29.03.2018 11:22, Misono Tomohiro wrote:
> "fi usage" shows the warning "RAID5/6 numbers will be incorrect" when
> running without root privilege even if raid5/6 is not used.  What
> happens is it cannot get the per device profile usage info, so change
> the message more appropriately.
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>
> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
> ---
>  cmds-fi-usage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmds-fi-usage.c b/cmds-fi-usage.c
> index de7ad668..f2307fe0 100644
> --- a/cmds-fi-usage.c
> +++ b/cmds-fi-usage.c
> @@ -629,7 +629,7 @@ int load_chunk_and_device_info(int fd, struct chunk_info **chunkinfo,
>  	ret = load_chunk_info(fd, chunkinfo, chunkcount);
>  	if (ret == -EPERM) {
>  		warning(
> -"cannot read detailed chunk info, RAID5/6 numbers will be incorrect, run as root");
> +"cannot read detailed chunk info. per device usage will not be shown, run as root");
                                    ^^^
nit: New sentences must start with a capital letter :)
>  	} else if (ret) {
>  		return ret;
>  	}
> 

Re: [PATCH v2 3/3] btrfs-progs: fi usage: cleanup unnecessary permission error check

[David Sterba]

On Thu, Mar 29, 2018 at 05:23:39PM +0900, Misono Tomohiro wrote:
> Since BTRFS_IOC_FS_INFO does not require root privilege, there is no
> need to check EPERM error.

Well, this has been changed in 3.16, but there's still 3.2 longterm
kernel. It does not hurt to have the check and verbose message so we may
need to keep it for a few more years.

Re: [PATCH v2 0/3] btrfs-progs: Some fix for fi usage

[David Sterba]

On Thu, Mar 29, 2018 at 05:21:29PM +0900, Misono Tomohiro wrote:
> v1->v2
>   These were sent several months ago, just rebased to current devel branch.
> 
> Patch 1 and 2 aims to fix the "fi du" to include the information of "fi df"
> even when running without root privilege.
> 
> Patch 3 is an independent cleanup.
> 
> Tomohiro Misono (3):
>   btrfs-progs: fi usage: change warning message more appropriately
>   btrfs-progs: fi usage: change to output more info without root privilege
>   btrfs-progs: fi usage: cleanup unnecessary permission error check

1-2 applied, thanks.