From 6e10767db74377628824e32cb683b463d5ab6d20 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 29 Oct 2026 18:21:17 +0800 Subject: [PATCH 1/2] engine: retry a volume mount while the device is not yet openable The guest publishes /sys/block and the /dev node before the kernel can open a hot-plugged disk, so mount occasionally failed with "not a valid block device" (3 of 620 mounts on cocoon-test2). Retry only that error on the existing poll interval within the setup timeout; any other mount failure still returns at once. --- sandboxd/engine/installca_test.go | 9 +++++++++ sandboxd/engine/volume.go | 23 ++++++++++++++++++++--- sandboxd/engine/volume_test.go | 17 +++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/sandboxd/engine/installca_test.go b/sandboxd/engine/installca_test.go index c71fabd8e..d810e7442 100644 --- a/sandboxd/engine/installca_test.go +++ b/sandboxd/engine/installca_test.go @@ -78,6 +78,7 @@ type fakeSilkd struct { execEnv map[string]string execCode int32 execFailAt int + notReady int execHook func([]string) realpath map[string]string noRealpath bool @@ -221,6 +222,11 @@ func (f *fakeSilkd) handleExec(conn net.Conn, argv []string, env map[string]stri if f.execFailAt > 0 && len(f.execCalls) != f.execFailAt { code, reply = 0, "" } + stderr := "" + if argv[0] == "mount" && f.notReady > 0 { + f.notReady-- + code, stderr = 32, "mount: /data: /dev/vdc is not a valid block device." + } if code == 0 && reply == "" && f.execHook != nil { f.execHook(argv) } @@ -234,6 +240,9 @@ func (f *fakeSilkd) handleExec(conn net.Conn, argv []string, env map[string]stri return } _, _ = io.WriteString(conn, `{"type":"started","pid":1}`+"\n") + if stderr != "" { + writeFakeSilkdResponse(conn, &wire.Stderr{Data: []byte(stderr)}) + } if argv[0] == "realpath" && code == 0 { writeFakeSilkdResponse(conn, &wire.Stdout{Data: []byte(cmp.Or(resolved, argv[len(argv)-1]) + "\n")}) } diff --git a/sandboxd/engine/volume.go b/sandboxd/engine/volume.go index 3032b2578..00dfa2bfb 100644 --- a/sandboxd/engine/volume.go +++ b/sandboxd/engine/volume.go @@ -144,10 +144,22 @@ func (e *Engine) mountDevice(ctx context.Context, vsockSocket, device, name, mou if err := e.silkdExec(ctx, vsockSocket, "mkdir", "-p", "--", mount); err != nil { return fmt.Errorf("create volume mount point %s: %w", mount, err) } - if err := e.silkdExec(ctx, vsockSocket, "mount", "-o", mountMode(rw), "--", device, mount); err != nil { - return fmt.Errorf("mount volume %s: %w", name, err) + ticker := time.NewTicker(volumePollInterval) + defer ticker.Stop() + for { + err := e.silkdExec(ctx, vsockSocket, "mount", "-o", mountMode(rw), "--", device, mount) + if err == nil { + return nil + } + if !deviceNotReady(err) { + return fmt.Errorf("mount volume %s: %w", name, err) + } + select { + case <-ctx.Done(): + return fmt.Errorf("mount volume %s: %w", name, err) + case <-ticker.C: + } } - return nil } func (e *Engine) diskAttachCopy(ctx context.Context, vmName, sourceFlag, source string) (string, error) { @@ -180,6 +192,11 @@ func (e *Engine) diskAttachArgs(vmName string, spec VolumeSpec) ([]string, error return append(args, "--directio", directIO), nil } +func deviceNotReady(err error) bool { + exit, ok := errors.AsType[*guestExitError](err) + return ok && strings.Contains(exit.output, "not a valid block device") +} + func isNotFound(err error) bool { respErr, ok := errors.AsType[*wire.ErrorResp](err) return ok && respErr.Kind == wire.KindNotFound diff --git a/sandboxd/engine/volume_test.go b/sandboxd/engine/volume_test.go index 17bde498c..8e26247b5 100644 --- a/sandboxd/engine/volume_test.go +++ b/sandboxd/engine/volume_test.go @@ -226,6 +226,23 @@ func TestMountVolumeWaitsForDelayedDevNode(t *testing.T) { } } +func TestMountVolumeRetriesUntilTheDeviceOpens(t *testing.T) { + path := sockPath(t) + fake := serveFakeSilkd(t, path) + configureVolumeDevices(fake) + fake.mu.Lock() + fake.notReady = 2 + fake.mu.Unlock() + if err := New("cocoon", nil, nil, false, false, "").MountVolume(t.Context(), path, "imagenet", "/datasets/training", false); err != nil { + t.Fatalf("MountVolume: %v", err) + } + fake.mu.Lock() + defer fake.mu.Unlock() + if mounts := len(fake.execCalls) - 1; mounts != 3 { + t.Fatalf("mount attempts = %d, want 3: %v", mounts, fake.execCalls) + } +} + func TestMountVolumeStopsAtFailedStage(t *testing.T) { for _, tt := range []struct { name string From 9571611556b236e11bdb92cd65b4ac01d11a3cb6 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 29 Oct 2026 20:43:23 +0800 Subject: [PATCH 2/2] engine: report the deadline when a volume mount retry times out A retry that ran into the setup deadline returned the last mount error, so the live volume API answered 500 instead of 504. Wrap ctx.Err() whenever the context has ended, both after a failed attempt and while waiting, keeping the last mount error in the message. --- sandboxd/engine/volume.go | 10 ++++++---- sandboxd/engine/volume_test.go | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/sandboxd/engine/volume.go b/sandboxd/engine/volume.go index 00dfa2bfb..e5631b551 100644 --- a/sandboxd/engine/volume.go +++ b/sandboxd/engine/volume.go @@ -148,15 +148,17 @@ func (e *Engine) mountDevice(ctx context.Context, vsockSocket, device, name, mou defer ticker.Stop() for { err := e.silkdExec(ctx, vsockSocket, "mount", "-o", mountMode(rw), "--", device, mount) - if err == nil { + switch { + case err == nil: return nil - } - if !deviceNotReady(err) { + case ctx.Err() != nil: + return fmt.Errorf("mount volume %s: %w: %w", name, ctx.Err(), err) + case !deviceNotReady(err): return fmt.Errorf("mount volume %s: %w", name, err) } select { case <-ctx.Done(): - return fmt.Errorf("mount volume %s: %w", name, err) + return fmt.Errorf("mount volume %s: %w: %w", name, ctx.Err(), err) case <-ticker.C: } } diff --git a/sandboxd/engine/volume_test.go b/sandboxd/engine/volume_test.go index 8e26247b5..c96a0d673 100644 --- a/sandboxd/engine/volume_test.go +++ b/sandboxd/engine/volume_test.go @@ -243,6 +243,21 @@ func TestMountVolumeRetriesUntilTheDeviceOpens(t *testing.T) { } } +func TestMountVolumeNotReadyUntilDeadlineReportsTheTimeout(t *testing.T) { + path := sockPath(t) + fake := serveFakeSilkd(t, path) + configureVolumeDevices(fake) + fake.mu.Lock() + fake.notReady = 1 << 20 + fake.mu.Unlock() + ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) + defer cancel() + err := New("cocoon", nil, nil, false, false, "").MountVolume(ctx, path, "imagenet", "/datasets/training", false) + if !errors.Is(err, context.DeadlineExceeded) || !strings.Contains(err.Error(), "mount volume imagenet") { + t.Fatalf("got %v, want the mount deadline", err) + } +} + func TestMountVolumeStopsAtFailedStage(t *testing.T) { for _, tt := range []struct { name string