Re: [PATCH 2/3] btrfs: add replace missing and replace RAID 5/6 to profile configs
On Tue, Jul 28, 2015 at 06:52:07PM +0200, David Sterba wrote: On Tue, Jul 28, 2015 at 12:22:56AM +0200, Brendan Hide wrote: It does not, I apparently forgot that you could use single to concatenate multiple devices. I'll fix that in v2. Thanks for reviewing! Late to the party. DUP *implies* single device but there are cases where dup is used on a multi-device fs. Even if the use-cases aren't good or intended to be long-term, they are still valid, right? You're right, DUP is reported by 'fi df' after 2nd device is added and this state (even if it's temporary) has to be taken into account. I wouldn't read too much into this, _btrfs_get_profile_configs is just for tests that mkfs a bunch of new filesystems for testing, and in that case, DUP is invalid for multiple devices. It's not meant to be an authoritative source of truth regarding what metadata/data profiles could be legal. Thanks, -- Omar -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] btrfs: add replace missing and replace RAID 5/6 to profile configs
On Tue, Jul 28, 2015 at 12:22:56AM +0200, Brendan Hide wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 2015/07/24 07:50 PM, Omar Sandoval wrote: On Fri, Jul 24, 2015 at 02:09:46PM +0200, David Sterba wrote: On Thu, Jul 23, 2015 at 01:51:50PM -0700, Omar Sandoval wrote: + # We can't do replace with these profiles because they + # imply only one device ($SCRATCH_DEV), and we need to + # keep $SCRATCH_DEV around for _scratch_mount + # and _check_scratch_fs. + local unsupported=( + single + dup DUP does imply single device, but why does 'single' ? It does not, I apparently forgot that you could use single to concatenate multiple devices. I'll fix that in v2. Thanks for reviewing! Late to the party. DUP *implies* single device but there are cases where dup is used on a multi-device fs. Even if the use-cases aren't good or intended to be long-term, they are still valid, right? Yes, but bear in mind that any new (presumably metadata) chunks allocated on the multi-device FS will end up as RAID-1, because of the automatic upgrade from DUP to RAID-1. Any balance will also do the upgrade. Hugo. PS. Can we get rid of that upgrade? It's a pain the arse in so many ways. - -- __ Brendan Hide http://swiftspirit.co.za/ http://www.webafrica.co.za/?AFF1E97 -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (MingW32) iQEcBAEBAgAGBQJVtq9AAAoJEE+uni74c4qN5eYIAJAGznsi3RD1tchbSLwhMXJk bJJ4ORB9taLXHykSfYTsHIaUoVpcVR6tT/I1jz5070DY3mKkQ16a8nwtSxPba4Lv QiS8YRegFiHMYzZbH1T7Tnm6R9g/RZsaU7GS3JhP9HUYG7hIWGRRuoiOjYn/hoLw uMXuIFOkPKGYDgyAhDIp3KDYlBjMHT6Oun7CcpvTjXiOnzJFFp3MSt3b6mmmdMVV YKWpWyKVh7qlENEoqKb4exqr1WGYKU+kBLXRs4wdm3xb66EcWYs0Er1u6v+K1trx nryFrfUxYtMJsSuR9ZJm88DOsXKAuX1LEdRKVOlq7krsIK8HlTizccMXAl10gKk= =ndkL -END PGP SIGNATURE- -- Hugo Mills | I'm on a 30-day diet. So far I've lost 18 days. hugo@... carfax.org.uk | http://carfax.org.uk/ | PGP: E2AB1DE4 | signature.asc Description: Digital signature
Re: [PATCH 2/3] btrfs: add replace missing and replace RAID 5/6 to profile configs
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 2015/07/24 07:50 PM, Omar Sandoval wrote: On Fri, Jul 24, 2015 at 02:09:46PM +0200, David Sterba wrote: On Thu, Jul 23, 2015 at 01:51:50PM -0700, Omar Sandoval wrote: + # We can't do replace with these profiles because they + # imply only one device ($SCRATCH_DEV), and we need to + # keep $SCRATCH_DEV around for _scratch_mount + # and _check_scratch_fs. +local unsupported=( + single + dup DUP does imply single device, but why does 'single' ? It does not, I apparently forgot that you could use single to concatenate multiple devices. I'll fix that in v2. Thanks for reviewing! Late to the party. DUP *implies* single device but there are cases where dup is used on a multi-device fs. Even if the use-cases aren't good or intended to be long-term, they are still valid, right? - -- __ Brendan Hide http://swiftspirit.co.za/ http://www.webafrica.co.za/?AFF1E97 -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.22 (MingW32) iQEcBAEBAgAGBQJVtq9AAAoJEE+uni74c4qN5eYIAJAGznsi3RD1tchbSLwhMXJk bJJ4ORB9taLXHykSfYTsHIaUoVpcVR6tT/I1jz5070DY3mKkQ16a8nwtSxPba4Lv QiS8YRegFiHMYzZbH1T7Tnm6R9g/RZsaU7GS3JhP9HUYG7hIWGRRuoiOjYn/hoLw uMXuIFOkPKGYDgyAhDIp3KDYlBjMHT6Oun7CcpvTjXiOnzJFFp3MSt3b6mmmdMVV YKWpWyKVh7qlENEoqKb4exqr1WGYKU+kBLXRs4wdm3xb66EcWYs0Er1u6v+K1trx nryFrfUxYtMJsSuR9ZJm88DOsXKAuX1LEdRKVOlq7krsIK8HlTizccMXAl10gKk= =ndkL -END PGP SIGNATURE- -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] btrfs: add replace missing and replace RAID 5/6 to profile configs
On Fri, Jul 24, 2015 at 02:09:46PM +0200, David Sterba wrote: On Thu, Jul 23, 2015 at 01:51:50PM -0700, Omar Sandoval wrote: + # We can't do replace with these profiles because they + # imply only one device ($SCRATCH_DEV), and we need to + # keep $SCRATCH_DEV around for _scratch_mount + # and _check_scratch_fs. + local unsupported=( + single + dup DUP does imply single device, but why does 'single' ? It does not, I apparently forgot that you could use single to concatenate multiple devices. I'll fix that in v2. Thanks for reviewing! -- Omar -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] btrfs: add replace missing and replace RAID 5/6 to profile configs
On Thu, Jul 23, 2015 at 01:51:50PM -0700, Omar Sandoval wrote: + # We can't do replace with these profiles because they + # imply only one device ($SCRATCH_DEV), and we need to + # keep $SCRATCH_DEV around for _scratch_mount + # and _check_scratch_fs. + local unsupported=( + single + dup DUP does imply single device, but why does 'single' ? -- To unsubscribe from this list: send the line unsubscribe linux-btrfs in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] btrfs: add replace missing and replace RAID 5/6 to profile configs
Replacing and scrubbing RAID 5/6 is now supported on Btrfs. Enable it in
_btrfs_get_profile_configs while making it more generic to also support
replace missing.
Signed-off-by: Omar Sandoval osan...@fb.com
---
common/rc | 96 ---
1 file changed, 49 insertions(+), 47 deletions(-)
diff --git a/common/rc b/common/rc
index 610045eab304..3e6fdb6ebcfa 100644
--- a/common/rc
+++ b/common/rc
@@ -2748,60 +2748,62 @@ _btrfs_get_profile_configs()
return
fi
- # no user specified btrfs profile configs, export the default configs
if [ -z $BTRFS_PROFILE_CONFIGS ]; then
- # default configs
- _btrfs_profile_configs=(
- -m single -d single
- -m dup -d single
- -m raid0 -d raid0
- -m raid1 -d raid0
- -m raid1 -d raid1
- -m raid10 -d raid10
- -m raid5 -d raid5
- -m raid6 -d raid6
+ # Default configurations to test.
+ local configs=(
+ single:single
+ dup:single
+ raid0:raid0
+ raid1:raid0
+ raid1:raid1
+ raid10:raid10
+ raid5:raid5
+ raid6:raid6
)
+ else
+ # User-provided configurations.
+ local configs=(${BTRFS_PROFILE_CONFIGS[@]})
+ fi
- # remove dup/raid5/raid6 profiles if we're doing device replace
- # dup profile indicates only one device being used
(SCRATCH_DEV),
- # but we don't want to replace SCRATCH_DEV, which will be used
in
- # _scratch_mount/_check_scratch_fs etc.
- # and raid5/raid6 doesn't support replace yet
+ _btrfs_profile_configs=()
+ for cfg in ${configs[@]}; do
+ local supported=true
+ local profiles=(${cfg/:/ })
if [ $1 == replace ]; then
- _btrfs_profile_configs=(
- -m single -d single
- -m raid0 -d raid0
- -m raid1 -d raid0
- -m raid1 -d raid1
- -m raid10 -d raid10
- # add these back when raid5/6 is working with
replace
- #-m raid5 -d raid5
- #-m raid6 -d raid6
+ # We can't do replace with these profiles because they
+ # imply only one device ($SCRATCH_DEV), and we need to
+ # keep $SCRATCH_DEV around for _scratch_mount
+ # and _check_scratch_fs.
+ local unsupported=(
+ single
+ dup
)
+ elif [ $1 == replace-missing ]; then
+ # We can't replace missing devices with these profiles
+ # because there isn't enough redundancy.
+ local unsupported=(
+ single
+ dup
+ raid0
+ )
+ else
+ local unsupported=()
fi
- export _btrfs_profile_configs
- return
- fi
-
- # parse user specified btrfs profile configs
- local i=0
- local cfg=
- for cfg in $BTRFS_PROFILE_CONFIGS; do
- # turn metadata:data format to -m metadata -d data
- # and assign it to _btrfs_profile_configs array
- cfg=`echo $cfg | sed -e 's/^/-m /' -e 's/:/ -d /'`
- _btrfs_profile_configs[$i]=$cfg
- let i=i+1
- done
-
- if [ $1 == replace ]; then
- if echo ${_btrfs_profile_configs[*]} | grep -q raid[56]; then
- _notrun RAID5/6 doesn't support btrfs device replace
yet
- fi
- if echo ${_btrfs_profile_configs[*]} | grep -q dup; then
- _notrun Do not set dup profile in btrfs device replace
test
+ for unsupp in ${unsupported[@]}; do
+ if [ ${profiles[0]} == $unsupp -o ${profiles[1]}
== $unsupp ]; then
+if [ -z $BTRFS_PROFILE_CONFIGS ]; then
+# For the default config, just omit it.
+supported=false
+else
+# For user-provided config, don't run the
test.
+_notrun Profile $unsupp not supported for
$1
+fi
+
Re: [PATCH 2/3] btrfs: add replace missing and replace RAID 5/6 to profile configs
On Thu, Jul 23, 2015 at 01:51:50PM -0700, Omar Sandoval wrote:
Replacing and scrubbing RAID 5/6 is now supported on Btrfs. Enable it in
_btrfs_get_profile_configs while making it more generic to also support
replace missing.
Signed-off-by: Omar Sandoval osan...@fb.com
Looks great! Tested with default configs and user-defined configs.
Reviewed-by: Eryu Guan eg...@redhat.com
---
common/rc | 96
---
1 file changed, 49 insertions(+), 47 deletions(-)
diff --git a/common/rc b/common/rc
index 610045eab304..3e6fdb6ebcfa 100644
--- a/common/rc
+++ b/common/rc
@@ -2748,60 +2748,62 @@ _btrfs_get_profile_configs()
return
fi
- # no user specified btrfs profile configs, export the default configs
if [ -z $BTRFS_PROFILE_CONFIGS ]; then
- # default configs
- _btrfs_profile_configs=(
- -m single -d single
- -m dup -d single
- -m raid0 -d raid0
- -m raid1 -d raid0
- -m raid1 -d raid1
- -m raid10 -d raid10
- -m raid5 -d raid5
- -m raid6 -d raid6
+ # Default configurations to test.
+ local configs=(
+ single:single
+ dup:single
+ raid0:raid0
+ raid1:raid0
+ raid1:raid1
+ raid10:raid10
+ raid5:raid5
+ raid6:raid6
)
+ else
+ # User-provided configurations.
+ local configs=(${BTRFS_PROFILE_CONFIGS[@]})
+ fi
- # remove dup/raid5/raid6 profiles if we're doing device replace
- # dup profile indicates only one device being used
(SCRATCH_DEV),
- # but we don't want to replace SCRATCH_DEV, which will be used
in
- # _scratch_mount/_check_scratch_fs etc.
- # and raid5/raid6 doesn't support replace yet
+ _btrfs_profile_configs=()
+ for cfg in ${configs[@]}; do
+ local supported=true
+ local profiles=(${cfg/:/ })
if [ $1 == replace ]; then
- _btrfs_profile_configs=(
- -m single -d single
- -m raid0 -d raid0
- -m raid1 -d raid0
- -m raid1 -d raid1
- -m raid10 -d raid10
- # add these back when raid5/6 is working with
replace
- #-m raid5 -d raid5
- #-m raid6 -d raid6
+ # We can't do replace with these profiles because they
+ # imply only one device ($SCRATCH_DEV), and we need to
+ # keep $SCRATCH_DEV around for _scratch_mount
+ # and _check_scratch_fs.
+ local unsupported=(
+ single
+ dup
)
+ elif [ $1 == replace-missing ]; then
+ # We can't replace missing devices with these profiles
+ # because there isn't enough redundancy.
+ local unsupported=(
+ single
+ dup
+ raid0
+ )
+ else
+ local unsupported=()
fi
- export _btrfs_profile_configs
- return
- fi
-
- # parse user specified btrfs profile configs
- local i=0
- local cfg=
- for cfg in $BTRFS_PROFILE_CONFIGS; do
- # turn metadata:data format to -m metadata -d data
- # and assign it to _btrfs_profile_configs array
- cfg=`echo $cfg | sed -e 's/^/-m /' -e 's/:/ -d /'`
- _btrfs_profile_configs[$i]=$cfg
- let i=i+1
- done
-
- if [ $1 == replace ]; then
- if echo ${_btrfs_profile_configs[*]} | grep -q raid[56]; then
- _notrun RAID5/6 doesn't support btrfs device replace
yet
- fi
- if echo ${_btrfs_profile_configs[*]} | grep -q dup; then
- _notrun Do not set dup profile in btrfs device replace
test
+ for unsupp in ${unsupported[@]}; do
+ if [ ${profiles[0]} == $unsupp -o ${profiles[1]}
== $unsupp ]; then
+ if [ -z $BTRFS_PROFILE_CONFIGS ]; then
+ # For the default config, just omit it.
+ supported=false
+ else
+ # For user-provided config, don't run the
test.
+
