From 9d53b9479423df4f422fcf1cf6f2a8398c77cabc Mon Sep 17 00:00:00 2001 From: David Scott Date: Thu, 23 Jul 2020 14:05:54 +0100 Subject: [PATCH 1/4] go: initialise go mod This allows a native `go build` and `go test`. Signed-off-by: David Scott --- go.mod | 8 ++++++++ go.sum | 4 ++++ 2 files changed, 12 insertions(+) create mode 100644 go.mod create mode 100644 go.sum diff --git a/go.mod b/go.mod new file mode 100644 index 00000000..9e761f04 --- /dev/null +++ b/go.mod @@ -0,0 +1,8 @@ +module github.com/moby/hyperkit + +go 1.14 + +require ( + github.com/mitchellh/go-ps v1.0.0 + golang.org/x/sys v0.0.0-20200722175500-76b94024e4b6 +) diff --git a/go.sum b/go.sum new file mode 100644 index 00000000..fe2853dc --- /dev/null +++ b/go.sum @@ -0,0 +1,4 @@ +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= +golang.org/x/sys v0.0.0-20200722175500-76b94024e4b6 h1:X9xIZ1YU8bLZA3l6gqDUHSFiD0GFI9S548h6C8nDtOY= +golang.org/x/sys v0.0.0-20200722175500-76b94024e4b6/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= From c0e53d4c091d0ba439191c4a5966cfc77c69794f Mon Sep 17 00:00:00 2001 From: David Scott Date: Thu, 23 Jul 2020 14:04:09 +0100 Subject: [PATCH 2/4] go: add a new API for configuring serial ports The previous API didn't allow us to disable the PTY but keep the ring buffer, for example. These should be orthogonal, so add a new API and deprecate the old one. Signed-off-by: David Scott --- go/hyperkit.go | 128 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 106 insertions(+), 22 deletions(-) diff --git a/go/hyperkit.go b/go/hyperkit.go index 10263374..8c286b40 100644 --- a/go/hyperkit.go +++ b/go/hyperkit.go @@ -40,11 +40,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 +120,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 +140,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 +222,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 +233,41 @@ 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 { + for _, 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 && !isTerminal(os.Stdout) { + return fmt.Errorf("If StdioInteractiveConsole is set, stdio must be a TTY") + } + 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 +422,42 @@ 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 StdioInteractiveConsole: + cfg += fmt.Sprintf(",stdio") + case TTYInteractiveConsole: + cfg += fmt.Sprintf(",autopty=%s/tty", h.StateDir) + } + 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 +522,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 == "" { From 02dfe99f457fdeab1e7eddf32108c4e45dfe0180 Mon Sep 17 00:00:00 2001 From: David Scott Date: Thu, 23 Jul 2020 14:12:28 +0100 Subject: [PATCH 3/4] go: add unit tests for the hyperkit cmdline Signed-off-by: David Scott --- go.mod | 1 + go.sum | 10 ++++++++++ go/hyperkit_test.go | 31 +++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) create mode 100644 go/hyperkit_test.go diff --git a/go.mod b/go.mod index 9e761f04..7c4ed5d2 100644 --- a/go.mod +++ b/go.mod @@ -4,5 +4,6 @@ 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 ) diff --git a/go.sum b/go.sum index fe2853dc..383b35b8 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +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= diff --git a/go/hyperkit_test.go b/go/hyperkit_test.go new file mode 100644 index 00000000..80ad8b7f --- /dev/null +++ b/go/hyperkit_test.go @@ -0,0 +1,31 @@ +package hyperkit + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLegacyConsole(t *testing.T) { + h, err := New("hyperkit", "", "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("hyperkit", "", "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/tty,log=state-dir/console-ring", "-f", "kexec,,,earlyprintk=serial "}, h.Arguments) +} From 04d209cccb69c987f2c61512fbd5e455a3fd2a35 Mon Sep 17 00:00:00 2001 From: David Scott Date: Thu, 23 Jul 2020 11:54:06 +0100 Subject: [PATCH 4/4] serial: allow the device to be set to `null` It's useful to be able to log the serial console output to a ring buffer (or even ASL) without necessarily having a real host console attached to it (i.e. no TTY or `stdio`). The code requires an open file descriptor so add a new type `null` which opens `/dev/null`. Signed-off-by: David Scott --- go/hyperkit.go | 45 ++++++++++++++++++++++++++++++++++++--------- go/hyperkit_test.go | 19 ++++++++++++++++--- src/lib/uart_emul.c | 10 +++++++++- 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/go/hyperkit.go b/go/hyperkit.go index 8c286b40..5352ee46 100644 --- a/go/hyperkit.go +++ b/go/hyperkit.go @@ -31,6 +31,7 @@ import ( "os" "os/exec" "os/user" + "path" "path/filepath" "strconv" "strings" @@ -237,12 +238,19 @@ func (h *HyperKit) checkLegacyConsole() error { } func (h *HyperKit) checkSerials() error { - for _, serial := range h.Serials { + 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 && !isTerminal(os.Stdout) { - return fmt.Errorf("If StdioInteractiveConsole is set, stdio must be a TTY") + 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 ") @@ -442,10 +450,12 @@ func (h *HyperKit) serialArgs() []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", h.StateDir) + cfg += fmt.Sprintf(",autopty=%s/tty%d", h.StateDir, i+1) } if serial.LogToASL { cfg += fmt.Sprintf(",asl") @@ -543,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) @@ -558,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) { @@ -580,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 { diff --git a/go/hyperkit_test.go b/go/hyperkit_test.go index 80ad8b7f..8dd3ffa2 100644 --- a/go/hyperkit_test.go +++ b/go/hyperkit_test.go @@ -8,7 +8,7 @@ import ( ) func TestLegacyConsole(t *testing.T) { - h, err := New("hyperkit", "", "state-dir") + h, err := New("sh", "", "state-dir") require.Nil(t, err) h.Console = ConsoleFile @@ -17,7 +17,7 @@ func TestLegacyConsole(t *testing.T) { } func TestNewSerial(t *testing.T) { - h, err := New("hyperkit", "", "state-dir") + h, err := New("sh", "", "state-dir") require.Nil(t, err) h.Serials = []Serial{ @@ -27,5 +27,18 @@ func TestNewSerial(t *testing.T) { }, } 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) + 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) } diff --git a/src/lib/uart_emul.c b/src/lib/uart_emul.c index e4ab0cc8..7be69a15 100644 --- a/src/lib/uart_emul.c +++ b/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;