[RFC][PATCH] btrfs-progs: add warning for mixed prfofiles filesystem

[RFC][PATCH] btrfs-progs: add warning for mixed prfofiles filesystem

[Goffredo Baroncelli]

Hi all,

the aim of this patch set is to issue a warning when a mixed profiles
filesystem is detected. This happens when the filesystems contains
(i.e.) raid1c3 and single chunk for data.

BTRFS has the capability to support a filesystem with mixed profiles.
However this could lead to an unexpected behavior when a new chunk is
allocated (i.e. the chunk profile is not what is wanted). Moreover
if the user is not aware of this, he could assume a redundancy which
doesn't exist (for example because there is some 'single' chunk when
it is expected the filesystem to be full raid1).
A possible cause of a mixed profiles filesystem is an interrupted
balance operation or a not fully balance due to very specific filter.

In this first attempt the check is only added to the 'btrfs fi us'
command. I hope to receive suggestion about which commands should
have this check (I think all the commands which interact with a
mounted filesystem).

Example of output:

$ sudo ./btrfs fi us /tmp/t/
WARNING: ------------------------------------------------------
WARNING: Detection of multiple profiles for a block group type:
WARNING:
WARNING: * DATA ->          [raid1c3, single]
WARNING: * METADATA ->      [raid1, single]
WARNING:
WARNING: Please consider using 'btrfs balance ...' commands set
WARNING: to solve this issue.
WARNING: ------------------------------------------------------
Overall:
    Device size:		  30.00GiB
    Device allocated:		   7.78GiB
    Device unallocated:		  22.22GiB
    Device missing:		     0.00B
    Used:			   1.84GiB
    Free (estimated):		  11.93GiB	(min: 9.82GiB)
    Data ratio:			      2.33
    Metadata ratio:		      1.50
    Global reserve:		   3.25MiB	(used: 0.00B)

Data,single: Size:1.00GiB, Used:1023.91MiB (99.99%)
   /dev/loop0	   1.00GiB

Data,RAID1C3: Size:2.00GiB, Used:128.90MiB (6.29%)
   /dev/loop0	   2.00GiB
   /dev/loop1	   2.00GiB
   /dev/loop2	   2.00GiB

Metadata,single: Size:256.00MiB, Used:88.67MiB (34.64%)
   /dev/loop1	 256.00MiB

Metadata,RAID1: Size:256.00MiB, Used:194.28MiB (75.89%)
   /dev/loop1	 256.00MiB
   /dev/loop2	 256.00MiB

System,single: Size:32.00MiB, Used:16.00KiB (0.05%)
   /dev/loop2	  32.00MiB

Unallocated:
   /dev/loop0	   7.00GiB
   /dev/loop1	   7.50GiB
   /dev/loop2	   7.72GiB


In this case there are two kind of chunks for data (raid1c3 and single)
and metadata (raid1, single).

Patch #1 and #2 are preparatory ones.
Patch #3 contains the code for the check.
Patch #4 adds the check to the command 'btrfs fi us'

Comments are welcome

BR
G.Baroncelli

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

[PATCH 1/4] btrfs-progs: Add missing fields to btrfs_raid_array[] for raid1c[34].

[Goffredo Baroncelli]

From: Goffredo Baroncelli <kreijack@inwind.it>

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 volumes.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/volumes.c b/volumes.c
index b46bf598..9e37f986 100644
--- a/volumes.c
+++ b/volumes.c
@@ -65,6 +65,8 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
 		.tolerated_failures = 2,
 		.devs_increment	= 3,
 		.ncopies	= 3,
+		.raid_name	= "raid1c3",
+		.bg_flag	= BTRFS_BLOCK_GROUP_RAID1C3,
 	},
 	[BTRFS_RAID_RAID1C4] = {
 		.sub_stripes	= 1,
@@ -74,6 +76,8 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
 		.tolerated_failures = 3,
 		.devs_increment	= 4,
 		.ncopies	= 4,
+		.raid_name	= "raid1c4",
+		.bg_flag	= BTRFS_BLOCK_GROUP_RAID1C4,
 	},
 	[BTRFS_RAID_DUP] = {
 		.sub_stripes	= 1,
-- 
2.26.0.rc2

[PATCH 2/4] btrfs-progs: Add BTRFS_EXTENDED_PROFILE_MASK mask.

[Goffredo Baroncelli]

From: Goffredo Baroncelli <kreijack@inwind.it>

Add BTRFS_EXTENDED_PROFILE_MASK to consider also the
BTRFS_AVAIL_ALLOC_BIT_SINGLE bit.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 ctree.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ctree.h b/ctree.h
index 36f62732..017ac067 100644
--- a/ctree.h
+++ b/ctree.h
@@ -1005,6 +1005,9 @@ enum btrfs_raid_types {
 /* used in struct btrfs_balance_args fields */
 #define BTRFS_AVAIL_ALLOC_BIT_SINGLE	(1ULL << 48)
 
+#define BTRFS_EXTENDED_PROFILE_MASK	(BTRFS_BLOCK_GROUP_PROFILE_MASK | \
+					 BTRFS_AVAIL_ALLOC_BIT_SINGLE)
+
 /*
  * GLOBAL_RSV does not exist as a on-disk block group type and is used
  * internally for exporting info about global block reserve from space infos
-- 
2.26.0.rc2

[PATCH 3/4] btrfs-progs: Add btrfs_check_for_mixed_profiles_by_* function

[Goffredo Baroncelli]

From: Goffredo Baroncelli <kreijack@inwind.it>

Show a warning if a mixed profiles filesystem
is detected.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 common/utils.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++
 common/utils.h |   3 ++
 2 files changed, 129 insertions(+)

diff --git a/common/utils.c b/common/utils.c
index 4ce36836..e7cd66eb 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -1710,3 +1710,129 @@ void print_all_devices(struct list_head *devices)
 		print_device_info(dev, "\t");
 	printf("\n");
 }
+
+static int bit_count(u64 x)
+{
+	int ret = 0;
+
+	while (x) {
+		if (x & 1)
+			ret++;
+		x >>= 1;
+	}
+	return ret;
+}
+
+static void print_profiles(FILE *out, u64 profiles)
+{
+	int i;
+	int first = true;
+
+	for (i = 0 ; i < BTRFS_NR_RAID_TYPES ; i++) {
+		if (!(btrfs_raid_array[i].bg_flag & profiles))
+			continue;
+
+		if (!first)
+			fprintf(out, ", ");
+		fprintf(out, "%s", btrfs_raid_array[i].raid_name);
+		first = false;
+	}
+	if (profiles & BTRFS_AVAIL_ALLOC_BIT_SINGLE) {
+		if (!first)
+			fprintf(out, ", ");
+		fprintf(out, "%s",
+			btrfs_raid_array[BTRFS_RAID_SINGLE].raid_name);
+	}
+}
+
+int btrfs_check_for_mixed_profiles_by_path(char *path)
+{
+	int fd;
+	int ret;
+	DIR *dirstream;
+
+	fd = btrfs_open_dir(path, &dirstream, 0);
+	if (fd < 0)
+		return -1;
+	closedir(dirstream);
+
+	ret = btrfs_check_for_mixed_profiles_by_fd(fd);
+	close(fd);
+
+	return ret;
+}
+
+int btrfs_check_for_mixed_profiles_by_fd(int fd)
+{
+	int ret;
+	int i;
+	struct btrfs_ioctl_space_args *sargs;
+	u64 data_profiles = 0;
+	u64 metadata_profiles = 0;
+	u64 system_profiles = 0;
+	u64 mixed_profiles = 0;
+	static const u64 mixed_profile_fl = BTRFS_BLOCK_GROUP_METADATA |
+		BTRFS_BLOCK_GROUP_DATA;
+
+	ret = get_df(fd, &sargs);
+	if (ret < 0)
+		return -1;
+
+	for (i = 0 ; i < sargs->total_spaces ; i++) {
+		u64 flags = sargs->spaces[i].flags;
+
+		if (!(flags & BTRFS_BLOCK_GROUP_PROFILE_MASK))
+			flags |= BTRFS_AVAIL_ALLOC_BIT_SINGLE;
+
+		if ((flags & mixed_profile_fl) == mixed_profile_fl)
+			mixed_profiles |= flags;
+		else if (flags & BTRFS_BLOCK_GROUP_DATA)
+			data_profiles |= flags;
+		else if (flags & BTRFS_BLOCK_GROUP_METADATA)
+			metadata_profiles |= flags;
+		else if (flags & BTRFS_BLOCK_GROUP_SYSTEM)
+			system_profiles |= flags;
+	}
+	free(sargs);
+
+	data_profiles &= BTRFS_EXTENDED_PROFILE_MASK;
+	system_profiles &= BTRFS_EXTENDED_PROFILE_MASK;
+	mixed_profiles &= BTRFS_EXTENDED_PROFILE_MASK;
+	metadata_profiles &= BTRFS_EXTENDED_PROFILE_MASK;
+
+	if ((bit_count(data_profiles) <= 1) &&
+	    (bit_count(metadata_profiles) <= 1) &&
+	    (bit_count(system_profiles) <= 1) &&
+	    (bit_count(mixed_profiles) <= 1))
+		return 0;
+
+	fprintf(stderr, "WARNING: ------------------------------------------------------\n");
+	fprintf(stderr, "WARNING: Detection of multiple profiles for a block group type:\n");
+	fprintf(stderr, "WARNING:\n");
+	if (bit_count(data_profiles) > 1) {
+		fprintf(stderr, "WARNING: * DATA ->          [");
+		print_profiles(stderr, data_profiles);
+		fprintf(stderr, "]\n");
+	}
+	if (bit_count(metadata_profiles) > 1) {
+		fprintf(stderr, "WARNING: * METADATA ->      [");
+		print_profiles(stderr, metadata_profiles);
+		fprintf(stderr, "]\n");
+	}
+	if (bit_count(mixed_profiles) > 1) {
+		fprintf(stderr, "WARNING: * DATA+METADATA -> [");
+		print_profiles(stderr, mixed_profiles);
+		fprintf(stderr, "]\n");
+	}
+	if (bit_count(system_profiles) > 1) {
+		fprintf(stderr, "WARNING: * SYSTEM ->        [");
+		print_profiles(stderr, system_profiles);
+		fprintf(stderr, "]\n");
+	}
+	fprintf(stderr, "WARNING:\n");
+	fprintf(stderr, "WARNING: Please consider using 'btrfs balance ...' commands set\n");
+	fprintf(stderr, "WARNING: to solve this issue.\n");
+	fprintf(stderr, "WARNING: ------------------------------------------------------\n");
+
+	return 1;
+}
diff --git a/common/utils.h b/common/utils.h
index 5c1afda9..662c9e38 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -137,4 +137,7 @@ u64 rand_u64(void);
 unsigned int rand_range(unsigned int upper);
 void init_rand_seed(u64 seed);
 
+int btrfs_check_for_mixed_profiles_by_path(char *path);
+int btrfs_check_for_mixed_profiles_by_fd(int fd);
+
 #endif
-- 
2.26.0.rc2

[PATCH 4/4] btrfs-progs: Add mixed profiles check to 'btrfs fi us'

[Goffredo Baroncelli]

From: Goffredo Baroncelli <kreijack@inwind.it>

Add a check in 'btrfs fi us' command to detect if a filesystem has
mixed profiles for data/metadata/system. In this case
a warning is showed.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 cmds/filesystem-usage.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
index aa7065d5..b867dc15 100644
--- a/cmds/filesystem-usage.c
+++ b/cmds/filesystem-usage.c
@@ -1030,6 +1030,8 @@ static int cmd_filesystem_usage(const struct cmd_struct *cmd,
 		if (more_than_one)
 			printf("\n");
 
+		btrfs_check_for_mixed_profiles_by_fd(fd);
+
 		ret = load_chunk_and_device_info(fd, &chunkinfo, &chunkcount,
 				&devinfo, &devcount);
 		if (ret)
-- 
2.26.0.rc2

Re: [PATCH 1/4] btrfs-progs: Add missing fields to btrfs_raid_array[] for raid1c[34].

[Qu Wenruo]

On 2020/3/26 上午4:10, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli <kreijack@inwind.it>
>
> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
> ---
>  volumes.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/volumes.c b/volumes.c
> index b46bf598..9e37f986 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -65,6 +65,8 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>  		.tolerated_failures = 2,
>  		.devs_increment	= 3,
>  		.ncopies	= 3,
> +		.raid_name	= "raid1c3",
> +		.bg_flag	= BTRFS_BLOCK_GROUP_RAID1C3,

Since you're here, mind to also add .mindev_error?

Thanks,
Qu
>  	},
>  	[BTRFS_RAID_RAID1C4] = {
>  		.sub_stripes	= 1,
> @@ -74,6 +76,8 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>  		.tolerated_failures = 3,
>  		.devs_increment	= 4,
>  		.ncopies	= 4,
> +		.raid_name	= "raid1c4",
> +		.bg_flag	= BTRFS_BLOCK_GROUP_RAID1C4,
>  	},
>  	[BTRFS_RAID_DUP] = {
>  		.sub_stripes	= 1,
>

Re: [PATCH 1/4] btrfs-progs: Add missing fields to btrfs_raid_array[] for raid1c[34].

[Goffredo Baroncelli]

On 3/26/20 2:59 AM, Qu Wenruo wrote:
> 
> 
> On 2020/3/26 上午4:10, Goffredo Baroncelli wrote:
>> From: Goffredo Baroncelli <kreijack@inwind.it>
>>
>> Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
>> ---
>>   volumes.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/volumes.c b/volumes.c
>> index b46bf598..9e37f986 100644
>> --- a/volumes.c
>> +++ b/volumes.c
>> @@ -65,6 +65,8 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>>   		.tolerated_failures = 2,
>>   		.devs_increment	= 3,
>>   		.ncopies	= 3,
>> +		.raid_name	= "raid1c3",
>> +		.bg_flag	= BTRFS_BLOCK_GROUP_RAID1C3,
> 
> Since you're here, mind to also add .mindev_error?

I will take care of
BR
G.Baroncelli

> 
> Thanks,
> Qu
>>   	},
>>   	[BTRFS_RAID_RAID1C4] = {
>>   		.sub_stripes	= 1,
>> @@ -74,6 +76,8 @@ const struct btrfs_raid_attr btrfs_raid_array[BTRFS_NR_RAID_TYPES] = {
>>   		.tolerated_failures = 3,
>>   		.devs_increment	= 4,
>>   		.ncopies	= 4,
>> +		.raid_name	= "raid1c4",
>> +		.bg_flag	= BTRFS_BLOCK_GROUP_RAID1C4,
>>   	},
>>   	[BTRFS_RAID_DUP] = {
>>   		.sub_stripes	= 1,
>>


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

[PATCH 3/4] btrfs-progs: Add btrfs_check_for_mixed_profiles_by_* function

[Goffredo Baroncelli]

From: Goffredo Baroncelli <kreijack@inwind.it>

Show a warning if a mixed profiles filesystem
is detected.

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 common/utils.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++
 common/utils.h |   3 ++
 2 files changed, 129 insertions(+)

diff --git a/common/utils.c b/common/utils.c
index a7761683..e3f18709 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -1710,3 +1710,129 @@ void print_all_devices(struct list_head *devices)
 		print_device_info(dev, "\t");
 	printf("\n");
 }
+
+static int bit_count(u64 x)
+{
+	int ret = 0;
+
+	while (x) {
+		if (x & 1)
+			ret++;
+		x >>= 1;
+	}
+	return ret;
+}
+
+static void print_profiles(FILE *out, u64 profiles)
+{
+	int i;
+	int first = true;
+
+	for (i = 0 ; i < BTRFS_NR_RAID_TYPES ; i++) {
+		if (!(btrfs_raid_array[i].bg_flag & profiles))
+			continue;
+
+		if (!first)
+			fprintf(out, ", ");
+		fprintf(out, "%s", btrfs_raid_array[i].raid_name);
+		first = false;
+	}
+	if (profiles & BTRFS_AVAIL_ALLOC_BIT_SINGLE) {
+		if (!first)
+			fprintf(out, ", ");
+		fprintf(out, "%s",
+			btrfs_raid_array[BTRFS_RAID_SINGLE].raid_name);
+	}
+}
+
+int btrfs_check_for_mixed_profiles_by_path(char *path)
+{
+	int fd;
+	int ret;
+	DIR *dirstream;
+
+	fd = btrfs_open_dir(path, &dirstream, 0);
+	if (fd < 0)
+		return -1;
+	closedir(dirstream);
+
+	ret = btrfs_check_for_mixed_profiles_by_fd(fd);
+	close(fd);
+
+	return ret;
+}
+
+int btrfs_check_for_mixed_profiles_by_fd(int fd)
+{
+	int ret;
+	int i;
+	struct btrfs_ioctl_space_args *sargs;
+	u64 data_profiles = 0;
+	u64 metadata_profiles = 0;
+	u64 system_profiles = 0;
+	u64 mixed_profiles = 0;
+	static const u64 mixed_profile_fl = BTRFS_BLOCK_GROUP_METADATA |
+		BTRFS_BLOCK_GROUP_DATA;
+
+	ret = get_df(fd, &sargs);
+	if (ret < 0)
+		return -1;
+
+	for (i = 0 ; i < sargs->total_spaces ; i++) {
+		u64 flags = sargs->spaces[i].flags;
+
+		if (!(flags & BTRFS_BLOCK_GROUP_PROFILE_MASK))
+			flags |= BTRFS_AVAIL_ALLOC_BIT_SINGLE;
+
+		if ((flags & mixed_profile_fl) == mixed_profile_fl)
+			mixed_profiles |= flags;
+		else if (flags & BTRFS_BLOCK_GROUP_DATA)
+			data_profiles |= flags;
+		else if (flags & BTRFS_BLOCK_GROUP_METADATA)
+			metadata_profiles |= flags;
+		else if (flags & BTRFS_BLOCK_GROUP_SYSTEM)
+			system_profiles |= flags;
+	}
+	free(sargs);
+
+	data_profiles &= BTRFS_EXTENDED_PROFILE_MASK;
+	system_profiles &= BTRFS_EXTENDED_PROFILE_MASK;
+	mixed_profiles &= BTRFS_EXTENDED_PROFILE_MASK;
+	metadata_profiles &= BTRFS_EXTENDED_PROFILE_MASK;
+
+	if ((bit_count(data_profiles) <= 1) &&
+	    (bit_count(metadata_profiles) <= 1) &&
+	    (bit_count(system_profiles) <= 1) &&
+	    (bit_count(mixed_profiles) <= 1))
+		return 0;
+
+	fprintf(stderr, "WARNING: ------------------------------------------------------\n");
+	fprintf(stderr, "WARNING: Detection of multiple profiles for a block group type:\n");
+	fprintf(stderr, "WARNING:\n");
+	if (bit_count(data_profiles) > 1) {
+		fprintf(stderr, "WARNING: * DATA ->          [");
+		print_profiles(stderr, data_profiles);
+		fprintf(stderr, "]\n");
+	}
+	if (bit_count(metadata_profiles) > 1) {
+		fprintf(stderr, "WARNING: * METADATA ->      [");
+		print_profiles(stderr, metadata_profiles);
+		fprintf(stderr, "]\n");
+	}
+	if (bit_count(mixed_profiles) > 1) {
+		fprintf(stderr, "WARNING: * DATA+METADATA -> [");
+		print_profiles(stderr, mixed_profiles);
+		fprintf(stderr, "]\n");
+	}
+	if (bit_count(system_profiles) > 1) {
+		fprintf(stderr, "WARNING: * SYSTEM ->        [");
+		print_profiles(stderr, system_profiles);
+		fprintf(stderr, "]\n");
+	}
+	fprintf(stderr, "WARNING:\n");
+	fprintf(stderr, "WARNING: Please consider using 'btrfs balance ...' commands set\n");
+	fprintf(stderr, "WARNING: to solve this issue.\n");
+	fprintf(stderr, "WARNING: ------------------------------------------------------\n");
+
+	return 1;
+}
diff --git a/common/utils.h b/common/utils.h
index 5c1afda9..662c9e38 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -137,4 +137,7 @@ u64 rand_u64(void);
 unsigned int rand_range(unsigned int upper);
 void init_rand_seed(u64 seed);
 
+int btrfs_check_for_mixed_profiles_by_path(char *path);
+int btrfs_check_for_mixed_profiles_by_fd(int fd);
+
 #endif
-- 
2.26.0