After introducing 64k variant kernel on aarch64, an example kernel name looks like "vmlinuz-5.14.0-316.el9.aarch64+64k". To match the plus symbol, it demands an escape charater.
Signed-off-by: Pingfan Liu piliu@redhat.com --- v1 -> v2 add test case
kdumpctl | 3 +++ spec/kdumpctl_general_spec.sh | 17 +++++++++++++++++ 2 files changed, 20 insertions(+)
diff --git a/kdumpctl b/kdumpctl index 48cd33d..f86a365 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1406,6 +1406,9 @@ _filter_grubby_kernel_str() _find_kernel_path_by_release() { local _release="$1" _grubby_kernel_str _kernel_path + + # Insert '/' before '+' to cope with grep's EREs + _release=$(echo "$_release" | sed 's/+/\+/g') _grubby_kernel_str=$(grubby --info ALL | grep -E "^kernel=.*$_release(/\w+)?"$") _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str") if [[ -z $_kernel_path ]]; then diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index bb5755c..7304cd4 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -222,6 +222,23 @@ Describe 'kdumpctl' End End
+ Describe '_find_kernel_path_by_release()' + grubby() { + echo -e 'kernel="/boot/vmlinuz-6.2.11-200.fc37.x86_64"\nkernel="/boot/vmlinuz-5.14.0-322.el9.aarch64"\nkernel="/boot/vmlinuz-5.14.0-316.el9.aarch64+64k"' + } + + Parameters + # parameter answer + vmlinuz-6.2.11-200.fc37.x86_64 /boot/vmlinuz-6.2.11-200.fc37.x86_64 + vmlinuz-5.14.0-322.el9.aarch64 /boot/vmlinuz-5.14.0-322.el9.aarch64 + vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-316.el9.aarch64+64k + End + It 'returns the kernel path for the given release' + When call _find_kernel_path_by_release "$1" + The output should equal "$2" + End + End + Describe 'parse_config()' bad_kdump_conf=$(mktemp -t bad_kdump_conf.XXXXXXXXXX) cleanup() {
Hi Pingfan,
On Fri, 9 Jun 2023 16:04:29 +0800 Pingfan Liu piliu@redhat.com wrote:
After introducing 64k variant kernel on aarch64, an example kernel name looks like "vmlinuz-5.14.0-316.el9.aarch64+64k". To match the plus symbol, it demands an escape charater.
Signed-off-by: Pingfan Liu piliu@redhat.com
v1 -> v2 add test case
kdumpctl | 3 +++ spec/kdumpctl_general_spec.sh | 17 +++++++++++++++++ 2 files changed, 20 insertions(+)
diff --git a/kdumpctl b/kdumpctl index 48cd33d..f86a365 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1406,6 +1406,9 @@ _filter_grubby_kernel_str() _find_kernel_path_by_release() { local _release="$1" _grubby_kernel_str _kernel_path
- # Insert '/' before '+' to cope with grep's EREs
- _release=$(echo "$_release" | sed 's/+/\+/g')
You could drop the pipe to sed by using the bash builtin string replacement, i.e.
_release=${_release//+/\+}
But that's probably more a matter of taste here. Other than that the patch looks fine to me.
Reviewed-by: Philipp Rudo prudo@redhat.com
_grubby_kernel_str=$(grubby --info ALL | grep -E "^kernel=.*$_release(/\w+)?"$") _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str") if [[ -z $_kernel_path ]]; then diff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index bb5755c..7304cd4 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -222,6 +222,23 @@ Describe 'kdumpctl' End End
- Describe '_find_kernel_path_by_release()'
grubby() {echo -e 'kernel="/boot/vmlinuz-6.2.11-200.fc37.x86_64"\nkernel="/boot/vmlinuz-5.14.0-322.el9.aarch64"\nkernel="/boot/vmlinuz-5.14.0-316.el9.aarch64+64k"'}Parameters# parameter answervmlinuz-6.2.11-200.fc37.x86_64 /boot/vmlinuz-6.2.11-200.fc37.x86_64vmlinuz-5.14.0-322.el9.aarch64 /boot/vmlinuz-5.14.0-322.el9.aarch64vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-316.el9.aarch64+64kEndIt 'returns the kernel path for the given release'When call _find_kernel_path_by_release "$1"The output should equal "$2"End- End
- Describe 'parse_config()' bad_kdump_conf=$(mktemp -t bad_kdump_conf.XXXXXXXXXX) cleanup() {
On Mon, Jun 12, 2023 at 11:00 PM Philipp Rudo prudo@redhat.com wrote:
Hi Pingfan,
On Fri, 9 Jun 2023 16:04:29 +0800 Pingfan Liu piliu@redhat.com wrote:
After introducing 64k variant kernel on aarch64, an example kernel name looks like "vmlinuz-5.14.0-316.el9.aarch64+64k". To match the plus symbol, it demands an escape charater.
Signed-off-by: Pingfan Liu piliu@redhat.com
v1 -> v2 add test case
kdumpctl | 3 +++ spec/kdumpctl_general_spec.sh | 17 +++++++++++++++++ 2 files changed, 20 insertions(+)
diff --git a/kdumpctl b/kdumpctl index 48cd33d..f86a365 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1406,6 +1406,9 @@ _filter_grubby_kernel_str() _find_kernel_path_by_release() { local _release="$1" _grubby_kernel_str _kernel_path
# Insert '/' before '+' to cope with grep's EREs_release=$(echo "$_release" | sed 's/+/\\+/g')You could drop the pipe to sed by using the bash builtin string replacement, i.e.
_release=${_release//+/\+}
But that's probably more a matter of taste here. Other than that the patch looks fine to me.
It is neat. I like it.
Thanks,
Pingfan
Reviewed-by: Philipp Rudo prudo@redhat.com
_grubby_kernel_str=$(grubby --info ALL | grep -E "^kernel=.*$_release(\/\w+)?\"$") _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str") if [[ -z $_kernel_path ]]; thendiff --git a/spec/kdumpctl_general_spec.sh b/spec/kdumpctl_general_spec.sh index bb5755c..7304cd4 100644 --- a/spec/kdumpctl_general_spec.sh +++ b/spec/kdumpctl_general_spec.sh @@ -222,6 +222,23 @@ Describe 'kdumpctl' End End
Describe '_find_kernel_path_by_release()'grubby() {echo -e 'kernel="/boot/vmlinuz-6.2.11-200.fc37.x86_64"\nkernel="/boot/vmlinuz-5.14.0-322.el9.aarch64"\nkernel="/boot/vmlinuz-5.14.0-316.el9.aarch64+64k"'}Parameters# parameter answervmlinuz-6.2.11-200.fc37.x86_64 /boot/vmlinuz-6.2.11-200.fc37.x86_64vmlinuz-5.14.0-322.el9.aarch64 /boot/vmlinuz-5.14.0-322.el9.aarch64vmlinuz-5.14.0-316.el9.aarch64+64k /boot/vmlinuz-5.14.0-316.el9.aarch64+64kEndIt 'returns the kernel path for the given release'When call _find_kernel_path_by_release "$1"The output should equal "$2"EndEndDescribe 'parse_config()' bad_kdump_conf=$(mktemp -t bad_kdump_conf.XXXXXXXXXX) cleanup() {
-
Philipp Rudo -
Pingfan Liu