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 {
+func (inst *instance) Diagnose() ([]byte, bool) {
-	return false
+	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 {
+func (inst *instance) Diagnose() ([]byte, bool) {
-	return false
+	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 {
+func (inst *instance) Diagnose() ([]byte, bool) {
-	return false
+	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,9 +263,16 @@ func (mon *monitor) extractError(defaultError string) *report.Report {
 		}
 		return rep
 	}
-	if !crashed && mon.inst.Diagnose() {
+	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 {
 		panic(fmt.Sprintf("reporter.ContainsCrash/Parse disagree:\n%s", mon.output[mon.matchPos:]))

vm/vm_test.go

@@ -34,6 +34,7 @@ type testInstance struct {
 	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() {
@@ -77,6 +85,7 @@ 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
+	DiagnoseNoWait bool // Diagnose returns output directly rather than to console
 	Body           func(outc chan []byte, errc chan error)
 	Report         *report.Report
 }
@@ -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
+	// Diagnose retrieves additional debugging info from the VM (e.g. by
-	// (e.g. sending some sys-rq's or SIGABORT'ing a Go program).
+	// sending some sys-rq's or SIGABORT'ing a Go program).
-	// Returns true if it did anything.
+	//
-	Diagnose() bool
+	// 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 {
+func (inst *instance) Diagnose() ([]byte, bool) {
-	return vmimpl.DiagnoseOpenBSD(inst.consolew)
+	return nil, vmimpl.DiagnoseOpenBSD(inst.consolew)
 }
 
 // Run the given vmctl(8) command and wait for it to finish.