[PR #131] [MERGED] fix(overlaybd): isolate runtime and CLI cache directories #199

New issue

Closed

opened 2026-10-29 14:20:56 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:20:56 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/131
Author: @huajq
Created: 8/6/2026
Status: ✅ Merged
Merged: 8/7/2026
Merged by: @yingdi-shan

Base: main ← Head: fix/overlaybd-cache-isolation

📝 Commits (1)

15f8dd9 fix(overlaybd): isolate runtime and CLI cache directories

📊 Changes

10 files changed (+449 additions, -50 deletions)

View changed files

📝 docs/src/internals/persistence-artifact-inventory.md (+3 -2)
📝 src/cfg.rs (+97 -22)
📝 src/image/resolver.rs (+3 -3)
📝 src/sandbox/ublk/device.rs (+4 -0)
📝 src/setup/deps.rs (+207 -12)
📝 storage/ublk-daemon/src/client.rs (+4 -0)
📝 storage/ublk-daemon/src/main.rs (+5 -0)
📝 storage/ublk-daemon/src/runtime.rs (+74 -7)
📝 storage/ublk-daemon/src/server.rs (+34 -2)
📝 storage/ublk-daemon/tests/ublk_daemon_test.rs (+18 -2)

📄 Description

What

Give every OverlayBD cache consumer its own cacheDir instead of sharing {image-cache}/remote-blocks:

rootfs runtime (Rust FileCacheBackend) → remote-blocks (unchanged)
memory snapshot (Rust FileCacheBackend) → memory-blocks
overlaybd-apply (C++ file cache, OCI → overlaybd conversion) → convert-blocks
overlaybd-resize (C++ file cache) → resize-blocks

overlaybd-resize receives its dedicated generated global config (resize-overlaybd-global.json) through a new daemon CLI argument, and daemon resize invocations are serialized under a daemon-wide permit. Config validation now also rejects user-configured rootfs/memory global config paths that collide with the derived convert/resize config paths.

Why

All generated OverlayBD global configs pointed at the same cacheDir (remote-blocks), with two distinct failure modes:

The offline C++ tools (overlaybd-apply, overlaybd-resize) initialize the C++ flat file cache on the shared directory. That cache recursively indexes every file it finds — including the Rust cache's <digest>/data and <digest>/meta.bin — and its eviction does truncate(0)+unlink on them. With the Rust cache above ~90% of cacheSizeGB, any tool invocation past the 10s eviction timer batch-empties Rust per-entry directories, and truncate(0) on a live daemon mmap raises SIGBUS, killing the ublk daemon and all its devices.
The daemon runs two independent Rust FileCacheBackend instances (rootfs and memory) over the same directory. Each keeps its own bitmap/mmap/LRU and eviction worker, so one backend can hole-punch or delete entries the other still considers valid — the same class of corruption without any C++ process involved.

overlaybd-apply has no functional need for the cache at all (conversion only opens local layers). overlaybd-resize legitimately reads remote lowers, so it keeps cacheType=file with on-demand reads in its own directory; background download is disabled for both tool configs.

N/A — no tracking issue.

Scope and non-goals

Included:

Four isolated cache directories with generated configs rewritten on every setup/startup.
resize-overlaybd-global.json plumbed from AppConfig into the daemon (--resize-global-config) and used only for the resize child's --service_config_path; base-size resolution and post-resize verification keep using the runtime config.
Daemon-wide tokio::sync::Mutex permit around overlaybd-resize (the C++ cache has no cross-process coordination), released on success, non-zero exit, and timeout (kill + reap).
Validation that rootfs/memory/convert/resize global config paths are pairwise distinct (lexical + canonical alias detection).
Unit tests for config generation, path validation, resize argv, permit release after failure/timeout, and the fresh-launch (known_source_virtual_size=None) shape.

Non-goals (deliberately excluded):

Shared process-wide Rust CachePool with QoS for rootfs/memory (would avoid duplicated caching but is a larger cache-architecture change).
Dynamic cross-cache disk quota management; each cache currently inherits image.cache.remote_blocks.max_size_gb independently.
C++ cache cross-process locking (upstream OverlayBD concern).

Design and behavior changes

Generated config layout:

{home}/overlaybd/overlaybd-global.json          -> {image-cache}/remote-blocks   (rootfs)
{home}/overlaybd/mem-overlaybd-global.json      -> {image-cache}/memory-blocks   (memory)
{home}/overlaybd/convert-overlaybd-global.json  -> {image-cache}/convert-blocks  (apply, download off)
{home}/overlaybd/resize-overlaybd-global.json   -> {image-cache}/resize-blocks   (resize, download off)

Resize config flow: AppConfig::resolved_overlaybd_resize_global_config_path() → UblkDaemonConfig → UblkDaemonSpawnConfig → daemon --resize-global-config → UblkDaemonServer → per-connection MaterializeOverlaybdRuntimeRequest → run_resize_tool. The resize permit is acquired immediately before spawning the child and dropped before Rust-side verification.

Compatibility and operations

Public API or generated protocol: unchanged. The daemon gains a required --resize-global-config CLI arg; server and daemon ship together, and the in-repo client is the only spawner.
Configuration or defaults: config.validate() now rejects rootfs/memory global config paths that alias the derived convert/resize config paths (previously silently overwritten at setup). No default behavior change.
Snapshot manifest, artifact layout, or storage format: unchanged. New cache directories are created on demand; memory-blocks starts cold once (existing remote-blocks content is not migrated).
Upgrade and rollback: rollback to the previous version is safe (it ignores the new config files/directories).
Host requirements, permissions, ports, or dependencies: unchanged.

Validation

make fmt
make clippy
make test-unit (see note below)
Relevant Rust integration tests
make -C services test (required when services/ changes) — N/A, no services/ changes
Generated clients/server regenerated with the documented make target — N/A, no generated code touched
Documentation updated
Benchmarks or performance comparison completed — N/A, no steady-state performance change intended

Commands and results (Linux host with KVM/ublk):

make fmt                                    # pass
make clippy                                 # pass (-D warnings)
make test-unit                              # 693 passed, 1 failed (see note)
cargo test -p uvm-ublk-daemon               # 46 unit + 40 integration passed
cargo test -p agentenv setup::deps --lib    # 17 passed
cargo test -p agentenv --lib validate_rejects_  # 9 passed

End-to-end on the same host: created a sandbox with diskSizeMB larger than the base image (300 GiB on a 256 GiB image); observed the real invocation

overlaybd-resize --config .../image.json --size ... --service_config_path .../resize-overlaybd-global.json

with resize-blocks/ gaining the C++ cache entry and remote-blocks/ containing only Rust per-entry directories afterwards; sandbox booted and the device showed the requested size.

make test-unit note: the single failure is privileges::tests::scoped_spawn_clears_child_capabilities_without_mutating_caller (src/privileges.rs, untouched by this PR). It asserts a spawned child retains no CapPrm, which does not hold when tests run as root on this host; pre-existing and environment-dependent.

Skipped checks and reasons: services tests, codegen, benchmarks — no related changes.

Risks and reviewer notes

Permit queueing vs client RPC timeout: serialized resizes can queue; the client runtime-device timeout is sized for one resize, so a burst of concurrent oversized cold-starts can time out client-side while the daemon finishes the device (orphaned until restart). Pre-existing failure class amplified by serialization; tracked as a follow-up (observability + orphan cleanup), not addressed here.
Each of the four caches inherits the full image.cache.remote_blocks.max_size_gb, so worst-case aggregate cache usage grows; convert-blocks stays near-empty in practice (local-layer conversion only).
Concurrent overlaybd-apply processes share convert-blocks, but in the current pipeline conversion opens only local layers, so the C++ cache produces no remote entries there; revisit if conversion ever opens remote lowers.
Most important files: src/setup/deps.rs (four generated configs), src/cfg.rs (derived paths + collision validation), storage/ublk-daemon/src/runtime.rs (resize child config + permit).

Checklist

The PR contains one coherent change and no unrelated formatting or refactoring.
New behavior is covered by tests, or I explained why testing is impractical.
Logs and examples contain no credentials, tokens, or private registry information.
I did not manually edit generated code without updating its source and regenerating it.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/AgentENV/pull/131 **Author:** [@huajq](https://github.com/huajq) **Created:** 8/6/2026 **Status:** ✅ Merged **Merged:** 8/7/2026 **Merged by:** [@yingdi-shan](https://github.com/yingdi-shan) **Base:** `main` ← **Head:** `fix/overlaybd-cache-isolation` --- ### 📝 Commits (1) - [`15f8dd9`](https://github.com/kvcache-ai/AgentENV/commit/15f8dd9b5f2fc9a132d819a3ca44bf855eb4e604) fix(overlaybd): isolate runtime and CLI cache directories ### 📊 Changes **10 files changed** (+449 additions, -50 deletions) <details> <summary>View changed files</summary> 📝 `docs/src/internals/persistence-artifact-inventory.md` (+3 -2) 📝 `src/cfg.rs` (+97 -22) 📝 `src/image/resolver.rs` (+3 -3) 📝 `src/sandbox/ublk/device.rs` (+4 -0) 📝 `src/setup/deps.rs` (+207 -12) 📝 `storage/ublk-daemon/src/client.rs` (+4 -0) 📝 `storage/ublk-daemon/src/main.rs` (+5 -0) 📝 `storage/ublk-daemon/src/runtime.rs` (+74 -7) 📝 `storage/ublk-daemon/src/server.rs` (+34 -2) 📝 `storage/ublk-daemon/tests/ublk_daemon_test.rs` (+18 -2) </details> ### 📄 Description ## What Give every OverlayBD cache consumer its own `cacheDir` instead of sharing `{image-cache}/remote-blocks`: - rootfs runtime (Rust `FileCacheBackend`) → `remote-blocks` (unchanged) - memory snapshot (Rust `FileCacheBackend`) → `memory-blocks` - `overlaybd-apply` (C++ file cache, OCI → overlaybd conversion) → `convert-blocks` - `overlaybd-resize` (C++ file cache) → `resize-blocks` `overlaybd-resize` receives its dedicated generated global config (`resize-overlaybd-global.json`) through a new daemon CLI argument, and daemon resize invocations are serialized under a daemon-wide permit. Config validation now also rejects user-configured rootfs/memory global config paths that collide with the derived convert/resize config paths. ## Why All generated OverlayBD global configs pointed at the same `cacheDir` (`remote-blocks`), with two distinct failure modes: 1. The offline C++ tools (`overlaybd-apply`, `overlaybd-resize`) initialize the C++ flat file cache on the shared directory. That cache recursively indexes every file it finds — including the Rust cache's `<digest>/data` and `<digest>/meta.bin` — and its eviction does `truncate(0)`+`unlink` on them. With the Rust cache above ~90% of `cacheSizeGB`, any tool invocation past the 10s eviction timer batch-empties Rust per-entry directories, and `truncate(0)` on a live daemon mmap raises SIGBUS, killing the ublk daemon and all its devices. 2. The daemon runs two independent Rust `FileCacheBackend` instances (rootfs and memory) over the same directory. Each keeps its own bitmap/mmap/LRU and eviction worker, so one backend can hole-punch or delete entries the other still considers valid — the same class of corruption without any C++ process involved. `overlaybd-apply` has no functional need for the cache at all (conversion only opens local layers). `overlaybd-resize` legitimately reads remote lowers, so it keeps `cacheType=file` with on-demand reads in its own directory; background download is disabled for both tool configs. ## Related issue N/A — no tracking issue. ## Scope and non-goals Included: - Four isolated cache directories with generated configs rewritten on every setup/startup. - `resize-overlaybd-global.json` plumbed from `AppConfig` into the daemon (`--resize-global-config`) and used only for the resize child's `--service_config_path`; base-size resolution and post-resize verification keep using the runtime config. - Daemon-wide `tokio::sync::Mutex` permit around `overlaybd-resize` (the C++ cache has no cross-process coordination), released on success, non-zero exit, and timeout (kill + reap). - Validation that rootfs/memory/convert/resize global config paths are pairwise distinct (lexical + canonical alias detection). - Unit tests for config generation, path validation, resize argv, permit release after failure/timeout, and the fresh-launch (`known_source_virtual_size=None`) shape. Non-goals (deliberately excluded): - Shared process-wide Rust `CachePool` with QoS for rootfs/memory (would avoid duplicated caching but is a larger cache-architecture change). - Dynamic cross-cache disk quota management; each cache currently inherits `image.cache.remote_blocks.max_size_gb` independently. - C++ cache cross-process locking (upstream OverlayBD concern). ## Design and behavior changes Generated config layout: ```text {home}/overlaybd/overlaybd-global.json -> {image-cache}/remote-blocks (rootfs) {home}/overlaybd/mem-overlaybd-global.json -> {image-cache}/memory-blocks (memory) {home}/overlaybd/convert-overlaybd-global.json -> {image-cache}/convert-blocks (apply, download off) {home}/overlaybd/resize-overlaybd-global.json -> {image-cache}/resize-blocks (resize, download off) ``` Resize config flow: `AppConfig::resolved_overlaybd_resize_global_config_path()` → `UblkDaemonConfig` → `UblkDaemonSpawnConfig` → daemon `--resize-global-config` → `UblkDaemonServer` → per-connection `MaterializeOverlaybdRuntimeRequest` → `run_resize_tool`. The resize permit is acquired immediately before spawning the child and dropped before Rust-side verification. ## Compatibility and operations - Public API or generated protocol: unchanged. The daemon gains a required `--resize-global-config` CLI arg; server and daemon ship together, and the in-repo client is the only spawner. - Configuration or defaults: `config.validate()` now rejects rootfs/memory global config paths that alias the derived convert/resize config paths (previously silently overwritten at setup). No default behavior change. - Snapshot manifest, artifact layout, or storage format: unchanged. New cache directories are created on demand; `memory-blocks` starts cold once (existing `remote-blocks` content is not migrated). - Upgrade and rollback: rollback to the previous version is safe (it ignores the new config files/directories). - Host requirements, permissions, ports, or dependencies: unchanged. ## Validation - [x] `make fmt` - [x] `make clippy` - [x] `make test-unit` (see note below) - [x] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) — N/A, no `services/` changes - [ ] Generated clients/server regenerated with the documented `make` target — N/A, no generated code touched - [x] Documentation updated - [ ] Benchmarks or performance comparison completed — N/A, no steady-state performance change intended Commands and results (Linux host with KVM/ublk): ```text make fmt # pass make clippy # pass (-D warnings) make test-unit # 693 passed, 1 failed (see note) cargo test -p uvm-ublk-daemon # 46 unit + 40 integration passed cargo test -p agentenv setup::deps --lib # 17 passed cargo test -p agentenv --lib validate_rejects_ # 9 passed ``` End-to-end on the same host: created a sandbox with `diskSizeMB` larger than the base image (300 GiB on a 256 GiB image); observed the real invocation ```text overlaybd-resize --config .../image.json --size ... --service_config_path .../resize-overlaybd-global.json ``` with `resize-blocks/` gaining the C++ cache entry and `remote-blocks/` containing only Rust per-entry directories afterwards; sandbox booted and the device showed the requested size. `make test-unit` note: the single failure is `privileges::tests::scoped_spawn_clears_child_capabilities_without_mutating_caller` (`src/privileges.rs`, untouched by this PR). It asserts a spawned child retains no `CapPrm`, which does not hold when tests run as root on this host; pre-existing and environment-dependent. Skipped checks and reasons: services tests, codegen, benchmarks — no related changes. ## Risks and reviewer notes - Permit queueing vs client RPC timeout: serialized resizes can queue; the client runtime-device timeout is sized for one resize, so a burst of concurrent oversized cold-starts can time out client-side while the daemon finishes the device (orphaned until restart). Pre-existing failure class amplified by serialization; tracked as a follow-up (observability + orphan cleanup), not addressed here. - Each of the four caches inherits the full `image.cache.remote_blocks.max_size_gb`, so worst-case aggregate cache usage grows; `convert-blocks` stays near-empty in practice (local-layer conversion only). - Concurrent `overlaybd-apply` processes share `convert-blocks`, but in the current pipeline conversion opens only local layers, so the C++ cache produces no remote entries there; revisit if conversion ever opens remote lowers. - Most important files: `src/setup/deps.rs` (four generated configs), `src/cfg.rs` (derived paths + collision validation), `storage/ublk-daemon/src/runtime.rs` (resize child config + permit). ## Checklist - [x] The PR contains one coherent change and no unrelated formatting or refactoring. - [x] New behavior is covered by tests, or I explained why testing is impractical. - [x] Logs and examples contain no credentials, tokens, or private registry information. - [x] I did not manually edit generated code without updating its source and regenerating it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 14:20:56 +00:00