fix: avoid misleading journalctl warning in logs on first boot#190
fix: avoid misleading journalctl warning in logs on first boot#190sarmahaj wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a check in src/main.rs to verify if a previous boot exists using journalctl --list-boots before attempting to query the journal for the previous boot (-b -1). This prevents errors on systems during their first boot. The review feedback suggests optimizing the check for the number of boots by using .nth(1).is_none() instead of counting all lines, which avoids unnecessary iteration over the entire command output.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
de51ed4 to
e0432e6
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a check to verify if a previous boot exists using journalctl --list-boots before querying its journal, skipping the rollback check if no previous boot is found. It also improves a log message format. The feedback suggests inverting the success check for journalctl --list-boots to return early on failure, which reduces nesting and improves readability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
LGTM, I like gemini comment that prevents the use of else . |
|
Can we do the implementation of checking if its a first time boot of the system only, systemd should provide such info, as i think system with non-persistent journald will skip the rollback check everytime without reporting error. |
knecasov
left a comment
knecasov
left a comment
There was a problem hiding this comment.
I added a few comments and gave a thumbs-up to the Gemini note.
e0432e6 to
3511b80
Compare
@say-paul Thanks for suggesting ! |
knecasov
left a comment
knecasov
left a comment
There was a problem hiding this comment.
LGTM, I would only recommend editing the PR description to reflect the final implementation.
3511b80 to
abc7f15
Compare
|
Thanks for review and approval @mcattamoredhat and @knecasov . Applied changes suggested by you @knecasov Thanks! |
knecasov
left a comment
knecasov
left a comment
There was a problem hiding this comment.
Thank you so much for fixes! The PR description looks good now - only the formatting seems to be broken.
Anyway, it is not blocking merging, so I am approving it!
Fixes misleading warning on first boot and on systems with non-persistent journald storage. Signed-off-by: Sarita Mahajan <sarmahaj@redhat.com>
abc7f15 to
fa0cafe
Compare
4 participants
Use systemd-analyze condition ConditionFirstBoot=yes to detect first boot before querying the previous boot journal. This prevents a misleading warning on first boot when journalctl -b -1 fails because there is no prior boot. This also works correctly on systems with non-persistent journald storage.