fix: make OpenCore NBD injection lifecycle safe by czmDeRepository · Pull Request #31 · cocoonstack/cocoon-macos

czmDeRepository · 2026-08-23T14:59:23Z

Summary

This change makes the OpenCore qcow2 injection flow safe under concurrent VM creation, request cancellation, and stale host NBD state.

The issue is not specific to a particular machine. It can affect any Linux host running concurrent cocoon-macos VM creation through qemu-nbd.

Background

Before starting a macOS VM, cocoon-macos creates an OpenCore qcow2 overlay and temporarily exposes it as a Linux block device:

OpenCore.qcow2
  -> qemu-nbd
  -> /dev/nbdN
  -> wait for EFI partition
  -> mount
  -> patch config.plist and SMBIOS
  -> unmount
  -> disconnect NBD

Each VM requires a unique SMBIOS identity, so this operation is part of the normal VM creation path.

Root Cause

1. `qemu-nbd` was started in foreground mode

The previous command did not use --fork:

qemu-nbd --connect=/dev/nbdN OpenCore.qcow2

After establishing the mapping, qemu-nbd remained in the foreground. The caller could remain blocked and never reach the mount, patch, unmount, or disconnect steps.

When the request later timed out or was cancelled, cleanup could be skipped or interrupted, leaving processes, NBD mappings, and incomplete VM directories behind.

2. NBD selection had a TOCTOU race

The old flow checked whether an NBD device was free and connected it in separate operations:

check /dev/nbdN
  -> device appears free
  -> connect qemu-nbd

Two concurrent VM creation requests could observe the same device as free before either request completed the connection.

Because /dev/nbdN is a host-global resource, this allowed multiple requests to compete for the same block device.

3. Cleanup reused a cancelled request context

Creation and cleanup shared the same request context.

If the create request timed out, cleanup commands such as unmount and disconnect inherited an already-cancelled context and could exit immediately without releasing resources.

This is equivalent to trying to roll back a transaction using an already-expired request context.

4. A wedged NBD device could appear free

A previous failed operation could leave a command such as:

partprobe /dev/nbd0

blocked in Linux D state while waiting for an uninterruptible kernel I/O operation.

In this condition, /sys/block/nbd0/pid may already be absent even though a userspace process still references /dev/nbd0.

Checking only the sysfs PID could therefore incorrectly classify the device as free. Reusing that device could block the next VM creation flow.

5. Repeated `partprobe` calls amplified the problem

The old partition wait loop repeatedly executed partprobe.

When the NBD device was already unhealthy, this could create more blocked partprobe processes instead of recovering the device.

Solution

Run `qemu-nbd` as a tracked background process

The connection now uses:

qemu-nbd \
  --fork \
  --pid-file=<OpenCore.qcow2.nbd.pid> \
  --connect=/dev/nbdN \
  -f qcow2 \
  <OpenCore.qcow2>

--fork allows the VM creation flow to continue after the mapping is established.

--pid-file records the exact daemon PID so cleanup can target the correct process without relying only on broad process matching.

Serialize the complete NBD transaction

A host-wide flock protects the complete OpenCore injection transaction:

acquire host-wide NBD lock
  -> select a healthy NBD device
  -> connect qcow2
  -> wait for the partition
  -> mount
  -> patch config.plist
  -> unmount
  -> disconnect NBD
release host-wide NBD lock

This removes the check-to-use race and prevents concurrent requests from observing or modifying an intermediate NBD state.

The lock is host-wide because all /dev/nbdN devices are shared host resources.

Only the short OpenCore injection transaction is serialized. VM boot and normal VM execution remain concurrent.

Use an independent cleanup context

Unmount, disconnect, process termination, and cleanup polling now use a fresh background context with explicit timeouts.

Cleanup can therefore continue even when the original create request has timed out or been cancelled.

Verify kernel and userspace state

Before selecting /dev/nbdN, the implementation now checks:

Whether the device exists.
Whether /sys/block/nbdN/pid indicates an active mapping.
Whether any process in /proc/*/cmdline still references:
- /dev/nbdN
- /dev/nbdNp1 or another partition
- --connect=/dev/nbdN

If a process still references the device, the device is treated as busy and skipped.

This protects new VM creation requests from stale partprobe, mount, or qemu-nbd processes whose state is no longer fully represented by sysfs.

Remove repeated `partprobe` execution

The partition wait loop now waits for the kernel-created partition node instead of repeatedly starting partprobe.

This avoids creating additional uninterruptible processes when a device is unhealthy.

Perform precise cleanup

Cleanup now:

disconnects the selected NBD device;
waits for the qcow2 file to no longer be held;
verifies PID identity before terminating the recorded process;
only searches for qemu-nbd processes referencing the exact qcow2 path;
removes the PID file after cleanup;
uses independent timeouts even when the create request was cancelled.

This avoids terminating unrelated processes if a PID has been reused.

Why This Is a General Fix

This change does not contain any host name, node IP, machine identifier, or special handling for /dev/nbd0.

It applies to every configured /dev/nbdN device and to any Linux host using the cocoon-macos OpenCore injection flow.

A host containing historical blocked processes helped reproduce the issue, but the underlying defects were generic:

incorrect external process lifecycle management;
host-global resource contention;
a TOCTOU race during device selection;
cancellation interrupting rollback;
incomplete block-device health detection;
unsafe reuse of stale devices.

The same design principles can be applied to other qemu-nbd users, although the implementation in this MR is scoped to the cocoon-macos OpenCore injection path.

Compatibility

No public API changes.
No CLI parameter changes.
No VM record format changes.
No image format changes.
Healthy NBD devices continue to work as before.
Devices referenced by stale userspace processes are skipped.
If all NBD devices are unavailable, creation fails explicitly instead of blocking indefinitely.
VM boot and execution concurrency are not reduced.

Validation

The following checks passed:

go test -race ./...
make fmt-check vet lint

The tests cover:

use of qemu-nbd --fork;
creation and use of the PID file;
50 concurrent callers competing for the host-wide NBD lease;
cancellation while waiting for the lease;
detection of /dev/nbdN references;
detection of partition references such as /dev/nbdNp1;
detection of --connect=/dev/nbdN;
avoidance of false matches such as /dev/nbd01.

The fix was also validated in an integration cluster:

a single-instance batch completed successfully;
a 10-instance batch completed successfully;
50- and 100-instance batches completed the infrastructure-level VM creation path;
no new blocked partprobe process was created;
no qemu-nbd process remained after instance release;
no failed VM directory remained after cleanup;
all created instances were released successfully.

Some large-batch application-level readiness samples were delayed by guest control-server or guest IP readiness. These delays occurred after the NBD injection and QEMU launch stages and are independent of the NBD lifecycle issue fixed by this MR.

Operational Note

Processes already blocked in Linux D state generally cannot be removed with SIGKILL.

This fix prevents new requests from reusing affected devices and prevents new leaks. Existing historical D-state processes may still require a controlled host reboot.

After controlled rolling reboots, the integration hosts were verified with:

qemu-nbd processes: 0
partprobe processes: 0
failed VM directories: 0

CMGS

the substance is good — host-wide flock over the whole connect/mount/patch/unmount/disconnect transaction closes the toctou, --fork + --pid-file fixes the foreground-hang, fresh cleanup contexts survive create cancellation, and the /proc scan catches wedged devices sysfs no longer reflects. reuse is right (cocoon's lock/flock; cocoon has no nbd handling to borrow). i traced the concurrency + cleanup and didn't find a hard defect — notably disconnect() returns nil when the device released even if the --disconnect command was noisy, so a successful inject won't be flipped to failure.

blocker is the structural gate: asl ./... fails on both GOOS (3 findings), and one exported symbol shouldn't be exported. see inline. everything else is green: lint (linux+darwin) 0 issues, go test -race ./qemu, gofmt, vet.

one behavioral note, not blocking: dropping the partprobe retry in waitForPart means creation now hard-depends on the kernel auto-creating nbdXp1 within 5s of connect. that's the right call for the D-state pileup, and your 50/100-batch runs exercise it — just calling it out as the one healthy-host regression surface to keep an eye on.

CMGS · 2026-08-23T16:32:35Z

+	nbdCommandTimeout = 5 * time.Second
+)
+
+type nbdConnection struct {


asl (both GOOS): type block must be atomic — nbdConnection is split from its only method disconnect() (line 113) by InjectConfig/withNBDLease/waitForPart/cleanupNBDMount. move disconnect() up to sit immediately below the struct, or move the struct down next to it.

CMGS · 2026-08-23T16:32:35Z

+// CleanupNBDForPath terminates qemu-nbd processes whose command line references
+// path. PID identity is verified before signaling, so an unrelated process is
+// never killed even if a PID was reused.
+func CleanupNBDForPath(ctx context.Context, path string) error {


CleanupNBDForPath is exported but has no caller outside this file (only disconnect() and connectFreeNBD use it). unexport it → cleanupNBDForPath. that also clears the asl 'unexported isFileHeld above exported CleanupNBDForPath' finding at line 154 (both become unexported, order stops mattering).

CMGS · 2026-08-23T16:32:35Z

+}
+
 // sampleConfig mirrors OSX-KVM's config.plist so patchPlist round-trips a realistic input.
 const sampleConfig = `<?xml version="1.0" encoding="UTF-8"?>


asl (both GOOS): const sampleConfig sits below the first test func. in _test.go every Test* comes first, then helpers/fixtures — move this const up into a top-of-file block above TestQemuNBDConnectArgsForkAndTrackServer.

CMGS

follow-up on comment budget (house rule: unexported funcs get at most a one-line WHY, and only when the name can't carry it; exported godoc is one line). the fix is genuinely well-explained, but five of the new comments are 2-3 line narratives where one WHY line does the job — first sentence usually restates what the code already says, keep only the non-obvious half. suggested trims inline. the single-line ones (InjectConfig, waitForPart, isFileHeld) are fine. no other simplify/reuse findings beyond the structural ones already noted.

CMGS · 2026-08-23T16:34:27Z

+	})
+}
+
+// withNBDLease serializes the complete connect/mount/patch/unmount/disconnect


3 lines → 1; line 1 restates the func name. keep the WHY only:
// serialize the whole transaction: a free-device check then qemu-nbd --connect is not atomic across processes.

CMGS · 2026-08-23T16:34:27Z


-// disconnectNBD waits out qemu-nbd's asynchronous release, or the qemu launch races in and fails with "Failed to get shared write lock".
-func disconnectNBD(ctx context.Context, nbd, ocPath string) {
+// disconnect waits out qemu-nbd's asynchronous release. Cleanup deliberately


the 'waits out async release' half is what WaitFor already shows; keep the fresh-context WHY:
// fresh context, not the caller's: disconnect must still run when the caller is unwinding after a timeout.

CMGS · 2026-08-23T16:34:28Z

+	return len(pids) > 0, err
+}
+
+// CleanupNBDForPath terminates qemu-nbd processes whose command line references


godoc is one line (and this should be unexported per the other review):
// cleanupNBDForPath terminates qemu-nbd holders of path; PID identity is verified so a reused PID is never hit.

CMGS · 2026-08-23T16:34:28Z


-// connectFreeNBD claims a device by connecting: the connect itself is the exclusive operation, so a race with another VM create just advances to the next candidate.
-func connectFreeNBD(ctx context.Context, ocPath string) (string, error) {
+// connectFreeNBD is called while holding the host-wide NBD lease. --fork is


3 lines → 1, keep the --fork point:
// holds the nbd lease; --fork or qemu-nbd stays foreground and the caller never reaches mount/patch.

CMGS · 2026-08-23T16:34:28Z

+	return nil, errors.New("no free /dev/nbd device (is the nbd module loaded)")
+}
+
+// processReferencesBlockDevice catches userspace operations that are still


3 lines → 1:
// a device can stay held by a blocked userspace op after the kernel drops its sysfs pid; reusing it wedges the next attach.

czmDeRepository added 2 commits August 23, 2026 22:00

fix: serialize and clean up OpenCore NBD injection

74e1df3

fix: avoid reusing wedged NBD devices

711dea8

CMGS requested changes Aug 23, 2026

View reviewed changes

CMGS reviewed Aug 23, 2026

View reviewed changes

czmDeRepository mentioned this pull request Aug 24, 2026

[codex] fix: make macOS VM creation transactional #34

Draft

Navigation Menu

Uh oh!

fix: make OpenCore NBD injection lifecycle safe - #31

fix: make OpenCore NBD injection lifecycle safe#31
czmDeRepository wants to merge 2 commits into
cocoonstack:masterfrom
czmDeRepository:codex/fix-nbd-lifecycle

czmDeRepository commented Aug 23, 2026

Uh oh!

CMGS left a comment

Uh oh!

CMGS Aug 23, 2026

Uh oh!

CMGS Aug 23, 2026

Uh oh!

CMGS Aug 23, 2026

Uh oh!

CMGS left a comment

Uh oh!

CMGS Aug 23, 2026

Uh oh!

CMGS Aug 23, 2026

Uh oh!

CMGS Aug 23, 2026

Uh oh!

CMGS Aug 23, 2026

Uh oh!

CMGS Aug 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

czmDeRepository commented Aug 23, 2026

Summary

Background

Root Cause

1. qemu-nbd was started in foreground mode

2. NBD selection had a TOCTOU race

3. Cleanup reused a cancelled request context

4. A wedged NBD device could appear free

5. Repeated partprobe calls amplified the problem

Solution

Run qemu-nbd as a tracked background process

Serialize the complete NBD transaction

Use an independent cleanup context

Verify kernel and userspace state

Remove repeated partprobe execution

Perform precise cleanup

Why This Is a General Fix

Compatibility

Validation

Operational Note

Uh oh!

CMGS left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CMGS left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1. `qemu-nbd` was started in foreground mode

5. Repeated `partprobe` calls amplified the problem

Run `qemu-nbd` as a tracked background process

Remove repeated `partprobe` execution