thr3ads.net - Btrfs devel - [PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265 [Aug 2012]

If this information is useful, please help other people find it:
Share via:

Miao Xie

2012-Aug-24 03:16 UTC

[PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265

Case 265 need 4 devices to test RAID10, so we need 4 or more devices not 2.
and it is better that these 4 devices are independent devices, especially
the 2nd last one, so we add independent device check to check the devices
in SCRATCH_DEV_POOL.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 265       |    1 +
 README    |    4 ++--
 common.rc |   22 +++++++++++++++++++---
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/265 b/265
index ec8410c..947e65b 100755
--- a/265
+++ b/265
@@ -51,6 +51,7 @@ _supported_fs btrfs
 _supported_os Linux
 _require_scratch
 _require_scratch_dev_pool
+_require_independent_scratch_dev_pool
 _require_deletable_scratch_dev_pool
 
 # Test cases related to raid in btrfs
diff --git a/README b/README
index d81ede9..bb10dba 100644
--- a/README
+++ b/README
@@ -38,7 +38,7 @@ Preparing system for tests (IRIX and Linux):
               not be run.
               (SCRATCH and TEST must be two DIFFERENT partitions)
               OR
-        - for btrfs only: some btrfs test cases will need 3 or more independent
+        - for btrfs only: some btrfs test cases will need 4 or more independent
               SCRATCH disks which should be set using SCRATCH_DEV_POOL (for eg:
               SCRATCH_DEV_POOL="/dev/sda /dev/sdb /dev/sdc") with
which
               SCRATCH_DEV should be unused by the tester, and for the legacy
@@ -50,7 +50,7 @@ Preparing system for tests (IRIX and Linux):
         - setenv TEST_DIR "mount point of TEST PARTITION"   
        	- optionally:
              - setenv SCRATCH_DEV "device containing SCRATCH
PARTITION" OR
-               (btrfs only) setenv SCRATCH_DEV_POOL "to 3 or more SCRATCH
disks for
+               (btrfs only) setenv SCRATCH_DEV_POOL "to 4 or more SCRATCH
disks for
                testing btrfs raid concepts"
              - setenv SCRATCH_MNT "mount point for SCRATCH PARTITION"
              - setenv TAPE_DEV "tape device for testing xfsdump"
diff --git a/common.rc b/common.rc
index 602513a..ede25fe 100644
--- a/common.rc
+++ b/common.rc
@@ -1699,12 +1699,14 @@ _require_scratch_dev_pool()
 		_notrun "this test requires a valid \$SCRATCH_DEV_POOL"
 	fi
 
-	# btrfs test case needs 2 or more scratch_dev_pool; other FS not sure
+	# btrfs test case needs 4 or more scratch_dev_pool; other FS not sure
 	# so fail it
+	# common.config has moved the first device to SCRATCH_DEV, so
+	# SCRATCH_DEV_POOL should have 3 or more disks.
 	case $FSTYP in
 	btrfs)
-		if [ "`echo $SCRATCH_DEV_POOL|wc -w`" -lt 2 ]; then
-			_notrun "btrfs and this test needs 2 or more disks in
SCRATCH_DEV_POOL"
+		if [ "`echo $SCRATCH_DEV_POOL|wc -w`" -lt 3 ]; then
+			_notrun "btrfs and this test needs 4 or more disks in
SCRATCH_DEV_POOL"
 		fi
 	;;
 	*)
@@ -1746,6 +1748,20 @@ _require_deletable_scratch_dev_pool()
 	done
 }
 
+# We will check if the device is independent device.
+_require_independent_scratch_dev_pool()
+{
+	local i
+	local dev
+	for i in $SCRATCH_DEV_POOL; do
+		dev=${i/*\//}
+		[[ ! $dev == md* && $dev == *[0-9] ]] && \
+			_notrun "$i is not a independent device"
+		[[ $dev == md* && $dev == md[0-9]*p[0-9]* ]] && \
+			_notrun "$i is not a independent device"
+	done
+}
+
 # We check for btrfs and (optionally) features of the btrfs command
 _require_btrfs()
 {
-- 
1.7.6.5
--
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

Dave Chinner

2012-Aug-24 04:18 UTC

head link

Re: [PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265

On Fri, Aug 24, 2012 at 11:16:11AM +0800, Miao Xie
wrote:> Case 265 need 4 devices to test RAID10, so we need 4 or more devices not 2.
> and it is better that these 4 devices are independent devices, especially
> the 2nd last one, so we add independent device check to check the devices
> in SCRATCH_DEV_POOL.
I don''t see any reason for requiring the devices to be independent.
You''re basically checking if the devices are on an MD device, which
isn''t really a check for indpendent devices. e.g. my 4 devices could
be loopback devices with files all the in the same filesystem, or on
a VM using images at that are all hosted on the same device, or LVM
volumes on top of a single MD device, hardware lun, etc. They are
most certainly not independent, but your test won''t pick up any of
them.

Hence the test does not require the devices to be independent to run
correctly.  Sure, the test will run faster if each device is on an
independent spindle, but it''s not a requirement for test success or
failure....
> diff --git a/common.rc b/common.rc
> index 602513a..ede25fe 100644
> --- a/common.rc
> +++ b/common.rc
> @@ -1699,12 +1699,14 @@ _require_scratch_dev_pool()
>  		_notrun "this test requires a valid \$SCRATCH_DEV_POOL"
>  	fi
>  
> -	# btrfs test case needs 2 or more scratch_dev_pool; other FS not sure
> +	# btrfs test case needs 4 or more scratch_dev_pool; other FS not sure
>  	# so fail it
> +	# common.config has moved the first device to SCRATCH_DEV, so
> +	# SCRATCH_DEV_POOL should have 3 or more disks.
>  	case $FSTYP in
>  	btrfs)
> -		if [ "`echo $SCRATCH_DEV_POOL|wc -w`" -lt 2 ]; then
> -			_notrun "btrfs and this test needs 2 or more disks in
SCRATCH_DEV_POOL"
> +		if [ "`echo $SCRATCH_DEV_POOL|wc -w`" -lt 3 ]; then
> +			_notrun "btrfs and this test needs 4 or more disks in
SCRATCH_DEV_POOL"
>  		fi
>  	;;
>  	*)
Rather than changing this every time a new number of disks is
required, change it so that the number of devices required by the
test is passed as a parameter to _require_scratch_dev_pool.

Cheers,

Dave.

-- 
Dave Chinner
david@fromorbit.com
--
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

Miao Xie

2012-Aug-24 05:50 UTC

head link

Re: [PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265

On fri, 24 Aug 2012 14:18:04 +1000, Dave Chinner wrote:> On Fri, Aug 24, 2012 at 11:16:11AM +0800, Miao Xie wrote:
>> Case 265 need 4 devices to test RAID10, so we need 4 or more devices
not 2.
>> and it is better that these 4 devices are independent devices,
especially
>> the 2nd last one, so we add independent device check to check the
devices
>> in SCRATCH_DEV_POOL.
> 
> I don''t see any reason for requiring the devices to be
independent.
README said we need independent devices. I think the reason is:
Case 265 will remove/add the 2nd last device in SCRATCH_DEV_POOL, if this device
is a partition of a device, not a independent device, it is easy to make a
mistake
for the users that the other partitions are used while doing the test. If so,
the name of the device will be changed, and it will make the next test cases
fail.
> You''re basically checking if the devices are on an MD device,
which
> isn''t really a check for indpendent devices. e.g. my 4 devices
could
> be loopback devices with files all the in the same filesystem, or on
> a VM using images at that are all hosted on the same device, or LVM
> volumes on top of a single MD device, hardware lun, etc. They are
> most certainly not independent, but your test won''t pick up any of
> them.
The check _require_deletable_scratch_dev_pool will make sure the device is not
a virtual device. My check just make sure the devices are not partitions.
Maybe I should change the name of the my check.

P.S. I made a mistake, I needn''t take the soft raid into account
because
the soft raid devices are also virtual disks.
> Hence the test does not require the devices to be independent to run
> correctly.  Sure, the test will run faster if each device is on an
> independent spindle, but it''s not a requirement for test success
or
> failure....
> 
>> diff --git a/common.rc b/common.rc
>> index 602513a..ede25fe 100644
>> --- a/common.rc
>> +++ b/common.rc
>> @@ -1699,12 +1699,14 @@ _require_scratch_dev_pool()
>>  		_notrun "this test requires a valid \$SCRATCH_DEV_POOL"
>>  	fi
>>  
>> -	# btrfs test case needs 2 or more scratch_dev_pool; other FS not sure
>> +	# btrfs test case needs 4 or more scratch_dev_pool; other FS not sure
>>  	# so fail it
>> +	# common.config has moved the first device to SCRATCH_DEV, so
>> +	# SCRATCH_DEV_POOL should have 3 or more disks.
>>  	case $FSTYP in
>>  	btrfs)
>> -		if [ "`echo $SCRATCH_DEV_POOL|wc -w`" -lt 2 ]; then
>> -			_notrun "btrfs and this test needs 2 or more disks in
SCRATCH_DEV_POOL"
>> +		if [ "`echo $SCRATCH_DEV_POOL|wc -w`" -lt 3 ]; then
>> +			_notrun "btrfs and this test needs 4 or more disks in
SCRATCH_DEV_POOL"
>>  		fi
>>  	;;
>>  	*)
> 
> Rather than changing this every time a new number of disks is
> required, change it so that the number of devices required by the
> test is passed as a parameter to _require_scratch_dev_pool.
Yes, I''ll update my patch.

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

Miao Xie

2012-Aug-24 06:55 UTC

head link

Re: [PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265

On fri, 24 Aug 2012 13:50:27 +0800, Miao Xie wrote:> On fri, 24 Aug 2012 14:18:04 +1000, Dave Chinner wrote:
>> On Fri, Aug 24, 2012 at 11:16:11AM +0800, Miao Xie wrote:
>>> Case 265 need 4 devices to test RAID10, so we need 4 or more
devices not 2.
>>> and it is better that these 4 devices are independent devices,
especially
>>> the 2nd last one, so we add independent device check to check the
devices
>>> in SCRATCH_DEV_POOL.
>>
>> I don''t see any reason for requiring the devices to be
independent.
> 
> README said we need independent devices. I think the reason is:
> Case 265 will remove/add the 2nd last device in SCRATCH_DEV_POOL, if this
device
> is a partition of a device, not a independent device, it is easy to make a
mistake
> for the users that the other partitions are used while doing the test. If
so,
> the name of the device will be changed, and it will make the next test
cases fail.
I find all the partitions and the virtual devices don''t have the delete
entry-point
in sysfs, so we can avoid the above problem by checking the delete entry-point.
>> You''re basically checking if the devices are on an MD device,
which
>> isn''t really a check for indpendent devices. e.g. my 4 devices
could
>> be loopback devices with files all the in the same filesystem, or on
>> a VM using images at that are all hosted on the same device, or LVM
>> volumes on top of a single MD device, hardware lun, etc. They are
>> most certainly not independent, but your test won''t pick up
any of
>> them.
> 
> The check _require_deletable_scratch_dev_pool will make sure the device is
not
> a virtual device. My check just make sure the devices are not partitions.
> Maybe I should change the name of the my check.
> 
> P.S. I made a mistake, I needn''t take the soft raid into account
because
> the soft raid devices are also virtual disks.
> 
>> Hence the test does not require the devices to be independent to run
>> correctly.  Sure, the test will run faster if each device is on an
>> independent spindle, but it''s not a requirement for test
success or
>> failure....
If the 2nd last device in SCRATCH_DEV_POOL is a partition, case 265 will
fail because case 265 is designed for independent device test and
doesn''t
take the partitions into account.

Thanks
Miao
>>
>>> diff --git a/common.rc b/common.rc
>>> index 602513a..ede25fe 100644
>>> --- a/common.rc
>>> +++ b/common.rc
>>> @@ -1699,12 +1699,14 @@ _require_scratch_dev_pool()
>>>  		_notrun "this test requires a valid
\$SCRATCH_DEV_POOL"
>>>  	fi
>>>  
>>> -	# btrfs test case needs 2 or more scratch_dev_pool; other FS not
sure
>>> +	# btrfs test case needs 4 or more scratch_dev_pool; other FS not
sure
>>>  	# so fail it
>>> +	# common.config has moved the first device to SCRATCH_DEV, so
>>> +	# SCRATCH_DEV_POOL should have 3 or more disks.
>>>  	case $FSTYP in
>>>  	btrfs)
>>> -		if [ "`echo $SCRATCH_DEV_POOL|wc -w`" -lt 2 ]; then
>>> -			_notrun "btrfs and this test needs 2 or more disks in
SCRATCH_DEV_POOL"
>>> +		if [ "`echo $SCRATCH_DEV_POOL|wc -w`" -lt 3 ]; then
>>> +			_notrun "btrfs and this test needs 4 or more disks in
SCRATCH_DEV_POOL"
>>>  		fi
>>>  	;;
>>>  	*)
>>
>> Rather than changing this every time a new number of disks is
>> required, change it so that the number of devices required by the
>> test is passed as a parameter to _require_scratch_dev_pool.
> 
> Yes, I''ll update my patch.
> 
> Thanks
> Miao
> --
> 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

Miao Xie

2012-Aug-24 08:08 UTC

head link

[PATCH V2 3/3] xfstests: fix wrong number of the required devices and wrong deletable device check method for case 265

Case 265 need 4 devices to test RAID10, so we need 4 or more devices not 2.
And the deletable device check method is also wrong, the virtual devices in
the VMs which are drived by virtio are also not deletable(no delete entry
point), but it is not managed in the virtual directory in sysfs, so the current
method will make a mistake and thinks they are deletable. Fix it by check
the delete entry point. This fix method can also avoid the users use
partitions to run case 265.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
Changelog v1 -> v2:
- drop the independent device check
- modify the deletable device check
- do not modify README
---
 265       |    2 +-
 common.rc |   18 ++++++++++--------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/265 b/265
index ec8410c..0687b62 100755
--- a/265
+++ b/265
@@ -50,7 +50,7 @@ _need_to_be_root
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
-_require_scratch_dev_pool
+_require_scratch_dev_pool 4
 _require_deletable_scratch_dev_pool
 
 # Test cases related to raid in btrfs
diff --git a/common.rc b/common.rc
index 602513a..a254e0e 100644
--- a/common.rc
+++ b/common.rc
@@ -1695,16 +1695,20 @@ _test_inode_extsz()
 _require_scratch_dev_pool()
 {
 	local i
+	local ndevs=$1
 	if [ -z "$SCRATCH_DEV_POOL" ]; then
 		_notrun "this test requires a valid \$SCRATCH_DEV_POOL"
 	fi
 
-	# btrfs test case needs 2 or more scratch_dev_pool; other FS not sure
+	# btrfs test case needs scratch_dev_pool; other FS not sure
 	# so fail it
 	case $FSTYP in
 	btrfs)
-		if [ "`echo $SCRATCH_DEV_POOL|wc -w`" -lt 2 ]; then
-			_notrun "btrfs and this test needs 2 or more disks in
SCRATCH_DEV_POOL"
+		# We have moved a device from SCRATCH_DEV_POOL tp SCRATCH_DEV,
+		# so we must make it into account.
+		if [ $((`echo $SCRATCH_DEV_POOL | wc -w` + 1)) -lt $ndevs ]
+		then
+			_notrun "btrfs and this test needs $ndevs or more disks in
SCRATCH_DEV_POOL"
 		fi
 	;;
 	*)
@@ -1731,17 +1735,15 @@ _require_scratch_dev_pool()
 	done
 }
 
-# We will check if the device is virtual (eg: loop device) since it does not
-# have the delete entry-point. Otherwise SCSI and USB devices are fine. 
+# We will check if the device is deletable. 
 _require_deletable_scratch_dev_pool()
 {
 	local i
 	local x
 	for i in $SCRATCH_DEV_POOL; do
 		x=`echo $i | cut -d"/" -f 3`
-		ls -l /sys/class/block/${x} | grep -q "virtual" 
-		if [ $? == "0" ]; then
-			_notrun "$i is a virtual device which is not deletable"
+		if [ ! -f /sys/class/block/${x}/device/delete ]; then
+			_notrun "$i is a device which is not deletable"
 		fi
 	done
 }
-- 
1.7.6.5

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

Btrfs devel - Aug 2012 - [PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265

[PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265

Re: [PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265

Re: [PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265

Re: [PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265

[PATCH V2 3/3] xfstests: fix wrong number of the required devices and wrong deletable device check method for case 265