On Tue, May 10, 2022 at 05:40:27PM +0200, Philipp Rudo wrote:
Hi Coiby,
On Sat, 7 May 2022 16:48:56 +0800
Coiby Xu <coxu(a)redhat.com> wrote:
> Signed-off-by: Coiby Xu <coxu(a)redhat.com>
> ---
> spec/kdump-lib_spec.sh | 46 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
> create mode 100644 spec/kdump-lib_spec.sh
>
> diff --git a/spec/kdump-lib_spec.sh b/spec/kdump-lib_spec.sh
> new file mode 100644
> index 0000000..78d4c46
> --- /dev/null
> +++ b/spec/kdump-lib_spec.sh
> @@ -0,0 +1,46 @@
> +#!/bin/bash
> +Describe 'kdump-lib'
> + Include ./kdump-lib.sh
> +
> + Describe 'get_system_size()'
> +
> + It 'should return correct system RAM size when there are one System RAM
entry'
> + grep() {
> + %text
> + #|000000-3fffffff : System RAM
> + }
> + When call get_system_size
> + The output should equal 1
> + End
> +
> + It 'should return correct system RAM size when there are three System RAM
entries'
> + grep() {
> + %text
> + #|000000-3fffffff : System RAM
> + #|000000-3fffffff : System RAM
> + #|000000-3fffffff : System RAM
> + }
> + When call get_system_size
> + The output should equal 3
> + End
> +
> + End
> +
> + Describe 'get_recommend_size()'
> + It 'is inclusive for the lower bound of the range of crashkernel'
> + When call get_recommend_size 1 1G-4G:192M
> + The output should equal 192M
> + End
> +
> + It 'is exclusive for the lower bound of the range of crashkernel'
s/lower/upper/
Applied, thanks!
> + When call get_recommend_size 4 1G-4G:192M
> + The output should not equal 192M
> + End
> +
> + It 'does not yet support ranges not sorted in increasing order'
> + When call get_recommend_size 1 4G-64G:256M,1G-4G:192M,64G-102400T:512M
> + The output should not equal 192M
sorry, I only wanted to suggest that you switch from what it shouldn't
print to what it should print, i.e. '0M'. But then I started looking at
get_recommend_size...
My main problem with the current implementation is that it assumes that
the start of the first entry has only a single digit and it is in
gigabytes. So the function will fail when e.g. $mem_size=2 and
_ck_cmdline='10G-:256M' or _ck_cmdline='1T-:256M'.
So how about this instead? This will also drop the requirement that the
ranges are in increasing order.
---
get_recommend_size()
{
local mem_size=$1
local _ck_cmdline=$2
local range start start_unit end end_unit size
while read -d , range; do
# need to use non-default IFS as double spaces are used as a
# single delimiter while commas aren't...
IFS=, read start start_unit end end_unit size <<< \
"$(echo "$range" | sed -n
"s/\([0-9]\+\)\([GT]\?\)-\([0-9]*\)\([GT]\?\):\([0-9]\+[MG]\)/\1,\2,\3,\4,\5/p")"
# aka. 102400T
end=${end:-104857600}
[[ "$end_unit" == T ]] && end=$(($end * 1024))
[[ "$start_unit" == T ]] && start=$(($start * 1024))
if [[ $mem_size -ge $start ]] && [[ $mem_size -lt $end ]]; then
echo "$size"
return
fi
# append a ',' as read expects the 'file' to end with a delimiter
done <<< "$_ck_cmdline,"
# no matching range found
echo "0M"
}
Thanks for providing a more general solution!
ck="4G-64G:256M,2G-4G:192M,64G-1T:512M,1T-:12345M"
get_recommend_size "1" "$ck"
get_recommend_size "2" "$ck"
get_recommend_size "64" "$ck"
get_recommend_size "1024" "$ck"
get_recommend_size "$((64*1024))" "$ck"
In v2, I have used your test cases instead since they are more complete.
---
Anyway, the test is fine ;-)
Reviewed-by: Philipp Rudo <prudo(a)redhat.com>
> + End
> + End
> +
> +End
--
Best regards,
Coiby