From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-btrfs-owner@vger.kernel.org>
Received: from mail-ie0-f175.google.com ([209.85.223.175]:56526 "EHLO
	mail-ie0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753801AbaBSQKo (ORCPT
	<rfc822;linux-btrfs@vger.kernel.org>);
	Wed, 19 Feb 2014 11:10:44 -0500
Received: by mail-ie0-f175.google.com with SMTP id at1so373691iec.34
        for <linux-btrfs@vger.kernel.org>; Wed, 19 Feb 2014 08:10:44 -0800 (PST)
Received: from [10.0.1.18] (user-0cdvcvs.cable.mindspring.com. [24.223.179.252])
        by mx.google.com with ESMTPSA id y9sm28916634igl.7.2014.02.19.08.10.42
        for <linux-btrfs@vger.kernel.org>
        (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128);
        Wed, 19 Feb 2014 08:10:42 -0800 (PST)
Message-ID: <5304D781.6020309@gmail.com>
Date: Wed, 19 Feb 2014 11:10:41 -0500
From: Austin S Hemmelgarn <ahferroin7@gmail.com>
MIME-Version: 1.0
To: linux-btrfs@vger.kernel.org
Subject: [PATCH] btrfs: Allow forced conversion of metadata to dup profile
 on multiple devices
Content-Type: text/plain; charset=UTF-8
Sender: linux-btrfs-owner@vger.kernel.org
List-ID: <linux-btrfs.vger.kernel.org>

Currently, btrfs balance start fails when trying to convert metadata or
system chunks to dup profile on filesystems with multiple devices.  This
requires that a conversion from a multi-device filesystem to a single
device filesystem use the following methodology:
    1. btrfs balance start -dconvert=single -mconvert=single \
       -sconvert=single -f /
    2. btrfs device delete / /dev/sdx
    3. btrfs balance start -mconvert=dup -sconvert=dup /
This results in a period of time (possibly very long if the devices are
big) where you don't have the protection guarantees of multiple copies
of metadata chunks.

After applying this patch, one can instead use the following methodology
for conversion from a multi-device filesystem to a single device
filesystem:
    1. btrfs balance start -dconvert=single -mconvert=dup \
       -sconvert=dup -f /
    2. btrfs device delete / /dev/sdx
This greatly reduces the chances of the operation causing data loss due
to a read error during the device delete.

Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
---
 fs/btrfs/volumes.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 07629e9..38a9522 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3152,10 +3152,8 @@ int btrfs_balance(struct btrfs_balance_control
*bctl,
 		num_devices--;
 	}
 	btrfs_dev_replace_unlock(&fs_info->dev_replace);
-	allowed = BTRFS_AVAIL_ALLOC_BIT_SINGLE;
-	if (num_devices == 1)
-		allowed |= BTRFS_BLOCK_GROUP_DUP;
-	else if (num_devices > 1)
+	allowed = BTRFS_AVAIL_ALLOC_BIT_SINGLE | BTRFS_BLOCK_GROUP_DUP;
+	if (num_devices > 1)
 		allowed |= (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1);
 	if (num_devices > 2)
 		allowed |= BTRFS_BLOCK_GROUP_RAID5;
@@ -3221,6 +3219,21 @@ int btrfs_balance(struct btrfs_balance_control
*bctl,
 				goto out;
 			}
 		}
+		if (((bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) &&
+		    (bctl->sys.target & ~BTRFS_BLOCK_GROUP_DUP) ||
+		    (bctl->meta.flags & BTRFS_BALANCE_ARGS_CONVERT) &&
+		    (bctl->meta.target & ~BTRFS_BLOCK_GROUP_DUP)) &&
+		    (num_devs > 1)) {
+			if (bctl->flags & BTRFS_BALANCE_FORCE) {
+				btrfs_info(fs_info, "force conversion of metadata "
+					   "to dup profile on multiple devices");
+			} else {
+				btrfs_err(fs_info, "balance will reduce metadata "
+					  "integrity, use force if you want this");
+				ret = -EINVAL;
+				goto out;
+			}
+		}
 	} while (read_seqretry(&fs_info->profiles_lock, seq));
  	if (bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) {
-- 
1.8.5.4


From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-btrfs-owner@vger.kernel.org>
Received: from cantor2.suse.de ([195.135.220.15]:57405 "EHLO mx2.suse.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1755141AbaBTQ5p (ORCPT <rfc822;linux-btrfs@vger.kernel.org>);
	Thu, 20 Feb 2014 11:57:45 -0500
Date: Thu, 20 Feb 2014 17:57:43 +0100
From: David Sterba <dsterba@suse.cz>
To: Austin S Hemmelgarn <ahferroin7@gmail.com>
Cc: linux-btrfs@vger.kernel.org, idryomov@gmail.com
Subject: Re: [PATCH] btrfs: Allow forced conversion of metadata to dup
 profile on multiple devices
Message-ID: <20140220165743.GT16073@twin.jikos.cz>
Reply-To: dsterba@suse.cz
References: <5304D781.6020309@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
In-Reply-To: <5304D781.6020309@gmail.com>
Sender: linux-btrfs-owner@vger.kernel.org
List-ID: <linux-btrfs.vger.kernel.org>

On Wed, Feb 19, 2014 at 11:10:41AM -0500, Austin S Hemmelgarn wrote:
> Currently, btrfs balance start fails when trying to convert metadata or
> system chunks to dup profile on filesystems with multiple devices.  This
> requires that a conversion from a multi-device filesystem to a single
> device filesystem use the following methodology:
>     1. btrfs balance start -dconvert=single -mconvert=single \
>        -sconvert=single -f /
>     2. btrfs device delete / /dev/sdx
>     3. btrfs balance start -mconvert=dup -sconvert=dup /
> This results in a period of time (possibly very long if the devices are
> big) where you don't have the protection guarantees of multiple copies
> of metadata chunks.
> 
> After applying this patch, one can instead use the following methodology
> for conversion from a multi-device filesystem to a single device
> filesystem:
>     1. btrfs balance start -dconvert=single -mconvert=dup \
>        -sconvert=dup -f /
>     2. btrfs device delete / /dev/sdx
> This greatly reduces the chances of the operation causing data loss due
> to a read error during the device delete.
> 
> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.cz>

Sounds useful. The muliple devices + DUP is allowed setup when the
device is added, this patch only adds the 'delete' counterpart. The
imroved data loss protection during the process is a good thing.

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-btrfs-owner@vger.kernel.org>
Received: from mail-wi0-f181.google.com ([209.85.212.181]:56993 "EHLO
	mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752301AbaBXNhH (ORCPT
	<rfc822;linux-btrfs@vger.kernel.org>);
	Mon, 24 Feb 2014 08:37:07 -0500
Received: by mail-wi0-f181.google.com with SMTP id hi5so3045034wib.8
        for <linux-btrfs@vger.kernel.org>; Mon, 24 Feb 2014 05:37:05 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <20140220165743.GT16073@twin.jikos.cz>
References: <5304D781.6020309@gmail.com>
	<20140220165743.GT16073@twin.jikos.cz>
Date: Mon, 24 Feb 2014 15:37:05 +0200
Message-ID: <CAOi1vP9hEqBcDgY-zthijx37+_cowhoMQaTZyWG9OUqOQvJR2A@mail.gmail.com>
Subject: Re: [PATCH] btrfs: Allow forced conversion of metadata to dup profile
 on multiple devices
From: Ilya Dryomov <idryomov@gmail.com>
To: David Sterba <dsterba@suse.cz>, Austin S Hemmelgarn <ahferroin7@gmail.com>,
        linux-btrfs@vger.kernel.org, Ilya Drymov <idryomov@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Sender: linux-btrfs-owner@vger.kernel.org
List-ID: <linux-btrfs.vger.kernel.org>

On Thu, Feb 20, 2014 at 6:57 PM, David Sterba <dsterba@suse.cz> wrote:
> On Wed, Feb 19, 2014 at 11:10:41AM -0500, Austin S Hemmelgarn wrote:
>> Currently, btrfs balance start fails when trying to convert metadata or
>> system chunks to dup profile on filesystems with multiple devices.  This
>> requires that a conversion from a multi-device filesystem to a single
>> device filesystem use the following methodology:
>>     1. btrfs balance start -dconvert=single -mconvert=single \
>>        -sconvert=single -f /
>>     2. btrfs device delete / /dev/sdx
>>     3. btrfs balance start -mconvert=dup -sconvert=dup /
>> This results in a period of time (possibly very long if the devices are
>> big) where you don't have the protection guarantees of multiple copies
>> of metadata chunks.
>>
>> After applying this patch, one can instead use the following methodology
>> for conversion from a multi-device filesystem to a single device
>> filesystem:
>>     1. btrfs balance start -dconvert=single -mconvert=dup \
>>        -sconvert=dup -f /
>>     2. btrfs device delete / /dev/sdx
>> This greatly reduces the chances of the operation causing data loss due
>> to a read error during the device delete.
>>
>> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
> Reviewed-by: David Sterba <dsterba@suse.cz>
>
> Sounds useful. The muliple devices + DUP is allowed setup when the
> device is added, this patch only adds the 'delete' counterpart. The
> imroved data loss protection during the process is a good thing.

Hi,

Have you actually tried to queue it?  Unless I'm missing something, it won't
compile, and on top of that, it seems to be corrupted too..

IIRC muliple devices + DUP is "allowed" only until the first balance, has that
changed?

Thanks,

                Ilya

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-btrfs-owner@vger.kernel.org>
Received: from mail-ig0-f172.google.com ([209.85.213.172]:58101 "EHLO
	mail-ig0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752399AbaBXNoZ (ORCPT
	<rfc822;linux-btrfs@vger.kernel.org>);
	Mon, 24 Feb 2014 08:44:25 -0500
Received: by mail-ig0-f172.google.com with SMTP id h3so4712422igd.5
        for <linux-btrfs@vger.kernel.org>; Mon, 24 Feb 2014 05:44:24 -0800 (PST)
Message-ID: <530B4CB4.5010909@gmail.com>
Date: Mon, 24 Feb 2014 08:44:20 -0500
From: Austin S Hemmelgarn <ahferroin7@gmail.com>
MIME-Version: 1.0
To: Ilya Dryomov <idryomov@gmail.com>, David Sterba <dsterba@suse.cz>,
        linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: Allow forced conversion of metadata to dup profile
 on multiple devices
References: <5304D781.6020309@gmail.com>	<20140220165743.GT16073@twin.jikos.cz> <CAOi1vP9hEqBcDgY-zthijx37+_cowhoMQaTZyWG9OUqOQvJR2A@mail.gmail.com>
In-Reply-To: <CAOi1vP9hEqBcDgY-zthijx37+_cowhoMQaTZyWG9OUqOQvJR2A@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Sender: linux-btrfs-owner@vger.kernel.org
List-ID: <linux-btrfs.vger.kernel.org>

On 2014-02-24 08:37, Ilya Dryomov wrote:
> On Thu, Feb 20, 2014 at 6:57 PM, David Sterba <dsterba@suse.cz> wrote:
>> On Wed, Feb 19, 2014 at 11:10:41AM -0500, Austin S Hemmelgarn wrote:
>>> Currently, btrfs balance start fails when trying to convert metadata or
>>> system chunks to dup profile on filesystems with multiple devices.  This
>>> requires that a conversion from a multi-device filesystem to a single
>>> device filesystem use the following methodology:
>>>     1. btrfs balance start -dconvert=single -mconvert=single \
>>>        -sconvert=single -f /
>>>     2. btrfs device delete / /dev/sdx
>>>     3. btrfs balance start -mconvert=dup -sconvert=dup /
>>> This results in a period of time (possibly very long if the devices are
>>> big) where you don't have the protection guarantees of multiple copies
>>> of metadata chunks.
>>>
>>> After applying this patch, one can instead use the following methodology
>>> for conversion from a multi-device filesystem to a single device
>>> filesystem:
>>>     1. btrfs balance start -dconvert=single -mconvert=dup \
>>>        -sconvert=dup -f /
>>>     2. btrfs device delete / /dev/sdx
>>> This greatly reduces the chances of the operation causing data loss due
>>> to a read error during the device delete.
>>>
>>> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>> Reviewed-by: David Sterba <dsterba@suse.cz>
>>
>> Sounds useful. The muliple devices + DUP is allowed setup when the
>> device is added, this patch only adds the 'delete' counterpart. The
>> imroved data loss protection during the process is a good thing.
> 
> Hi,
> 
> Have you actually tried to queue it?  Unless I'm missing something, it won't
> compile, and on top of that, it seems to be corrupted too..
The patch itself was made using git, AFAICT it should be fine.  I've
personally built and tested it using UML.
> 
> IIRC muliple devices + DUP is "allowed" only until the first balance, has that
> changed?

This is just a limitation of how the kernel handles balances, DUP
profiles with multiple devices work, it's just terribly inefficient.
The primary use case is converting a multi-device FS with RAID for
metadata to a single device FS without having to reduce integrity.
> Thanks,
> 
>                 Ilya
> 

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-btrfs-owner@vger.kernel.org>
Received: from mail-wg0-f49.google.com ([74.125.82.49]:48868 "EHLO
	mail-wg0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752249AbaBXOM5 (ORCPT
	<rfc822;linux-btrfs@vger.kernel.org>);
	Mon, 24 Feb 2014 09:12:57 -0500
Received: by mail-wg0-f49.google.com with SMTP id y10so4643511wgg.4
        for <linux-btrfs@vger.kernel.org>; Mon, 24 Feb 2014 06:12:56 -0800 (PST)
MIME-Version: 1.0
In-Reply-To: <530B4CB4.5010909@gmail.com>
References: <5304D781.6020309@gmail.com>
	<20140220165743.GT16073@twin.jikos.cz>
	<CAOi1vP9hEqBcDgY-zthijx37+_cowhoMQaTZyWG9OUqOQvJR2A@mail.gmail.com>
	<530B4CB4.5010909@gmail.com>
Date: Mon, 24 Feb 2014 16:12:56 +0200
Message-ID: <CAOi1vP9-sFDxomB7E-xspfufoUn5iqjgtW=3oFfkdjq_=cmzmg@mail.gmail.com>
Subject: Re: [PATCH] btrfs: Allow forced conversion of metadata to dup profile
 on multiple devices
From: Ilya Dryomov <idryomov@gmail.com>
To: Austin S Hemmelgarn <ahferroin7@gmail.com>
Cc: David Sterba <dsterba@suse.cz>, linux-btrfs@vger.kernel.org
Content-Type: text/plain; charset=ISO-8859-1
Sender: linux-btrfs-owner@vger.kernel.org
List-ID: <linux-btrfs.vger.kernel.org>

On Mon, Feb 24, 2014 at 3:44 PM, Austin S Hemmelgarn
<ahferroin7@gmail.com> wrote:
> On 2014-02-24 08:37, Ilya Dryomov wrote:
>> On Thu, Feb 20, 2014 at 6:57 PM, David Sterba <dsterba@suse.cz> wrote:
>>> On Wed, Feb 19, 2014 at 11:10:41AM -0500, Austin S Hemmelgarn wrote:
>>>> Currently, btrfs balance start fails when trying to convert metadata or
>>>> system chunks to dup profile on filesystems with multiple devices.  This
>>>> requires that a conversion from a multi-device filesystem to a single
>>>> device filesystem use the following methodology:
>>>>     1. btrfs balance start -dconvert=single -mconvert=single \
>>>>        -sconvert=single -f /
>>>>     2. btrfs device delete / /dev/sdx
>>>>     3. btrfs balance start -mconvert=dup -sconvert=dup /
>>>> This results in a period of time (possibly very long if the devices are
>>>> big) where you don't have the protection guarantees of multiple copies
>>>> of metadata chunks.
>>>>
>>>> After applying this patch, one can instead use the following methodology
>>>> for conversion from a multi-device filesystem to a single device
>>>> filesystem:
>>>>     1. btrfs balance start -dconvert=single -mconvert=dup \
>>>>        -sconvert=dup -f /
>>>>     2. btrfs device delete / /dev/sdx
>>>> This greatly reduces the chances of the operation causing data loss due
>>>> to a read error during the device delete.
>>>>
>>>> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>>> Reviewed-by: David Sterba <dsterba@suse.cz>
>>>
>>> Sounds useful. The muliple devices + DUP is allowed setup when the
>>> device is added, this patch only adds the 'delete' counterpart. The
>>> imroved data loss protection during the process is a good thing.
>>
>> Hi,
>>
>> Have you actually tried to queue it?  Unless I'm missing something, it won't
>> compile, and on top of that, it seems to be corrupted too..
> The patch itself was made using git, AFAICT it should be fine.  I've
> personally built and tested it using UML.

It doesn't look fine.  It was generated with git, but it got corrupted
on the way: either how you pasted it or the email client you use is the
problem.

On Wed, Feb 19, 2014 at 6:10 PM, Austin S Hemmelgarn
<ahferroin7@gmail.com> wrote:
> Currently, btrfs balance start fails when trying to convert metadata or
> system chunks to dup profile on filesystems with multiple devices.  This
> requires that a conversion from a multi-device filesystem to a single
> device filesystem use the following methodology:
>     1. btrfs balance start -dconvert=single -mconvert=single \
>        -sconvert=single -f /
>     2. btrfs device delete / /dev/sdx
>     3. btrfs balance start -mconvert=dup -sconvert=dup /
> This results in a period of time (possibly very long if the devices are
> big) where you don't have the protection guarantees of multiple copies
> of metadata chunks.
>
> After applying this patch, one can instead use the following methodology
> for conversion from a multi-device filesystem to a single device
> filesystem:
>     1. btrfs balance start -dconvert=single -mconvert=dup \
>        -sconvert=dup -f /
>     2. btrfs device delete / /dev/sdx
> This greatly reduces the chances of the operation causing data loss due
> to a read error during the device delete.
>
> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
> ---
>  fs/btrfs/volumes.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 07629e9..38a9522 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3152,10 +3152,8 @@ int btrfs_balance(struct btrfs_balance_control
> *bctl,

^^^, that should be a single line

>                 num_devices--;
>         }
>         btrfs_dev_replace_unlock(&fs_info->dev_replace);
> -       allowed = BTRFS_AVAIL_ALLOC_BIT_SINGLE;
> -       if (num_devices == 1)
> -               allowed |= BTRFS_BLOCK_GROUP_DUP;
> -       else if (num_devices > 1)
> +       allowed = BTRFS_AVAIL_ALLOC_BIT_SINGLE | BTRFS_BLOCK_GROUP_DUP;
> +       if (num_devices > 1)
>                 allowed |= (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1);
>         if (num_devices > 2)
>                 allowed |= BTRFS_BLOCK_GROUP_RAID5;
> @@ -3221,6 +3219,21 @@ int btrfs_balance(struct btrfs_balance_control
> *bctl,

^^^, ditto

>                                 goto out;
>                         }
>                 }
> +               if (((bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) &&
> +                   (bctl->sys.target & ~BTRFS_BLOCK_GROUP_DUP) ||
> +                   (bctl->meta.flags & BTRFS_BALANCE_ARGS_CONVERT) &&
> +                   (bctl->meta.target & ~BTRFS_BLOCK_GROUP_DUP)) &&
> +                   (num_devs > 1)) {
> +                       if (bctl->flags & BTRFS_BALANCE_FORCE) {
> +                               btrfs_info(fs_info, "force conversion of metadata "
> +                                          "to dup profile on multiple devices");
> +                       } else {
> +                               btrfs_err(fs_info, "balance will reduce metadata "
> +                                         "integrity, use force if you want this");
> +                               ret = -EINVAL;
> +                               goto out;
> +                       }
> +               }
>         } while (read_seqretry(&fs_info->profiles_lock, seq));
>         if (bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) {

^^^, there should be 3 lines of context here.  It looks like an empty
line between "} while ..." and "if (bctl..." is missing.

There is probably more, those just stand out.

Thanks,

                Ilya

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-btrfs-owner@vger.kernel.org>
Received: from mail-ie0-f173.google.com ([209.85.223.173]:40243 "EHLO
	mail-ie0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751948AbaBXOlC (ORCPT
	<rfc822;linux-btrfs@vger.kernel.org>);
	Mon, 24 Feb 2014 09:41:02 -0500
Received: by mail-ie0-f173.google.com with SMTP id lx4so3509365iec.32
        for <linux-btrfs@vger.kernel.org>; Mon, 24 Feb 2014 06:41:01 -0800 (PST)
Message-ID: <530B59FA.3010909@gmail.com>
Date: Mon, 24 Feb 2014 09:40:58 -0500
From: Austin S Hemmelgarn <ahferroin7@gmail.com>
MIME-Version: 1.0
To: Ilya Dryomov <idryomov@gmail.com>
CC: David Sterba <dsterba@suse.cz>, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: Allow forced conversion of metadata to dup profile
 on multiple devices
References: <5304D781.6020309@gmail.com>	<20140220165743.GT16073@twin.jikos.cz>	<CAOi1vP9hEqBcDgY-zthijx37+_cowhoMQaTZyWG9OUqOQvJR2A@mail.gmail.com>	<530B4CB4.5010909@gmail.com> <CAOi1vP9-sFDxomB7E-xspfufoUn5iqjgtW=3oFfkdjq_=cmzmg@mail.gmail.com>
In-Reply-To: <CAOi1vP9-sFDxomB7E-xspfufoUn5iqjgtW=3oFfkdjq_=cmzmg@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Sender: linux-btrfs-owner@vger.kernel.org
List-ID: <linux-btrfs.vger.kernel.org>

On 2014-02-24 09:12, Ilya Dryomov wrote:
> On Mon, Feb 24, 2014 at 3:44 PM, Austin S Hemmelgarn
> <ahferroin7@gmail.com> wrote:
>> On 2014-02-24 08:37, Ilya Dryomov wrote:
>>> On Thu, Feb 20, 2014 at 6:57 PM, David Sterba <dsterba@suse.cz> wrote:
>>>> On Wed, Feb 19, 2014 at 11:10:41AM -0500, Austin S Hemmelgarn wrote:
>>>>> Currently, btrfs balance start fails when trying to convert metadata or
>>>>> system chunks to dup profile on filesystems with multiple devices.  This
>>>>> requires that a conversion from a multi-device filesystem to a single
>>>>> device filesystem use the following methodology:
>>>>>     1. btrfs balance start -dconvert=single -mconvert=single \
>>>>>        -sconvert=single -f /
>>>>>     2. btrfs device delete / /dev/sdx
>>>>>     3. btrfs balance start -mconvert=dup -sconvert=dup /
>>>>> This results in a period of time (possibly very long if the devices are
>>>>> big) where you don't have the protection guarantees of multiple copies
>>>>> of metadata chunks.
>>>>>
>>>>> After applying this patch, one can instead use the following methodology
>>>>> for conversion from a multi-device filesystem to a single device
>>>>> filesystem:
>>>>>     1. btrfs balance start -dconvert=single -mconvert=dup \
>>>>>        -sconvert=dup -f /
>>>>>     2. btrfs device delete / /dev/sdx
>>>>> This greatly reduces the chances of the operation causing data loss due
>>>>> to a read error during the device delete.
>>>>>
>>>>> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>>>> Reviewed-by: David Sterba <dsterba@suse.cz>
>>>>
>>>> Sounds useful. The muliple devices + DUP is allowed setup when the
>>>> device is added, this patch only adds the 'delete' counterpart. The
>>>> imroved data loss protection during the process is a good thing.
>>>
>>> Hi,
>>>
>>> Have you actually tried to queue it?  Unless I'm missing something, it won't
>>> compile, and on top of that, it seems to be corrupted too..
>> The patch itself was made using git, AFAICT it should be fine.  I've
>> personally built and tested it using UML.
> 
> It doesn't look fine.  It was generated with git, but it got corrupted
> on the way: either how you pasted it or the email client you use is the
> problem.
> 
> On Wed, Feb 19, 2014 at 6:10 PM, Austin S Hemmelgarn
> <ahferroin7@gmail.com> wrote:
>> Currently, btrfs balance start fails when trying to convert metadata or
>> system chunks to dup profile on filesystems with multiple devices.  This
>> requires that a conversion from a multi-device filesystem to a single
>> device filesystem use the following methodology:
>>     1. btrfs balance start -dconvert=single -mconvert=single \
>>        -sconvert=single -f /
>>     2. btrfs device delete / /dev/sdx
>>     3. btrfs balance start -mconvert=dup -sconvert=dup /
>> This results in a period of time (possibly very long if the devices are
>> big) where you don't have the protection guarantees of multiple copies
>> of metadata chunks.
>>
>> After applying this patch, one can instead use the following methodology
>> for conversion from a multi-device filesystem to a single device
>> filesystem:
>>     1. btrfs balance start -dconvert=single -mconvert=dup \
>>        -sconvert=dup -f /
>>     2. btrfs device delete / /dev/sdx
>> This greatly reduces the chances of the operation causing data loss due
>> to a read error during the device delete.
>>
>> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
>> ---
>>  fs/btrfs/volumes.c | 21 +++++++++++++++++----
>>  1 file changed, 17 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 07629e9..38a9522 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -3152,10 +3152,8 @@ int btrfs_balance(struct btrfs_balance_control
>> *bctl,
> 
> ^^^, that should be a single line
> 
>>                 num_devices--;
>>         }
>>         btrfs_dev_replace_unlock(&fs_info->dev_replace);
>> -       allowed = BTRFS_AVAIL_ALLOC_BIT_SINGLE;
>> -       if (num_devices == 1)
>> -               allowed |= BTRFS_BLOCK_GROUP_DUP;
>> -       else if (num_devices > 1)
>> +       allowed = BTRFS_AVAIL_ALLOC_BIT_SINGLE | BTRFS_BLOCK_GROUP_DUP;
>> +       if (num_devices > 1)
>>                 allowed |= (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1);
>>         if (num_devices > 2)
>>                 allowed |= BTRFS_BLOCK_GROUP_RAID5;
>> @@ -3221,6 +3219,21 @@ int btrfs_balance(struct btrfs_balance_control
>> *bctl,
> 
> ^^^, ditto
> 
>>                                 goto out;
>>                         }
>>                 }
>> +               if (((bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) &&
>> +                   (bctl->sys.target & ~BTRFS_BLOCK_GROUP_DUP) ||
>> +                   (bctl->meta.flags & BTRFS_BALANCE_ARGS_CONVERT) &&
>> +                   (bctl->meta.target & ~BTRFS_BLOCK_GROUP_DUP)) &&
>> +                   (num_devs > 1)) {
>> +                       if (bctl->flags & BTRFS_BALANCE_FORCE) {
>> +                               btrfs_info(fs_info, "force conversion of metadata "
>> +                                          "to dup profile on multiple devices");
>> +                       } else {
>> +                               btrfs_err(fs_info, "balance will reduce metadata "
>> +                                         "integrity, use force if you want this");
>> +                               ret = -EINVAL;
>> +                               goto out;
>> +                       }
>> +               }
>>         } while (read_seqretry(&fs_info->profiles_lock, seq));
>>         if (bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) {
> 
> ^^^, there should be 3 lines of context here.  It looks like an empty
> line between "} while ..." and "if (bctl..." is missing.
> 
> There is probably more, those just stand out.
> 
> Thanks,
> 
>                 Ilya
> 
I'll rebase and regenerate the patch tonight when I get off of work, I'm
thinking my mail client might have mangled it somehow.

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-btrfs-owner@vger.kernel.org>
Received: from mail-ie0-f181.google.com ([209.85.223.181]:36625 "EHLO
	mail-ie0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751155AbaBZOXU (ORCPT
	<rfc822;linux-btrfs@vger.kernel.org>);
	Wed, 26 Feb 2014 09:23:20 -0500
Received: by mail-ie0-f181.google.com with SMTP id lx4so681096iec.40
        for <linux-btrfs@vger.kernel.org>; Wed, 26 Feb 2014 06:23:20 -0800 (PST)
Received: from [10.0.1.18] (user-0cdvcvs.cable.mindspring.com. [24.223.179.252])
        by mx.google.com with ESMTPSA id gd5sm191940igd.5.2014.02.26.06.23.18
        for <linux-btrfs@vger.kernel.org>
        (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128);
        Wed, 26 Feb 2014 06:23:19 -0800 (PST)
Message-ID: <530DF8D5.7010808@gmail.com>
Date: Wed, 26 Feb 2014 09:23:17 -0500
From: Austin S Hemmelgarn <ahferroin7@gmail.com>
MIME-Version: 1.0
To: "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Subject: [PATCH] btrfs: Allow forced conversion of metadata to dup profile
 on, multiple devices
Content-Type: text/plain; charset=UTF-8
Sender: linux-btrfs-owner@vger.kernel.org
List-ID: <linux-btrfs.vger.kernel.org>

Currently, btrfs balance start fails when trying to convert metadata or
system chunks to dup profile on filesystems with multiple devices.  This
requires that a conversion from a multi-device filesystem to a single
device filesystem use the following methodology:
    1. btrfs balance start -dconvert=single -mconvert=single \
       -sconvert=single -f /
    2. btrfs device delete / /dev/sdx
    3. btrfs balance start -mconvert=dup -sconvert=dup /
This results in a period of time (possibly very long if the devices are
big) where you don't have the protection guarantees of multiple copies
of metadata chunks.

After applying this patch, one can instead use the following methodology
for conversion from a multi-device filesystem to a single device
filesystem:
    1. btrfs balance start -dconvert=single -mconvert=dup \
       -sconvert=dup -f /
    2. btrfs device delete / /dev/sdx
This greatly reduces the chances of the operation causing data loss due
to a read error during the device delete.

Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com>
---
 fs/btrfs/volumes.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 07629e9..38a9522 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3152,10 +3152,8 @@ int btrfs_balance(struct btrfs_balance_control *bctl,
 		num_devices--;
 	}
 	btrfs_dev_replace_unlock(&fs_info->dev_replace);
-	allowed = BTRFS_AVAIL_ALLOC_BIT_SINGLE;
-	if (num_devices == 1)
-		allowed |= BTRFS_BLOCK_GROUP_DUP;
-	else if (num_devices > 1)
+	allowed = BTRFS_AVAIL_ALLOC_BIT_SINGLE | BTRFS_BLOCK_GROUP_DUP;
+	if (num_devices > 1)
 		allowed |= (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1);
 	if (num_devices > 2)
 		allowed |= BTRFS_BLOCK_GROUP_RAID5;
@@ -3221,6 +3219,21 @@ int btrfs_balance(struct btrfs_balance_control *bctl,
 				goto out;
 			}
 		}
+		if (((bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) &&
+		    (bctl->sys.target & ~BTRFS_BLOCK_GROUP_DUP) ||
+		    (bctl->meta.flags & BTRFS_BALANCE_ARGS_CONVERT) &&
+		    (bctl->meta.target & ~BTRFS_BLOCK_GROUP_DUP)) &&
+		    (num_devs > 1)) {
+			if (bctl->flags & BTRFS_BALANCE_FORCE) {
+				btrfs_info(fs_info, "force conversion of metadata "
+					   "to dup profile on multiple devices");
+			} else {
+				btrfs_err(fs_info, "balance will reduce metadata "
+					  "integrity, use force if you want this");
+				ret = -EINVAL;
+				goto out;
+			}
+		}
 	} while (read_seqretry(&fs_info->profiles_lock, seq));
 
 	if (bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) {

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-btrfs-owner@vger.kernel.org>
Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:63587 "EHLO
	mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1752892AbaCGSFr (ORCPT
	<rfc822;linux-btrfs@vger.kernel.org>);
	Fri, 7 Mar 2014 13:05:47 -0500
Message-ID: <531A0A75.6000506@fb.com>
Date: Fri, 7 Mar 2014 13:05:41 -0500
From: Josef Bacik <jbacik@fb.com>
MIME-Version: 1.0
To: Austin S Hemmelgarn <ahferroin7@gmail.com>,
        "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH] btrfs: Allow forced conversion of metadata to dup profile
 on, multiple devices
References: <530DF8D5.7010808@gmail.com>
In-Reply-To: <530DF8D5.7010808@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-btrfs-owner@vger.kernel.org
List-ID: <linux-btrfs.vger.kernel.org>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 02/26/2014 09:23 AM, Austin S Hemmelgarn wrote:
> Currently, btrfs balance start fails when trying to convert
> metadata or system chunks to dup profile on filesystems with
> multiple devices.  This requires that a conversion from a
> multi-device filesystem to a single device filesystem use the
> following methodology: 1. btrfs balance start -dconvert=single
> -mconvert=single \ -sconvert=single -f / 2. btrfs device delete /
> /dev/sdx 3. btrfs balance start -mconvert=dup -sconvert=dup / This
> results in a period of time (possibly very long if the devices are 
> big) where you don't have the protection guarantees of multiple
> copies of metadata chunks.
> 
> After applying this patch, one can instead use the following
> methodology for conversion from a multi-device filesystem to a
> single device filesystem: 1. btrfs balance start -dconvert=single
> -mconvert=dup \ -sconvert=dup -f / 2. btrfs device delete /
> /dev/sdx This greatly reduces the chances of the operation causing
> data loss due to a read error during the device delete.
> 
> Signed-off-by: Austin S. Hemmelgarn <ahferroin7@gmail.com> ---

Fails to build, wait for me to push an updated btrfs-next and rebase
and resubmit

fs/btrfs/volumes.c: In function ‘btrfs_balance’:
fs/btrfs/volumes.c:3223:55: warning: suggest parentheses around ‘&&’
within ‘||’ [-Wparentheses]
   if (((bctl->sys.flags & BTRFS_BALANCE_ARGS_CONVERT) &&
                                                       ^
fs/btrfs/volumes.c:3227:8: error: ‘num_devs’ undeclared (first use in
this function)
       (num_devs > 1)) {
        ^
fs/btrfs/volumes.c:3227:8: note: each undeclared identifier is
reported only once for each function it appears in
make[1]: *** [fs/btrfs/volumes.o] Error 1
make: *** [_module_fs/btrfs] Error 2

Thanks,

Josef

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJTGgp0AAoJEANb+wAKly3BrkkQANOaAAk6LJ4VO86f9/N7Hkom
Hf4FK5lQiMkzlLc8pz+RxHeIHSXScXdrVKPv8bIPoGwd171OIrMA1HZa/9iuez8z
OInjO6Zwj6l/+N3/QckAJKkNmbInNH2wgdhUrFyUw+gDFvVnl1YGUcTwx6/udqTB
kfsls2ivD7S9kfSXyaM4oxVMN+tZWpZtOs1TMpf7BDMIz92gr87VADKIocrlu5qh
EZ2YyWGyIP87jz+7zzNPUQ00/BAgC1lPnbZf+ei0L1KQbDtjII3Rl1/rlg78Zdtb
VQfYMBz0hpaOE+UNfVsClgDgMjAkphob7BXTrkzJaChOagJ5tIlSGZRUaHPt3qaz
so1R/3BaPzlTdv7gPCpMg+nSYdNl0x3w5CauCB4EX3L8PtbxK9tQdFtKmpl5GNrZ
gBADD+AAbyPoz4GO2lfQyIN6Hd5FA+drK6cErF/rY+dPlieyihdYfBWRU+nao9RY
ju07FN2UHJaAlSh/K8ZLv4lZUULDObr/tOdg5M1h5piSL9H9jOHUdaLmRzSx+7hZ
j96qRY9/W4L6avEQQLphGbgn3v+aOHcgxLS0MyO/fhXANah2z69PjpS5J7GOXGZ5
UDGTneQIeRqueYms1yqpmZA0Ctwavq0TGoDvYsOlf/x7vucJE6a02qMRitUJbuvV
6m+8a6PFRudGng7zQYef
=KCPn
-----END PGP SIGNATURE-----

