Skip to content

Commit 661159a

Browse files
committed
Address review comments
1 parent c95c45d commit 661159a

File tree

3 files changed

+37
-3
lines changed

3 files changed

+37
-3
lines changed

lib/hypervisor/cloudhypervisor/process.go

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ package cloudhypervisor
33
import (
44
"context"
55
"fmt"
6+
"syscall"
67

78
"github.com/onkernel/hypeman/lib/hypervisor"
89
"github.com/onkernel/hypeman/lib/paths"
910
"github.com/onkernel/hypeman/lib/vmm"
11+
"gvisor.dev/gvisor/pkg/cleanup"
1012
)
1113

1214
func init() {
@@ -53,6 +55,12 @@ func (s *Starter) StartVM(ctx context.Context, p *paths.Paths, version string, s
5355
return 0, nil, fmt.Errorf("start process: %w", err)
5456
}
5557

58+
// Setup cleanup to kill the process if subsequent steps fail
59+
cu := cleanup.Make(func() {
60+
syscall.Kill(pid, syscall.SIGKILL)
61+
})
62+
defer cu.Clean()
63+
5664
// 2. Create the HTTP client
5765
hv, err := New(socketPath)
5866
if err != nil {
@@ -78,6 +86,8 @@ func (s *Starter) StartVM(ctx context.Context, p *paths.Paths, version string, s
7886
return 0, nil, fmt.Errorf("boot vm failed with status %d: %s", bootResp.StatusCode(), string(bootResp.Body))
7987
}
8088

89+
// Success - release cleanup to prevent killing the process
90+
cu.Release()
8191
return pid, hv, nil
8292
}
8393

@@ -96,6 +106,12 @@ func (s *Starter) RestoreVM(ctx context.Context, p *paths.Paths, version string,
96106
return 0, nil, fmt.Errorf("start process: %w", err)
97107
}
98108

109+
// Setup cleanup to kill the process if subsequent steps fail
110+
cu := cleanup.Make(func() {
111+
syscall.Kill(pid, syscall.SIGKILL)
112+
})
113+
defer cu.Clean()
114+
99115
// 2. Create the HTTP client
100116
hv, err := New(socketPath)
101117
if err != nil {
@@ -110,14 +126,14 @@ func (s *Starter) RestoreVM(ctx context.Context, p *paths.Paths, version string,
110126
}
111127
resp, err := hv.client.PutVmRestoreWithResponse(ctx, restoreConfig)
112128
if err != nil {
113-
hv.Shutdown(ctx) // Cleanup on failure
114129
return 0, nil, fmt.Errorf("restore: %w", err)
115130
}
116131
if resp.StatusCode() != 204 {
117-
hv.Shutdown(ctx) // Cleanup on failure
118132
return 0, nil, fmt.Errorf("restore failed with status %d: %s", resp.StatusCode(), string(resp.Body))
119133
}
120134

135+
// Success - release cleanup to prevent killing the process
136+
cu.Release()
121137
return pid, hv, nil
122138
}
123139

lib/hypervisor/qemu/process.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/onkernel/hypeman/lib/hypervisor"
1616
"github.com/onkernel/hypeman/lib/paths"
17+
"gvisor.dev/gvisor/pkg/cleanup"
1718
)
1819

1920
func init() {
@@ -121,6 +122,12 @@ func (s *Starter) StartVM(ctx context.Context, p *paths.Paths, version string, s
121122

122123
pid := cmd.Process.Pid
123124

125+
// Setup cleanup to kill the process if subsequent steps fail
126+
cu := cleanup.Make(func() {
127+
syscall.Kill(pid, syscall.SIGKILL)
128+
})
129+
defer cu.Clean()
130+
124131
// Wait for socket to be ready
125132
if err := waitForSocket(socketPath, 10*time.Second); err != nil {
126133
vmmLogPath := filepath.Join(logsDir, "vmm.log")
@@ -136,6 +143,8 @@ func (s *Starter) StartVM(ctx context.Context, p *paths.Paths, version string, s
136143
return 0, nil, fmt.Errorf("create client: %w", err)
137144
}
138145

146+
// Success - release cleanup to prevent killing the process
147+
cu.Release()
139148
return pid, hv, nil
140149
}
141150

lib/hypervisor/qemu/vsock.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,16 @@ func (c *vsockConn) Read(b []byte) (int, error) {
172172
}
173173

174174
func (c *vsockConn) Write(b []byte) (int, error) {
175-
return unix.Write(c.fd, b)
175+
n, err := unix.Write(c.fd, b)
176+
// Ensure we never return negative n (violates io.Writer contract)
177+
// This can happen when the vsock fd becomes invalid (VM died)
178+
if n < 0 {
179+
if err == nil {
180+
err = io.ErrClosedPipe
181+
}
182+
return 0, err
183+
}
184+
return n, err
176185
}
177186

178187
func (c *vsockConn) Close() error {

0 commit comments

Comments
 (0)