All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem
@ 2020-04-04 10:32 Goffredo Baroncelli
  2020-04-04 10:32 ` [PATCH 1/5] btrfs-progs: Add code for checking mixed profile function Goffredo Baroncelli
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2020-04-04 10:32 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Zygo Blaxell, Qu Wenruo, David Sterba, Graham Cobb


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.

The check is added to the following btrfs commands:
- btrfs balance pause
- btrfs balance cancel
- btrfs device add
- btrfs device del

The warning is shorter than the before one. Below an example and
it is printed after the normal output of the command.

   WARNING: Multiple profiles detected.  See 'man btrfs(5)'.
   WARNING: data -> [raid1c3, single], metadata -> [raid1, single]

The command "btrfs fi us" doesn't show the warning above, instead
it was added a further line in the "Overall" section. The output now
is this:

$ sudo ./btrfs fi us /tmp/t/
[sudo] password for ghigo: 
Overall:
    Device size:		  30.00GiB
    Device allocated:		   4.78GiB
    Device unallocated:		  25.22GiB
    Device missing:		     0.00B
    Used:			   1.95GiB
    Free (estimated):		  13.87GiB	(min: 9.67GiB)
    Data ratio:			      2.00
    Metadata ratio:		      1.50
    Global reserve:		   3.25MiB	(used: 0.00B)
    Multiple profile:		       YES

Data,single: Size:1.00GiB, Used:974.04MiB (95.12%)
   /dev/loop0	   1.00GiB

Data,RAID1C3: Size:1.00GiB, Used:178.59MiB (17.44%)
   /dev/loop0	   1.00GiB
   /dev/loop1	   1.00GiB
   /dev/loop2	   1.00GiB

Metadata,single: Size:256.00MiB, Used:76.22MiB (29.77%)
   /dev/loop1	 256.00MiB

Metadata,RAID1: Size:256.00MiB, Used:206.92MiB (80.83%)
   /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	   8.00GiB
   /dev/loop1	   8.50GiB
   /dev/loop2	   8.72GiB


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

As the previous patch set, the warning is added also to the command
'btrfs fi df' and 'btrfs dev us' as separate patch. If even in this
review nobody likes it, we can simply drop this patch.

Suggestion about which commands should (not) have this check are 
welcome.

v1 
- first issue
v2 
- add some needed missing pieces about raid1c[34]
- add the check to more btrfs commands
v3
- add a section in btrfs(5) 'FILESYSTEM WITH MULTIPLE PROFILES'
- 'btrfs fi us': changed the worning in a info in the 'overall' section

Patch #1 contains the code for the check.
Patch #3 adds the check to the command 'btrfs dev {add,del}' and 'btrfs
bal {pause, stop}'
Patch #3 adds the check to the command 'btrfs fi us'
Patch #5 add the check to the command 'btrfs fi df' and 'btrfs dev us'
Patch #5 add the info in btrfs(5) man page

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






^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/5] btrfs-progs: Add code for checking mixed profile  function
  2020-04-04 10:32 [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem Goffredo Baroncelli
@ 2020-04-04 10:32 ` Goffredo Baroncelli
  2020-04-04 10:32 ` [PATCH 2/5] btrfs-progs: Add mixed profiles check to some btrfs sub-commands Goffredo Baroncelli
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2020-04-04 10:32 UTC (permalink / raw)
  To: linux-btrfs
  Cc: Zygo Blaxell, Qu Wenruo, David Sterba, Graham Cobb, Goffredo Baroncelli

From: Goffredo Baroncelli <kreijack@inwind.it>

Add code to show a warning if a mixed profiles filesystem is detected.

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

diff --git a/common/utils.c b/common/utils.c
index a7761683..5c9ff562 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -1710,3 +1710,191 @@ 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 sprint_profiles(char **ptr, u64 profiles)
+{
+	int i;
+	int first = true;
+	int l = 1;
+
+	*ptr = NULL;
+
+	for (i = 0 ; i < BTRFS_NR_RAID_TYPES ; i++)
+		l += strlen(btrfs_raid_array[i].raid_name) + 2;
+
+	*ptr = malloc(l);
+	if (!*ptr)
+		return;
+	**ptr = 0;
+
+	for (i = 0 ; i < BTRFS_NR_RAID_TYPES ; i++) {
+		if (!(btrfs_raid_array[i].bg_flag & profiles))
+			continue;
+
+		if (!first)
+			strcat(*ptr, ", ");
+		strcat(*ptr, btrfs_raid_array[i].raid_name);
+		first = false;
+	}
+	if (profiles & BTRFS_AVAIL_ALLOC_BIT_SINGLE) {
+		if (!first)
+			strcat(*ptr, ", ");
+		strcat(*ptr, btrfs_raid_array[BTRFS_RAID_SINGLE].raid_name);
+	}
+}
+
+int btrfs_string_check_for_mixed_profiles_by_fd(int fd, char **data_ret,
+							char **metadata_ret,
+							char **mixed_ret,
+							char **system_ret)
+{
+	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;
+
+	if (data_ret) {
+		if (bit_count(data_profiles) > 1)
+			sprint_profiles(data_ret, data_profiles);
+		else
+			*data_ret = NULL;
+	}
+	if (metadata_ret) {
+		if (bit_count(metadata_profiles) > 1)
+			sprint_profiles(metadata_ret, metadata_profiles);
+		else
+			*metadata_ret = NULL;
+	}
+	if (mixed_ret) {
+		if (bit_count(mixed_profiles) > 1)
+			sprint_profiles(mixed_ret, mixed_profiles);
+		else
+			*mixed_ret = NULL;
+	}
+	if (system_ret) {
+		if (bit_count(system_profiles) > 1)
+			sprint_profiles(system_ret, system_profiles);
+		else
+			*system_ret = NULL;
+	}
+
+	return 1;
+}
+
+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 first = true;
+	char *data_prof, *mixed_prof, *metadata_prof, *system_prof;
+
+	ret = btrfs_string_check_for_mixed_profiles_by_fd(fd, &data_prof,
+			&metadata_prof, &mixed_prof, &system_prof);
+
+	if (ret != 1)
+		return ret;
+
+	fprintf(stderr,
+		"WARNING: Multiple profiles detected.  See 'man btrfs(5)'.\n");
+	fprintf(stderr, "WARNING: ");
+	if (data_prof) {
+		fprintf(stderr, "data -> [%s]", data_prof);
+		first = false;
+	}
+	if (metadata_prof) {
+		if (!first)
+			fprintf(stderr, ", ");
+		fprintf(stderr, "metadata -> [%s]", metadata_prof);
+		first = false;
+	}
+	if (mixed_prof) {
+		if (!first)
+			fprintf(stderr, ", ");
+		fprintf(stderr, "data+metadata -> [%s]", mixed_prof);
+		first = false;
+	}
+	if (system_prof) {
+		if (!first)
+			fprintf(stderr, ", ");
+		fprintf(stderr, "system -> [%s]", system_prof);
+		first = false;
+	}
+
+	fprintf(stderr, "\n");
+
+	if (data_prof)
+		free(data_prof);
+	if (metadata_prof)
+		free(metadata_prof);
+	if (mixed_prof)
+		free(mixed_prof);
+	if (system_prof)
+		free(system_prof);
+
+	return 1;
+}
diff --git a/common/utils.h b/common/utils.h
index 5c1afda9..c4e6e935 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -137,4 +137,15 @@ u64 rand_u64(void);
 unsigned int rand_range(unsigned int upper);
 void init_rand_seed(u64 seed);
 
+int btrfs_string_check_for_mixed_profiles_by_fd(int fd, char **data_ret,
+							char **metadata_ret,
+							char **mixed_ret,
+							char **system_ret);
+static inline int btrfs_test_for_mixed_profiles_by_fd(int fd)
+{
+	return btrfs_string_check_for_mixed_profiles_by_fd(fd, 0L, 0L, 0L, 0L);
+}
+int btrfs_check_for_mixed_profiles_by_path(char *path);
+int btrfs_check_for_mixed_profiles_by_fd(int fd);
+
 #endif
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/5] btrfs-progs: Add mixed profiles check to some btrfs sub-commands.
  2020-04-04 10:32 [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem Goffredo Baroncelli
  2020-04-04 10:32 ` [PATCH 1/5] btrfs-progs: Add code for checking mixed profile function Goffredo Baroncelli
@ 2020-04-04 10:32 ` Goffredo Baroncelli
  2020-04-04 10:32 ` [PATCH 3/5] Add check for multiple profile in btrfs fi us Goffredo Baroncelli
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2020-04-04 10:32 UTC (permalink / raw)
  To: linux-btrfs
  Cc: Zygo Blaxell, Qu Wenruo, David Sterba, Graham Cobb, Goffredo Baroncelli

From: Goffredo Baroncelli <kreijack@inwind.it>

Add a check in some btrfs subcommands 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/balance.c | 2 ++
 cmds/device.c  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/cmds/balance.c b/cmds/balance.c
index 5392a604..20d0ebc1 100644
--- a/cmds/balance.c
+++ b/cmds/balance.c
@@ -716,6 +716,7 @@ static int cmd_balance_pause(const struct cmd_struct *cmd,
 			ret = 1;
 	}
 
+	btrfs_check_for_mixed_profiles_by_fd(fd);
 	close_file_or_dir(fd, dirstream);
 	return ret;
 }
@@ -756,6 +757,7 @@ static int cmd_balance_cancel(const struct cmd_struct *cmd,
 			ret = 1;
 	}
 
+	btrfs_check_for_mixed_profiles_by_fd(fd);
 	close_file_or_dir(fd, dirstream);
 	return ret;
 }
diff --git a/cmds/device.c b/cmds/device.c
index 24158308..401b32b9 100644
--- a/cmds/device.c
+++ b/cmds/device.c
@@ -143,6 +143,7 @@ static int cmd_device_add(const struct cmd_struct *cmd,
 	}
 
 error_out:
+	btrfs_check_for_mixed_profiles_by_fd(fdmnt);
 	close_file_or_dir(fdmnt, dirstream);
 	return !!ret;
 }
@@ -225,6 +226,7 @@ static int _cmd_device_remove(const struct cmd_struct *cmd,
 		}
 	}
 
+	btrfs_check_for_mixed_profiles_by_fd(fdmnt);
 	close_file_or_dir(fdmnt, dirstream);
 	return !!ret;
 }
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/5] Add check for multiple profile in btrfs fi us
  2020-04-04 10:32 [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem Goffredo Baroncelli
  2020-04-04 10:32 ` [PATCH 1/5] btrfs-progs: Add code for checking mixed profile function Goffredo Baroncelli
  2020-04-04 10:32 ` [PATCH 2/5] btrfs-progs: Add mixed profiles check to some btrfs sub-commands Goffredo Baroncelli
@ 2020-04-04 10:32 ` Goffredo Baroncelli
  2020-04-04 10:32 ` [PATCH 4/5] Add further check in btrfs subcommand Goffredo Baroncelli
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2020-04-04 10:32 UTC (permalink / raw)
  To: linux-btrfs
  Cc: Zygo Blaxell, Qu Wenruo, David Sterba, Graham Cobb, Goffredo Baroncelli

From: Goffredo Baroncelli <kreijack@inwind.it>

A new line in the "Overall" section is added to inform that a 
'Multiple profile' is present.

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

diff --git a/cmds/filesystem-usage.c b/cmds/filesystem-usage.c
index aa7065d5..742b4ea4 100644
--- a/cmds/filesystem-usage.c
+++ b/cmds/filesystem-usage.c
@@ -492,6 +492,11 @@ static int print_filesystem_usage_overall(int fd, struct chunk_info *chunkinfo,
 	printf("    Global reserve:\t\t%*s\t(used: %s)\n", width,
 		pretty_size_mode(l_global_reserve, unit_mode),
 		pretty_size_mode(l_global_reserve_used, unit_mode));
+	if (btrfs_test_for_mixed_profiles_by_fd(fd) > 0)
+		printf("    Multiple profile:\t\t%*s\n", width, "YES");
+	else
+		printf("    Multiple profile:\t\t%*s\n", width, "no");
+
 
 exit:
 
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/5] Add further check in btrfs subcommand
  2020-04-04 10:32 [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem Goffredo Baroncelli
                   ` (2 preceding siblings ...)
  2020-04-04 10:32 ` [PATCH 3/5] Add check for multiple profile in btrfs fi us Goffredo Baroncelli
@ 2020-04-04 10:32 ` Goffredo Baroncelli
  2020-04-04 10:32 ` [PATCH 5/5] Update the man page in order to give a guideline about mixed profiles Goffredo Baroncelli
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2020-04-04 10:32 UTC (permalink / raw)
  To: linux-btrfs
  Cc: Zygo Blaxell, Qu Wenruo, David Sterba, Graham Cobb, Goffredo Baroncelli

From: Goffredo Baroncelli <kreijack@inwind.it>

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

diff --git a/cmds/device.c b/cmds/device.c
index 401b32b9..d83f92a7 100644
--- a/cmds/device.c
+++ b/cmds/device.c
@@ -661,6 +661,7 @@ static int cmd_device_usage(const struct cmd_struct *cmd, int argc, char **argv)
 		}
 
 		ret = _cmd_device_usage(fd, argv[i], unit_mode);
+		btrfs_check_for_mixed_profiles_by_fd(fd);
 		close_file_or_dir(fd, dirstream);
 
 		if (ret)
diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 4f22089a..c4bb13dd 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -111,6 +111,7 @@ static int cmd_filesystem_df(const struct cmd_struct *cmd,
 		error("get_df failed: %m");
 	}
 
+	btrfs_check_for_mixed_profiles_by_fd(fd);
 	close_file_or_dir(fd, dirstream);
 	return !!ret;
 }
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/5] Update the man page in order to give a guideline about mixed profiles.
  2020-04-04 10:32 [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem Goffredo Baroncelli
                   ` (3 preceding siblings ...)
  2020-04-04 10:32 ` [PATCH 4/5] Add further check in btrfs subcommand Goffredo Baroncelli
@ 2020-04-04 10:32 ` Goffredo Baroncelli
  2020-04-06 17:57 ` [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem David Sterba
  2020-04-30 13:37 ` Johannes Thumshirn
  6 siblings, 0 replies; 10+ messages in thread
From: Goffredo Baroncelli @ 2020-04-04 10:32 UTC (permalink / raw)
  To: linux-btrfs
  Cc: Zygo Blaxell, Qu Wenruo, David Sterba, Graham Cobb, Goffredo Baroncelli

From: Goffredo Baroncelli <kreijack@inwind.it>

Signed-off-by: Goffredo Baroncelli <kreijack@inwind.it>
---
 Documentation/btrfs-man5.asciidoc | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/Documentation/btrfs-man5.asciidoc b/Documentation/btrfs-man5.asciidoc
index 56b1ed58..88f1c5ed 100644
--- a/Documentation/btrfs-man5.asciidoc
+++ b/Documentation/btrfs-man5.asciidoc
@@ -846,6 +846,37 @@ work and a workaround would need to be used to mount a multi-device filesystem.
 The mount option 'device' can trigger the device scanning during mount.
 
 
+FILESYSTEM WITH MULTIPLE PROFILES
+---------------------------------
+
+It is possible that a btrfs filesystem features block group of the same
+type (e.g. data) with different profiles.
+This could happen when a profile conversion is interrupted (see
+`btrfs-balance(8)`).
+Some 'btrfs' commands perform a test to detect this kind of condition. In such
+case a warning like this is showed:
+
+--------------------
+WARNING: Multiple profiles detected.  See 'man btrfs(5)'.
+WARNING: data -> [raid1, single], metadata -> [raid1, single]
+--------------------
+
+In a case like this, it is suggested to complete the conversion running
+`btrfs balance`. This because the next block group allocation
+is performed on the basis of the set of the profiles present on the disks,
+according to the following priorities:
+
+* RAID6
+* RAID5
+* RAID10
+* RAID1
+* RAID0
+
+For example if both the profile RAID6 and RAID1 are present on the disks,
+the next block group allocation will be RAID6, regardeless of the last
+`btrfs balance`.
+
+
 SEE ALSO
 --------
 `acl`(5),
-- 
2.26.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem
  2020-04-04 10:32 [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem Goffredo Baroncelli
                   ` (4 preceding siblings ...)
  2020-04-04 10:32 ` [PATCH 5/5] Update the man page in order to give a guideline about mixed profiles Goffredo Baroncelli
@ 2020-04-06 17:57 ` David Sterba
  2020-04-30 16:09   ` David Sterba
  2020-04-30 13:37 ` Johannes Thumshirn
  6 siblings, 1 reply; 10+ messages in thread
From: David Sterba @ 2020-04-06 17:57 UTC (permalink / raw)
  To: Goffredo Baroncelli
  Cc: linux-btrfs, Zygo Blaxell, Qu Wenruo, David Sterba, Graham Cobb

On Sat, Apr 04, 2020 at 12:32:07PM +0200, Goffredo Baroncelli wrote:
> 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.
> 
> The check is added to the following btrfs commands:
> - btrfs balance pause
> - btrfs balance cancel
> - btrfs device add
> - btrfs device del
> 
> The warning is shorter than the before one. Below an example and
> it is printed after the normal output of the command.
> 
>    WARNING: Multiple profiles detected.  See 'man btrfs(5)'.
>    WARNING: data -> [raid1c3, single], metadata -> [raid1, single]
> 
> The command "btrfs fi us" doesn't show the warning above, instead
> it was added a further line in the "Overall" section. The output now
> is this:
> 
>     Multiple profile:		       YES

Yeah the summary section seems suitable, I think this could be more
specific, for which profile type it applies, eg. 'data, metadata', or
just one of them.

I'll add the patches to devel so we can see how it works in practice.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem
  2020-04-04 10:32 [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem Goffredo Baroncelli
                   ` (5 preceding siblings ...)
  2020-04-06 17:57 ` [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem David Sterba
@ 2020-04-30 13:37 ` Johannes Thumshirn
  2020-04-30 14:04   ` David Sterba
  6 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2020-04-30 13:37 UTC (permalink / raw)
  To: Goffredo Baroncelli, linux-btrfs, David Sterba
  Cc: Zygo Blaxell, Qu Wenruo, Graham Cobb

On 04/04/2020 12:32, Goffredo Baroncelli wrote:
> 
> 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.
> 
> The check is added to the following btrfs commands:
> - btrfs balance pause
> - btrfs balance cancel
> - btrfs device add
> - btrfs device del
> 
> The warning is shorter than the before one. Below an example and
> it is printed after the normal output of the command.
> 
>     WARNING: Multiple profiles detected.  See 'man btrfs(5)'.
>     WARNING: data -> [raid1c3, single], metadata -> [raid1, single]
> 
> The command "btrfs fi us" doesn't show the warning above, instead
> it was added a further line in the "Overall" section. The output now
> is this:
> 
> $ sudo ./btrfs fi us /tmp/t/
> [sudo] password for ghigo:
> Overall:
>      Device size:		  30.00GiB
>      Device allocated:		   4.78GiB
>      Device unallocated:		  25.22GiB
>      Device missing:		     0.00B
>      Used:			   1.95GiB
>      Free (estimated):		  13.87GiB	(min: 9.67GiB)
>      Data ratio:			      2.00
>      Metadata ratio:		      1.50
>      Global reserve:		   3.25MiB	(used: 0.00B)
>      Multiple profile:		       YES
> 
> Data,single: Size:1.00GiB, Used:974.04MiB (95.12%)
>     /dev/loop0	   1.00GiB
> 
> Data,RAID1C3: Size:1.00GiB, Used:178.59MiB (17.44%)
>     /dev/loop0	   1.00GiB
>     /dev/loop1	   1.00GiB
>     /dev/loop2	   1.00GiB
> 
> Metadata,single: Size:256.00MiB, Used:76.22MiB (29.77%)
>     /dev/loop1	 256.00MiB
> 
> Metadata,RAID1: Size:256.00MiB, Used:206.92MiB (80.83%)
>     /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	   8.00GiB
>     /dev/loop1	   8.50GiB
>     /dev/loop2	   8.72GiB
> 
> 
> In this case there are two kind of chunks for data (raid1c3 and single)
> and metadata (raid1, single).
> 
> As the previous patch set, the warning is added also to the command
> 'btrfs fi df' and 'btrfs dev us' as separate patch. If even in this
> review nobody likes it, we can simply drop this patch.
> 
> Suggestion about which commands should (not) have this check are
> welcome.
> 
> v1
> - first issue
> v2
> - add some needed missing pieces about raid1c[34]
> - add the check to more btrfs commands
> v3
> - add a section in btrfs(5) 'FILESYSTEM WITH MULTIPLE PROFILES'
> - 'btrfs fi us': changed the worning in a info in the 'overall' section
> 
> Patch #1 contains the code for the check.
> Patch #3 adds the check to the command 'btrfs dev {add,del}' and 'btrfs
> bal {pause, stop}'
> Patch #3 adds the check to the command 'btrfs fi us'
> Patch #5 add the check to the command 'btrfs fi df' and 'btrfs dev us'
> Patch #5 add the info in btrfs(5) man page

Btw with this patchset applied fstests choke on some tests (e.g. 
btrfs/003) in my setup:

btrfs/003       - output mismatch (see 
/home/johannes/src/xfstests-dev/results//btrfs/003.out.bad)
     --- tests/btrfs/003.out     2020-01-02 08:43:50.000000000 +0000
     +++ /home/johannes/src/xfstests-dev/results//btrfs/003.out.bad 
2020-04-30 13:20:43.050569551 +0000
     @@ -1,2 +1,4 @@
      QA output created by 003
     +WARNING: Multiple profiles detected.  See 'man btrfs(5)'.
     +WARNING:
      Silence is golden
     ...
     (Run 'diff -u /home/johannes/src/xfstests-dev/tests/btrfs/003.out 
/home/johannes/src/xfstests-dev/results//btrfs/003.out.bad'  to see the 
entire diff)


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem
  2020-04-30 13:37 ` Johannes Thumshirn
@ 2020-04-30 14:04   ` David Sterba
  0 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2020-04-30 14:04 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: Goffredo Baroncelli, linux-btrfs, David Sterba, Zygo Blaxell,
	Qu Wenruo, Graham Cobb

On Thu, Apr 30, 2020 at 01:37:39PM +0000, Johannes Thumshirn wrote:
> On 04/04/2020 12:32, Goffredo Baroncelli wrote:
> > 
> > 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.
> > 
> > The check is added to the following btrfs commands:
> > - btrfs balance pause
> > - btrfs balance cancel
> > - btrfs device add
> > - btrfs device del
> > 
> > The warning is shorter than the before one. Below an example and
> > it is printed after the normal output of the command.
> > 
> >     WARNING: Multiple profiles detected.  See 'man btrfs(5)'.
> >     WARNING: data -> [raid1c3, single], metadata -> [raid1, single]
> > 
> > The command "btrfs fi us" doesn't show the warning above, instead
> > it was added a further line in the "Overall" section. The output now
> > is this:
> > 
> > $ sudo ./btrfs fi us /tmp/t/
> > [sudo] password for ghigo:
> > Overall:
> >      Device size:		  30.00GiB
> >      Device allocated:		   4.78GiB
> >      Device unallocated:		  25.22GiB
> >      Device missing:		     0.00B
> >      Used:			   1.95GiB
> >      Free (estimated):		  13.87GiB	(min: 9.67GiB)
> >      Data ratio:			      2.00
> >      Metadata ratio:		      1.50
> >      Global reserve:		   3.25MiB	(used: 0.00B)
> >      Multiple profile:		       YES
> > 
> > Data,single: Size:1.00GiB, Used:974.04MiB (95.12%)
> >     /dev/loop0	   1.00GiB
> > 
> > Data,RAID1C3: Size:1.00GiB, Used:178.59MiB (17.44%)
> >     /dev/loop0	   1.00GiB
> >     /dev/loop1	   1.00GiB
> >     /dev/loop2	   1.00GiB
> > 
> > Metadata,single: Size:256.00MiB, Used:76.22MiB (29.77%)
> >     /dev/loop1	 256.00MiB
> > 
> > Metadata,RAID1: Size:256.00MiB, Used:206.92MiB (80.83%)
> >     /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	   8.00GiB
> >     /dev/loop1	   8.50GiB
> >     /dev/loop2	   8.72GiB
> > 
> > 
> > In this case there are two kind of chunks for data (raid1c3 and single)
> > and metadata (raid1, single).
> > 
> > As the previous patch set, the warning is added also to the command
> > 'btrfs fi df' and 'btrfs dev us' as separate patch. If even in this
> > review nobody likes it, we can simply drop this patch.
> > 
> > Suggestion about which commands should (not) have this check are
> > welcome.
> > 
> > v1
> > - first issue
> > v2
> > - add some needed missing pieces about raid1c[34]
> > - add the check to more btrfs commands
> > v3
> > - add a section in btrfs(5) 'FILESYSTEM WITH MULTIPLE PROFILES'
> > - 'btrfs fi us': changed the worning in a info in the 'overall' section
> > 
> > Patch #1 contains the code for the check.
> > Patch #3 adds the check to the command 'btrfs dev {add,del}' and 'btrfs
> > bal {pause, stop}'
> > Patch #3 adds the check to the command 'btrfs fi us'
> > Patch #5 add the check to the command 'btrfs fi df' and 'btrfs dev us'
> > Patch #5 add the info in btrfs(5) man page
> 
> Btw with this patchset applied fstests choke on some tests (e.g. 
> btrfs/003) in my setup:
> 
> btrfs/003       - output mismatch (see 
> /home/johannes/src/xfstests-dev/results//btrfs/003.out.bad)
>      --- tests/btrfs/003.out     2020-01-02 08:43:50.000000000 +0000
>      +++ /home/johannes/src/xfstests-dev/results//btrfs/003.out.bad 
> 2020-04-30 13:20:43.050569551 +0000
>      @@ -1,2 +1,4 @@
>       QA output created by 003
>      +WARNING: Multiple profiles detected.  See 'man btrfs(5)'.
>      +WARNING:

Well, the tests need to be adjusted to filter that message out.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem
  2020-04-06 17:57 ` [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem David Sterba
@ 2020-04-30 16:09   ` David Sterba
  0 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2020-04-30 16:09 UTC (permalink / raw)
  To: dsterba, Goffredo Baroncelli, linux-btrfs, Zygo Blaxell,
	Qu Wenruo, David Sterba, Graham Cobb

On Mon, Apr 06, 2020 at 07:57:17PM +0200, David Sterba wrote:
> On Sat, Apr 04, 2020 at 12:32:07PM +0200, Goffredo Baroncelli wrote:
> > 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.
> > 
> > The check is added to the following btrfs commands:
> > - btrfs balance pause
> > - btrfs balance cancel
> > - btrfs device add
> > - btrfs device del
> > 
> > The warning is shorter than the before one. Below an example and
> > it is printed after the normal output of the command.
> > 
> >    WARNING: Multiple profiles detected.  See 'man btrfs(5)'.
> >    WARNING: data -> [raid1c3, single], metadata -> [raid1, single]
> > 
> > The command "btrfs fi us" doesn't show the warning above, instead
> > it was added a further line in the "Overall" section. The output now
> > is this:
> > 
> >     Multiple profile:		       YES
> 
> Yeah the summary section seems suitable, I think this could be more
> specific, for which profile type it applies, eg. 'data, metadata', or
> just one of them.
> 
> I'll add the patches to devel so we can see how it works in practice.

I've added test for all the commands that print the warning, the output
can be found in tests/cli-tests-results.txt after

  $ make TEST=014\* test-cli

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-04-30 16:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-04 10:32 [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem Goffredo Baroncelli
2020-04-04 10:32 ` [PATCH 1/5] btrfs-progs: Add code for checking mixed profile function Goffredo Baroncelli
2020-04-04 10:32 ` [PATCH 2/5] btrfs-progs: Add mixed profiles check to some btrfs sub-commands Goffredo Baroncelli
2020-04-04 10:32 ` [PATCH 3/5] Add check for multiple profile in btrfs fi us Goffredo Baroncelli
2020-04-04 10:32 ` [PATCH 4/5] Add further check in btrfs subcommand Goffredo Baroncelli
2020-04-04 10:32 ` [PATCH 5/5] Update the man page in order to give a guideline about mixed profiles Goffredo Baroncelli
2020-04-06 17:57 ` [PATCH v3] btrfs-progs: add warning for mixed profiles filesystem David Sterba
2020-04-30 16:09   ` David Sterba
2020-04-30 13:37 ` Johannes Thumshirn
2020-04-30 14:04   ` David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.