Merge pull request #292 from djs55/improve-serial-console

go: improve serial console

go.mod

@@ -0,0 +1,9 @@
+module github.com/moby/hyperkit
+
+go 1.14
+
+require (
+	github.com/mitchellh/go-ps v1.0.0
+	github.com/stretchr/testify v1.6.1
+	golang.org/x/sys v0.0.0-20200722175500-76b94024e4b6
+)

go.sum

@@ -0,0 +1,14 @@
+github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
+github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/mitchellh/go-ps v1.0.0 h1:i6ampVEEF4wQFF+bkYfwYgY+F/uYJDktmvLPf7qIgjc=
+github.com/mitchellh/go-ps v1.0.0/go.mod h1:J4lOc8z8yJs6vUwklHw2XEIiT4z4C40KtWVN3nvg8Pg=
+github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
+github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+golang.org/x/sys v0.0.0-20200722175500-76b94024e4b6 h1:X9xIZ1YU8bLZA3l6gqDUHSFiD0GFI9S548h6C8nDtOY=
+golang.org/x/sys v0.0.0-20200722175500-76b94024e4b6/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

go/hyperkit.go

@@ -31,6 +31,7 @@ import (
 	"os"
 	"os/exec"
 	"os/user"
+	"path"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -40,11 +41,11 @@ import (
 )
 
 const (
-	// ConsoleStdio configures console to use Stdio
+	// ConsoleStdio configures console to use Stdio (deprecated)
 	ConsoleStdio = iota
-	// ConsoleFile configures console to a tty and output to a file
+	// ConsoleFile configures console to a tty and output to a file (deprecated)
 	ConsoleFile
-	// ConsoleLog configures console to a tty and sends its contents to the logs
+	// ConsoleLog configures console to a tty and sends its contents to the logs (deprecated)
 	ConsoleLog
 
 	legacyVPNKitSock  = "Library/Containers/com.docker.docker/Data/s50"
@@ -120,9 +121,13 @@ type HyperKit struct {
 	// Memory is the amount of megabytes of memory for the VM.
 	Memory int `json:"memory"`
 
-	// Console defines where the console of the VM should be connected to.
+	// Console defines where the console of the VM should be connected to. (deprecated)
 	Console int `json:"console"`
 
+	// Serials defines what happens to the I/O on the serial ports. If this is not nil
+	// it overrides the Console setting.
+	Serials []Serial `json:"serials"`
+
 	// Below here are internal members, but they are exported so
 	// that they are written to the state json file, if configured.
 
@@ -136,6 +141,28 @@ type HyperKit struct {
 	process *os.Process
 }
 
+// Serial port.
+type Serial struct {
+	// InteractiveConsole allows a user to connect to a live VM serial console.
+	InteractiveConsole InteractiveConsole
+	// LogToRingBuffer will write console output to a fixed size ring buffer file.
+	LogToRingBuffer bool
+	// LogToASL will write console output to the Apple System Log.
+	LogToASL bool
+}
+
+// InteractiveConsole is an optional interactive VM console.
+type InteractiveConsole int
+
+const (
+	// NoInteractiveConsole disables the interactive console.
+	NoInteractiveConsole = InteractiveConsole(iota)
+	// StdioInteractiveConsole creates a console on stdio.
+	StdioInteractiveConsole
+	// TTYInteractiveConsole creates a console on a TTY.
+	TTYInteractiveConsole
+)
+
 // New creates a template config structure.
 // - If hyperkit can't be found an error is returned.
 // - If vpnkitsock is empty no networking is configured. If it is set
@@ -196,11 +223,7 @@ func (h *HyperKit) Start(cmdline string) (chan error, error) {
 	return errCh, nil
 }
 
-// check validates `h`.  It also creates the disks if needed.
-func (h *HyperKit) check() error {
-	log.Debugf("hyperkit: check %#v", h)
-	var err error
-	// Sanity checks on configuration
+func (h *HyperKit) checkLegacyConsole() error {
 	switch h.Console {
 	case ConsoleFile, ConsoleLog:
 		if h.StateDir == "" {
@@ -211,6 +234,48 @@ func (h *HyperKit) check() error {
 			return fmt.Errorf("If ConsoleStdio is set but stdio is not a terminal, StateDir must be specified")
 		}
 	}
+	return nil
+}
+
+func (h *HyperKit) checkSerials() error {
+	stdioConsole := -1
+	for i, serial := range h.Serials {
+		if serial.LogToRingBuffer && h.StateDir == "" {
+			return fmt.Errorf("If VM is to log to a ring buffer, StateDir must be specified")
+		}
+		if serial.InteractiveConsole == StdioInteractiveConsole {
+			if isTerminal(os.Stdout) {
+				return fmt.Errorf("If StdioInteractiveConsole is set, stdio must be a TTY")
+			}
+			if stdioConsole != -1 {
+				return fmt.Errorf("Only one serial port can be nominated as the stdio interactive console")
+			}
+			stdioConsole = i
+		}
+		if serial.InteractiveConsole == TTYInteractiveConsole && h.StateDir == "" {
+			return fmt.Errorf("If TTYInteractiveConsole is set, StateDir must be specified ")
+		}
+		if serial.LogToRingBuffer && h.StateDir == "" {
+			return fmt.Errorf("If LogToRingBuffer is set, StateDir must be specified")
+		}
+	}
+	return nil
+}
+
+// check validates `h`.  It also creates the disks if needed.
+func (h *HyperKit) check() error {
+	log.Debugf("hyperkit: check %#v", h)
+	var err error
+	// Sanity checks on configuration
+	if h.Serials == nil {
+		if err := h.checkLegacyConsole(); err != nil {
+			return err
+		}
+	} else {
+		if err := h.checkSerials(); err != nil {
+			return err
+		}
+	}
 	for _, image := range h.ISOImages {
 		if _, err = os.Stat(image); os.IsNotExist(err) {
 			return fmt.Errorf("ISO %s does not exist", image)
@@ -365,6 +430,44 @@ func intArrayToString(i []int, sep string) string {
 	return strings.Join(s, sep)
 }
 
+func (h *HyperKit) legacyConsoleArgs() []string {
+	cfg := "com1"
+	if h.Console == ConsoleStdio && isTerminal(os.Stdout) {
+		cfg += fmt.Sprintf(",stdio")
+	} else {
+		cfg += fmt.Sprintf(",autopty=%s/tty", h.StateDir)
+	}
+	if h.Console == ConsoleLog {
+		cfg += fmt.Sprintf(",asl")
+	} else {
+		cfg += fmt.Sprintf(",log=%s/console-ring", h.StateDir)
+	}
+	return []string{"-l", cfg}
+}
+
+func (h *HyperKit) serialArgs() []string {
+	results := []string{}
+	for i, serial := range h.Serials {
+		cfg := fmt.Sprintf("com%d", i+1)
+		switch serial.InteractiveConsole {
+		case NoInteractiveConsole:
+			cfg += ",null"
+		case StdioInteractiveConsole:
+			cfg += fmt.Sprintf(",stdio")
+		case TTYInteractiveConsole:
+			cfg += fmt.Sprintf(",autopty=%s/tty%d", h.StateDir, i+1)
+		}
+		if serial.LogToASL {
+			cfg += fmt.Sprintf(",asl")
+		}
+		if serial.LogToRingBuffer {
+			cfg += fmt.Sprintf(",log=%s/console-ring", h.StateDir)
+		}
+		results = append(results, "-l", cfg)
+	}
+	return results
+}
+
 func (h *HyperKit) buildArgs(cmdline string) {
 	a := []string{"-A", "-u"}
 	if h.StateDir != "" {
@@ -429,19 +532,10 @@ func (h *HyperKit) buildArgs(cmdline string) {
 	}
 
 	// -l: LPC device configuration.
-	{
-		cfg := "com1"
-		if h.Console == ConsoleStdio && isTerminal(os.Stdout) {
-			cfg += fmt.Sprintf(",stdio")
-		} else {
-			cfg += fmt.Sprintf(",autopty=%s/tty", h.StateDir)
-		}
-		if h.Console == ConsoleLog {
-			cfg += fmt.Sprintf(",asl")
-		} else {
-			cfg += fmt.Sprintf(",log=%s/console-ring", h.StateDir)
-		}
-		a = append(a, "-l", cfg)
+	if h.Serials == nil {
+		a = append(a, h.legacyConsoleArgs()...)
+	} else {
+		a = append(a, h.serialArgs()...)
 	}
 
 	if h.Bootrom == "" {
@@ -459,8 +553,8 @@ func (h *HyperKit) buildArgs(cmdline string) {
 }
 
 // openTTY opens the tty files for reading, and returns it.
-func (h *HyperKit) openTTY() *os.File {
-	path := fmt.Sprintf("%s/tty", h.StateDir)
+func (h *HyperKit) openTTY(filename string) *os.File {
+	path := path.Join(h.StateDir, filename)
 	for {
 		if res, err := os.OpenFile(path, os.O_RDONLY, 0); err != nil {
 			log.Infof("hyperkit: openTTY: %v, retrying", err)
@@ -474,6 +568,21 @@ func (h *HyperKit) openTTY() *os.File {
 	}
 }
 
+func (h *HyperKit) findStdioTTY() string {
+	if h.Serials != nil {
+		for i, serial := range h.Serials {
+			if serial.InteractiveConsole == StdioInteractiveConsole {
+				return fmt.Sprintf("tty%d", i)
+			}
+		}
+		return ""
+	}
+	if h.Console == ConsoleStdio {
+		return "tty"
+	}
+	return ""
+}
+
 // execute forges the command to run hyperkit, runs and returns it.
 // It also plumbs stdin/stdout/stderr.
 func (h *HyperKit) execute() (*exec.Cmd, error) {
@@ -496,19 +605,21 @@ func (h *HyperKit) execute() (*exec.Cmd, error) {
 	//
 	// If a logger is specified, use it for stdout/stderr
 	// logging. Otherwise use the default /dev/null.
-	if h.Console == ConsoleStdio {
+	filename := h.findStdioTTY()
+	if filename != "" {
 		if isTerminal(os.Stdout) {
 			cmd.Stdin = os.Stdin
 			cmd.Stdout = os.Stdout
 			cmd.Stderr = os.Stderr
 		} else {
 			go func() {
-				tty := h.openTTY()
+				tty := h.openTTY(filename)
 				defer tty.Close()
 				io.Copy(os.Stdout, tty)
 			}()
 		}
-	} else if log != nil {
+	}
+	if log != nil {
 		log.Debugf("hyperkit: Redirecting stdout/stderr to logger")
 		stdout, err := cmd.StdoutPipe()
 		if err != nil {

go/hyperkit_test.go

@@ -0,0 +1,44 @@
+package hyperkit
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestLegacyConsole(t *testing.T) {
+	h, err := New("sh", "", "state-dir")
+	require.Nil(t, err)
+
+	h.Console = ConsoleFile
+	h.buildArgs("")
+	assert.EqualValues(t, []string{"-A", "-u", "-F", "state-dir/hyperkit.pid", "-c", "1", "-m", "1024M", "-s", "0:0,hostbridge", "-s", "31,lpc", "-s", "1,virtio-rnd", "-l", "com1,autopty=state-dir/tty,log=state-dir/console-ring", "-f", "kexec,,,earlyprintk=serial "}, h.Arguments)
+}
+
+func TestNewSerial(t *testing.T) {
+	h, err := New("sh", "", "state-dir")
+	require.Nil(t, err)
+
+	h.Serials = []Serial{
+		{
+			InteractiveConsole: TTYInteractiveConsole,
+			LogToRingBuffer:    true,
+		},
+	}
+	h.buildArgs("")
+	assert.EqualValues(t, []string{"-A", "-u", "-F", "state-dir/hyperkit.pid", "-c", "1", "-m", "1024M", "-s", "0:0,hostbridge", "-s", "31,lpc", "-s", "1,virtio-rnd", "-l", "com1,autopty=state-dir/tty1,log=state-dir/console-ring", "-f", "kexec,,,earlyprintk=serial "}, h.Arguments)
+}
+
+func TestNullSerial(t *testing.T) {
+	h, err := New("sh", "", "state-dir")
+	require.Nil(t, err)
+
+	h.Serials = []Serial{
+		{
+			LogToRingBuffer: true,
+		},
+	}
+	h.buildArgs("")
+	assert.EqualValues(t, []string{"-A", "-u", "-F", "state-dir/hyperkit.pid", "-c", "1", "-m", "1024M", "-s", "0:0,hostbridge", "-s", "31,lpc", "-s", "1,virtio-rnd", "-l", "com1,null,log=state-dir/console-ring", "-f", "kexec,,,earlyprintk=serial "}, h.Arguments)
+}

src/lib/uart_emul.c

@@ -722,7 +722,15 @@ uart_set_backend(struct uart_softc *sc, const char *backend, const char *devname
 		if (next)
 			next[0] = '\0';
 
-		if (strcmp("stdio", backend) == 0 && !uart_stdio) {
+		if (strcmp("null", backend) == 0) {
+			sc->tty.fd = open("/dev/null", O_RDWR | O_NONBLOCK);
+			if (sc->tty.fd == -1) {
+				fprintf(stderr, "error opening /dev/null\n");
+				goto err;
+			}
+			sc->tty.opened = true;
+			retval = 0;
+		} else if (strcmp("stdio", backend) == 0 && !uart_stdio) {
 			sc->tty.fd = STDIN_FILENO;
 			sc->tty.opened = true;
 			uart_stdio = true;