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.j...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to