Skip to content

Commit 134a270

Browse files
committed
Report its exit status from Wait() correctly
`err` was actually the variable outside of the goroutine. Signed-off-by: Kazuyoshi Kato <[email protected]>
1 parent ef87884 commit 134a270

File tree

2 files changed

+59
-5
lines changed

2 files changed

+59
-5
lines changed

machine.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -459,17 +459,19 @@ func (m *Machine) startVMM(ctx context.Context) error {
459459

460460
errCh := make(chan error)
461461
go func() {
462-
if err := m.cmd.Wait(); err != nil {
463-
m.logger.Warnf("firecracker exited: %s", err.Error())
462+
waitErr := m.cmd.Wait()
463+
if waitErr != nil {
464+
m.logger.Warnf("firecracker exited: %s", waitErr.Error())
464465
} else {
465466
m.logger.Printf("firecracker exited: status=0")
466467
}
467468

468-
if err := m.doCleanup(); err != nil {
469-
m.logger.Errorf("failed to cleanup after VM exit: %v", err)
469+
cleanupErr := m.doCleanup()
470+
if cleanupErr != nil {
471+
m.logger.Errorf("failed to cleanup after VM exit: %v", cleanupErr)
470472
}
471473

472-
errCh <- err
474+
errCh <- multierror.Append(waitErr, cleanupErr).ErrorOrNil()
473475

474476
// Notify subscribers that there will be no more values.
475477
// When err is nil, two reads are performed (waitForSocket and close exitCh goroutine),

machine_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,3 +1075,55 @@ func TestCaptureFifoToFile_leak(t *testing.T) {
10751075
expected := `io.Copy failed to copy contents of fifo pipe: read testdata/TestCaptureFifoToFileLeak.fifo: file already closed`
10761076
assert.Contains(t, loggerBuffer.String(), expected)
10771077
}
1078+
1079+
func TestWait(t *testing.T) {
1080+
fctesting.RequiresRoot(t)
1081+
ctx := context.Background()
1082+
1083+
socketPath := filepath.Join(testDataPath, t.Name())
1084+
defer os.Remove(socketPath)
1085+
1086+
cfg := Config{
1087+
SocketPath: socketPath,
1088+
KernelImagePath: getVmlinuxPath(t),
1089+
MachineCfg: models.MachineConfiguration{
1090+
VcpuCount: Int64(2),
1091+
CPUTemplate: models.CPUTemplate(models.CPUTemplateT2),
1092+
MemSizeMib: Int64(256),
1093+
HtEnabled: Bool(false),
1094+
},
1095+
Drives: []models.Drive{
1096+
{
1097+
DriveID: String("root"),
1098+
IsRootDevice: Bool(true),
1099+
IsReadOnly: Bool(true),
1100+
PathOnHost: String(testRootfs),
1101+
},
1102+
},
1103+
}
1104+
1105+
cmd := VMCommandBuilder{}.
1106+
WithSocketPath(cfg.SocketPath).
1107+
WithBin(getFirecrackerBinaryPath()).
1108+
Build(ctx)
1109+
1110+
m, err := NewMachine(ctx, cfg, WithProcessRunner(cmd))
1111+
require.NoError(t, err)
1112+
1113+
err = m.Start(ctx)
1114+
require.NoError(t, err)
1115+
1116+
go func() {
1117+
pid, err := m.PID()
1118+
require.NoError(t, err)
1119+
1120+
process, err := os.FindProcess(pid)
1121+
require.NoError(t, err)
1122+
1123+
err = process.Kill()
1124+
require.NoError(t, err)
1125+
}()
1126+
1127+
err = m.Wait(ctx)
1128+
require.Error(t, err, "Firecracker was killed and it must be reported")
1129+
}

0 commit comments

Comments
 (0)