[v2] Btrfs: Check metadata redundancy on balance

Commit Message

Sam Tygier Dec. 8, 2015, 9:25 a.m. UTC

Resending as previous comments did not need any changes.

Currently BTRFS allows you to make bad choices of data and 
metadata levels. For example -d raid1 -m raid0 means you can
only use half your total disk space, but will loose everything
if 1 disk fails. It should give a warning in these cases.

This patch is a follow up to
[PATCH v2] btrfs-progs: check metadata redundancy
in order to cover the case of using balance to convert to such
a set of raid levels.

A simple example to hit this is to create a single device fs, 
which will default to single:dup, then to add a second device and
attempt to convert to raid1 with the command
btrfs balance start -dconvert=raid1  /mnt
this will result in a filesystem with raid1:dup, which will not
survive the loss of one drive. I personally don't see why the tools
should allow this, but in the previous thread a warning was
considered sufficient.

Changes in v2
Use btrfs_get_num_tolerated_disk_barrier_failures()

Signed-off-by: Sam Tygier <[e-mail]>

From: Sam Tygier <[e-mail]>
Date: Sat, 3 Oct 2015 16:43:48 +0100
Subject: [PATCH] Btrfs: Check metadata redundancy on balance

When converting a filesystem via balance check that metadata mode
is at least as redundant as the data mode. For example give warning
when:
-dconvert=raid1 -mconvert=single
---
 fs/btrfs/volumes.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

David Sterba Jan. 5, 2016, 2:41 p.m. UTC | #1

On Tue, Dec 08, 2015 at 09:25:03AM +0000, sam tygier wrote:
> Signed-off-by: Sam Tygier <[e-mail]>
> 
> From: Sam Tygier <[e-mail]>
> Date: Sat, 3 Oct 2015 16:43:48 +0100
> Subject: [PATCH] Btrfs: Check metadata redundancy on balance
> 
> When converting a filesystem via balance check that metadata mode
> is at least as redundant as the data mode. For example give warning
> when:
> -dconvert=raid1 -mconvert=single

Missing signed-off-by

> ---
>  fs/btrfs/volumes.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6fc73586..40247e9 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3584,6 +3584,12 @@ int btrfs_balance(struct btrfs_balance_control *bctl,
>  		}
>  	} while (read_seqretry(&fs_info->profiles_lock, seq));
>  
> +	if (btrfs_get_num_tolerated_disk_barrier_failures(bctl->meta.target) <
> +		btrfs_get_num_tolerated_disk_barrier_failures(bctl->data.target)) {
> +		btrfs_info(fs_info,
> +			"Warning: metatdata has lower redundancy than data\n");

If it's a warning then please use btrfs_warn. The message gets a higher
priority and would be caught by log scanners as an issue worth attention.

Also, explicitly mentioning the profiles for data and metadata would be
better.

And sorry that it takes so long to get this patch merged.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6fc73586..40247e9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3584,6 +3584,12 @@  int btrfs_balance(struct btrfs_balance_control *bctl,
 		}
 	} while (read_seqretry(&fs_info->profiles_lock, seq));
 
+	if (btrfs_get_num_tolerated_disk_barrier_failures(bctl->meta.target) <
+		btrfs_get_num_tolerated_disk_barrier_failures(bctl->data.target)) {
+		btrfs_info(fs_info,
+			"Warning: metatdata has lower redundancy than data\n");
+	}
+
 	if (bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) {
 		fs_info->num_tolerated_disk_barrier_failures = min(
 			btrfs_calc_num_tolerated_disk_barrier_failures(fs_info),