e2b-dev / e2b-dev/runtime

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

Open Beginner friendly
#3,273 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Go
Stars
1.6k
Forks
438
PR merge metrics
No merged PRs in 30d

Description

Describe the bug

When a Firecracker VM exits abnormally (OOM-kill, crash, etc.), NBDProvider.Close() in packages/orchestrator/pkg/sandbox/rootfs/nbd.go calls sync() before mnt.Close(). Because the NBD device is already in an error state after the VM dies, the ioctl(BLKFLSBUF) + fsync inside sync() returns EIO. This causes every sandbox cleanup after an abnormal VM exit to log:

failed to cleanup sandbox: error flushing cow device: failed to fsync path: input/output error

even though no data loss has occurred — the error is an expected consequence of the VM being dead, not a real storage failure.

Root cause

NBDProvider.Close() (line 130 of rootfs/nbd.go):

func (o *NBDProvider) Close(ctx context.Context) error {
    var errs []error

    err := o.sync(ctx)         // ← calls BLKFLSBUF ioctl + fsync on /dev/nbdX
    if err != nil {
        errs = append(errs, fmt.Errorf("error flushing cow device: %w", err))
    }

    err = o.mnt.Close(ctx)     // ← disconnects the NBD device (too late)
    ...
}

sync() (line 162) opens /dev/nbdX, issues BLKFLSBUF, then calls flush() which does syscall.Fsync. When the VM has already crashed, the NBD device is in error state and Fsync returns syscall.EIO. The error propagates up through Sandbox.Close()setupSandboxLifecycle() which logs it at ERROR level.

Expected behavior

EIO from sync() during sandbox cleanup should be treated as an expected condition (VM died, no data to flush) and logged at WARN level rather than propagating as an error that triggers the "failed to cleanup sandbox" ERROR log.

Impact

  • Spurious "failed to cleanup sandbox" ERROR log on every abnormal VM exit
  • Noise in production alerting / on-call signals

Suggested fix

In NBDProvider.Close(), detect syscall.EIO from sync() and downgrade to a warning:

err := o.sync(ctx)
if err != nil {
    if errors.Is(err, syscall.EIO) {
        logger.L().Warn(ctx, "error flushing cow device (VM likely crashed, ignoring)", zap.Error(err))
    } else {
        errs = append(errs, fmt.Errorf("error flushing cow device: %w", err))
    }
}

Environment

  • Bare-metal deployment with Nomad
  • Observed under high sandbox creation load (thundering herd from client retries)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/orchestrator/pkg/sandbox/rootfs/nbd.go, focusing on NBDProvider.Close() and sync(). Verify cleanup behavior after a VM crash: EIO from sync() should be logged at WARN without propagating as a cleanup error, while other sync errors should still propagate.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
infrastructure
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
75/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.