Skip to content

fix(orchestrator): suppress EIO/ENXIO on sandbox cleanup after VM crash#3276

Open
AdaAibaby wants to merge 3 commits into
e2b-dev:mainfrom
AdaAibaby:fix/nbd-close-sync-eio
Open

fix(orchestrator): suppress EIO/ENXIO on sandbox cleanup after VM crash#3276
AdaAibaby wants to merge 3 commits into
e2b-dev:mainfrom
AdaAibaby:fix/nbd-close-sync-eio

Conversation

@AdaAibaby

@AdaAibaby AdaAibaby commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #3273

When a VM crashes or is OOM-killed, NBDProvider.Close() calls sync() which issues BLKFLSBUF ioctl + fsync on /dev/nbdX. Because the NBD device is already in error state after the VM dies, the kernel returns:

  • EIO — device is in error state (broken NBD connection)
  • ENXIO — device is already disconnected/cleared

Both are expected — there is nothing to flush because the VM is gone. This caused every abnormal VM exit to emit a spurious ERROR:

level=error msg="failed to cleanup sandbox, will remove from cache"
  error="failed to cleanup sandbox: error flushing cow device: failed to fsync path: input/output error"

Why not suppress in NBDProvider.Close() directly?

The initial approach (commit 82a2ec2) caught EIO in NBDProvider.Close() itself. This was flagged by code review (Codex) as too broad: Sandbox.Shutdown() also calls s.Close() with the explicit comment "This should properly flush rootfs to the underlying device" — a genuine EIO there (e.g. host disk full) must still propagate as an error or a corrupt template could be silently published.

Actual fix

Handle EIO/ENXIO in setupSandboxLifecycle() in sandboxes.go — the only call site that is guaranteed to run after VM exit. The Sandbox.Shutdown() path is unaffected and still treats EIO as a hard error.

packages/orchestrator/pkg/server/sandboxes.go:

// Before
cleanupErr := sbx.Close(ctx)
if cleanupErr != nil {
    sbxlogger.I(sbx).Error(ctx, "failed to cleanup sandbox, will remove from cache", zap.Error(cleanupErr))
}

// After
cleanupErr := sbx.Close(ctx)
if cleanupErr != nil {
    if errors.Is(cleanupErr, syscall.EIO) || errors.Is(cleanupErr, syscall.ENXIO) {
        // After a VM crash the NBD device is in error state. sync() on
        // /dev/nbdX returns EIO (device error) or ENXIO (device already
        // disconnected). Both are expected — no data is lost because
        // the VM already exited.
        sbxlogger.I(sbx).Warn(ctx, "failed to flush sandbox device after VM crash (ignoring)", zap.Error(cleanupErr))
    } else {
        sbxlogger.I(sbx).Error(ctx, "failed to cleanup sandbox, will remove from cache", zap.Error(cleanupErr))
    }
}

Before / After

Before (every abnormal VM exit):

level=error msg="failed to cleanup sandbox, will remove from cache"
  error="failed to cleanup sandbox: error flushing cow device: failed to fsync path: input/output error"

After:

level=warn msg="failed to flush sandbox device after VM crash (ignoring)"
  error="failed to cleanup sandbox: error flushing cow device: failed to fsync path: input/output error"

Real non-EIO/ENXIO failures still surface at ERROR.

Test plan

  • Kill a Firecracker VM with kill -9; confirm no ERROR log for "failed to cleanup sandbox" and a WARN appears instead
  • Normal Sandbox.Shutdown() (pause/snapshot): confirm a genuine disk error still propagates as ERROR

When a VM crashes or is OOM-killed, NBDProvider.Close() calls sync()
before disconnecting the NBD device. If the device is already in an
error state the BLKFLSBUF ioctl or fsync returns EIO, causing every
abnormal VM exit to log "failed to cleanup sandbox: error flushing cow
device" at ERROR level even though no data loss has occurred.

Detect syscall.EIO from sync() and log at Warn instead of propagating
the error, so the cleanup path completes cleanly after a VM crash.

Fixes: e2b-dev#3273

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Close method in nbd.go to handle syscall.EIO errors when flushing the copy-on-write (COW) device, treating them as warnings since they likely indicate a VM crash. The reviewer suggested extending this logic to also ignore syscall.ENXIO errors, which can occur if the NBD device is already disconnected or cleared during a crash.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread packages/orchestrator/pkg/sandbox/rootfs/nbd.go Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82a2ec22e1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread packages/orchestrator/pkg/sandbox/rootfs/nbd.go Outdated
After a VM crashes, NBDProvider.Close() calls sync() which issues
BLKFLSBUF ioctl + fsync on /dev/nbdX. Because the NBD device is in
error state the kernel returns EIO (or ENXIO if the device is already
disconnected). Both errors are expected: the VM is gone and there is
nothing to flush.

Instead of suppressing the error in NBDProvider.Close() (which would
also hide genuine disk-error EIO during Sandbox.Shutdown's flush-before-
snapshot path), handle it in setupSandboxLifecycle — the one call site
that is guaranteed to run only after VM exit. Non-EIO/ENXIO errors still
propagate as ERROR, preserving alerts for real failures.

Fixes: e2b-dev#3273
@AdaAibaby
AdaAibaby force-pushed the fix/nbd-close-sync-eio branch from 2cbcbcd to 90a738c Compare July 11, 2026 12:47
@AdaAibaby AdaAibaby changed the title fix(orchestrator): treat EIO from NBD sync as warning on sandbox cleanup after VM crash fix(orchestrator): suppress EIO/ENXIO on sandbox cleanup after VM crash Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orchestrator: NBDProvider.Close() syncs device before disconnecting, causing spurious EIO on sandbox cleanup after VM crash

1 participant