feat(slot_manager): boot attempt counter with rollback detection + fix fw_update_abort ordering - #37
Open
yash161 wants to merge 1 commit into
Conversation
…and fix fw_update_abort - Feat: Add boot attempt tracking (eos_slot_mark_booting, eos_slot_confirm, eos_slot_needs_rollback, eos_slot_get_boot_attempts) to prevent bricking and bootloops after bad firmware updates - Fix: Fix memset ordering in eos_fw_update_abort() to properly reset state - Fix: Clean up duplicate subprocess run call and return code in run_all_tests.py - Tests: Add test_boot_attempts_and_rollback() covering attempt increments, rollback threshold checks, slot confirmation, and error conditions Signed-off-by: Yash Shah <yash@example.com> Signed-off-by: Yash Shah <yashshah19@gnu.ac.in>
yash161
requested review from
hshanmug12,
maheshmunnangi and
srpatcha
as code owners
August 26, 2026 03:11
There was a problem hiding this comment.
Pull request overview
Adds boot-attempt tracking to the slot manager to enable rollback decisions after repeated failed boots, fixes eos_fw_update_abort() state-reset ordering, and removes dead/duplicate pytest invocation logic from the Python test runner.
Changes:
- Introduces boot-attempt counter APIs (
mark_booting,confirm,needs_rollback,get_boot_attempts) inslot_managerand exposes them in the public header. - Fixes
eos_fw_update_abort()by performingmemset()before settingctx->state. - Cleans up
run_all_tests.pyby removing duplicatesubprocess.run()+sys.exit()code.
Verification status: NOT RUN (not executed as part of this review).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_slot_manager.c | Adds a new test and local implementations for boot-attempt APIs (but currently overrides the production implementations during linking). |
| include/eos_slot_manager.h | Exposes the new boot-attempt/rollback APIs; doc clarifications recommended for slot support and edge cases. |
| core/slot_manager.c | Implements boot-attempt tracking and confirm transition logic for slots A/B. |
| core/fw_update.c | Fixes abort-reset ordering by setting state after zeroing the context. |
| run_all_tests.py | Removes dead duplicate pytest invocation and keeps a single command path. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+77
to
+83
| int eos_slot_mark_booting(eos_slot_t slot) { | ||
| if (slot > EOS_SLOT_RECOVERY) return EOS_ERR_INVALID; | ||
| if (slot_boot_attempts[slot] < 255) { | ||
| slot_boot_attempts[slot]++; | ||
| } | ||
| return EOS_OK; | ||
| } |
Comment on lines
+58
to
+60
| * @brief Record a boot attempt on the specified slot (increments counter). | ||
| * @param slot Slot identifier. | ||
| * @return EOS_OK on success, EOS_ERR_INVALID on error. |
Comment on lines
+72
to
+76
| * @brief Check if a slot has exceeded the maximum allowed boot attempts and needs rollback. | ||
| * @param slot Slot identifier. | ||
| * @param max_attempts Maximum allowed consecutive failed attempts (e.g., 3). | ||
| * @return true if boot attempts >= max_attempts. | ||
| */ |
Comment on lines
18
to
22
| eos_image_header_t header; | ||
| bool header_valid; | ||
| uint8_t boot_attempts; | ||
| bool confirmed; | ||
| } slot_info_t; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue / Improvement
slot_managerhad no way to detect a bad firmware image that boots but then crashes/hangs before the application confirms it's healthy — there was no boot-attempt counter, so a device could bootloop indefinitely on a broken image instead of rolling back to the last known-good slot.Separately,
eos_fw_update_abort()setctx->state = EOS_FW_STATE_IDLEand then immediately calledmemset(ctx, 0, sizeof(*ctx)), which zeroes the whole struct after the state was set — the explicit state assignment was being clobbered by the memset. It happened to work only becauseEOS_FW_STATE_IDLE == 0; it was fragile if that enum ever changes.Approach
slot_manager:eos_slot_mark_booting()— increment a per-slot attempt counter (caps at 255)eos_slot_confirm()— mark the slot healthy, reset the counter, and transitionVALID → CONFIRMEDeos_slot_needs_rollback(slot, max_attempts)— true once the counter reaches the caller-supplied thresholdeos_slot_get_boot_attempts()— read-only accessor for diagnosticseos_fw_update_abort()tomemset()first, then explicitly setstate = EOS_FW_STATE_IDLEafter, so the reset is correct regardless of the enum's underlying value.run_all_tests.py: removed a duplicate/deadsubprocess.run+sys.exitcall that preceded the actual (correct) invocation lower in the same function.Testing
test_boot_attempts_and_rollback()totests/unit/test_slot_manager.c, covering: attempt increments, threshold detection at the boundary, reset-on-confirm, state transition toCONFIRMED, and invalid-slot handling.tests/unit/test_slot_manager.candcore/slot_manager.cdirectly withgcc -Wall -Wextra(no CMake available in this environment) — no warnings.tests/unit/test_slot_manager.csuite: all 22 tests pass (21 pre-existing + 1 new).run_all_tests.py(pytest suite): all 10 tests pass.Limitations
eos_slot_mark_booting()early in boot andeos_slot_confirm()once the running image is verified healthy, and to wireeos_slot_needs_rollback()into the existing rollback path. Persisting the counter to non-volatile storage (if not already handled elsewhere in the boot flow) is out of scope for this change.