* [PATCH] Btrfs: Check metadata redundancy on balance
@ 2015-08-31 18:48 sam tygier
0 siblings, 0 replies; 4+ messages in thread
From: sam tygier @ 2015-08-31 18:48 UTC (permalink / raw)
To: linux-btrfs
Resending, as I received no comments on previous submission.
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.
Signed-off-by: Sam Tygier <samtygier@yahoo.co.uk>
From: Sam Tygier <samtygier@yahoo.co.uk>
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(+)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index fbe7c10..a0ce1f7 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -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;
--
2.4.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] Btrfs: Check metadata redundancy on balance
@ 2015-09-16 8:42 sam tygier
2015-09-16 10:15 ` Zhao Lei
0 siblings, 1 reply; 4+ messages in thread
From: sam tygier @ 2015-09-16 8:42 UTC (permalink / raw)
To: linux-btrfs
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.
Signed-off-by: Sam Tygier <samtygier@yahoo.co.uk>
From: Sam Tygier <samtygier@yahoo.co.uk>
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(+)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index fbe7c10..a0ce1f7 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -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;
--
2.4.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH] Btrfs: Check metadata redundancy on balance
2015-09-16 8:42 sam tygier
@ 2015-09-16 10:15 ` Zhao Lei
2015-09-20 8:31 ` sam tygier
0 siblings, 1 reply; 4+ messages in thread
From: Zhao Lei @ 2015-09-16 10:15 UTC (permalink / raw)
To: 'sam tygier', linux-btrfs
Hi, sam tygier
> -----Original Message-----
> From: linux-btrfs-owner@vger.kernel.org
> [mailto:linux-btrfs-owner@vger.kernel.org] On Behalf Of sam tygier
> Sent: Wednesday, September 16, 2015 4:42 PM
> To: linux-btrfs@vger.kernel.org
> Subject: [PATCH] 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.
>
Can we check and show warning of balance operation in btrfs-progs,
just like above patch?
> 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 <samtygier@yahoo.co.uk>
>
> From: Sam Tygier <samtygier@yahoo.co.uk>
> 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(+)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index fbe7c10..a0ce1f7
> 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -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;
> + }
> +}
> +
Maybe btrfs_get_num_tolerated_disk_barrier_failures()
fits above request, better to use existence function if possible.
Thanks
Zhaolei
> /*
> * 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Btrfs: Check metadata redundancy on balance
2015-09-16 10:15 ` Zhao Lei
@ 2015-09-20 8:31 ` sam tygier
0 siblings, 0 replies; 4+ messages in thread
From: sam tygier @ 2015-09-20 8:31 UTC (permalink / raw)
To: linux-btrfs
On 16/09/15 11:15, Zhao Lei wrote:
> Hi, sam tygier
>
>> -----Original Message-----
>> From: linux-btrfs-owner@vger.kernel.org
>> [mailto:linux-btrfs-owner@vger.kernel.org] On Behalf Of sam tygier
>> Sent: Wednesday, September 16, 2015 4:42 PM
>> To: linux-btrfs@vger.kernel.org
>> Subject: [PATCH] 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.
>>
>
> 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.
>> 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 <samtygier@yahoo.co.uk>
>>
>> From: Sam Tygier <samtygier@yahoo.co.uk>
>> 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(+)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index fbe7c10..a0ce1f7
>> 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -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;
>> + }
>> +}
>> +
>
> 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()
> Thanks
> Zhaolei
>
>
>> /*
>> * 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-09-20 8:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-31 18:48 [PATCH] Btrfs: Check metadata redundancy on balance sam tygier
-- strict thread matches above, loose matches on Subject: below --
2015-09-16 8:42 sam tygier
2015-09-16 10:15 ` Zhao Lei
2015-09-20 8:31 ` sam tygier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).