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
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.
--- 265 | 1 + README | 4 ++-- common.rc | 22 +++++++++++++++++++--- 3 files changed, 22 insertions(+), 5 deletions(-)
265 +1 -0
@@ -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
README +2 -2
@@ -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"
common.rc +19 -3
@@ -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() {
Signature · 6 lines
1.7.6.5 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 4 lines · Miao Xie
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....
Quoted 12 lines
common.rc +2
-1@@ -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
common.rc _require_scratch_dev_pool() · line 1712
Quoted 10 lines
common.rc +3
-2+ # 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.
Signature · 7 lines
Dave Chinner [email protected] _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Dave Chinner
Quoted 4 lines · Miao Xie
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.
Quoted 7 lines
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.
Quoted 23 lines
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....
Quoted 18 lines
common.rc +4
-3@@ -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
Quoted 8 lines
common.rc _require_scratch_dev_pool() · line 1712
Quoted 4 lines
common.rc +1
-0+ _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
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Quoted 7 lines · Miao Xie
Quoted 6 lines · Dave Chinner
Quoted 4 lines · Miao Xie
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.
Quoted 7 lines
Quoted 1 line
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 inSCRATCH_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.
Quoted 8 lines
Quoted 7 lines
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.
Quoted 11 lines
The check
_require_deletable_scratch_dev_poolwill 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.Quoted 4 lines
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
Quoted 28 lines
Quoted 19 lines
Quoted 19 lines
common.rc +4
-3@@ -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 ]; thenQuoted 8 lines
common.rc _require_scratch_dev_pool() · line 1712
Quoted 4 lines
common.rc +1
-0+ _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.
Quoted 9 lines
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH v2 3/3 xfstests: fix wrong number of the required devices and wrong deletable device check method for case 265
In reply to Miao XieCase 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.
---
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(-)
265 +1 -1
@@ -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
common.rc +10 -8
@@ -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 }
Signature · 6 lines
1.7.6.5 _______________________________________________ xfs mailing list [email protected] http://oss.sgi.com/mailman/listinfo/xfs