Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved rollback, journal cleanup, and durability failures can discard acknowledged writes or persist an inconsistent layer stack.
7 open issues
Handle snapshot cleanup failures before reopening the device · New Parse the exact configuration bytes published in the journal · New Fsync parent directories before publishing new upper files · New Block writes until configuration publication is durable · New Block writes and snapshots after rollback failure · New Retire rollback metadata under lock before resuming writers · New Avoid zero-length rollback before capturing the original size · New
What changed in this PR
This PR reworks live snapshots into a recoverable transaction, aiming to make retries idempotent and preserve a consistent layer stack across failures and restarts.
Changes:
- Splits restacking into preparation, config publication, and in-memory switching.
- Adds rollback journals, startup recovery, strict layer validation, and snapshot serialization.
- Adds regression tests and documents failure and recovery semantics.
| File | Description |
|---|---|
src/test/image_service_test.cpp |
Adds retry, validation, rollback, sequential snapshot, and recovery tests. |
src/overlaybd/lsmt/index.h |
Declares index preparation and application APIs. |
src/overlaybd/lsmt/index.cpp |
Prepares merged indexes without modifying the active backing index. |
src/overlaybd/lsmt/file.h |
Adds transaction callbacks and file-capacity reservation. |
src/overlaybd/lsmt/file.cpp |
Implements staged restacking, seal rollback, and fsync error propagation. |
src/image_file.h |
Declares recovery helpers and a snapshot mutex. |
src/image_file.cpp |
Implements journals, recovery, validation, and idempotent snapshot handling. |
docs/live-snapshot.md |
Documents transaction semantics, recovery, and limitations. |
docs/live-snapshot_zh.md |
Adds corresponding Chinese documentation. |
Review effort: Balanced
a44d8eb to
d12179e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved destination-validation, journal-recovery, and writer-synchronization flaws can overwrite or delete layer data, lose writes, and prevent devices from reopening.
11 open issues
Use lstat to reject dangling symlink move targets · New Reject snapshot journal paths as move destinations · New Use atomic no-replace moves and transaction ownership checks · New Acquire write mutex before flushing group commits · New Gate all writes and discards during snapshot sealing · New Retire rollback metadata under lock before resuming writers Block writes and snapshots after rollback failure Block writes until configuration publication is durable Fsync parent directories before publishing new upper files Handle snapshot cleanup failures before reopening the device Reject stale journals before staging new snapshots · New
2 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Journal-path collisions can cause recovery to delete layer data, while symlink moves and unescaped newline paths can leave images unreopenable or prevent restoration after a crash.
3 open issues
11 resolved since last review
Gate all writes and discards during snapshot sealing Acquire write mutex before flushing group commits Use atomic no-replace moves and transaction ownership checks Reject snapshot journal paths as move destinations Use lstat to reject dangling symlink move targets Retire rollback metadata under lock before resuming writers Block writes and snapshots after rollback failure Block writes until configuration publication is durable Fsync parent directories before publishing new upper files Handle snapshot cleanup failures before reopening the device Reject stale journals before staging new snapshots
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Journal collision checks miss backing files and path aliases, risking layer-data deletion during startup recovery, while direct use of renameat2() breaks the supported CentOS 7 build.
2 open issues
3 resolved since last review
Review effort: Balanced
Signed-off-by: huangzhaoxiang.hzx <huangzhaoxiang.hzx@alibaba-inc.com>


What this PR does / why we need it:
consumed: a retry failed to parse, and a re-created identical request sealed the upper layer a second
time instead of returning success.
the config was updated, so a failed update left the running device and the on-disk config describing
different stacks, with no record from which a restart could recover.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
<config>.snapshotand published byrename(), so the caller's fileis no longer consumed.
confis updated to the committed config, so an identical request returns success without touchinganything.
rename()now runs inside thecommitcallback, under the write lock and before thein-memory switch; a failure discards the prepared stack and both memory and disk stay on the old one.
Recovery covers process crashes only; the snapshot path performs no fsync.
Please check the following list: