vm: combine Run and MonitorExecution

All callers of Run always call MonitorExecution right after it.
Combine these 2 methods. This allows to hide some implementation
details and simplify users of vm package.

10 files changed, 82 insertions, 89 deletions

diff --git a/pkg/build/netbsd.go b/pkg/build/netbsd.go
index e4390872a..962b6e82c 100644
--- a/pkg/build/netbsd.go
+++ b/pkg/build/netbsd.go
@@ -155,12 +155,11 @@ func (ctx netbsd) copyKernelToDisk(targetArch, vmType, outputDir, kernel string)
 	}
 	commands = append(commands, "mknod /dev/vhci c 355 0")
 	commands = append(commands, "sync") // Run sync so that the copied image is stored properly.
-	outc, errc, err := inst.Run(time.Minute, nil, strings.Join(commands, ";"))
+	_, rep, err := inst.Run(time.Minute, reporter, strings.Join(commands, ";"))
 	if err != nil {
 		return fmt.Errorf("error syncing the instance %w", err)
 	}
 	// Make sure that the command has executed properly.
-	rep := inst.MonitorExecution(outc, errc, reporter, vm.ExitNormal)
 	if rep != nil {
 		return fmt.Errorf("error executing sync: %v", rep.Title)
 	}
diff --git a/pkg/instance/execprog.go b/pkg/instance/execprog.go
index 9e97dcd0c..acade5e8e 100644
--- a/pkg/instance/execprog.go
+++ b/pkg/instance/execprog.go
@@ -37,7 +37,8 @@ type ExecProgInstance struct {
 }
 
 type RunResult struct {
-	vm.ExecutionResult
+	Output []byte
+	Report *report.Report
 }
 
 func SetupExecProg(vmInst *vm.Instance, mgrCfg *mgrconfig.Config, reporter *report.Reporter,
@@ -107,25 +108,23 @@ func (inst *ExecProgInstance) runCommand(command string, duration time.Duration)
 		command = inst.StraceBin + filterCalls + ` -s 100 -x -f ` + command
 		prefixOutput = []byte(fmt.Sprintf("%s\n\n<...>\n", command))
 	}
-	outc, errc, err := inst.VMInstance.Run(duration, nil, command)
+	opts := []any{inst.ExitCondition}
+	if inst.BeforeContextLen != 0 {
+		opts = append(opts, vm.OutputSize(inst.BeforeContextLen))
+	}
+	output, rep, err := inst.VMInstance.Run(duration, inst.reporter, command, opts...)
 	if err != nil {
 		return nil, fmt.Errorf("failed to run command in VM: %w", err)
 	}
-	result := &RunResult{
-		ExecutionResult: *inst.VMInstance.MonitorExecutionRaw(outc, errc,
-			inst.reporter, inst.ExitCondition, inst.BeforeContextLen),
-	}
-	if len(prefixOutput) > 0 {
-		result.RawOutput = append(prefixOutput, result.RawOutput...)
-	}
-	if result.Report == nil {
+	if rep == nil {
 		inst.Logf(2, "program did not crash")
 	} else {
-		if err := inst.reporter.Symbolize(result.Report); err != nil {
+		if err := inst.reporter.Symbolize(rep); err != nil {
 			inst.Logf(0, "failed to symbolize report: %v", err)
 		}
-		inst.Logf(2, "program crashed: %v", result.Report.Title)
+		inst.Logf(2, "program crashed: %v", rep.Title)
 	}
+	result := &RunResult{append(prefixOutput, output...), rep}
 	return result, nil
 }
 
diff --git a/pkg/instance/instance.go b/pkg/instance/instance.go
index 1e4787c94..4c61b1d24 100644
--- a/pkg/instance/instance.go
+++ b/pkg/instance/instance.go
@@ -388,11 +388,11 @@ func (inst *inst) testInstance() error {
 
 	cmd := OldFuzzerCmd(fuzzerBin, executorBin, targets.TestOS, inst.cfg.TargetOS, inst.cfg.TargetArch, fwdAddr,
 		inst.cfg.Sandbox, inst.cfg.SandboxArg, 0, inst.cfg.Cover, true, inst.optionalFlags, inst.cfg.Timeouts.Slowdown)
-	outc, errc, err := inst.vm.Run(10*time.Minute*inst.cfg.Timeouts.Scale, nil, cmd)
+	timeout := 10 * time.Minute * inst.cfg.Timeouts.Scale
+	_, rep, err := inst.vm.Run(timeout, inst.reporter, cmd)
 	if err != nil {
 		return fmt.Errorf("failed to run binary in VM: %w", err)
 	}
-	rep := inst.vm.MonitorExecution(outc, errc, inst.reporter, vm.ExitNormal)
 	if rep != nil {
 		if err := inst.reporter.Symbolize(rep); err != nil {
 			// TODO(dvyukov): send such errors to dashboard.
@@ -424,9 +424,9 @@ func (inst *inst) testRepro() ([]byte, error) {
 			return nil, err
 		}
 		if res != nil && res.Report != nil {
-			return res.RawOutput, &CrashError{Report: res.Report}
+			return res.Output, &CrashError{Report: res.Report}
 		}
-		return res.RawOutput, nil
+		return res.Output, nil
 	}
 	out := []byte{}
 	if len(inst.reproSyz) > 0 {
diff --git a/pkg/repro/strace.go b/pkg/repro/strace.go
index 87d3cf64e..ef4f7c934 100644
--- a/pkg/repro/strace.go
+++ b/pkg/repro/strace.go
@@ -51,7 +51,7 @@ func RunStrace(result *Result, cfg *mgrconfig.Config, reporter *report.Reporter,
 	}
 	return &StraceResult{
 		Report: runRes.Report,
-		Output: runRes.RawOutput,
+		Output: runRes.Output,
 	}
 }
 
diff --git a/syz-manager/manager.go b/syz-manager/manager.go
index 58744d35a..e3c7e4fb2 100644
--- a/syz-manager/manager.go
+++ b/syz-manager/manager.go
@@ -827,23 +827,19 @@ func (mgr *Manager) runInstanceInner(index int, instanceName string) (*report.Re
 		},
 	}
 	cmd := instance.FuzzerCmd(args)
-	outc, errc, err := inst.Run(mgr.cfg.Timeouts.VMRunningTime, mgr.vmStop, cmd)
+	_, rep, err := inst.Run(mgr.cfg.Timeouts.VMRunningTime, mgr.reporter, cmd, vm.ExitTimeout, vm.StopChan(mgr.vmStop))
 	if err != nil {
 		return nil, nil, fmt.Errorf("failed to run fuzzer: %w", err)
 	}
-
-	var vmInfo []byte
-	rep := inst.MonitorExecution(outc, errc, mgr.reporter, vm.ExitTimeout)
 	if rep == nil {
 		// This is the only "OK" outcome.
 		log.Logf(0, "%s: running for %v, restarting", instanceName, time.Since(start))
-	} else {
-		vmInfo, err = inst.Info()
-		if err != nil {
-			vmInfo = []byte(fmt.Sprintf("error getting VM info: %v\n", err))
-		}
+		return nil, nil, nil
+	}
+	vmInfo, err := inst.Info()
+	if err != nil {
+		vmInfo = []byte(fmt.Sprintf("error getting VM info: %v\n", err))
 	}
-
 	return rep, vmInfo, nil
 }
 
diff --git a/syz-verifier/verifier.go b/syz-verifier/verifier.go
index d173b81bd..295a9da93 100644
--- a/syz-verifier/verifier.go
+++ b/syz-verifier/verifier.go
@@ -266,13 +266,10 @@ func (vrf *Verifier) createAndManageInstance(pi *poolInfo, poolID, vmID int) {
 	}
 
 	cmd := instance.RunnerCmd(runnerBin, fwdAddr, vrf.target.OS, vrf.target.Arch, poolID, 0, false, vrf.newEnv)
-	outc, errc, err := inst.Run(pi.cfg.Timeouts.VMRunningTime, vrf.vmStop, cmd)
+	_, _, err = inst.Run(pi.cfg.Timeouts.VMRunningTime, pi.Reporter, cmd, vm.ExitTimeout, vm.StopChan(vrf.vmStop))
 	if err != nil {
 		log.Fatalf("failed to start runner: %v", err)
 	}
-
-	inst.MonitorExecution(outc, errc, pi.Reporter, vm.ExitTimeout)
-
 	log.Logf(0, "reboot the VM in pool %d", poolID)
 }
 
diff --git a/tools/syz-crush/crush.go b/tools/syz-crush/crush.go
index 0ee550e79..f4e778a00 100644
--- a/tools/syz-crush/crush.go
+++ b/tools/syz-crush/crush.go
@@ -143,7 +143,7 @@ func storeCrash(cfg *mgrconfig.Config, res *instance.RunResult) {
 	if err := osutil.WriteFile(filepath.Join(dir, "description"), []byte(rep.Title+"\n")); err != nil {
 		log.Printf("failed to write crash description: %v", err)
 	}
-	if err := osutil.WriteFile(filepath.Join(dir, fmt.Sprintf("log%v", index)), res.RawOutput); err != nil {
+	if err := osutil.WriteFile(filepath.Join(dir, fmt.Sprintf("log%v", index)), res.Output); err != nil {
 		log.Printf("failed to write crash log: %v", err)
 	}
 	if err := osutil.WriteFile(filepath.Join(dir, fmt.Sprintf("tag%v", index)), []byte(cfg.Tag)); err != nil {
diff --git a/tools/syz-runtest/runtest.go b/tools/syz-runtest/runtest.go
index e90c5d70b..b61dfdad8 100644
--- a/tools/syz-runtest/runtest.go
+++ b/tools/syz-runtest/runtest.go
@@ -194,11 +194,10 @@ func (mgr *Manager) boot(name string, index int) (*report.Report, error) {
 		},
 	}
 	cmd := instance.FuzzerCmd(args)
-	outc, errc, err := inst.Run(time.Hour, mgr.vmStop, cmd)
+	_, rep, err := inst.Run(time.Hour, mgr.reporter, cmd, vm.StopChan(mgr.vmStop))
 	if err != nil {
 		return nil, fmt.Errorf("failed to run fuzzer: %w", err)
 	}
-	rep := inst.MonitorExecution(outc, errc, mgr.reporter, vm.ExitNormal)
 	return rep, nil
 }
 
diff --git a/vm/vm.go b/vm/vm.go
index 44ff3de38..6d3ee7fff 100644
--- a/vm/vm.go
+++ b/vm/vm.go
@@ -185,9 +185,58 @@ func (inst *Instance) Forward(port int) (string, error) {
 	return inst.impl.Forward(port)
 }
 
-func (inst *Instance) Run(timeout time.Duration, stop <-chan bool, command string) (
-	outc <-chan []byte, errc <-chan error, err error) {
-	return inst.impl.Run(timeout, stop, command)
+type ExitCondition int
+
+const (
+	// The program is allowed to exit after timeout.
+	ExitTimeout = ExitCondition(1 << iota)
+	// The program is allowed to exit with no errors.
+	ExitNormal
+	// The program is allowed to exit with errors.
+	ExitError
+)
+
+type StopChan <-chan bool
+type OutputSize int
+
+// Run runs cmd inside of the VM (think of ssh cmd) and monitors command execution
+// and the kernel console output. It detects kernel oopses in output, lost connections, hangs, etc.
+// Returns command+kernel output and a non-symbolized crash report (nil if no error happens).
+// Accepted options:
+//   - StopChan: stop channel can be used to prematurely stop the command
+//   - ExitCondition: says which exit modes should be considered as errors/OK
+//   - OutputSize: how much output to keep/return
+func (inst *Instance) Run(timeout time.Duration, reporter *report.Reporter, command string, opts ...any) (
+	[]byte, *report.Report, error) {
+	exit := ExitNormal
+	var stop <-chan bool
+	outputSize := beforeContextDefault
+	for _, o := range opts {
+		switch opt := o.(type) {
+		case ExitCondition:
+			exit = opt
+		case StopChan:
+			stop = opt
+		case OutputSize:
+			outputSize = int(opt)
+		default:
+			panic(fmt.Sprintf("unknown option %#v", opt))
+		}
+	}
+	outc, errc, err := inst.impl.Run(timeout, stop, command)
+	if err != nil {
+		return nil, nil, err
+	}
+	mon := &monitor{
+		inst:          inst,
+		outc:          outc,
+		errc:          errc,
+		reporter:      reporter,
+		beforeContext: outputSize,
+		exit:          exit,
+	}
+	rep := mon.monitorExecution()
+	return mon.output, rep, nil
 }
 
 func (inst *Instance) Info() ([]byte, error) {
@@ -222,51 +271,6 @@ func (inst *Instance) Close() {
 	inst.onClose()
 }
 
-type ExitCondition int
-
-const (
-	// The program is allowed to exit after timeout.
-	ExitTimeout = ExitCondition(1 << iota)
-	// The program is allowed to exit with no errors.
-	ExitNormal
-	// The program is allowed to exit with errors.
-	ExitError
-)
-
-// MonitorExecution monitors execution of a program running inside of a VM.
-// It detects kernel oopses in output, lost connections, hangs, etc.
-// outc/errc is what vm.Instance.Run returns, reporter parses kernel output for oopses.
-// Exit says which exit modes should be considered as errors/OK.
-// Returns a non-symbolized crash report, or nil if no error happens.
-func (inst *Instance) MonitorExecution(outc <-chan []byte, errc <-chan error,
-	reporter *report.Reporter, exit ExitCondition) (rep *report.Report) {
-	return inst.MonitorExecutionRaw(outc, errc, reporter, exit, 0).Report
-}
-
-type ExecutionResult struct {
-	Report    *report.Report
-	RawOutput []byte
-}
-
-func (inst *Instance) MonitorExecutionRaw(outc <-chan []byte, errc <-chan error,
-	reporter *report.Reporter, exit ExitCondition, beforeContextSize int) (res *ExecutionResult) {
-	if beforeContextSize == 0 {
-		beforeContextSize = beforeContextDefault
-	}
-	mon := &monitor{
-		inst:          inst,
-		outc:          outc,
-		errc:          errc,
-		reporter:      reporter,
-		beforeContext: beforeContextSize,
-		exit:          exit,
-	}
-	return &ExecutionResult{
-		Report:    mon.monitorExecution(),
-		RawOutput: mon.output,
-	}
-}
-
 type monitor struct {
 	inst          *Instance
 	outc          <-chan []byte
diff --git a/vm/vm_test.go b/vm/vm_test.go
index 62b93f4c1..a83adbe26 100644
--- a/vm/vm_test.go
+++ b/vm/vm_test.go
@@ -366,10 +366,6 @@ func testMonitorExecution(t *testing.T, test *Test) {
 		t.Fatal(err)
 	}
 	defer inst.Close()
-	outc, errc, err := inst.Run(time.Second, nil, "")
-	if err != nil {
-		t.Fatal(err)
-	}
 	testInst := inst.impl.(*testInstance)
 	testInst.diagnoseBug = test.DiagnoseBug
 	testInst.diagnoseNoWait = test.DiagnoseNoWait
@@ -378,7 +374,10 @@ func testMonitorExecution(t *testing.T, test *Test) {
 		test.Body(testInst.outc, testInst.errc)
 		done <- true
 	}()
-	rep := inst.MonitorExecution(outc, errc, reporter, test.Exit)
+	_, rep, err := inst.Run(time.Second, reporter, "", test.Exit)
+	if err != nil {
+		t.Fatal(err)
+	}
 	<-done
 	if test.Report != nil && rep == nil {
 		t.Fatalf("got no report")