Re: [PATCH] btrfs/124: add balance --full-balance option

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





On 12/05/2017 04:40 PM, Eryu Guan wrote:
On Tue, Dec 05, 2017 at 04:30:33PM +0800, Qu Wenruo wrote:


On 2017年12月05日 16:26, Anand Jain wrote:
btrfs balance needs --full-balance option since 4.6, so check the
version and then use it.

As this may be useful for other btrfs tests as well, so this patch
adds _btrfs_full_balance_option() to the common/btrfs file.

Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
---
  common/btrfs    | 13 +++++++++++++
  tests/btrfs/124 |  3 ++-
  2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/common/btrfs b/common/btrfs
index c09206c6f292..8ca6486b26dc 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -356,3 +356,16 @@ _btrfs_compression_algos()
  		echo "${feature#/sys/fs/btrfs/features/compress_}"
  	done
  }
+
+#btrfs-progs adds required --full-balance option since v4.6 so check for
+#that and then return the required option.
+_btrfs_full_balance_option()
+{
+	ver=$($BTRFS_UTIL_PROG --version|awk '{print $2}'|cut -d"-" -f1 | cut -d"v" -f2)

I'm never a fan of version check.
Backporting or modified package version can easily screw this up.

Agreed, avoid version checking if possible.


Why not directly checking the help message of "btrfs balance start"?

Thanks,
Qu

+	tup1=$(echo $ver| cut -d"." -f1)
+	tup2=$(echo $ver| cut -d"." -f2)

And please declare local variables as 'local'.

 ok.

+
+	RET=""
+	(("$tup1" > "4")) && RET="--full-balance"
+	(("$tup1" == "4")) && (("tup2" >= "6")) && RET="--full-balance"
+}
diff --git a/tests/btrfs/124 b/tests/btrfs/124
index a6486270a972..ec1d24b5ef42 100755
--- a/tests/btrfs/124
+++ b/tests/btrfs/124
@@ -129,7 +129,8 @@ _run_btrfs_util_prog device scan
  _scratch_mount >> $seqres.full 2>&1
  _run_btrfs_util_prog filesystem show
  echo >> $seqres.full
-_run_btrfs_util_prog balance start ${SCRATCH_MNT}
+_btrfs_full_balance_option
+_run_btrfs_util_prog balance start $RET ${SCRATCH_MNT}

This usage depends on the implicit global variable RET, which seems a
bit hard to follow. How about let the new helper return the expected
option string, and use that string in test, either assign it to a local
var or use it in command line directly.

Think about it more, perhaps we can introduce a new helper, e.g.
_run_btrfs_balance, and hide all these details, just call balance with
--full-balance when possible?

 Ah. That's much better. Will do.

Thanks, Anand


Thanks,
Eryu

checkpoint2=`md5sum $SCRATCH_MNT/tf2`
  echo $checkpoint2 >> $seqres.full 2>&1





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

--
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