Conversation
For BLS complaint bootloaders, just systemd for now, we were still looking for entries in `boot/loader/entries` which was failing as entries for these bootloaders will be stored in the ESP. Also, refactor to use `TempMount::mount_dev` for mounting ESP Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue with locating boot entries for systemd-boot by mounting the ESP. The refactoring to use TempMount for managing the mount lifecycle is a solid improvement for code clarity and safety. I've identified a critical compilation bug related to a use-after-move in the new logic for determining boot entry titles, and a minor issue with discarded error information. I've provided suggestions to address these points. Overall, the changes are well-aligned with the pull request's goal.
391d95c to
991e4ee
Compare
jeckersb
left a comment
There was a problem hiding this comment.
Generally looks fine to me, just a few minor things inline.
089831a to
12e8ca6
Compare
For Type1 boot entries, get the version and title from /usr/lib/os-release only defaulting to "sort_key" and "verity hash" respectively if we fail to find them in the file. Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
12e8ca6 to
3052021
Compare
|
Tests for centos9 seem to be failing with I can't figure out what the macro actually expands to, but the problem is probably in the following control flow |
|
Hm the macros must have been updated in c9s? I'll look into that, I had to figure out that mess the first time around. |
Yep, that's indeed the case - https://gitlab.com/redhat/centos-stream/rpms/rust/-/commit/54e4b48d95a9fe0d459f464b2a672325673254a8 I'll fix it in a separate PR and then we can just rebase this one. |
|
Actually nm, those jobs aren't required, we can just merge this and then fix the macros issue. |
For BLS complaint bootloaders, just systemd for now, we were still
looking for entries in
boot/loader/entrieswhich was failing asentries for these bootloaders will be stored in the ESP.
Also, refactor to use
TempMount::mount_devfor mounting ESP