Btrfs devel - Nov 2011 - Don't prevent removal of devices that break raid reqs

Instead of preventing the removal of devices that would render existing
raid10 or raid1 impossible, warn but go ahead with it; the rebalancing
code is smart enough to use different block group types.

Should the refusal remain, so that we''d only proceed with a
newly-introduced --force option or so?

Signed-off-by: Alexandre Oliva <oliva@lsd.ic.unicamp.br>
---
 fs/btrfs/volumes.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 4d5b29f..507afca 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1281,18 +1281,14 @@ int btrfs_rm_device(struct btrfs_root *root, char
*device_path)
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) &&
 	    root->fs_info->fs_devices->num_devices <= 4) {
-		printk(KERN_ERR "btrfs: unable to go below four devices "
-		       "on raid10\n");
-		ret = -EINVAL;
-		goto out;
+		printk(KERN_ERR "btrfs: going below four devices "
+		       "will turn raid10 into raid1\n");
 	}
 
 	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) &&
 	    root->fs_info->fs_devices->num_devices <= 2) {
-		printk(KERN_ERR "btrfs: unable to go below two "
-		       "devices on raid1\n");
-		ret = -EINVAL;
-		goto out;
+		printk(KERN_ERR "btrfs: going below two devices "
+		       "will lose raid1 redundancy\n");
 	}
 
 	if (strcmp(device_path, "missing") == 0) {
-- 
1.7.4.4


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer
--
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

On Thu, Nov 10, 2011 at 05:32:48PM -0200, Alexandre Oliva
wrote:
> Instead of preventing the removal of devices that would render existing
> raid10 or raid1 impossible, warn but go ahead with it; the rebalancing
> code is smart enough to use different block group types.
> 
> Should the refusal remain, so that we''d only proceed with a
> newly-introduced --force option or so?
Hmm, going to three devices on raid10 doesn''t turn it into
raid1.  It turns it into a degraded raid10.

We''ll need a --force or some kind.  There are definitely cases users
have wanted to do this but it is rarely a good idea ;)

-chris
> 
> Signed-off-by: Alexandre Oliva <oliva@lsd.ic.unicamp.br>
> ---
>  fs/btrfs/volumes.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 4d5b29f..507afca 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1281,18 +1281,14 @@ int btrfs_rm_device(struct btrfs_root *root, char
*device_path)
>  
>  	if ((all_avail & BTRFS_BLOCK_GROUP_RAID10) &&
>  	    root->fs_info->fs_devices->num_devices <= 4) {
> -		printk(KERN_ERR "btrfs: unable to go below four devices "
> -		       "on raid10\n");
> -		ret = -EINVAL;
> -		goto out;
> +		printk(KERN_ERR "btrfs: going below four devices "
> +		       "will turn raid10 into raid1\n");
>  	}
>  
>  	if ((all_avail & BTRFS_BLOCK_GROUP_RAID1) &&
>  	    root->fs_info->fs_devices->num_devices <= 2) {
> -		printk(KERN_ERR "btrfs: unable to go below two "
> -		       "devices on raid1\n");
> -		ret = -EINVAL;
> -		goto out;
> +		printk(KERN_ERR "btrfs: going below two devices "
> +		       "will lose raid1 redundancy\n");
>  	}
>  
>  	if (strcmp(device_path, "missing") == 0) {
> -- 
> 1.7.4.4
> 
> 
> -- 
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer
> --
> 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
--
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

On Thu, Nov 10, 2011 at 09:21:00PM -0500, Chris Mason
wrote:
> On Thu, Nov 10, 2011 at 05:32:48PM -0200, Alexandre Oliva wrote:
> > Instead of preventing the removal of devices that would render
existing
> > raid10 or raid1 impossible, warn but go ahead with it; the rebalancing
> > code is smart enough to use different block group types.
> > 
> > Should the refusal remain, so that we''d only proceed with a
> > newly-introduced --force option or so?
> 
> Hmm, going to three devices on raid10 doesn''t turn it into
> raid1.  It turns it into a degraded raid10.
> 
> We''ll need a --force or some kind.  There are definitely cases
users
> have wanted to do this but it is rarely a good idea ;)
I''m not sure about use cases Chris talks about, but sans those I think
we should prevent breaking raids.  If user wants to downgrade his FS he
can do that explicitly with restriper.  As for the relocation code
''smartness'', we already have a confusing case where balancing
silently
upgrades single to raid0.

Chris, can you describe those cases in detail so I can integrate and
align this whole thing with restriper before it''s merged ?  (I added a
--force option for some of the transitions, probably best not to add
another closely related one)

Thanks,

		Ilya
--
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

On Tue, Nov 15, 2011 at 11:37:13AM +0200, Ilya Dryomov
wrote:
> On Thu, Nov 10, 2011 at 09:21:00PM -0500, Chris Mason wrote:
> > On Thu, Nov 10, 2011 at 05:32:48PM -0200, Alexandre Oliva wrote:
> > > Instead of preventing the removal of devices that would render
existing
> > > raid10 or raid1 impossible, warn but go ahead with it; the
rebalancing
> > > code is smart enough to use different block group types.
> > > 
> > > Should the refusal remain, so that we''d only proceed
with a
> > > newly-introduced --force option or so?
> > 
> > Hmm, going to three devices on raid10 doesn''t turn it into
> > raid1.  It turns it into a degraded raid10.
> > 
> > We''ll need a --force or some kind.  There are definitely
cases users
> > have wanted to do this but it is rarely a good idea ;)
> 
> I''m not sure about use cases Chris talks about, but sans those I
think
> we should prevent breaking raids.  If user wants to downgrade his FS he
> can do that explicitly with restriper.  As for the relocation code
> ''smartness'', we already have a confusing case where
balancing silently
> upgrades single to raid0.
> 
> Chris, can you describe those cases in detail so I can integrate and
> align this whole thing with restriper before it''s merged ?  (I
added a
> --force option for some of the transitions, probably best not to add
> another closely related one)
There are a few valid use cases where people want to be able to
"break"
a raid1.  I''d put it at the very bottom of the list of interesting
things, just because I see a long list of bug reports that start with:

my FS was broken, so I told it to remove device xxyzzz, and that didn''t
work so I ran --force, and then (sad story follows).

-chris

--
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

On Nov 15, 2011, Ilya Dryomov <idryomov@gmail.com> wrote:
> On Thu, Nov 10, 2011 at 09:21:00PM -0500, Chris Mason wrote:
>> On Thu, Nov 10, 2011 at 05:32:48PM -0200, Alexandre Oliva wrote:
>> > Instead of preventing the removal of devices that would render
existing
>> > raid10 or raid1 impossible, warn but go ahead with it; the
rebalancing
>> > code is smart enough to use different block group types.
>> > 
>> > Should the refusal remain, so that we''d only proceed with
a
>> > newly-introduced --force option or so?
>> 
>> Hmm, going to three devices on raid10 doesn''t turn it into
>> raid1.  It turns it into a degraded raid10.
Oops.  I *knew* there was some case I had failed to test.  I thought
I''d
the two relevant cases, but I must have tested only raid0 to single and
raid1 to dup.
> If user wants to downgrade his FS he
> can do that explicitly with restriper.
Didn''t I just read in another thread that restriper won''t let
one switch
from raid1 to dup without going through single in between?  That''s what
I actually needed to do that drove me to write the patch a while ago
after accidentally adding a device to the wrong filesystem, and if
restriper won''t do it without going through a riskier stage in between
(metadata loss due to e.g. a bad block), I wish it was improved so that
this is possible.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer
--
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

On Nov 11, 2011, Chris Mason <chris.mason@oracle.com> wrote:
> On Thu, Nov 10, 2011 at 05:32:48PM -0200, Alexandre Oliva wrote:
>> Instead of preventing the removal of devices that would render existing
>> raid10 or raid1 impossible, warn but go ahead with it; the rebalancing
>> code is smart enough to use different block group types.
> We''ll need a --force or some kind.  There are definitely cases
users
> have wanted to do this but it is rarely a good idea ;)
Even if it''s just metadata that will turn from raid1 to dup, as in the
revised patch below?







-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

On 11/10/2011 2:32 PM, Alexandre Oliva wrote:
> Instead of preventing the removal of devices that would render existing
> raid10 or raid1 impossible, warn but go ahead with it; the rebalancing
> code is smart enough to use different block group types.
>
> Should the refusal remain, so that we''d only proceed with a
> newly-introduced --force option or so?
I just thought of something.  When adding the second device, balance 
converts DUP to RAID1 automatically, and it is the RAID1 that prevents 
removing the second disk.  What if the chunks were left with both the 
DUP and RAID1 flags set?  That way if you explicitly requested raid1, 
then it won''t let you accidentally drop below two disks, but if it were
auto promoted from DUP, then going back to DUP is ok.

--
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