Skip to content

Commit c07cbbc

Browse files
committed
fix: give vm.snapshot / vm.restore a 10-minute heavy-op timeout
The CH REST client utils.NewSocketHTTPClient sets http.Client.Timeout to 30 seconds, which fires regardless of any context.WithTimeout at the call site. vm.snapshot and vm.restore write/read the entire VM memory image; on multi-GiB Windows snapshots over GCE standard persistent disk, or for mid-boot snapshots still dirty with Windows init activity, 30s is marginal and occasionally exceeded. DoWithRetry masks the symptom by retrying 4x, but each attempt hits the same 30s ceiling, so the user-visible effect is a ~2-minute wait followed by "Client.Timeout exceeded while awaiting headers". Fix: add heavyClient(hc) that clones the caller's http.Client by value and raises Timeout to 10 minutes. The Transport (socket dialer and connection pool) stays shared, so light endpoints like vm.pause and vm.resume still use the original 30s client unchanged. Only vm.snapshot and vm.restore route through heavyClient. 10 minutes was chosen to give multi-GiB cold-start restores real headroom on slow disks while still bounding a genuine CH hang. The global HTTPTimeout stays 30s to keep fast endpoints responsive. Observed: epoch snapshot round-trip test against cocoonset-node-2 (2 GiB memory, freshly-booted Windows 11) — vm.restore timed out with the default 30s client during cocoon vm clone even though the memory-ranges file, disk, and network were all healthy.
1 parent f5a6937 commit c07cbbc

10 files changed

Lines changed: 53 additions & 19 deletions

File tree

hypervisor/backend.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,16 @@ const (
2828
// CreatingStateGCGrace is how long a VM can stay in "creating" state
2929
// before GC treats it as a crash remnant and cleans it up.
3030
CreatingStateGCGrace = 24 * time.Hour
31+
32+
// VMMemTransferTimeout is the per-call HTTP timeout for hypervisor API
33+
// endpoints that move the entire guest memory (snapshot/restore). The
34+
// default utils.HTTPTimeout (30s) is marginal for multi-GiB transfers
35+
// on slow storage, and these operations have no useful retry semantics:
36+
// they are non-idempotent (a partial snapshot overwrites files; a
37+
// partial restore leaves the hypervisor in an undefined state), so
38+
// callers must bypass the per-backend retry wrapper and issue a single
39+
// call with this deadline.
40+
VMMemTransferTimeout = 10 * time.Minute
3141
)
3242

3343
// BackendConfig provides backend-specific values needed by shared Backend methods.

hypervisor/cloudhypervisor/clone.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,10 +206,10 @@ func (ch *CloudHypervisor) restoreAndResumeClone(
206206
}
207207
}()
208208

209-
hc := utils.NewSocketHTTPClient(sockPath)
210-
if err = restoreVM(ctx, hc, runDir); err != nil {
209+
if err = restoreVM(ctx, sockPath, runDir); err != nil {
211210
return fmt.Errorf("vm.restore: %w", err)
212211
}
212+
hc := utils.NewSocketHTTPClient(sockPath)
213213

214214
// Hot-swap NICs while paused: remove snapshot's virtio-net devices (which carry
215215
// the old MAC baked in binary device state), then add fresh ones with correct MAC.

hypervisor/cloudhypervisor/helper.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,24 +66,31 @@ func resumeVM(ctx context.Context, hc *http.Client) error {
6666
return vmAPI(ctx, hc, "vm.resume", nil)
6767
}
6868

69-
func snapshotVM(ctx context.Context, hc *http.Client, destDir string) error {
69+
// snapshotVM and restoreVM bypass vmAPI's retry layer — see hypervisor.VMMemTransferTimeout.
70+
func snapshotVM(ctx context.Context, sockPath, destDir string) error {
7071
body, err := json.Marshal(map[string]string{
7172
"destination_url": "file://" + destDir,
7273
})
7374
if err != nil {
7475
return fmt.Errorf("marshal snapshot request: %w", err)
7576
}
76-
return vmAPI(ctx, hc, "vm.snapshot", body)
77+
hc := utils.NewSocketHTTPClientWithTimeout(sockPath, hypervisor.VMMemTransferTimeout)
78+
_, err = utils.DoAPI(ctx, hc, http.MethodPut,
79+
"http://localhost/api/v1/vm.snapshot", body, http.StatusNoContent)
80+
return err
7781
}
7882

79-
func restoreVM(ctx context.Context, hc *http.Client, sourceDir string) error {
83+
func restoreVM(ctx context.Context, sockPath, sourceDir string) error {
8084
body, err := json.Marshal(map[string]string{
8185
"source_url": "file://" + sourceDir,
8286
})
8387
if err != nil {
8488
return fmt.Errorf("marshal restore request: %w", err)
8589
}
86-
return vmAPI(ctx, hc, "vm.restore", body)
90+
hc := utils.NewSocketHTTPClientWithTimeout(sockPath, hypervisor.VMMemTransferTimeout)
91+
_, err = utils.DoAPI(ctx, hc, http.MethodPut,
92+
"http://localhost/api/v1/vm.restore", body, http.StatusNoContent)
93+
return err
8794
}
8895

8996
func addDiskVM(ctx context.Context, hc *http.Client, disk chDisk) error {

hypervisor/cloudhypervisor/restore.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,10 @@ func (ch *CloudHypervisor) restoreAfterExtract(ctx context.Context, vmID string,
113113
}
114114
}()
115115

116-
hc := utils.NewSocketHTTPClient(sockPath)
117-
if err = restoreVM(ctx, hc, rec.RunDir); err != nil {
116+
if err = restoreVM(ctx, sockPath, rec.RunDir); err != nil {
118117
return nil, fmt.Errorf("vm.restore: %w", err)
119118
}
119+
hc := utils.NewSocketHTTPClient(sockPath)
120120
if err = resumeVM(ctx, hc); err != nil {
121121
return nil, fmt.Errorf("vm.resume: %w", err)
122122
}

hypervisor/cloudhypervisor/snapshot.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func (ch *CloudHypervisor) Snapshot(ctx context.Context, ref string) (*types.Sna
6969
}
7070
defer doResume()
7171

72-
if err := snapshotVM(ctx, hc, tmpDir); err != nil {
72+
if err := snapshotVM(ctx, sockPath, tmpDir); err != nil {
7373
return fmt.Errorf("snapshot: %w", err)
7474
}
7575

hypervisor/firecracker/api.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"path/filepath"
1010
"slices"
1111

12+
"github.com/cocoonstack/cocoon/hypervisor"
1213
"github.com/cocoonstack/cocoon/utils"
1314
)
1415

@@ -196,20 +197,25 @@ type fcSnapshotMemBE struct {
196197
}
197198

198199
// createSnapshotFC creates a full VM snapshot (vmstate + memory file) in destDir.
199-
func createSnapshotFC(ctx context.Context, hc *http.Client, destDir string) error {
200+
// Bypasses fcAPI's retry layer — see hypervisor.VMMemTransferTimeout.
201+
func createSnapshotFC(ctx context.Context, sockPath, destDir string) error {
200202
body, err := json.Marshal(fcSnapshotCreate{
201203
SnapshotPath: filepath.Join(destDir, snapshotVMStateFile),
202204
MemFilePath: filepath.Join(destDir, snapshotMemFile),
203205
})
204206
if err != nil {
205207
return fmt.Errorf("marshal snapshot/create request: %w", err)
206208
}
207-
return fcAPI(ctx, hc, http.MethodPut, "/snapshot/create", body)
209+
hc := utils.NewSocketHTTPClientWithTimeout(sockPath, hypervisor.VMMemTransferTimeout)
210+
_, err = utils.DoAPI(ctx, hc, http.MethodPut,
211+
"http://localhost/snapshot/create", body, http.StatusNoContent)
212+
return err
208213
}
209214

210215
// loadSnapshotFC loads a VM snapshot from sourceDir into a freshly started FC process.
211216
// networkOverrides replaces TAP devices from the snapshot with new ones.
212-
func loadSnapshotFC(ctx context.Context, hc *http.Client, sourceDir string, networkOverrides []fcNetworkOverride) error {
217+
// Bypasses fcAPI's retry layer — see hypervisor.VMMemTransferTimeout.
218+
func loadSnapshotFC(ctx context.Context, sockPath, sourceDir string, networkOverrides []fcNetworkOverride) error {
213219
body, err := json.Marshal(fcSnapshotLoad{
214220
SnapshotPath: filepath.Join(sourceDir, snapshotVMStateFile),
215221
MemBackend: fcSnapshotMemBE{
@@ -221,5 +227,8 @@ func loadSnapshotFC(ctx context.Context, hc *http.Client, sourceDir string, netw
221227
if err != nil {
222228
return fmt.Errorf("marshal snapshot/load request: %w", err)
223229
}
224-
return fcAPI(ctx, hc, http.MethodPut, "/snapshot/load", body)
230+
hc := utils.NewSocketHTTPClientWithTimeout(sockPath, hypervisor.VMMemTransferTimeout)
231+
_, err = utils.DoAPI(ctx, hc, http.MethodPut,
232+
"http://localhost/snapshot/load", body, http.StatusNoContent)
233+
return err
225234
}

hypervisor/firecracker/clone.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,17 +177,17 @@ func (fc *Firecracker) restoreAndResumeClone(
177177
}
178178
}()
179179

180-
hc := utils.NewSocketHTTPClient(sockPath)
181180
// Use network_overrides to provide clone's TAP devices during snapshot/load.
182181
// FC re-creates network devices from vmstate — overrides replace the
183182
// source TAP with the clone's TAP so FC opens the right device.
184183
netOverrides := buildNetworkOverrides(networkConfigs)
185184
// FC opens drive files during snapshot/load and holds the fds.
186185
// The symlink redirect ensures FC opens the clone's COW, not the source's.
187186
// No drive reconfiguration needed — fds survive symlink cleanup.
188-
if err = loadSnapshotFC(ctx, hc, runDir, netOverrides); err != nil {
187+
if err = loadSnapshotFC(ctx, sockPath, runDir, netOverrides); err != nil {
189188
return fmt.Errorf("snapshot/load: %w", err)
190189
}
190+
hc := utils.NewSocketHTTPClient(sockPath)
191191
if err = resumeVM(ctx, hc); err != nil {
192192
return fmt.Errorf("resume: %w", err)
193193
}

hypervisor/firecracker/restore.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,13 @@ func (fc *Firecracker) restoreAfterExtract(ctx context.Context, vmID string, vmC
120120
}
121121
}()
122122

123-
hc := utils.NewSocketHTTPClient(sockPath)
124123
// Restore uses the same VM — TAP and drives are unchanged.
125124
// No network_overrides or drive reconfiguration needed.
126-
if err = loadSnapshotFC(ctx, hc, rec.RunDir, nil); err != nil {
125+
if err = loadSnapshotFC(ctx, sockPath, rec.RunDir, nil); err != nil {
127126
return nil, fmt.Errorf("snapshot/load: %w", err)
128127
}
129128

129+
hc := utils.NewSocketHTTPClient(sockPath)
130130
if err = resumeVM(ctx, hc); err != nil {
131131
return nil, fmt.Errorf("resume: %w", err)
132132
}

hypervisor/firecracker/snapshot.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func (fc *Firecracker) Snapshot(ctx context.Context, ref string) (*types.Snapsho
6969
}
7070
defer doResume()
7171

72-
if err := createSnapshotFC(ctx, hc, tmpDir); err != nil {
72+
if err := createSnapshotFC(ctx, sockPath, tmpDir); err != nil {
7373
return fmt.Errorf("snapshot: %w", err)
7474
}
7575
if err := utils.ReflinkCopy(filepath.Join(tmpDir, cowFileName), cowPath); err != nil {

utils/http.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,16 @@ func (e *APIError) Error() string { return e.Message }
2727

2828
// NewSocketHTTPClient creates an HTTP client that dials a Unix socket.
2929
func NewSocketHTTPClient(socketPath string) *http.Client {
30+
return NewSocketHTTPClientWithTimeout(socketPath, HTTPTimeout)
31+
}
32+
33+
// NewSocketHTTPClientWithTimeout is like NewSocketHTTPClient but with a custom
34+
// per-call Timeout. Use this for endpoints where the default HTTPTimeout is too
35+
// tight (e.g. hypervisor snapshot/restore which transfer the entire guest memory
36+
// synchronously and can take minutes on multi-GiB VMs or slow storage).
37+
func NewSocketHTTPClientWithTimeout(socketPath string, timeout time.Duration) *http.Client {
3038
return &http.Client{
31-
Timeout: HTTPTimeout,
39+
Timeout: timeout,
3240
Transport: &http.Transport{
3341
DialContext: func(ctx context.Context, _, _ string) (net.Conn, error) {
3442
var d net.Dialer

0 commit comments

Comments
 (0)