Skip to content

Commit 20cd951

Browse files
committed
fix: pre-build VMM command before reporting container as started
Add BuildExecCmd() method to VMM interface to validate command construction before sending StartSuccess message. This prevents reporting a container as "Running" when the VMM command cannot be built (e.g., due to invalid arguments or failed config writes). The fix preserves the existing PID 1 execution model where the reexec process uses syscall.Exec to become the VMM.
1 parent ce7e5c0 commit 20cd951

File tree

7 files changed

+59
-14
lines changed

7 files changed

+59
-14
lines changed

pkg/unikontainers/hypervisors/firecracker.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (fc *Firecracker) Path() string {
9797
return fc.binaryPath
9898
}
9999

100-
func (fc *Firecracker) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
100+
func (fc *Firecracker) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
101101
// FIXME: Note for getting unikernel specific options.
102102
// Due to the way FC operates, we have not encountered any guest specific
103103
// options yet. However, we need to revisit how we can use guest specific
@@ -189,12 +189,19 @@ func (fc *Firecracker) Execve(args types.ExecArgs, ukernel types.Unikernel) erro
189189
}
190190
FCConfigJSON, _ := json.Marshal(FCConfig)
191191
if err := os.WriteFile(JSONConfigFile, FCConfigJSON, 0o644); err != nil { //nolint: gosec
192-
return fmt.Errorf("failed to save Firecracker json config: %w", err)
192+
return nil, fmt.Errorf("failed to save Firecracker json config: %w", err)
193193
}
194194
vmmLog.WithField("Json", string(FCConfigJSON)).Debug("Firecracker json config")
195195

196196
exArgs := strings.Split(cmdString, " ")
197-
vmmLog.WithField("Firecracker command", exArgs).Debug("Ready to execve Firecracker")
197+
return exArgs, nil
198+
}
198199

200+
func (fc *Firecracker) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
201+
exArgs, err := fc.BuildExecCmd(args, ukernel)
202+
if err != nil {
203+
return err
204+
}
205+
vmmLog.WithField("Firecracker command", exArgs).Debug("Ready to execve Firecracker")
199206
return syscall.Exec(fc.Path(), exArgs, args.Environment) //nolint: gosec
200207
}

pkg/unikontainers/hypervisors/hedge.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ func (h *Hedge) Path() string {
5050
return ""
5151
}
5252

53+
func (h *Hedge) BuildExecCmd(_ types.ExecArgs, _ types.Unikernel) ([]string, error) {
54+
return nil, fmt.Errorf("hedge not implemented yet")
55+
}
56+
5357
func (h *Hedge) Execve(_ types.ExecArgs, _ types.Unikernel) error {
5458
return fmt.Errorf("hedge not implemented yet")
5559
}

pkg/unikontainers/hypervisors/hvt.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ func (h *HVT) Ok() error {
148148
return nil
149149
}
150150

151-
func (h *HVT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
151+
func (h *HVT) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
152152
hvtMem := BytesToStringMB(args.MemSizeB)
153153
cmdString := h.binaryPath + " --mem=" + hvtMem
154154
if args.Net.TapDev != "" {
@@ -164,12 +164,20 @@ func (h *HVT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
164164
cmdString = appendNonEmpty(cmdString, " ", extraMonArgs.OtherArgs)
165165
cmdString += " " + args.UnikernelPath + " " + args.Command
166166
cmdArgs := strings.Split(cmdString, " ")
167+
return cmdArgs, nil
168+
}
169+
170+
func (h *HVT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
171+
cmdArgs, err := h.BuildExecCmd(args, ukernel)
172+
if err != nil {
173+
return err
174+
}
167175
if args.Seccomp {
168176
err := applySeccompFilter()
169177
if err != nil {
170178
return err
171179
}
172180
}
173-
vmmLog.WithField("hvt command", cmdString).Debug("Ready to execve hvt")
181+
vmmLog.WithField("hvt command", cmdArgs).Debug("Ready to execve hvt")
174182
return syscall.Exec(h.binaryPath, cmdArgs, args.Environment) //nolint: gosec
175183
}

pkg/unikontainers/hypervisors/qemu.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (q *Qemu) Path() string {
5555
return q.binaryPath
5656
}
5757

58-
func (q *Qemu) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
58+
func (q *Qemu) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
5959
qemuMem := BytesToStringMB(args.MemSizeB)
6060
cmdString := q.binaryPath + " -m " + qemuMem + "M"
6161
cmdString += " -L /usr/share/qemu" // Set the path for qemu bios/data
@@ -137,6 +137,14 @@ func (q *Qemu) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
137137

138138
exArgs := strings.Split(cmdString, " ")
139139
exArgs = append(exArgs, "-append", args.Command)
140+
return exArgs, nil
141+
}
142+
143+
func (q *Qemu) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
144+
exArgs, err := q.BuildExecCmd(args, ukernel)
145+
if err != nil {
146+
return err
147+
}
140148
vmmLog.WithField("qemu command", exArgs).Debug("Ready to execve qemu")
141149
return syscall.Exec(q.Path(), exArgs, args.Environment) //nolint: gosec
142150
}

pkg/unikontainers/hypervisors/spt.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ func (s *SPT) Ok() error {
6060
return nil
6161
}
6262

63-
func (s *SPT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
63+
func (s *SPT) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
6464
sptMem := BytesToStringMB(args.MemSizeB)
6565
cmdString := s.binaryPath + " --mem=" + sptMem
6666
if args.Net.TapDev != "" {
@@ -76,6 +76,14 @@ func (s *SPT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
7676
cmdString = appendNonEmpty(cmdString, " ", extraMonArgs.OtherArgs)
7777
cmdString += " " + args.UnikernelPath + " " + args.Command
7878
cmdArgs := strings.Split(cmdString, " ")
79-
vmmLog.WithField("spt command", cmdString).Debug("Ready to execve spt")
79+
return cmdArgs, nil
80+
}
81+
82+
func (s *SPT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
83+
cmdArgs, err := s.BuildExecCmd(args, ukernel)
84+
if err != nil {
85+
return err
86+
}
87+
vmmLog.WithField("spt command", cmdArgs).Debug("Ready to execve spt")
8088
return syscall.Exec(s.binaryPath, cmdArgs, args.Environment) //nolint: gosec
8189
}

pkg/unikontainers/types/types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ type Unikernel interface {
2525
}
2626

2727
type VMM interface {
28+
// BuildExecCmd builds and validates the VMM command arguments without executing.
29+
// This is used to verify the command can be built before reporting container as started.
30+
BuildExecCmd(args ExecArgs, ukernel Unikernel) ([]string, error)
31+
// Execve replaces the current process with the VMM using syscall.Exec.
2832
Execve(args ExecArgs, ukernel Unikernel) error
2933
Stop(int) error
3034
Path() string

pkg/unikontainers/unikontainers.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -493,12 +493,18 @@ func (u *Unikontainer) Exec(metrics m.Writer) error {
493493

494494
uniklog.Debug("calling vmm execve")
495495
metrics.Capture(m.TS18)
496-
// metrics.Wait()
497-
// TODO: We set the state to running and notify urunc Start that the monitor
498-
// started, but we might encounter issues with the monitor execution. We need
499-
// to revisit this and check if a failed monitor execution affects this approach.
500-
// If it affects then we need to re-design the whole spawning of the monitor.
501-
// Notify urunc start
496+
497+
// Pre-build the VMM command to verify it can be constructed successfully.
498+
// This ensures we don't report the container as started if command building fails.
499+
_, err = vmm.BuildExecCmd(vmmArgs, unikernel)
500+
if err != nil {
501+
uniklog.WithError(err).Error("failed to build VMM command")
502+
return err
503+
}
504+
505+
// Notify urunc start that the monitor is ready to execute.
506+
// We send this after BuildExecCmd succeeds to avoid reporting a container
507+
// as started when the VMM command cannot be built.
502508
err = u.SendMessage(StartSuccess)
503509
if err != nil {
504510
return err

0 commit comments

Comments
 (0)