feat: make live snapshot restack restartable and idempotent - #440
Open
andrewathud wants to merge 2 commits into
Open
feat: make live snapshot restack restartable and idempotent#440andrewathud wants to merge 2 commits into
andrewathud wants to merge 2 commits into
Conversation
Validate exact lower-chain extension, journal restack intent across mutation, update in-memory config after success, and serialize device registry lifetime so repeated D1/D2 checkpoints are safe. Signed-off-by: Andrew Ho <andyminhtuanho@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
LayerConfig accessors are non-const in the generated helpers, and set_failed must be available to the inline ImageFile constructor. Signed-off-by: Andrew Ho <andyminhtuanho@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Author
|
@Asadullo mind reviewing when you get a chance? |
Author
|
@solvemproblr mind reviewing when you get a chance? (GitHub won't let me formally request review on this upstream PR.) |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves OverlayBD live-snapshot “restack” behavior to be restartable and idempotent after mid-operation failures, and updates the HTTP snapshot API + tests to exercise the new semantics.
Changes:
- Add restack journaling/intent publishing logic to make
ImageFile::create_snapshot()restartable/idempotent. - Add locking around ImageService device registry operations and a “restack while holding registry lock” entrypoint for the API server.
- Expand unit tests to cover method validation, duplicate query params, and repeated snapshot/restack behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/test/image_service_test.cpp | Adds coverage for HTTP verb validation, duplicate query params, and repeated snapshot/restack scenarios. |
| src/image_service.h | Introduces device-registry rwlock and a create_snapshot_for_device() helper that holds the registry lock during restack. |
| src/image_service.cpp | Wraps register/unregister/find in rwlocks and implements create_snapshot_for_device() for safe restack invocation. |
| src/image_file.h | Adds IO/restack rwlock, updates error text, and declares config/journal helper utilities. |
| src/image_file.cpp | Implements restack journal + idempotency logic and adds helpers for config matching and durable config publishing. |
| src/api_server.cpp | Enforces POST-only snapshot endpoint, rejects malformed/duplicate query params, and uses the registry-locked restack helper. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+637
to
+641
| if (lfs->rename(tmp.c_str(), config_path.c_str()) != 0) { | ||
| unlink(tmp.c_str()); | ||
| return -1; | ||
| } | ||
| return 0; |
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.
Summary
Context
Used by HUD sandbox filesystem checkpoints (see hud-evals/hud-monorepo#1103). We intend to pin a HUD-owned fork once
hud-evals/overlaybdexists; this PR lands the upstream-ready changes for review.Tip
f6d477d12c3515891089767a43d4ac6ff16671f8Test plan
Made with Cursor