diff --git a/packages/orchestrator/pkg/sandbox/nbd/pool.go b/packages/orchestrator/pkg/sandbox/nbd/pool.go index 34548bbc5d..14aa25b806 100644 --- a/packages/orchestrator/pkg/sandbox/nbd/pool.go +++ b/packages/orchestrator/pkg/sandbox/nbd/pool.go @@ -243,7 +243,62 @@ func (d *DevicePool) isDeviceFree(slot DeviceSlot) (bool, error) { return false, fmt.Errorf("failed to parse size: %w", err) } - return size == 0, nil + if size != 0 { + return false, nil + } + + // size==0 and no pid only prove the synchronous half of NBD disconnect + // finished (capacity reset, connection torn down). The kernel still drains + // in-flight blk_mq requests and tears down the page cache on a workqueue + // afterwards; a slot handed out in that window races the tail of the + // previous device's teardown, which surfaces as the sector-0/partition-scan + // EIO seen in dmesg. Require the device to be quiescent -- no in-flight + // requests and no holders -- before calling it free. + return d.isDeviceQuiescent(slot) +} + +// isDeviceQuiescent reports whether the kernel has finished the asynchronous +// teardown for a disconnected device: no in-flight blk_mq requests and no +// holders (udev/partition-probe/mount references). Both signals are best-effort +// -- if the sysfs files are absent (older kernels, or the device node not yet +// materialized) the device is treated as quiescent so the pool never wedges on +// a signal that will never appear. +func (d *DevicePool) isDeviceQuiescent(slot DeviceSlot) (bool, error) { + inflightFile := fmt.Sprintf("%s/nbd%d/inflight", d.sysBlockDir, slot) + + data, err := os.ReadFile(inflightFile) + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + return false, fmt.Errorf("failed to read inflight file: %w", err) + } + // No inflight signal available: fall through to the holders check. + } else { + // The file is two whitespace-separated counters: reads and writes + // currently in flight. Any non-zero field means the device is not yet + // idle. + for _, field := range strings.Fields(string(data)) { + n, parseErr := strconv.ParseUint(field, 10, 64) + if parseErr != nil { + return false, fmt.Errorf("failed to parse inflight file: %w", parseErr) + } + if n != 0 { + return false, nil + } + } + } + + holdersDir := fmt.Sprintf("%s/nbd%d/holders", d.sysBlockDir, slot) + + entries, err := os.ReadDir(holdersDir) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return true, nil + } + + return false, fmt.Errorf("failed to read holders dir: %w", err) + } + + return len(entries) == 0, nil } func (d *DevicePool) getMaybeEmptySlot(start DeviceSlot) (DeviceSlot, func(), bool) { diff --git a/packages/orchestrator/pkg/sandbox/nbd/pool_quiescent_test.go b/packages/orchestrator/pkg/sandbox/nbd/pool_quiescent_test.go new file mode 100644 index 0000000000..ba2035f96e --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/nbd/pool_quiescent_test.go @@ -0,0 +1,131 @@ +//go:build linux + +package nbd + +import ( + "os" + "path/filepath" + "testing" + + "github.com/bits-and-blooms/bitset" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// quiescentPool builds a pool whose device-state reads resolve against a +// temporary /sys/block stand-in, so isDeviceFree can be driven without the nbd +// module loaded. +func quiescentPool(t *testing.T) *DevicePool { + t.Helper() + + return &DevicePool{ + done: make(chan struct{}), + usedSlots: bitset.New(16), + slots: make(chan DeviceSlot, 1), + sysBlockDir: t.TempDir(), + } +} + +// writeDeviceState lays out /sys/block/nbd/{size,inflight} and the +// holders directory for a fake device. A negative holder count means the +// holders directory is absent entirely (older-kernel path). +func writeDeviceState(t *testing.T, dir string, slot DeviceSlot, size, inflight string, holders int) { + t.Helper() + + base := filepath.Join(dir, "nbd"+itoa(slot)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + if size != "" { + require.NoError(t, os.WriteFile(filepath.Join(base, "size"), []byte(size), 0o644)) + } + if inflight != "" { + require.NoError(t, os.WriteFile(filepath.Join(base, "inflight"), []byte(inflight), 0o644)) + } + if holders >= 0 { + holdersDir := filepath.Join(base, "holders") + require.NoError(t, os.MkdirAll(holdersDir, 0o755)) + for i := 0; i < holders; i++ { + require.NoError(t, os.MkdirAll(filepath.Join(holdersDir, "dm-"+itoa(DeviceSlot(i))), 0o755)) + } + } +} + +func itoa(v DeviceSlot) string { + if v == 0 { + return "0" + } + var buf [10]byte + i := len(buf) + for v > 0 { + i-- + buf[i] = byte('0' + v%10) + v /= 10 + } + + return string(buf[i:]) +} + +// A disconnected device that is fully quiescent -- size 0, no in-flight +// requests, no holders -- is free. +func TestIsDeviceFreeQuiescent(t *testing.T) { + t.Parallel() + + pool := quiescentPool(t) + writeDeviceState(t, pool.sysBlockDir, 0, "0\n", " 0 0\n", 0) + + free, err := pool.isDeviceFree(0) + require.NoError(t, err) + assert.True(t, free, "quiescent disconnected device should be free") +} + +// In-flight requests mean the kernel has not finished draining the previous +// connection, so the device is not free even with size 0. +func TestIsDeviceFreeInflightNotFree(t *testing.T) { + t.Parallel() + + pool := quiescentPool(t) + writeDeviceState(t, pool.sysBlockDir, 0, "0\n", " 0 3\n", 0) + + free, err := pool.isDeviceFree(0) + require.NoError(t, err) + assert.False(t, free, "device with in-flight requests must not be free") +} + +// A holder (partition probe, dm, mount) still referencing the device keeps it +// out of the free pool. +func TestIsDeviceFreeHeldNotFree(t *testing.T) { + t.Parallel() + + pool := quiescentPool(t) + writeDeviceState(t, pool.sysBlockDir, 0, "0\n", " 0 0\n", 1) + + free, err := pool.isDeviceFree(0) + require.NoError(t, err) + assert.False(t, free, "device with a holder must not be free") +} + +// A non-zero size means the device is still connected/backed, so it is not free +// regardless of the quiescence signals. +func TestIsDeviceFreeNonZeroSizeNotFree(t *testing.T) { + t.Parallel() + + pool := quiescentPool(t) + writeDeviceState(t, pool.sysBlockDir, 0, "2048\n", " 0 0\n", 0) + + free, err := pool.isDeviceFree(0) + require.NoError(t, err) + assert.False(t, free, "device with non-zero size must not be free") +} + +// When the kernel does not expose inflight/holders (older kernels), size 0 with +// no pid is sufficient: the quiescence check must not wedge on absent signals. +func TestIsDeviceFreeMissingSignalsFallsBack(t *testing.T) { + t.Parallel() + + pool := quiescentPool(t) + writeDeviceState(t, pool.sysBlockDir, 0, "0\n", "", -1) + + free, err := pool.isDeviceFree(0) + require.NoError(t, err) + assert.True(t, free, "absent inflight/holders signals should fall back to size-only free") +}