| Message ID | 1463495530-425-1-git-send-email-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
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@oracle.com> 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 linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks for reviewing. On 06/12/2016 12:40 PM, Eryu Guan wrote: > 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@oracle.com> > > 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 added in v2. >> +_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. ah, copy paste typo. fixed it in v2. >> + 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. ok, updated. Thanks, Anand > Thanks, > Eryu > -- 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
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 +# +_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" + 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" + fi + + SCRATCH_DEV_POOL_SAVED=${SCRATCH_DEV_POOL} + export SCRATCH_DEV_POOL_SAVED + SCRATCH_DEV_POOL=${devs[@]:0:$test_ndevs} + export SCRATCH_DEV_POOL +} + +_scratch_dev_pool_put() +{ + typeset -p SCRATCH_DEV_POOL_SAVED >/dev/null 2>&1 + if [ $? != 0 ]; then + _fail "Bug: unset val, must call _scratch_dev_pool_get before _scratch_dev_pool_put" + fi + + if [ -z "$SCRATCH_DEV_POOL_SAVED" ]; then + _fail "Bug: str empty, must call _scratch_dev_pool_get before _scratch_dev_pool_put" + fi + + export SCRATCH_DEV_POOL=$SCRATCH_DEV_POOL_SAVED + export SCRATCH_DEV_POOL_SAVED="" +} + _scratch_pool_mkfs() { case $FSTYP in
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@oracle.com> --- common/rc | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)