From 5472279b816a6c9581075f321cd943ff35cae26c Mon Sep 17 00:00:00 2001 From: Benjamin Elder Date: Mon, 10 Aug 2026 22:51:17 -0700 Subject: [PATCH] ateom-microvm: stop paying a 50ms tick waiting for virtiofsd Restoring an actor rebuilds its virtio-fs plumbing before the guest can come back, and part of that is waiting for virtiofsd to bind its socket. The wait polled every 50ms while virtiofsd binds in single-digit milliseconds, so what it cost was decided by the interval rather than the work: every restore paid a full tick, measured at a flat 51ms each time, against 5-10ms for the bind mounts beside it. Two shares are started per restore, so it was paid twice. Poll finely enough to notice instead, and pull the wait out into a helper so a test can pin the behaviour rather than the constant. --- .../internal/kata/overlay_linux.go | 43 ++++++++-- .../internal/kata/overlay_wait_test.go | 84 +++++++++++++++++++ 2 files changed, 118 insertions(+), 9 deletions(-) create mode 100644 cmd/ateom-microvm/internal/kata/overlay_wait_test.go diff --git a/cmd/ateom-microvm/internal/kata/overlay_linux.go b/cmd/ateom-microvm/internal/kata/overlay_linux.go index abf0995df..741a1a446 100644 --- a/cmd/ateom-microvm/internal/kata/overlay_linux.go +++ b/cmd/ateom-microvm/internal/kata/overlay_linux.go @@ -131,20 +131,45 @@ func StartVirtiofsd(ctx context.Context, o VirtiofsdOptions) (*exec.Cmd, error) if err := cmd.Start(); err != nil { return nil, fmt.Errorf("starting virtiofsd: %w", err) } - deadline := time.Now().Add(10 * time.Second) - for time.Now().Before(deadline) { - if _, err := os.Stat(o.SocketPath); err == nil { - return cmd, nil + if err := waitForSocket(ctx, o.SocketPath, virtiofsdSocketTimeout); err != nil { + _ = cmd.Process.Kill() + return nil, err + } + return cmd, nil +} + +const ( + // virtiofsdSocketTimeout bounds how long we wait for virtiofsd to bind. + virtiofsdSocketTimeout = 10 * time.Second + // socketPollInterval is how often we look for it. This sits on the restore + // path, ahead of the guest coming back, and virtiofsd binds in single-digit + // milliseconds — so the interval, not the work, decides what this costs. At + // 50ms every restore paid a full tick; polling finely enough to notice makes + // it a few milliseconds instead, at the price of a handful of extra stats. + socketPollInterval = 1 * time.Millisecond +) + +// waitForSocket blocks until path exists, ctx is done, or timeout elapses. +func waitForSocket(ctx context.Context, path string, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + // One ticker rather than a timer per iteration: polling this finely, the + // allocations add up, and a ticker does not stretch the interval by however + // long the stat took. + ticker := time.NewTicker(socketPollInterval) + defer ticker.Stop() + for { + if _, err := os.Stat(path); err == nil { + return nil + } + if !time.Now().Before(deadline) { + return fmt.Errorf("virtiofsd socket %q did not appear within %s", path, timeout) } select { case <-ctx.Done(): - _ = cmd.Process.Kill() - return nil, ctx.Err() - case <-time.After(50 * time.Millisecond): + return ctx.Err() + case <-ticker.C: } } - _ = cmd.Process.Kill() - return nil, fmt.Errorf("virtiofsd socket %q did not appear", o.SocketPath) } // ReconstructSharedDirFromImage bind-mounts a container's OCI image rootfs at diff --git a/cmd/ateom-microvm/internal/kata/overlay_wait_test.go b/cmd/ateom-microvm/internal/kata/overlay_wait_test.go new file mode 100644 index 000000000..80eee7aae --- /dev/null +++ b/cmd/ateom-microvm/internal/kata/overlay_wait_test.go @@ -0,0 +1,84 @@ +//go:build linux + +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package kata + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + "time" +) + +// TestWaitForSocketReturnsPromptly pins the reason this helper exists: it sits on +// the restore path, so what it costs is decided by how often it looks, not by how +// long virtiofsd takes to bind. A socket appearing after 5ms should not cost a +// 50ms tick. +func TestWaitForSocketReturnsPromptly(t *testing.T) { + path := filepath.Join(t.TempDir(), "vfsd.sock") + go func() { + time.Sleep(5 * time.Millisecond) + f, err := os.Create(path) + if err == nil { + _ = f.Close() + } + }() + + start := time.Now() + if err := waitForSocket(context.Background(), path, 5*time.Second); err != nil { + t.Fatalf("waitForSocket: %v", err) + } + if elapsed := time.Since(start); elapsed > 40*time.Millisecond { + t.Errorf("waited %s for a socket that appeared after 5ms; the poll interval dominates", elapsed) + } +} + +func TestWaitForSocketAlreadyPresent(t *testing.T) { + path := filepath.Join(t.TempDir(), "vfsd.sock") + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + _ = f.Close() + if err := waitForSocket(context.Background(), path, time.Second); err != nil { + t.Errorf("waitForSocket on an existing path: %v", err) + } +} + +func TestWaitForSocketTimesOut(t *testing.T) { + path := filepath.Join(t.TempDir(), "never.sock") + err := waitForSocket(context.Background(), path, 20*time.Millisecond) + if err == nil { + t.Fatal("waitForSocket on a socket that never appears = nil, want error") + } + if !errors.Is(err, context.Canceled) && err.Error() == "" { + t.Errorf("unhelpful error: %v", err) + } +} + +func TestWaitForSocketHonoursContext(t *testing.T) { + path := filepath.Join(t.TempDir(), "never.sock") + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(10 * time.Millisecond) + cancel() + }() + if err := waitForSocket(ctx, path, 5*time.Second); !errors.Is(err, context.Canceled) { + t.Errorf("waitForSocket after cancel = %v, want context.Canceled", err) + } +}