[PR #342] test(ublk): reproduce stale placeholder pages on pooled device image switch #365

New issue

Open

opened 2026-10-29 14:22:39 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 14:22:39 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/AgentENV/pull/342
Author: @NickNYU
Created: 10/5/2026
Status: 🔄 Open

Base: main ← Head: test/issue-302-ublk-pool-stale-pages

📝 Commits (6)

9394c58 test(ublk): reproduce stale placeholder pages on pooled device image switch
ff0cb77 test(ublk): address review on pool stale-page reproducer
5a9a675 Apply batched suggestions from code review
1efdad1 test(ublk): drop unused alloc import after alloc_zeroed switch
3ddf935 test(ublk): address second OpenCodeReview round on stale-page reproducer
724d9a1 test(ublk): keep cleanup workdir alive until detached cleanup finishes

📊 Changes

1 file changed (+525 additions, -0 deletions)

View changed files

➕ storage/ublk-daemon/tests/pool_page_cache_handover.rs (+525 -0)

📄 Description

What

Adds an #[ignore]d integration test
storage/ublk-daemon/tests/pool_page_cache_handover.rs that deterministically
reproduces the stale-placeholder-page defect from #302: a pooled ublk device
rebound from the all-zero idle placeholder to a nonzero business image keeps
serving zero pages through the buffered path, while O_DIRECT reads of the
same offset return the correct content.

Test-only change; no production code is modified.

Why

Issue #302 documents the mechanism with a manual daemon-level experiment.
Encoding it as an executable test gives a shared, deterministic reference for
the fix discussion, and becomes permanent regression protection once the fix
lands: the key assertion (post-switch buffered read must return the business
image) is written against the correct behavior and currently fails, so the
test is ignored until it is un-ignored together with the fix.

Refs #302 — this PR adds the reproducer only; it does not fix the defect.

Scope and non-goals

Included: the integration test, using only public crate APIs
(UblkDaemonServer in-process, daemon RPC protocol, overlaybd LSMT helpers).
Excluded: the page-cache invalidation fix itself (direction under discussion
on the issue), and the EnvdInstance::wait_for_ready diagnostics
improvement mentioned in the issue, which would be a separate PR.

Design and behavior changes

No production behavior changes. The test:

Builds a 2 GiB read-only file-lower OverlayBD image (empty repoBlobUrl,
sparse on disk) with one known nonzero page at 0x7ffce000, using the
crate's own create_file_rw/close_seal — no external overlaybd tools.
Runs an in-process UblkDaemonServer with the issue's pool shape
(low=0, high=1, prewarm=false) and drives it through the public
length-prefixed JSON RPC protocol.
Follows the issue's 8-step procedure: baseline O_DIRECT read, release to
the placeholder, retained buffered FD + deliberate placeholder cache fill,
reacquire (asserts the same dev_id is reused), buffered vs O_DIRECT
comparison, and a BLKFLSBUF recovery control.
Skips cleanly on hosts without ublk; runs under the existing capability
runner used by make test-ublk.

Compatibility and operations

Public API or generated protocol: N/A — test-only.
Configuration or defaults: N/A.
Snapshot manifest, artifact layout, or storage format: N/A.
Upgrade and rollback: N/A.
Host requirements, permissions, ports, or dependencies: the test requires
Linux with ublk_drv and CAP_SYS_ADMIN (supplied by
scripts/run-with-capabilities.sh under make test-ublk); it is
#[ignore]d by default and self-skips when ublk is unavailable, so default
and CI suites are unaffected.

Validation

make fmt
make clippy
make test-unit
Relevant Rust integration tests
make -C services test (required when services/ changes)
Generated clients/server regenerated with the documented make target
Documentation updated
Benchmarks or performance comparison completed

Commands and results:

# Linux x86_64 VM, kernel 7.0.11, ublk_drv loaded
cargo fmt --all -- --check                      # clean
cargo clippy --workspace --all-targets --all-features -- -D warnings
                                                # clean
cargo test -p uvm-ublk-daemon                   # all binaries ok; new test: 1 ignored
sudo -E cargo test -p uvm-ublk-daemon --test pool_page_cache_handover -- --ignored
                                                # FAILS as designed at the regression
                                                # assertion: left=0 vs right=0x7ffce000
                                                # (stale placeholder page served)

Skipped checks and reasons:

make test-unit: no production code changed; the affected crate's full
suite (cargo test -p uvm-ublk-daemon) was run instead and is green.
make -C services test: no services/ changes.
Generated code, docs, benchmarks: not touched by this change.

Risks and reviewer notes

Zero risk to default suites: the test is ignored and self-skips without
ublk. Running it with --ignored on a ublk-capable host fails by design
until #302 is fixed; that failure is the reproduction.
The interesting part to review is the test's fidelity to the issue: see the
doc comment at the top of
storage/ublk-daemon/tests/pool_page_cache_handover.rs and the final
buffered assertion.

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/342 **Author:** [@NickNYU](https://github.com/NickNYU) **Created:** 10/5/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `test/issue-302-ublk-pool-stale-pages` --- ### 📝 Commits (6) - [`9394c58`](https://github.com/kvcache-ai/AgentENV/commit/9394c58761287cec309183981c9548542a18894e) test(ublk): reproduce stale placeholder pages on pooled device image switch - [`ff0cb77`](https://github.com/kvcache-ai/AgentENV/commit/ff0cb77a057f8ad790eb3ce31f5de72e10c7642c) test(ublk): address review on pool stale-page reproducer - [`5a9a675`](https://github.com/kvcache-ai/AgentENV/commit/5a9a675f72df7941cff8c96a3fc80d4d0829389c) Apply batched suggestions from code review - [`1efdad1`](https://github.com/kvcache-ai/AgentENV/commit/1efdad102ce187f8e2e0389d08fb7f336f15c3c3) test(ublk): drop unused alloc import after alloc_zeroed switch - [`3ddf935`](https://github.com/kvcache-ai/AgentENV/commit/3ddf935469bf4192a74e70fc35688dbae5c7477e) test(ublk): address second OpenCodeReview round on stale-page reproducer - [`724d9a1`](https://github.com/kvcache-ai/AgentENV/commit/724d9a17cb08fc6b383d025c165b28e2c501fac3) test(ublk): keep cleanup workdir alive until detached cleanup finishes ### 📊 Changes **1 file changed** (+525 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `storage/ublk-daemon/tests/pool_page_cache_handover.rs` (+525 -0) </details> ### 📄 Description ## What Adds an `#[ignore]`d integration test `storage/ublk-daemon/tests/pool_page_cache_handover.rs` that deterministically reproduces the stale-placeholder-page defect from #302: a pooled ublk device rebound from the all-zero idle placeholder to a nonzero business image keeps serving zero pages through the buffered path, while `O_DIRECT` reads of the same offset return the correct content. Test-only change; no production code is modified. ## Why Issue #302 documents the mechanism with a manual daemon-level experiment. Encoding it as an executable test gives a shared, deterministic reference for the fix discussion, and becomes permanent regression protection once the fix lands: the key assertion (post-switch buffered read must return the business image) is written against the correct behavior and currently fails, so the test is ignored until it is un-ignored together with the fix. ## Related issue Refs #302 — this PR adds the reproducer only; it does not fix the defect. ## Scope and non-goals - Included: the integration test, using only public crate APIs (`UblkDaemonServer` in-process, daemon RPC protocol, overlaybd LSMT helpers). - Excluded: the page-cache invalidation fix itself (direction under discussion on the issue), and the `EnvdInstance::wait_for_ready` diagnostics improvement mentioned in the issue, which would be a separate PR. ## Design and behavior changes No production behavior changes. The test: 1. Builds a 2 GiB read-only file-lower OverlayBD image (empty `repoBlobUrl`, sparse on disk) with one known nonzero page at `0x7ffce000`, using the crate's own `create_file_rw`/`close_seal` — no external overlaybd tools. 2. Runs an in-process `UblkDaemonServer` with the issue's pool shape (`low=0, high=1, prewarm=false`) and drives it through the public length-prefixed JSON RPC protocol. 3. Follows the issue's 8-step procedure: baseline `O_DIRECT` read, release to the placeholder, retained buffered FD + deliberate placeholder cache fill, reacquire (asserts the same `dev_id` is reused), buffered vs `O_DIRECT` comparison, and a `BLKFLSBUF` recovery control. 4. Skips cleanly on hosts without ublk; runs under the existing capability runner used by `make test-ublk`. ## Compatibility and operations - Public API or generated protocol: N/A — test-only. - Configuration or defaults: N/A. - Snapshot manifest, artifact layout, or storage format: N/A. - Upgrade and rollback: N/A. - Host requirements, permissions, ports, or dependencies: the test requires Linux with `ublk_drv` and `CAP_SYS_ADMIN` (supplied by `scripts/run-with-capabilities.sh` under `make test-ublk`); it is `#[ignore]`d by default and self-skips when ublk is unavailable, so default and CI suites are unaffected. ## Validation - [x] `make fmt` - [x] `make clippy` - [ ] `make test-unit` - [x] Relevant Rust integration tests - [ ] `make -C services test` (required when `services/` changes) - [ ] Generated clients/server regenerated with the documented `make` target - [ ] Documentation updated - [ ] Benchmarks or performance comparison completed Commands and results: ```text # Linux x86_64 VM, kernel 7.0.11, ublk_drv loaded cargo fmt --all -- --check # clean cargo clippy --workspace --all-targets --all-features -- -D warnings # clean cargo test -p uvm-ublk-daemon # all binaries ok; new test: 1 ignored sudo -E cargo test -p uvm-ublk-daemon --test pool_page_cache_handover -- --ignored # FAILS as designed at the regression # assertion: left=0 vs right=0x7ffce000 # (stale placeholder page served) ``` Skipped checks and reasons: - `make test-unit`: no production code changed; the affected crate's full suite (`cargo test -p uvm-ublk-daemon`) was run instead and is green. - `make -C services test`: no `services/` changes. - Generated code, docs, benchmarks: not touched by this change. ## Risks and reviewer notes - Zero risk to default suites: the test is ignored and self-skips without ublk. Running it with `--ignored` on a ublk-capable host fails by design until #302 is fixed; that failure is the reproduction. - The interesting part to review is the test's fidelity to the issue: see the doc comment at the top of `storage/ublk-daemon/tests/pool_page_cache_handover.rs` and the final buffered assertion. ## 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 added the

pull-request

label

2026-10-29 14:22:39 +00:00