Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zfsbootmenu-core: improve kernel searches #472

Closed
wants to merge 1 commit into from

Conversation

ahesford
Copy link
Member

@ahesford ahesford commented Sep 9, 2023

  • Remove obsolete default_kernel file
  • Avoid reversing kernels list in select_kernel
  • Avoid ls hack to find kernels in boot environment
  • Make initramfs/kernel pairing agnostic of actual kernel path

This seems a little cleaner and can be generalized in the future to finding kernels at arbitrary paths.

- Remove obsolete default_kernel file
- Avoid reversing kernels list in select_kernel
- Avoid `ls` hack to find kernels in boot environment
- Make initramfs/kernel pairing agnostic of actual kernel path
echo "${def_kernel##*/}" > "${def_kernel_file}"
return 0
# Remove an invalid kernel record if the search failed
zdebug "failed to find kernels on ${fs}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this debug message instead be an error message? Not having any kernels on a filesystem that ZBM expects to find them on is probably worth yelling about.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, an error should help leave some breadcrumbs if an expected boot environment is missing kernels. If somebody has a filesystem with mountpoint=/ that deliberately contains no kernels and unnecessarily triggers the error, org.zfsbootmenu:active=off is the fix.

Merged with this change.

@ahesford ahesford closed this Sep 10, 2023
@ahesford ahesford deleted the sanders_or_seasons branch September 10, 2023 01:41
@ahesford
Copy link
Member Author

1526bb8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants