* [PATCH] btrfs/124: add balance --full-balance option
@ 2017-12-05 8:26 Anand Jain
2017-12-05 8:30 ` Qu Wenruo
2017-12-05 11:36 ` [PATCH v2] " Anand Jain
0 siblings, 2 replies; 6+ messages in thread
From: Anand Jain @ 2017-12-05 8:26 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs
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@oracle.com>
---
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)
+ tup1=$(echo $ver| cut -d"." -f1)
+ tup2=$(echo $ver| cut -d"." -f2)
+
+ 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}
checkpoint2=`md5sum $SCRATCH_MNT/tf2`
echo $checkpoint2 >> $seqres.full 2>&1
--
2.15.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs/124: add balance --full-balance option
2017-12-05 8:26 [PATCH] btrfs/124: add balance --full-balance option Anand Jain
@ 2017-12-05 8:30 ` Qu Wenruo
2017-12-05 8:40 ` Eryu Guan
2017-12-05 8:54 ` Anand Jain
2017-12-05 11:36 ` [PATCH v2] " Anand Jain
1 sibling, 2 replies; 6+ messages in thread
From: Qu Wenruo @ 2017-12-05 8:30 UTC (permalink / raw)
To: Anand Jain, fstests; +Cc: linux-btrfs
[-- Attachment #1.1: Type: text/plain, Size: 1936 bytes --]
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@oracle.com>
> ---
> 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.
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)
> +
> + 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}
>
> checkpoint2=`md5sum $SCRATCH_MNT/tf2`
> echo $checkpoint2 >> $seqres.full 2>&1
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs/124: add balance --full-balance option
2017-12-05 8:30 ` Qu Wenruo
@ 2017-12-05 8:40 ` Eryu Guan
2017-12-05 9:02 ` Anand Jain
2017-12-05 8:54 ` Anand Jain
1 sibling, 1 reply; 6+ messages in thread
From: Eryu Guan @ 2017-12-05 8:40 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Anand Jain, fstests, linux-btrfs
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@oracle.com>
> > ---
> > 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'.
> > +
> > + 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?
Thanks,
Eryu
> >
> > checkpoint2=`md5sum $SCRATCH_MNT/tf2`
> > echo $checkpoint2 >> $seqres.full 2>&1
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs/124: add balance --full-balance option
2017-12-05 8:30 ` Qu Wenruo
2017-12-05 8:40 ` Eryu Guan
@ 2017-12-05 8:54 ` Anand Jain
1 sibling, 0 replies; 6+ messages in thread
From: Anand Jain @ 2017-12-05 8:54 UTC (permalink / raw)
To: Qu Wenruo, fstests; +Cc: linux-btrfs
On 12/05/2017 04:30 PM, 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@oracle.com>
>> ---
>> 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.
>
> Why not directly checking the help message of "btrfs balance start"?
Agreed. Will do.
Thanks, Anand
> Thanks,
> Qu
>
>> + tup1=$(echo $ver| cut -d"." -f1)
>> + tup2=$(echo $ver| cut -d"." -f2)
>> +
>> + 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}
>>
>> checkpoint2=`md5sum $SCRATCH_MNT/tf2`
>> echo $checkpoint2 >> $seqres.full 2>&1
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] btrfs/124: add balance --full-balance option
2017-12-05 8:40 ` Eryu Guan
@ 2017-12-05 9:02 ` Anand Jain
0 siblings, 0 replies; 6+ messages in thread
From: Anand Jain @ 2017-12-05 9:02 UTC (permalink / raw)
To: Eryu Guan, Qu Wenruo; +Cc: fstests, linux-btrfs
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@oracle.com>
>>> ---
>>> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] btrfs/124: add balance --full-balance option
2017-12-05 8:26 [PATCH] btrfs/124: add balance --full-balance option Anand Jain
2017-12-05 8:30 ` Qu Wenruo
@ 2017-12-05 11:36 ` Anand Jain
1 sibling, 0 replies; 6+ messages in thread
From: Anand Jain @ 2017-12-05 11:36 UTC (permalink / raw)
To: fstests; +Cc: linux-btrfs
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
also adds _run_btrfs_balance_start() to the common/btrfs file.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: Use help to find if --full-balance is supported
Make a local variable
run the balance instead of getting the options
common/btrfs | 11 +++++++++++
tests/btrfs/124 | 2 +-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/common/btrfs b/common/btrfs
index c09206c6f292..c3a62b29c70e 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -356,3 +356,14 @@ _btrfs_compression_algos()
echo "${feature#/sys/fs/btrfs/features/compress_}"
done
}
+
+#runs btrfs balance start with required --full-balance if available.
+_run_btrfs_balance_start()
+{
+ local bal_opt=""
+
+ $BTRFS_UTIL_PROG balance start --help | grep -q "full-balance"
+ (( $? == 0 )) && bal_opt="--full-balance"
+
+ run_check $BTRFS_UTIL_PROG balance start $bal_opt $*
+}
diff --git a/tests/btrfs/124 b/tests/btrfs/124
index a6486270a972..94c35fe1ede8 100755
--- a/tests/btrfs/124
+++ b/tests/btrfs/124
@@ -129,7 +129,7 @@ _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}
+_run_btrfs_balance_start ${SCRATCH_MNT}
checkpoint2=`md5sum $SCRATCH_MNT/tf2`
echo $checkpoint2 >> $seqres.full 2>&1
--
2.15.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-05 11:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-05 8:26 [PATCH] btrfs/124: add balance --full-balance option Anand Jain
2017-12-05 8:30 ` Qu Wenruo
2017-12-05 8:40 ` Eryu Guan
2017-12-05 9:02 ` Anand Jain
2017-12-05 8:54 ` Anand Jain
2017-12-05 11:36 ` [PATCH v2] " Anand Jain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).