fix(qemu): add overlay support to Windows and Linux containers#1255
fix(qemu): add overlay support to Windows and Linux containers#1255NikkeTryHard wants to merge 2 commits intotrycua:mainfrom
Conversation
|
@NikkeTryHard is attempting to deploy a commit to the Cua Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughContainer entrypoints now detect overlay mode—when Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
libs/qemu-docker/linux/src/entry.sh (1)
15-25: Consider extracting shared overlay logic to reduce duplication.The overlay setup block is identical in both Windows and Linux entrypoints. If the logic evolves (e.g., adding logging, metrics, or additional validation), both files must be updated in sync.
Consider extracting this into a shared script (e.g.,
/run/overlay-setup.sh) that both entrypoints source or invoke.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/qemu-docker/linux/src/entry.sh` around lines 15 - 25, Extract the duplicated overlay setup shell block into a reusable script (e.g., create an executable overlay-setup.sh that encapsulates the if [ -d "/golden" ] && [ -z "$(ls -A /storage 2>/dev/null)" ] check, the cp -al fallback to cp -a, and the echo messages), then replace the duplicated code in libs/qemu-docker/linux/src/entry.sh with a single invocation or source of overlay-setup.sh; ensure the new script returns non-zero on fatal errors and preserves existing behavior and messages so both Linux and the Windows entrypoint can call the same script.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/qemu-docker/linux/src/entry.sh`:
- Around line 15-25: The fallback cp -a in entry.sh has no error handling, so if
it fails the script continues with incomplete /storage; update the overlay setup
block (the cp -al fallback path) to check the exit status of the cp -a command
and on failure emit a clear error message (echo) and exit with non-zero status;
ensure the messages mirror the existing "Hard links not supported, falling back
to full copy..." and "Overlay setup complete (full copy)." flow but add a
failure branch that logs the cp error and calls exit 1 so the container/VM fails
fast instead of booting with bad state.
In `@libs/qemu-docker/windows/src/entry.sh`:
- Around line 15-25: The fallback copy in entry.sh silently continues if cp -a
/golden/. /storage/ fails and the initial empty-check using ls -A masks the case
where /storage does not exist; update the overlay setup to first validate that
/storage exists and is a writable directory (replace the ls -A test with an
explicit existence/permission check), then run the cp -a fallback and
immediately test its exit status—on failure log a descriptive error including
the cp error, remove any partially copied content or restore /storage to a safe
state, and exit non‑zero to prevent booting with an incomplete overlay; ensure
these checks are applied around the existing if-block that performs cp -al and
cp -a so both hard-link and fallback paths are covered.
---
Nitpick comments:
In `@libs/qemu-docker/linux/src/entry.sh`:
- Around line 15-25: Extract the duplicated overlay setup shell block into a
reusable script (e.g., create an executable overlay-setup.sh that encapsulates
the if [ -d "/golden" ] && [ -z "$(ls -A /storage 2>/dev/null)" ] check, the cp
-al fallback to cp -a, and the echo messages), then replace the duplicated code
in libs/qemu-docker/linux/src/entry.sh with a single invocation or source of
overlay-setup.sh; ensure the new script returns non-zero on fatal errors and
preserves existing behavior and messages so both Linux and the Windows
entrypoint can call the same script.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 39868817-57f4-4c96-8c2f-dbb60689eaa8
📒 Files selected for processing (2)
libs/qemu-docker/linux/src/entry.shlibs/qemu-docker/windows/src/entry.sh
1 participant
Summary
/goldenis mounted read-only and/storageis empty, populate storage via hard links (cp -al) for copy-on-write semanticscp -awhen hard links are not supported (cross-filesystem mounts)Test plan
/golden:ro+ empty/storage-- logs show "Overlay mode detected", VM boots from linked fileswindows.bootnot re-created (already present from golden)/goldenmounted -- no overlay message, normal boot path unchanged/goldenand/storageon different filesystems -- falls back to full copyCloses #699
Supersedes #730
Summary by CodeRabbit