Btrfs: Check metadata redundancy on balance
PATCH 1/1 Btrfs: Check metadata redundancy on balance
It was recommended that I resend after the merge window. No changes since last version.
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.
From: Sam Tygier <[email protected]>
Date: Sat, 13 Jun 2015 18:13:06 +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 | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
fs/btrfs/volumes.c +24 -0
@@ -3454,6 +3454,24 @@ static void __cancel_balance(struct btrfs_fs_info *fs_info)atomic_set (&fs_info->mutually_exclusive_operation_running,0 ); } +static int group_profile_max_safe_loss (u64 flag) +{ +switch (flag &BTRFS_BLOCK_GROUP_PROFILE_MASK ) { +case 0 :/* single */ +case BTRFS_BLOCK_GROUP_DUP : +case BTRFS_BLOCK_GROUP_RAID0 : +return 0 ; +case BTRFS_BLOCK_GROUP_RAID1 : +case BTRFS_BLOCK_GROUP_RAID5 : +case BTRFS_BLOCK_GROUP_RAID10 : +return 1 ; +case BTRFS_BLOCK_GROUP_RAID6 : +return 2 ; +default : +return -1 ; + } +} +/* * Should be called with both balance and volume mutexes held */
@@ -3572,6 +3590,12 @@ int btrfs_balance(struct btrfs_balance_control *bctl, } }while (read_seqretry (&fs_info->profiles_lock, seq)); +if (group_profile_max_safe_loss (bctl->meta.target) < +group_profile_max_safe_loss (bctl->data.target)){ +btrfs_info (fs_info, +"Warning: metatdata has lower redundancy than data\n" ); + } +if (bctl->sys.flags &BTRFS_BALANCE_ARGS_CONVERT ) {int num_tolerated_disk_barrier_failures;u64 target = bctl->sys.target;
Signature · 1 line
2.4.3
Hi, sam tygier
Quoted 10 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of sam tygier
Sent: Wednesday, September 16, 2015 4:42 PM
To: [email protected]
Subject: [PATCH] Btrfs: Check metadata redundancy on balanceIt was recommended that I resend after the merge window. No changes since last version.
Quoted 8 lines
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.
Can we check and show warning of balance operation in btrfs-progs, just like above patch?
Quoted 36 lines
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.
Signed-off-by Sam Tygier <[email protected]>From: Sam Tygier <[email protected]>
Date: Sat, 13 Jun 2015 18:13:06 +0100
Subject: [PATCH] Btrfs: Check metadata redundancy on balanceWhen converting a filesystem via balance check that metadata mode is at least as redundant as the data mode. For example give warning
when:(quoted diff) +0
-2-dconvert=raid1 -mconvert=single --- fs/btrfs/volumes.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)fs/btrfs/volumes.c index fbe7c10..a0ce1f7 renamed from fs/btrfs/volumes.c +0
-0100644
fs/btrfs/volumes.c +7
-0@@ -3454,6 +3454,24 @@ static void __cancel_balance(struct btrfs_fs_info *fs_info)atomic_set (&fs_info->mutually_exclusive_operation_running,0 ); } +static int group_profile_max_safe_loss (u64 flag) { +switch (flag &BTRFS_BLOCK_GROUP_PROFILE_MASK ) { +case 0 :/* single */ +case BTRFS_BLOCK_GROUP_DUP : +case BTRFS_BLOCK_GROUP_RAID0 : +return 0 ; +case BTRFS_BLOCK_GROUP_RAID1 :
fs/btrfs/volumes.c static void __cancel_balance(struct btrfs_fs_info · line 3473
Quoted 10 lines
fs/btrfs/volumes.c +10
-0+case BTRFS_BLOCK_GROUP_RAID5 : +case BTRFS_BLOCK_GROUP_RAID10 : +return 1 ; +case BTRFS_BLOCK_GROUP_RAID6 : +return 2 ; +default : +return -1 ; + } +} +
Maybe btrfs_get_num_tolerated_disk_barrier_failures() fits above request, better to use existence function if possible.
Thanks
Zhaolei
Quoted 25 lines
fs/btrfs/volumes.c +6
-0/* * Should be called with both balance and volume mutexes held */ @@ -3572,6 +3590,12 @@ int btrfs_balance(struct btrfs_balance_control *bctl, } }while (read_seqretry (&fs_info->profiles_lock, seq)); +if (group_profile_max_safe_loss (bctl->meta.target) < +group_profile_max_safe_loss (bctl->data.target)){ +btrfs_info (fs_info, +"Warning: metatdata has lower redundancy than data\n" ); + } +if (bctl->sys.flags &BTRFS_BALANCE_ARGS_CONVERT ) {int num_tolerated_disk_barrier_failures;u64 target =
bctl->sys.target;
--
2.4.3--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 16 lines · Zhao Lei
Hi, sam tygier
Quoted 14 lines
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of sam tygier
Sent: Wednesday, September 16, 2015 4:42 PM
To: [email protected]
Subject: [PATCH] Btrfs: Check metadata redundancy on balanceIt was recommended that I resend after the merge window. No changes since last version.
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.
Quoted 8 lines
Quoted 4 lines
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.Can we check and show warning of balance operation in btrfs-progs, just like above patch?
I was not completely sure if this was better suited in btrfs-progs or the kernel. The existing checks against reducing redundancy are on the kernel side. Currently it looks like btrfs-progs does look at the current levels, only passes the arguments through to the kernel. If this was put on the btrfs-progs side, would it be an issue that some other tool might call the kernel directly, and bypass the check.
Quoted 41 lines
Quoted 41 lines
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.
Signed-off-by Sam Tygier <[email protected]>From: Sam Tygier <[email protected]>
Date: Sat, 13 Jun 2015 18:13:06 +0100
Subject: [PATCH] Btrfs: Check metadata redundancy on balanceWhen converting a filesystem via balance check that metadata mode is at least as redundant as the data mode. For example give warning
when:(quoted diff) +0
-2-dconvert=raid1 -mconvert=single --- fs/btrfs/volumes.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)fs/btrfs/volumes.c index fbe7c10..a0ce1f7 renamed from fs/btrfs/volumes.c +0
-0100644
fs/btrfs/volumes.c +12
-0@@ -3454,6 +3454,24 @@ static void __cancel_balance(struct btrfs_fs_info *fs_info)atomic_set (&fs_info->mutually_exclusive_operation_running,0 ); } +static int group_profile_max_safe_loss (u64 flag) { +switch (flag &BTRFS_BLOCK_GROUP_PROFILE_MASK ) { +case 0 :/* single */ +case BTRFS_BLOCK_GROUP_DUP : +case BTRFS_BLOCK_GROUP_RAID0 : +return 0 ; +case BTRFS_BLOCK_GROUP_RAID1 : +case BTRFS_BLOCK_GROUP_RAID5 : +case BTRFS_BLOCK_GROUP_RAID10 : +return 1 ; +case BTRFS_BLOCK_GROUP_RAID6 : +return 2 ;
Quoted 8 lines
fs/btrfs/volumes.c static void __cancel_balance(struct btrfs_fs_info *fs_info) · line 3474
Quoted 5 lines
fs/btrfs/volumes.c +5
-0+default : +return -1 ; + } +} +Maybe
btrfs_get_num_tolerated_disk_barrier_failures()fits above request, better to use existence function if possible.
Sorry, I missed that. If it stays kernel side, I can switch to this. If it moves to btrfs-progs I'll use the existing group_profile_max_safe_loss()
Quoted 23 lines
Thanks
ZhaoleiQuoted 19 lines
fs/btrfs/volumes.c +6
-0/* * Should be called with both balance and volume mutexes held */ @@ -3572,6 +3590,12 @@ int btrfs_balance(struct btrfs_balance_control *bctl, } }while (read_seqretry (&fs_info->profiles_lock, seq)); +if (group_profile_max_safe_loss (bctl->meta.target) < +group_profile_max_safe_loss (bctl->data.target)){ +btrfs_info (fs_info, +"Warning: metatdata has lower redundancy than data\n" ); + } +if (bctl->sys.flags &BTRFS_BALANCE_ARGS_CONVERT ) {int num_tolerated_disk_barrier_failures;u64 target =
bctl->sys.target;
--
2.4.3