[PATCH] kdumpctl: Optimize _find_kernel_path_by_release regex string

[Lichen Liu]

Currently _find_kernel_path_by_release uses grubby and grep to
find the kernel path, if both the normal kernel and it's debug
varient exist, the grep will give more than one kernel strings.

```
kernel="/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x+debug"
kernel="/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x"
```

On the s390x arch, which uses zipl but not grub as bootloader,
this will cause an error when installing debug kernel.

```
The param "/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x+debug
/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x" is incorrect
```

Signed-off-by: Lichen Liu 
---
 kdumpctl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kdumpctl b/kdumpctl
index 7caaae5..63c78b0 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -1353,7 +1353,7 @@ _filter_grubby_kernel_str()
 _find_kernel_path_by_release()
 {
    local _release="$1" _grubby_kernel_str _kernel_path
-	_grubby_kernel_str=$(grubby --info ALL | grep "^kernel=.*$_release")
+	_grubby_kernel_str=$(grubby --info ALL | grep "^kernel=.*$_release"$")
    _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str")
    if [[ -z $_kernel_path ]]; then
    	derror "kernel $_release doesn't exist"

[Coiby Xu]

Hi Lichen,

The patch looks good to me except for the commit message.

On Mon, Nov 21, 2022 at 03:57:38PM +0800, Lichen Liu wrote:
> Currently _find_kernel_path_by_release uses grubby and grep to
> find the kernel path, if both the normal kernel and it's debug
> varient exist, the grep will give more than one kernel strings.
> kernel="/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x+debug"
> kernel="/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x"
>
> On the s390x arch, which uses zipl but not grub as bootloader,
> this will cause an error when installing debug kernel.
> The param "/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x+debug
> /boot/vmlinuz-5.14.0-139.kpq0.el9.s390x" is incorrect

This error has nothing to do with s390x using zipl as bootloader because
it's from grubby. And we shall hit the same error on other arches. And
I can confirm on x86 after running the following code,

touch /boot/vmlinuz-42.0.0-debug; mkdir -p /lib/modules/42.0.0-debug/
   kernel-install add 42.0.0-debug /boot/vmlinuz-42.0.0-debug
   kernelstr=$'/boot/vmlinuz-42.0.0-debug\n/boot/vmlinuz-42.0.0'
   grubby --info "$kernelstr"

I see the error
```
The param /boot/vmlinuz-42.0.0-debug
/boot/vmlinuz-42.0.0 is incorrect
```
> Signed-off-by: Lichen Liu
>
> kdumpctl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kdumpctl b/kdumpctl
> index 7caaae5..63c78b0 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -1353,7 +1353,7 @@ _filter_grubby_kernel_str()
> _find_kernel_path_by_release()
> {
>    local _release="$1" _grubby_kernel_str _kernel_path
> _grubby_kernel_str=$(grubby --info ALL | grep "^kernel=.*$_release")
> _grubby_kernel_str=$(grubby --info ALL | grep "^kernel=.*$_release"$")
> _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str")
> if [[ -z $_kernel_path ]]; then
> derror "kernel $_release doesn't exist"
>
> -- 
> 2.37.3
> _______________________________________________
> kexec mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue

[Lichen Liu]

Thank you Coiby, I will change the commit message and post a V2 patch

On Wed, Nov 23, 2022 at 5:25 PM Coiby Xu  wrote:
> Hi Lichen,
>
> The patch looks good to me except for the commit message.
>
> On Mon, Nov 21, 2022 at 03:57:38PM +0800, Lichen Liu wrote:
> > Currently _find_kernel_path_by_release uses grubby and grep to
> > find the kernel path, if both the normal kernel and it's debug
> > varient exist, the grep will give more than one kernel strings.
> > kernel="/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x+debug"
> > kernel="/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x"
> >
> > On the s390x arch, which uses zipl but not grub as bootloader,
> > this will cause an error when installing debug kernel.
> > The param "/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x+debug
> > /boot/vmlinuz-5.14.0-139.kpq0.el9.s390x" is incorrect
>
> This error has nothing to do with s390x using zipl as bootloader because
> it's from grubby. And we shall hit the same error on other arches. And
> I can confirm on x86 after running the following code,
>
> touch /boot/vmlinuz-42.0.0-debug; mkdir -p /lib/modules/42.0.0-debug/
>    kernel-install add 42.0.0-debug /boot/vmlinuz-42.0.0-debug
>    kernelstr=$'/boot/vmlinuz-42.0.0-debug\n/boot/vmlinuz-42.0.0'
>    grubby --info "$kernelstr"
>
> I see the error
> The param /boot/vmlinuz-42.0.0-debug
> /boot/vmlinuz-42.0.0 is incorrect
> > Signed-off-by: Lichen Liu
> >
> > kdumpctl | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kdumpctl b/kdumpctl
> > index 7caaae5..63c78b0 100755
> > --- a/kdumpctl
> > +++ b/kdumpctl
> > @@ -1353,7 +1353,7 @@ _filter_grubby_kernel_str()
> > _find_kernel_path_by_release()
> > {
> >       local _release="$1" _grubby_kernel_str _kernel_path
> >  _grubby_kernel_str=$(grubby --info ALL | grep "^kernel=.*$_release")
> >  _grubby_kernel_str=$(grubby --info ALL | grep "^kernel=.*$_release\"$")
> > _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str")
> > if [[ -z $_kernel_path ]]; then
> >         derror "kernel $_release doesn't exist"
> >
> > --
> > 2.37.3
> > _______________________________________________
> > kexec mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
> > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> > Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
>
> --
> Best regards,
> Coiby

[Philipp Rudo]

On Wed, 23 Nov 2022 17:23:59 +0800
Coiby Xu  wrote:
> Hi Lichen,
>
> The patch looks good to me except for the commit message.
>
> On Mon, Nov 21, 2022 at 03:57:38PM +0800, Lichen Liu wrote:
> > Currently _find_kernel_path_by_release uses grubby and grep to
> > find the kernel path, if both the normal kernel and it's debug
> > varient exist, the grep will give more than one kernel strings.
> > kernel="/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x+debug"
> > kernel="/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x"
> >
> > On the s390x arch, which uses zipl but not grub as bootloader,
> > this will cause an error when installing debug kernel.
> > The param "/boot/vmlinuz-5.14.0-139.kpq0.el9.s390x+debug
> > /boot/vmlinuz-5.14.0-139.kpq0.el9.s390x" is incorrect
>
> This error has nothing to do with s390x using zipl as bootloader because
> it's from grubby. And we shall hit the same error on other arches. And
> I can confirm on x86 after running the following code,
>
> touch /boot/vmlinuz-42.0.0-debug; mkdir -p /lib/modules/42.0.0-debug/
>    kernel-install add 42.0.0-debug /boot/vmlinuz-42.0.0-debug
>    kernelstr=$'/boot/vmlinuz-42.0.0-debug\n/boot/vmlinuz-42.0.0'
>    grubby --info "$kernelstr"
>
> I see the error
> The param /boot/vmlinuz-42.0.0-debug
> /boot/vmlinuz-42.0.0 is incorrect

In addition please also add a

Fixes: 945cbbd ("add helper functions to get kernel path by kernel release and the path of current running kernel")

Other than that the patch is fine
Reviewed-by: Philipp Rudo
> > Signed-off-by: Lichen Liu
> >
> > kdumpctl | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kdumpctl b/kdumpctl
> > index 7caaae5..63c78b0 100755
> > --- a/kdumpctl
> > +++ b/kdumpctl
> > @@ -1353,7 +1353,7 @@ _filter_grubby_kernel_str()
> > _find_kernel_path_by_release()
> > {
> >    local _release="$1" _grubby_kernel_str _kernel_path
> > _grubby_kernel_str=$(grubby --info ALL | grep "^kernel=.*$_release")
> > _grubby_kernel_str=$(grubby --info ALL | grep "^kernel=.*$_release"$")
> > _kernel_path=$(_filter_grubby_kernel_str "$_grubby_kernel_str")
> > if [[ -z $_kernel_path ]]; then
> > derror "kernel $_release doesn't exist"
> >
> > -- 
> > 2.37.3
> > _______________________________________________
> > kexec mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]
> > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives: https://lists.fedoraproject.org/archives/list/[e-mail]
> > Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue