[PATCH] btrfs: add a test of replace missing dev in diff raid

[PATCH] btrfs: add a test of replace missing dev in diff raid

[Wang Yanfeng]

Test of missing device replace in different raid modes.  This
test requires SCRATCH_DEV_POOL contain 5 same size devices.

This issue has been fixed by Omar's patch:
	Btrfs: RAID 5/6 missing device scrub+replace

Signed-off-by: Wang Yanfeng <wangyf-fnst@cn.fujitsu.com>
---
 tests/btrfs/095     | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/095.out |  2 ++
 tests/btrfs/group   |  1 +
 3 files changed, 96 insertions(+)
 create mode 100755 tests/btrfs/095
 create mode 100644 tests/btrfs/095.out

diff --git a/tests/btrfs/095 b/tests/btrfs/095
new file mode 100755
index 0000000..012c327
--- /dev/null
+++ b/tests/btrfs/095
@@ -0,0 +1,93 @@
+#! /bin/bash
+# FS QA Test No. btrfs/095
+#
+# Test of missing device replace in different raid mode
+#
+# Be sure $SCRATCH_DEV_POOL including 5 devices. And all
+# devices in pool must be in the same size.
+# 
+# To check the fs after replacing a dev, a scrub run is performed.
+#
+# This issue has been fixed by Omar Sandoval's patch:
+# 	Btrfs: RAID 5/6 missing device scrub+replace
+#-----------------------------------------------------------------------
+# Copyright (c) 2015 Fujitsu.  All Rights Reserved.
+# Author: Wang Yanfeng <wangyf-fnst@cn.fujitsu.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+status=1
+trap "exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_need_to_be_root
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+_require_scratch_dev_pool 5
+
+rm -f $seqres.full
+
+REPLACE_DEV=`echo $SCRATCH_DEV_POOL | awk '{print $5}'`
+export SCRATCH_DEV_POOL=`echo $SCRATCH_DEV_POOL | awk '{print $1,$2,$3,$4}'`
+
+_test () {
+	export MKFS_OPTIONS="-m $1 -d $1"
+	_scratch_pool_mkfs >>$seqres.full || _fail "mkfs failed"
+	_scratch_mount
+
+	PRUNE_DEV=`echo $SCRATCH_DEV_POOL | awk '{print $2}'`
+	PRUNE_DEV_ID=`$BTRFS_UTIL_PROG fi show $SCRATCH_MNT | grep $PRUNE_DEV | awk '{print $2}'`
+
+	# dd some data
+	dd if=/dev/urandom of="$SCRATCH_MNT"/file1 bs=1M count=1 \
+ >>$seqres.full 2>&1 || _fail "dd failed"
+	dd if=/dev/urandom of="$SCRATCH_MNT"/file2 bs=1M count=2 \
+ >>$seqres.full 2>&1 || _fail "dd failed"
+	dd if=/dev/urandom of="$SCRATCH_MNT"/file3 bs=1M count=4 \
+ >>$seqres.full 2>&1 || _fail "dd failed"
+
+	# prune the device PRUNE_DEV && remount by degraded mode
+	umount $SCRATCH_MNT
+	dd if=/dev/zero of=$PRUNE_DEV bs=1M count=1 >>$seqres.full 2>&1 \
+ || _fail "dd failed"
+	mount -o degraded $SCRATCH_DEV $SCRATCH_MNT
+
+	# replace the missing dev $PRUNE_DEV with $REPLACE_DEV and scrub it
+	$BTRFS_UTIL_PROG  replace start -B -r $PRUNE_DEV_ID $REPLACE_DEV \
+ $SCRATCH_MNT  -f >>$seqres.full 2>&1 || _fail "replace failed"
+	$BTRFS_UTIL_PROG scrub start -B $SCRATCH_MNT >>$seqres.full 2>&1 \
+ || _fail "scrub failed"
+
+	umount $SCRATCH_MNT
+}
+
+_test "raid1"
+_test "raid10"
+_test "raid5"
+_test "raid6"
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/btrfs/095.out b/tests/btrfs/095.out
new file mode 100644
index 0000000..80ad3b9
--- /dev/null
+++ b/tests/btrfs/095.out
@@ -0,0 +1,2 @@
+QA output created by 095
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index ffe18bf..f20a191 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -96,3 +96,4 @@
 092 auto quick send
 093 auto quick clone
 094 auto quick send
+095 auto
-- 
1.9.1

Re: [PATCH] btrfs: add a test of replace missing dev in diff raid

[Omar Sandoval]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

On Fri, Jun 26, 2015 at 01:21:03PM +0800, Wang Yanfeng wrote:
[snip]
> +_test "raid1"
> +_test "raid10"
> +_test "raid5"
> +_test "raid6"

Hi, Wang Yanfeng,

Thanks a lot for posting this. This looks similar to the stuff in, e.g.,
btrfs/071:

	_btrfs_get_profile_configs replace
	...
	echo "Silence is golden"
	for t in "${_btrfs_profile_configs[@]}"; do
		run_test "$t"
	done

It'd be nice to do it the same way in this test. _btrfs_profile_configs
would need to be updated to support RAID 5/6 and be aware of replace
missing. I'll attach a patch, feel free to use it. Additionally, we
could also update btrfs/011 to test RAID 5/6.

Thanks,
-- 
Omar

[-- Attachment #2: 0001-btrfs-add-replace-missing-and-replace-RAID-5-6-to-pr.patch --]
[-- Type: text/x-diff, Size: 3969 bytes --]

>From 2b038178335105055966a84173fa39b91591b43e Mon Sep 17 00:00:00 2001
Message-Id: <2b038178335105055966a84173fa39b91591b43e.1435356729.git.osandov@fb.com>
From: Omar Sandoval <osandov@fb.com>
Date: Fri, 26 Jun 2015 15:09:50 -0700
Subject: [PATCH] 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 <osandov@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
+			fi
+		done
+		if "$supported"; then
+			_btrfs_profile_configs+=("-m ${profiles[0]} -d ${profiles[1]}")
 		fi
-	fi
+	done
 	export _btrfs_profile_configs
 }
 
-- 
2.4.4

Re: [PATCH] btrfs: add a test of replace missing dev in diff raid

[wangyf]

Hello, Omar:

To update some cases to support RAID5/6 is necessary.
Like btrfs/011 btrfs/071 , and the man page of BTRFS-REPLACE,
they all should be modified.

But equipment damage and missing device is small probability event when
we use disks in our daily life. So it shouldn't be so important that we let
_btrfs_get_profile_confilgs know it.
Besides, every case tests a different direction of replace is good. e.g.
011 071  020 etc.

So I think to test it in a new case is better.  What is your opinion?

cheers,
wangyf


On 06/27/2015 06:24 AM, Omar Sandoval wrote:
> On Fri, Jun 26, 2015 at 01:21:03PM +0800, Wang Yanfeng wrote:
> [snip]
>> +_test "raid1"
>> +_test "raid10"
>> +_test "raid5"
>> +_test "raid6"
> Hi, Wang Yanfeng,
>
> Thanks a lot for posting this. This looks similar to the stuff in, e.g.,
> btrfs/071:
>
> 	_btrfs_get_profile_configs replace
> 	...
> 	echo "Silence is golden"
> 	for t in "${_btrfs_profile_configs[@]}"; do
> 		run_test "$t"
> 	done
>
> It'd be nice to do it the same way in this test. _btrfs_profile_configs
> would need to be updated to support RAID 5/6 and be aware of replace
> missing. I'll attach a patch, feel free to use it. Additionally, we
> could also update btrfs/011 to test RAID 5/6.
>
> Thanks,

Re: [PATCH] btrfs: add a test of replace missing dev in diff raid

[Omar Sandoval]

On 06/29/2015 02:13 AM, wangyf wrote:
> Hello, Omar:
> 
> To update some cases to support RAID5/6 is necessary.
> Like btrfs/011 btrfs/071 , and the man page of BTRFS-REPLACE,
> they all should be modified.
> 
> But equipment damage and missing device is small probability event when
> we use disks in our daily life. So it shouldn't be so important that we let
> _btrfs_get_profile_confilgs know it.
> Besides, every case tests a different direction of replace is good. e.g.
> 011 071  020 etc.
> 
> So I think to test it in a new case is better.  What is your opinion?
> 
> cheers,
> wangyf

Hi, Yanfeng,

Sorry, I think I might have been unclear. I definitely agree that the
missing case should be tested in a separate test case. However, I do
think that _btrfs_get_profile_configs should be updated so we can keep
things in one place and make sure that all profiles are thoroughly
tested. With _btrfs_get_profile_configs aware of RAID 5/6 and missing
device replacement, the new test case can just make use of that instead
of hardcoding the profiles to test. Does that sound alright?

Thanks!
-- 
Omar

Re: [PATCH] btrfs: add a test of replace missing dev in diff raid

[wangyf]

On 06/30/2015 02:06 AM, Omar Sandoval wrote:
> On 06/29/2015 02:13 AM, wangyf wrote:
>> Hello, Omar:
>>
>> To update some cases to support RAID5/6 is necessary.
>> Like btrfs/011 btrfs/071 , and the man page of BTRFS-REPLACE,
>> they all should be modified.
>>
>> But equipment damage and missing device is small probability event when
>> we use disks in our daily life. So it shouldn't be so important that we let
>> _btrfs_get_profile_confilgs know it.
>> Besides, every case tests a different direction of replace is good. e.g.
>> 011 071  020 etc.
>>
>> So I think to test it in a new case is better.  What is your opinion?
>>
>> cheers,
>> wangyf
> Hi, Yanfeng,
>
> Sorry, I think I might have been unclear. I definitely agree that the
> missing case should be tested in a separate test case. However, I do
> think that _btrfs_get_profile_configs should be updated so we can keep
> things in one place and make sure that all profiles are thoroughly
> tested. With _btrfs_get_profile_configs aware of RAID 5/6 and missing
> device replacement, the new test case can just make use of that instead
> of hardcoding the profiles to test. Does that sound alright?
>
> Thanks!

Hi, Omar:
I think it again, and now I think you are right.
To let _btrfs_get_profile_confilgs be aware of replace+missing
many redundant codes can be evitable.
We can use it in a new test case to test missing device.

regards.