Skip to content

fix: command exec inside minvmd - #1176

Merged
twitchyliquid64 merged 1 commit into
mainfrom
tom/sftp
Aug 5, 2026
Merged

fix: command exec inside minvmd#1176
twitchyliquid64 merged 1 commit into
mainfrom
tom/sftp

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Aug 5, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Fixes #1175

Summary by CodeRabbit

  • Bug Fixes

    • Improved session process startup errors with clearer diagnostics.
    • Ensured session re-execution continues to work after switching to the guest root filesystem.
    • Improved namespace shim selection for session operations.

  • Tests

    • Added coverage for shim resolution behavior.
    • Added end-to-end validation for non-interactive session commands, including output, working directory, exit codes, and failure diagnostics.

Note

Fix command exec inside minvmd by staging a runnable daemon binary for nsenter re-execs

  • After entering the upstream rootfs, guest.rs copies /proc/self/exe to a tmpfs path (/run/minimald) and registers it as the nsenter shim so subsequent re-execs resolve correctly.
  • nsenter.rs adds a process-wide OnceLock shim registration (set_shim_exe/shim_exe), with Injection::command now preferring an explicit per-injection shim, then the registered shim, then current_exe().
  • exec.rs wraps spawn failures with a contextual error message including the resolved shim path.
  • An e2e test in session-e2e.sh validates non-interactive exec behavior and exit-code propagation.
  • Behavioral Change: daemons running inside a rootfs where their own path is unrunnable now automatically stage and use a copy from tmpfs; staging failures are logged but do not abort boot.

Macroscope summarized eabd2b1.

@twitchyliquid64
twitchyliquid64 requested a review from a team as a code owner August 5, 2026 06:00
@twitchyliquid64
twitchyliquid64 enabled auto-merge (rebase) August 5, 2026 06:00
@coderabbitai

coderabbitai Bot commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change stages the guest executable at /run/minimald, registers it as the namespace shim, improves spawn error context, and adds non-interactive session execution coverage.

Changes

Guest shim execution

Layer / File(s) Summary
Shim registration and resolution
crates/minimald/src/nsenter.rs		 set_shim_exe and shim_exe provide first-registration-wins process-wide storage. Injection::command selects an explicit path, the registered path, or current_exe(). Tests cover this order.
Guest executable staging
crates/minimald/src/guest.rs		 enter_rootfs stages /proc/self/exe at /run/minimald, sets executable permissions, registers the path, and logs staging failures without aborting boot.
Session execution diagnostics and validation
crates/minimald/src/exec.rs, scripts/session-e2e.sh		 Spawn failures identify the shim path. The E2E script verifies /workbench, output capture, exit code 7, and failure diagnostics.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant enter_rootfs
  participant stage_self_exe
  participant nsenter
  participant SessionExec
  enter_rootfs->>stage_self_exe: Stage executable at /run/minimald
  stage_self_exe->>nsenter: Register /run/minimald
  SessionExec->>nsenter: Resolve namespace shim
  nsenter-->>SessionExec: Return registered shim path
  SessionExec->>SessionExec: Spawn session command and report errors
Loading

Possibly related PRs

Suggested reviewers: norrietaylor

Poem

A rabbit staged the shim with care,
At /run/minimald, ready there.
Namespaces now know where to hop,
Session commands start and stop.
Exit code seven lands with a thump.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the fix and references issue #1175, but it omits the required Testing and Checklist sections and provides no test evidence. Add the required Summary, Testing, and Checklist sections, and include commands or output for the e2e and relevant Rust tests.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #1175 by staging the shim, updating shim resolution, improving spawn errors, and adding in-guest execution coverage.
Out of Scope Changes check ✅ Passed All changed files support the linked issue by implementing the shim fix, diagnostics, or its end-to-end test.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title clearly summarizes the fix for command execution inside minvmd and follows the repository's Conventional Commit format.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tom/sftp

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@twitchyliquid64
twitchyliquid64 merged commit 4882ef3 into main Aug 5, 2026
30 checks passed
@twitchyliquid64
twitchyliquid64 deleted the tom/sftp branch August 5, 2026 16:49
@coderabbitai coderabbitai Bot mentioned this pull request Aug 7, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evanspearman evanspearman evanspearman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

diag: min session attach <name> -c '<cmd>' fails because the in‑guest minimald (running as PID 1, exec'd as th

2 participants

@twitchyliquid64 @evanspearman