Re: [PATCH 1/6] fstests: btrfs: add functions to set and reset required number of SCRATCH_DEV_POOL

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Tue, May 17, 2016 at 10:32:05PM +0800, Anand Jain wrote:
> This patch provides functions
>  _scratch_dev_pool_get()
>  _scratch_dev_pool_put()
> 
> Which will help to set/reset SCRATCH_DEV_POOL with the required
> number of devices. SCRATCH_DEV_POOL_SAVED will hold all the devices.
> 
> Usage:
>   _scratch_dev_pool_get() <ndevs>
>   :: do stuff
> 
>   _scratch_dev_pool_put()
> 
> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>

I'm not sure about the usefulness and the implementation of these
helpers (include the _spare_dev_get|_put helpers), but they look good to
me. It'd better to have btrfs developers review them as well.

> ---
>  common/rc | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 91e8f1c8e693..33632fd8e4a3 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -786,6 +786,53 @@ _scratch_mkfs()
>      esac
>  }
>  
> +#
> +# $1 Number of the scratch devs required
> +#

Some comments about its usage/purpose would be good, otherwise one has
to search for the commit log and look into it to understand its purpose

> +_scratch_dev_pool_get()
> +{
> +	if [ $# != 1 ]; then
> +		_fail "Usage: _scratch_dev_pool_get ndevs"
> +	fi
> +
> +	local test_ndevs=$1
> +	local config_ndevs=`echo $SCRATCH_DEV_POOL| wc -w`
> +	local devs[]="( $SCRATCH_DEV_POOL )"
> +
> +	typeset -p config_ndevs >/dev/null 2>&1
> +	if [ $? != 0 ]; then
> +		_fail "Bug: unset val, must call _scratch_dev_pool_get before _scratch_dev_pool_put"

This error message seems confusing, it's _scratch_dev_pool_get being
called here, not _put.

> +	fi
> +
> +	# _require_scratch_dev_pool $test_ndevs
> +	# must have already checked the min required devices
> +	# but just in case, trap here for any potential bugs
> +	# perpetuating any further
> +	if [ $config_ndevs -lt $test_ndevs ]; then
> +		_notrun "Need at least test requested number of ndevs $test_ndevs"

This message is not clear to me either.

Thanks,
Eryu
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux