vm: allow Diagnose to directly return diagnosis

Rather than writing the diagnosis to the kernel console, Diagnose can
now directly return the extra debugging info, which will be appended ot
the kernel console log.

vm/adb/adb.go

@@ -405,6 +405,6 @@ func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command strin
 	return vmimpl.Multiplex(adb, merger, tty, timeout, stop, inst.closed, inst.debug)
 }
 
-func (inst *instance) Diagnose() bool {
-	return false
+func (inst *instance) Diagnose() ([]byte, bool) {
+	return nil, false
 }

vm/gce/gce.go

@@ -366,11 +366,11 @@ func waitForConsoleConnect(merger *vmimpl.OutputMerger) error {
 	}
 }
 
-func (inst *instance) Diagnose() bool {
+func (inst *instance) Diagnose() ([]byte, bool) {
 	if inst.env.OS == "openbsd" {
-		return vmimpl.DiagnoseOpenBSD(inst.consolew)
+		return nil, vmimpl.DiagnoseOpenBSD(inst.consolew)
 	}
-	return false
+	return nil, false
 }
 
 func (pool *Pool) getSerialPortOutput(name, gceKey string) ([]byte, error) {

vm/gvisor/gvisor.go

@@ -327,9 +327,9 @@ func (inst *instance) guestProxy() (*os.File, error) {
 	return guestSock, nil
 }
 
-func (inst *instance) Diagnose() bool {
+func (inst *instance) Diagnose() ([]byte, bool) {
 	osutil.Run(time.Minute, inst.runscCmd("debug", "-signal=12", inst.name))
-	return true
+	return nil, true
 }
 
 func init() {

vm/isolated/isolated.go

@@ -303,8 +303,8 @@ func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command strin
 	return vmimpl.Multiplex(cmd, merger, dmesg, timeout, stop, inst.closed, inst.debug)
 }
 
-func (inst *instance) Diagnose() bool {
-	return false
+func (inst *instance) Diagnose() ([]byte, bool) {
+	return nil, false
 }
 
 func splitTargetPort(addr string) (string, int, error) {

vm/kvm/kvm.go

@@ -287,8 +287,8 @@ func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command strin
 	return outputC, errorC, nil
 }
 
-func (inst *instance) Diagnose() bool {
-	return false
+func (inst *instance) Diagnose() ([]byte, bool) {
+	return nil, false
 }
 
 const script = `#! /bin/bash

vm/qemu/qemu.go

@@ -512,12 +512,12 @@ func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command strin
 	return inst.merger.Output, errc, nil
 }
 
-func (inst *instance) Diagnose() bool {
+func (inst *instance) Diagnose() ([]byte, bool) {
 	select {
 	case inst.diagnose <- true:
 	default:
 	}
-	return false
+	return nil, false
 }
 
 // nolint: lll

vm/vm.go

@@ -126,7 +126,7 @@ func (inst *Instance) Run(timeout time.Duration, stop <-chan bool, command strin
 	return inst.impl.Run(timeout, stop, command)
 }
 
-func (inst *Instance) Diagnose() bool {
+func (inst *Instance) Diagnose() ([]byte, bool) {
 	return inst.impl.Diagnose()
 }
 
@@ -204,7 +204,12 @@ func (inst *Instance) MonitorExecution(outc <-chan []byte, errc <-chan error,
 			if time.Since(lastExecuteTime) < noOutputTimeout {
 				break
 			}
-			if inst.Diagnose() {
+			diag, wait := inst.Diagnose()
+			if len(diag) > 0 {
+				mon.output = append(mon.output, "DIAGNOSIS:\n"...)
+				mon.output = append(mon.output, diag...)
+			}
+			if wait {
 				mon.waitForOutput()
 			}
 			rep := &report.Report{
@@ -232,7 +237,12 @@ type monitor struct {
 func (mon *monitor) extractError(defaultError string) *report.Report {
 	crashed := defaultError != "" || !mon.canExit
 	if crashed {
-		mon.inst.Diagnose()
+		// N.B. we always wait below for other errors.
+		diag, _ := mon.inst.Diagnose()
+		if len(diag) > 0 {
+			mon.output = append(mon.output, "DIAGNOSIS:\n"...)
+			mon.output = append(mon.output, diag...)
+		}
 	}
 	// Give it some time to finish writing the error message.
 	mon.waitForOutput()
@@ -253,8 +263,15 @@ func (mon *monitor) extractError(defaultError string) *report.Report {
 		}
 		return rep
 	}
-	if !crashed && mon.inst.Diagnose() {
-		mon.waitForOutput()
+	if !crashed {
+		diag, wait := mon.inst.Diagnose()
+		if len(diag) > 0 {
+			mon.output = append(mon.output, "DIAGNOSIS:\n"...)
+			mon.output = append(mon.output, diag...)
+		}
+		if wait {
+			mon.waitForOutput()
+		}
 	}
 	rep := mon.reporter.Parse(mon.output[mon.matchPos:])
 	if rep == nil {

vm/vm_test.go

@@ -31,9 +31,10 @@ func (pool *testPool) Create(workdir string, index int) (vmimpl.Instance, error)
 }
 
 type testInstance struct {
-	outc        chan []byte
-	errc        chan error
-	diagnoseBug bool
+	outc           chan []byte
+	errc           chan error
+	diagnoseBug    bool
+	diagnoseNoWait bool
 }
 
 func (inst *testInstance) Copy(hostSrc string) (string, error) {
@@ -49,13 +50,20 @@ func (inst *testInstance) Run(timeout time.Duration, stop <-chan bool, command s
 	return inst.outc, inst.errc, nil
 }
 
-func (inst *testInstance) Diagnose() bool {
+func (inst *testInstance) Diagnose() ([]byte, bool) {
+	var diag []byte
 	if inst.diagnoseBug {
-		inst.outc <- []byte("BUG: DIAGNOSE\n")
+		diag = []byte("BUG: DIAGNOSE\n")
 	} else {
-		inst.outc <- []byte("DIAGNOSE\n")
+		diag = []byte("DIAGNOSE\n")
 	}
-	return true
+
+	if inst.diagnoseNoWait {
+		return diag, false
+	}
+
+	inst.outc <- diag
+	return nil, true
 }
 
 func (inst *testInstance) Close() {
@@ -74,11 +82,12 @@ func init() {
 }
 
 type Test struct {
-	Name        string
-	CanExit     bool // if the program is allowed to exit normally
-	DiagnoseBug bool // Diagnose produces output that is detected as kernel crash
-	Body        func(outc chan []byte, errc chan error)
-	Report      *report.Report
+	Name           string
+	CanExit        bool // if the program is allowed to exit normally
+	DiagnoseBug    bool // Diagnose produces output that is detected as kernel crash
+	DiagnoseNoWait bool // Diagnose returns output directly rather than to console
+	Body           func(outc chan []byte, errc chan error)
+	Report         *report.Report
 }
 
 var tests = []*Test{
@@ -120,6 +129,19 @@ var tests = []*Test{
 			),
 		},
 	},
+	{
+		Name: "diagnose-no-wait",
+		Body: func(outc chan []byte, errc chan error) {
+			errc <- nil
+		},
+		DiagnoseNoWait: true,
+		Report: &report.Report{
+			Title: lostConnectionCrash,
+			Output: []byte(
+				"DIAGNOSIS:\nDIAGNOSE\n",
+			),
+		},
+	},
 	{
 		Name: "kernel-crashes",
 		Body: func(outc chan []byte, errc chan error) {
@@ -280,6 +302,7 @@ func testMonitorExecution(t *testing.T, test *Test) {
 	}
 	testInst := inst.impl.(*testInstance)
 	testInst.diagnoseBug = test.DiagnoseBug
+	testInst.diagnoseNoWait = test.DiagnoseNoWait
 	done := make(chan bool)
 	go func() {
 		test.Body(testInst.outc, testInst.errc)

vm/vmimpl/vmimpl.go

@@ -43,10 +43,13 @@ type Instance interface {
 	// Command is terminated after timeout. Send on the stop chan can be used to terminate it earlier.
 	Run(timeout time.Duration, stop <-chan bool, command string) (outc <-chan []byte, errc <-chan error, err error)
 
-	// Diagnose forces VM to dump additional debugging info
-	// (e.g. sending some sys-rq's or SIGABORT'ing a Go program).
-	// Returns true if it did anything.
-	Diagnose() bool
+	// Diagnose retrieves additional debugging info from the VM (e.g. by
+	// sending some sys-rq's or SIGABORT'ing a Go program).
+	//
+	// Optionally returns (some or all) of the info directly. If wait ==
+	// true, the caller must wait for the VM to output info directly to its
+	// log.
+	Diagnose() (diagnosis []byte, wait bool)
 
 	// Close stops and destroys the VM.
 	Close()

vm/vmm/vmm.go

@@ -310,8 +310,8 @@ func (inst *instance) Run(timeout time.Duration, stop <-chan bool, command strin
 	return inst.merger.Output, errc, nil
 }
 
-func (inst *instance) Diagnose() bool {
-	return vmimpl.DiagnoseOpenBSD(inst.consolew)
+func (inst *instance) Diagnose() ([]byte, bool) {
+	return nil, vmimpl.DiagnoseOpenBSD(inst.consolew)
 }
 
 // Run the given vmctl(8) command and wait for it to finish.