From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cn.fujitsu.com ([222.73.24.84]:25213 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1755884Ab2HXHoZ (ORCPT ); Fri, 24 Aug 2012 03:44:25 -0400 Message-ID: <50372551.60609@cn.fujitsu.com> Date: Fri, 24 Aug 2012 14:55:13 +0800 From: Miao Xie Reply-To: miaox@cn.fujitsu.com MIME-Version: 1.0 To: Dave Chinner CC: Linux Btrfs , xfs@oss.sgi.com, anand.jain@oracle.com Subject: Re: [PATCH 3/3] xfstests: fix wrong number of the required devices and add independent device check for case 265 References: <5036F1FB.80205@cn.fujitsu.com> <20120824041804.GB19235@dastard> <50371623.9040505@cn.fujitsu.com> In-Reply-To: <50371623.9040505@cn.fujitsu.com> Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-btrfs-owner@vger.kernel.org List-ID: 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 >