[PATCH] btrfs-progs: Make "btrfs filesystem df" command to show upper case profile

[PATCH] btrfs-progs: Make "btrfs filesystem df" command to show upper case profile

[Qu Wenruo]

[BUG]
Since commit dad03fac3bb8 ("btrfs-progs: switch btrfs_group_profile_str
to use raid table"), fstests/btrfs/023 and btrfs/151 will always fail.

The failure of btrfs/151 explains the reason pretty well:

btrfs/151 1s ... - output mismatch
    --- tests/btrfs/151.out	2019-10-22 15:18:14.068965341 +0800
    +++ ~/xfstests-dev/results//btrfs/151.out.bad	2021-11-02 17:13:43.879999994 +0800
    @@ -1,2 +1,2 @@
     QA output created by 151
    -Data, RAID1
    +Data, raid1
    ...
    (Run 'diff -u ~/xfstests-dev/tests/btrfs/151.out ~/xfstests-dev/results//btrfs/151.out.bad'  to see the entire diff)

[CAUSE]
Commit dad03fac3bb8 ("btrfs-progs: switch btrfs_group_profile_str to use
raid table") will use btrfs_raid_array[index].raid_name, which is all
lower case.

[FIX]
There is no need to bring such output format change.

So here we adds a new helper function, btrfs_group_profile_upper_str()
to print the upper case profile name.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds/filesystem.c |  4 +++-
 common/utils.c    | 10 ++++++++++
 common/utils.h    |  3 +++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 6a9e46d2b7dc..9f49b7d0c9c5 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -72,6 +72,7 @@ static void print_df(int fd, struct btrfs_ioctl_space_args *sargs, unsigned unit
 {
 	u64 i;
 	struct btrfs_ioctl_space_info *sp = sargs->spaces;
+	char profile_buf[BTRFS_PROFILE_STR_LEN];
 	u64 unusable;
 	bool ok;
 
@@ -79,9 +80,10 @@ static void print_df(int fd, struct btrfs_ioctl_space_args *sargs, unsigned unit
 		unusable = device_get_zone_unusable(fd, sp->flags);
 		ok = (unusable != DEVICE_ZONE_UNUSABLE_UNKNOWN);
 
+		btrfs_group_profile_upper_str(sp->flags, profile_buf);
 		printf("%s, %s: total=%s, used=%s%s%s\n",
 			btrfs_group_type_str(sp->flags),
-			btrfs_group_profile_str(sp->flags),
+			profile_buf,
 			pretty_size_mode(sp->total_bytes, unit_mode),
 			pretty_size_mode(sp->used_bytes, unit_mode),
 			(ok ? ", zone_unusable=" : ""),
diff --git a/common/utils.c b/common/utils.c
index aee0eedc15fc..32ca6b2ef432 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -1038,6 +1038,16 @@ const char* btrfs_group_profile_str(u64 flag)
 	return btrfs_raid_array[index].raid_name;
 }
 
+void btrfs_group_profile_upper_str(u64 flags, char *ret)
+{
+	int i;
+
+	strncpy(ret, btrfs_group_profile_str(flags), BTRFS_PROFILE_STR_LEN);
+
+	for (i = 0; i < BTRFS_PROFILE_STR_LEN && ret[i]; i++)
+		ret[i] = toupper(ret[i]);
+}
+
 u64 div_factor(u64 num, int factor)
 {
 	if (factor == 10)
diff --git a/common/utils.h b/common/utils.h
index 6f84e3cbc98f..0c1b6baa7ae3 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -75,6 +75,9 @@ int find_next_key(struct btrfs_path *path, struct btrfs_key *key);
 const char* btrfs_group_type_str(u64 flag);
 const char* btrfs_group_profile_str(u64 flag);
 
+#define BTRFS_PROFILE_STR_LEN	(64)
+void btrfs_group_profile_upper_str(u64 flag, char *ret);
+
 int count_digits(u64 num);
 u64 div_factor(u64 num, int factor);
 
-- 
2.33.1

Re: [PATCH] btrfs-progs: Make "btrfs filesystem df" command to show upper case profile

[Su Yue]

On Tue 02 Nov 2021 at 18:47, Qu Wenruo <wqu@suse.com> wrote:

> [BUG]
> Since commit dad03fac3bb8 ("btrfs-progs: switch 
> btrfs_group_profile_str
> to use raid table"), fstests/btrfs/023 and btrfs/151 will always 
> fail.
>
> The failure of btrfs/151 explains the reason pretty well:
>
> btrfs/151 1s ... - output mismatch
>     --- tests/btrfs/151.out	2019-10-22 15:18:14.068965341 
>     +0800
>     +++ ~/xfstests-dev/results//btrfs/151.out.bad	2021-11-02 
>     17:13:43.879999994 +0800
>     @@ -1,2 +1,2 @@
>      QA output created by 151
>     -Data, RAID1
>     +Data, raid1
>     ...
>     (Run 'diff -u ~/xfstests-dev/tests/btrfs/151.out 
>     ~/xfstests-dev/results//btrfs/151.out.bad'  to see the 
>     entire diff)
>
> [CAUSE]
> Commit dad03fac3bb8 ("btrfs-progs: switch 
> btrfs_group_profile_str to use
> raid table") will use btrfs_raid_array[index].raid_name, which 
> is all
> lower case.
>
> [FIX]
> There is no need to bring such output format change.
>
> So here we adds a new helper function, 
> btrfs_group_profile_upper_str()
> to print the upper case profile name.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  cmds/filesystem.c |  4 +++-
>  common/utils.c    | 10 ++++++++++
>  common/utils.h    |  3 +++
>  3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> index 6a9e46d2b7dc..9f49b7d0c9c5 100644
> --- a/cmds/filesystem.c
> +++ b/cmds/filesystem.c
> @@ -72,6 +72,7 @@ static void print_df(int fd, struct 
> btrfs_ioctl_space_args *sargs, unsigned unit
>  {
>  	u64 i;
>  	struct btrfs_ioctl_space_info *sp = sargs->spaces;
> +	char profile_buf[BTRFS_PROFILE_STR_LEN];
>  	u64 unusable;
>  	bool ok;
>
> @@ -79,9 +80,10 @@ static void print_df(int fd, struct 
> btrfs_ioctl_space_args *sargs, unsigned unit
>  		unusable = device_get_zone_unusable(fd, sp->flags);
>  		ok = (unusable != DEVICE_ZONE_UNUSABLE_UNKNOWN);
>
> +		btrfs_group_profile_upper_str(sp->flags, profile_buf);
>  		printf("%s, %s: total=%s, used=%s%s%s\n",
>  			btrfs_group_type_str(sp->flags),
> -			btrfs_group_profile_str(sp->flags),
> +			profile_buf,
>  			pretty_size_mode(sp->total_bytes, unit_mode),
>  			pretty_size_mode(sp->used_bytes, unit_mode),
>  			(ok ? ", zone_unusable=" : ""),
> diff --git a/common/utils.c b/common/utils.c
> index aee0eedc15fc..32ca6b2ef432 100644
> --- a/common/utils.c
> +++ b/common/utils.c
> @@ -1038,6 +1038,16 @@ const char* btrfs_group_profile_str(u64 
> flag)
>  	return btrfs_raid_array[index].raid_name;
>  }
>
> +void btrfs_group_profile_upper_str(u64 flags, char *ret)
> +{
> +	int i;
> +
> +	strncpy(ret, btrfs_group_profile_str(flags), 
> BTRFS_PROFILE_STR_LEN);
> +
> +	for (i = 0; i < BTRFS_PROFILE_STR_LEN && ret[i]; i++)
> +		ret[i] = toupper(ret[i]);
> +}
> +
>  u64 div_factor(u64 num, int factor)
>  {
>  	if (factor == 10)
> diff --git a/common/utils.h b/common/utils.h
> index 6f84e3cbc98f..0c1b6baa7ae3 100644
> --- a/common/utils.h
> +++ b/common/utils.h
> @@ -75,6 +75,9 @@ int find_next_key(struct btrfs_path *path, 
> struct btrfs_key *key);
>  const char* btrfs_group_type_str(u64 flag);
>  const char* btrfs_group_profile_str(u64 flag);
>
> +#define BTRFS_PROFILE_STR_LEN	(64)
>
May it be 8?
struct btrfs_raid_attr {
...
       const char raid_name[8];
...
};

--
Su
> +void btrfs_group_profile_upper_str(u64 flag, char *ret);
> +
>  int count_digits(u64 num);
>  u64 div_factor(u64 num, int factor);

Re: [PATCH] btrfs-progs: Make "btrfs filesystem df" command to show upper case profile

[Qu Wenruo]

On 2021/11/2 19:52, Su Yue wrote:
> 
> On Tue 02 Nov 2021 at 18:47, Qu Wenruo <wqu@suse.com> wrote:
> 
>> [BUG]
>> Since commit dad03fac3bb8 ("btrfs-progs: switch btrfs_group_profile_str
>> to use raid table"), fstests/btrfs/023 and btrfs/151 will always fail.
>>
>> The failure of btrfs/151 explains the reason pretty well:
>>
>> btrfs/151 1s ... - output mismatch
>>     --- tests/btrfs/151.out    2019-10-22 15:18:14.068965341     +0800
>>     +++ ~/xfstests-dev/results//btrfs/151.out.bad    2021-11-02     
>> 17:13:43.879999994 +0800
>>     @@ -1,2 +1,2 @@
>>      QA output created by 151
>>     -Data, RAID1
>>     +Data, raid1
>>     ...
>>     (Run 'diff -u ~/xfstests-dev/tests/btrfs/151.out     
>> ~/xfstests-dev/results//btrfs/151.out.bad'  to see the     entire diff)
>>
>> [CAUSE]
>> Commit dad03fac3bb8 ("btrfs-progs: switch btrfs_group_profile_str to use
>> raid table") will use btrfs_raid_array[index].raid_name, which is all
>> lower case.
>>
>> [FIX]
>> There is no need to bring such output format change.
>>
>> So here we adds a new helper function, btrfs_group_profile_upper_str()
>> to print the upper case profile name.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  cmds/filesystem.c |  4 +++-
>>  common/utils.c    | 10 ++++++++++
>>  common/utils.h    |  3 +++
>>  3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
>> index 6a9e46d2b7dc..9f49b7d0c9c5 100644
>> --- a/cmds/filesystem.c
>> +++ b/cmds/filesystem.c
>> @@ -72,6 +72,7 @@ static void print_df(int fd, struct 
>> btrfs_ioctl_space_args *sargs, unsigned unit
>>  {
>>      u64 i;
>>      struct btrfs_ioctl_space_info *sp = sargs->spaces;
>> +    char profile_buf[BTRFS_PROFILE_STR_LEN];
>>      u64 unusable;
>>      bool ok;
>>
>> @@ -79,9 +80,10 @@ static void print_df(int fd, struct 
>> btrfs_ioctl_space_args *sargs, unsigned unit
>>          unusable = device_get_zone_unusable(fd, sp->flags);
>>          ok = (unusable != DEVICE_ZONE_UNUSABLE_UNKNOWN);
>>
>> +        btrfs_group_profile_upper_str(sp->flags, profile_buf);
>>          printf("%s, %s: total=%s, used=%s%s%s\n",
>>              btrfs_group_type_str(sp->flags),
>> -            btrfs_group_profile_str(sp->flags),
>> +            profile_buf,
>>              pretty_size_mode(sp->total_bytes, unit_mode),
>>              pretty_size_mode(sp->used_bytes, unit_mode),
>>              (ok ? ", zone_unusable=" : ""),
>> diff --git a/common/utils.c b/common/utils.c
>> index aee0eedc15fc..32ca6b2ef432 100644
>> --- a/common/utils.c
>> +++ b/common/utils.c
>> @@ -1038,6 +1038,16 @@ const char* btrfs_group_profile_str(u64 flag)
>>      return btrfs_raid_array[index].raid_name;
>>  }
>>
>> +void btrfs_group_profile_upper_str(u64 flags, char *ret)
>> +{
>> +    int i;
>> +
>> +    strncpy(ret, btrfs_group_profile_str(flags), BTRFS_PROFILE_STR_LEN);
>> +
>> +    for (i = 0; i < BTRFS_PROFILE_STR_LEN && ret[i]; i++)
>> +        ret[i] = toupper(ret[i]);
>> +}
>> +
>>  u64 div_factor(u64 num, int factor)
>>  {
>>      if (factor == 10)
>> diff --git a/common/utils.h b/common/utils.h
>> index 6f84e3cbc98f..0c1b6baa7ae3 100644
>> --- a/common/utils.h
>> +++ b/common/utils.h
>> @@ -75,6 +75,9 @@ int find_next_key(struct btrfs_path *path, struct 
>> btrfs_key *key);
>>  const char* btrfs_group_type_str(u64 flag);
>>  const char* btrfs_group_profile_str(u64 flag);
>>
>> +#define BTRFS_PROFILE_STR_LEN    (64)
>>
> May it be 8?
> struct btrfs_raid_attr {
> ...
>        const char raid_name[8];
> ...
> };

Right, and even UNKNOWN with tailing \0 can fit into 8 chars.

Will update in next version.

Thanks,
Qu

> 
> -- 
> Su
>> +void btrfs_group_profile_upper_str(u64 flag, char *ret);
>> +
>>  int count_digits(u64 num);
>>  u64 div_factor(u64 num, int factor);
> 

Re: [PATCH] btrfs-progs: Make "btrfs filesystem df" command to show upper case profile

[Wang Yugui]

Hi,

before Commit dad03fac3bb8 ("btrfs-progs: switch btrfs_group_profile_str to use
 raid table") , 'single' is lower case. others, such as RAID1 , are
upper case.

I don't know whether it is necessary to keep 'single' as lower case.

maybe we can change the array value directly?
./kernel-shared/volumes.c:62:   [BTRFS_RAID_RAID1C3] = {
./kernel-shared/volumes.c:71:           .raid_name      = "raid1c3",

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2021/11/02

> [BUG]
> Since commit dad03fac3bb8 ("btrfs-progs: switch btrfs_group_profile_str
> to use raid table"), fstests/btrfs/023 and btrfs/151 will always fail.
> 
> The failure of btrfs/151 explains the reason pretty well:
> 
> btrfs/151 1s ... - output mismatch
>     --- tests/btrfs/151.out	2019-10-22 15:18:14.068965341 +0800
>     +++ ~/xfstests-dev/results//btrfs/151.out.bad	2021-11-02 17:13:43.879999994 +0800
>     @@ -1,2 +1,2 @@
>      QA output created by 151
>     -Data, RAID1
>     +Data, raid1
>     ...
>     (Run 'diff -u ~/xfstests-dev/tests/btrfs/151.out ~/xfstests-dev/results//btrfs/151.out.bad'  to see the entire diff)
> 
> [CAUSE]
> Commit dad03fac3bb8 ("btrfs-progs: switch btrfs_group_profile_str to use
> raid table") will use btrfs_raid_array[index].raid_name, which is all
> lower case.
> 
> [FIX]
> There is no need to bring such output format change.
> 
> So here we adds a new helper function, btrfs_group_profile_upper_str()
> to print the upper case profile name.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  cmds/filesystem.c |  4 +++-
>  common/utils.c    | 10 ++++++++++
>  common/utils.h    |  3 +++
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
> index 6a9e46d2b7dc..9f49b7d0c9c5 100644
> --- a/cmds/filesystem.c
> +++ b/cmds/filesystem.c
> @@ -72,6 +72,7 @@ static void print_df(int fd, struct btrfs_ioctl_space_args *sargs, unsigned unit
>  {
>  	u64 i;
>  	struct btrfs_ioctl_space_info *sp = sargs->spaces;
> +	char profile_buf[BTRFS_PROFILE_STR_LEN];
>  	u64 unusable;
>  	bool ok;
>  
> @@ -79,9 +80,10 @@ static void print_df(int fd, struct btrfs_ioctl_space_args *sargs, unsigned unit
>  		unusable = device_get_zone_unusable(fd, sp->flags);
>  		ok = (unusable != DEVICE_ZONE_UNUSABLE_UNKNOWN);
>  
> +		btrfs_group_profile_upper_str(sp->flags, profile_buf);
>  		printf("%s, %s: total=%s, used=%s%s%s\n",
>  			btrfs_group_type_str(sp->flags),
> -			btrfs_group_profile_str(sp->flags),
> +			profile_buf,
>  			pretty_size_mode(sp->total_bytes, unit_mode),
>  			pretty_size_mode(sp->used_bytes, unit_mode),
>  			(ok ? ", zone_unusable=" : ""),
> diff --git a/common/utils.c b/common/utils.c
> index aee0eedc15fc..32ca6b2ef432 100644
> --- a/common/utils.c
> +++ b/common/utils.c
> @@ -1038,6 +1038,16 @@ const char* btrfs_group_profile_str(u64 flag)
>  	return btrfs_raid_array[index].raid_name;
>  }
>  
> +void btrfs_group_profile_upper_str(u64 flags, char *ret)
> +{
> +	int i;
> +
> +	strncpy(ret, btrfs_group_profile_str(flags), BTRFS_PROFILE_STR_LEN);
> +
> +	for (i = 0; i < BTRFS_PROFILE_STR_LEN && ret[i]; i++)
> +		ret[i] = toupper(ret[i]);
> +}
> +
>  u64 div_factor(u64 num, int factor)
>  {
>  	if (factor == 10)
> diff --git a/common/utils.h b/common/utils.h
> index 6f84e3cbc98f..0c1b6baa7ae3 100644
> --- a/common/utils.h
> +++ b/common/utils.h
> @@ -75,6 +75,9 @@ int find_next_key(struct btrfs_path *path, struct btrfs_key *key);
>  const char* btrfs_group_type_str(u64 flag);
>  const char* btrfs_group_profile_str(u64 flag);
>  
> +#define BTRFS_PROFILE_STR_LEN	(64)
> +void btrfs_group_profile_upper_str(u64 flag, char *ret);
> +
>  int count_digits(u64 num);
>  u64 div_factor(u64 num, int factor);
>  
> -- 
> 2.33.1

Re: [PATCH] btrfs-progs: Make "btrfs filesystem df" command to show upper case profile

[David Sterba]

On Tue, Nov 02, 2021 at 08:23:45PM +0800, Wang Yugui wrote:
> Hi,
> 
> before Commit dad03fac3bb8 ("btrfs-progs: switch btrfs_group_profile_str to use
>  raid table") , 'single' is lower case. others, such as RAID1 , are
> upper case.
> 
> I don't know whether it is necessary to keep 'single' as lower case.

Looks like it's better to keep it as it was, which would mean lowercase
single and uppercase the rest, with some additional conditions.

> maybe we can change the array value directly?
> ./kernel-shared/volumes.c:62:   [BTRFS_RAID_RAID1C3] = {
> ./kernel-shared/volumes.c:71:           .raid_name      = "raid1c3",

Either making the table name the one we want to print with the right
case, or add another that is same as kernel has and another for
printing.