On fri, 24 Aug 2012 13:50:27 +0800, Miao Xie wrote:

On fri, 24 Aug 2012 14:18:04 +1000, Dave Chinner wrote:

quoted

On Fri, Aug 24, 2012 at 11:16:11AM +0800, Miao Xie wrote:

quoted

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.

quoted

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

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

quoted

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