serial: Add --device virtio-serial,pty support

This new virtio-serial option allocates a pseudo-tty for the VM console.
It can then be accessed using `screen` for example.
This is a bit similar to the `--device virtio-serial,stdio` option,
except that the console is not tied to the terminal running vfkit, it's
possible to connect/disconnect from the pseudo-tty from any terminal.

This fixes crc-org#48

Signed-off-by: Christophe Fergeau <[email protected]>

doc/usage.md

@@ -225,11 +225,12 @@ This is useful in combination with usermode networking stacks such as [gvisor-ta
 #### Description
 
 The `--device virtio-serial` option adds a serial device to the virtual machine. This is useful to redirect text output from the virtual machine to a log file.
-The `logFilePath` and `stdio` arguments are mutually exclusive.
+The `logFilePath`, `stdio`, `pty` arguments are mutually exclusive.
 
 #### Arguments
 - `logFilePath`: path where the serial port output should be written.
 - `stdio`: uses stdin/stdout for the serial console input/output.
+- `pty`: allocates a pseudo-terminal for the serial console input/output.
 
 #### Example
 
@@ -244,6 +245,18 @@ launched from will be used as an interactive serial console for that device:
 --device virtio-serial,stdio
 ```
 
+This adds a virtio-serial device to the VM, and creates a pseudo-terminal for
+the console for that device:
+```
+--device virtio-serial,pty
+```
+Once the VM is running, you can connect to its console with:
+```
+screen /dev/ttys002
+```
+`/dev/ttys002` will vary between `vfkit` runs.
+The `/dev/ttys???` path to the pty is printed during vfkit startup.
+It's also available through the `/vm/inspect` endpoint of [REST API](#restful-service) in the `ptyName` field of the `virtio-serial` device.
 
 ### Random Number Generator
 

go.mod

@@ -9,6 +9,7 @@ require (
 	github.com/crc-org/crc/v2 v2.33.0
 	github.com/gin-gonic/gin v1.9.1
 	github.com/onsi/gocleanup v0.0.0-20140331211545-c1a5478700b5
+	github.com/pkg/term v1.1.0
 	github.com/prashantgupta24/mac-sleep-notifier v1.0.1
 	github.com/sirupsen/logrus v1.9.3
 	github.com/spf13/cobra v1.8.0

go.sum

@@ -116,6 +116,8 @@ github.com/pelletier/go-toml/v2 v2.1.1 h1:LWAJwfNvjQZCFIDKWYQaM62NcYeYViCmWIwmOS
 github.com/pelletier/go-toml/v2 v2.1.1/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+github.com/pkg/term v1.1.0 h1:xIAAdCMh3QIAy+5FrE8Ad8XoDhEU4ufwbaSozViP9kk=
+github.com/pkg/term v1.1.0/go.mod h1:E25nymQcrSllhX42Ok8MRm1+hyBdHY0dCeiKZ9jpNGw=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
 github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
@@ -195,6 +197,7 @@ golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200909081042-eff7692f9009/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

pkg/config/json_test.go

@@ -232,7 +232,7 @@ var jsonStabilityTests = map[string]jsonStabilityTest{
 	},
 	"VirtioSerial": {
 		obj:          &VirtioSerial{},
-		expectedJSON: `{"kind":"virtioserial","logFile":"LogFile","usesStdio":true}`,
+		expectedJSON: `{"kind":"virtioserial","logFile":"LogFile","ptyName":"PtyName","usesPty":true,"usesStdio":true}`,
 	},
 	"VirtioVsock": {
 		obj:          &VirtioVsock{},

pkg/config/virtio.go

@@ -104,6 +104,10 @@ type VirtioNet struct {
 type VirtioSerial struct {
 	LogFile   string `json:"logFile,omitempty"`
 	UsesStdio bool   `json:"usesStdio,omitempty"`
+	UsesPty   bool   `json:"usesPty,omitempty"`
+	// PtyName must not be set when creating the VM, from a user perspective, it's read-only,
+	// vfkit will set it during VM startup.
+	PtyName string `json:"ptyName,omitempty"`
 }
 
 // TODO: Add VirtioBalloon
@@ -195,12 +199,24 @@ func VirtioSerialNewStdio() (VirtioDevice, error) {
 	}, nil
 }
 
+func VirtioSerialNewPty() (VirtioDevice, error) {
+	return &VirtioSerial{
+		UsesPty: true,
+	}, nil
+}
+
 func (dev *VirtioSerial) validate() error {
 	if dev.LogFile != "" && dev.UsesStdio {
 		return fmt.Errorf("'logFilePath' and 'stdio' cannot be set at the same time")
 	}
-	if dev.LogFile == "" && !dev.UsesStdio {
-		return fmt.Errorf("one of 'logFilePath' or 'stdio' must be set")
+	if dev.LogFile != "" && dev.UsesPty {
+		return fmt.Errorf("'logFilePath' and 'pty' cannot be set at the same time")
+	}
+	if dev.UsesStdio && dev.UsesPty {
+		return fmt.Errorf("'stdio' and 'pty' cannot be set at the same time")
+	}
+	if dev.LogFile == "" && !dev.UsesStdio && !dev.UsesPty {
+		return fmt.Errorf("one of 'logFilePath', 'stdio' or 'pty' must be set")
 	}
 
 	return nil
@@ -210,11 +226,16 @@ func (dev *VirtioSerial) ToCmdLine() ([]string, error) {
 	if err := dev.validate(); err != nil {
 		return nil, err
 	}
-	if dev.UsesStdio {
+	switch {
+	case dev.UsesStdio:
 		return []string{"--device", "virtio-serial,stdio"}, nil
+	case dev.UsesPty:
+		return []string{"--device", "virtio-serial,pty"}, nil
+	case dev.LogFile != "":
+		fallthrough
+	default:
+		return []string{"--device", fmt.Sprintf("virtio-serial,logFilePath=%s", dev.LogFile)}, nil
 	}
-
-	return []string{"--device", fmt.Sprintf("virtio-serial,logFilePath=%s", dev.LogFile)}, nil
 }
 
 func (dev *VirtioSerial) FromOptions(options []option) error {
@@ -227,6 +248,8 @@ func (dev *VirtioSerial) FromOptions(options []option) error {
 				return fmt.Errorf("unexpected value for virtio-serial 'stdio' option: %s", option.value)
 			}
 			dev.UsesStdio = true
+		case "pty":
+			dev.UsesPty = true
 		default:
 			return fmt.Errorf("unknown option for virtio-serial devices: %s", option.key)
 		}

pkg/config/virtio_test.go

@@ -120,6 +120,13 @@ var virtioDevTests = map[string]virtioDevTest{
 		},
 		expectedCmdLine: []string{"--device", "virtio-serial,stdio"},
 	},
+	"NewVirtioSerialPty": {
+		newDev: VirtioSerialNewPty,
+		expectedDev: &VirtioSerial{
+			UsesPty: true,
+		},
+		expectedCmdLine: []string{"--device", "virtio-serial,pty"},
+	},
 	"NewVirtioNet": {
 		newDev: func() (VirtioDevice, error) { return VirtioNetNew("") },
 		expectedDev: &VirtioNet{

pkg/vf/virtio.go

@@ -7,10 +7,12 @@ import (
 	"syscall"
 
 	"github.com/crc-org/vfkit/pkg/config"
+	"github.com/onsi/gocleanup"
+	"golang.org/x/sys/unix"
 
 	"github.com/Code-Hex/vz/v3"
+	"github.com/pkg/term/termios"
 	log "github.com/sirupsen/logrus"
-	"golang.org/x/sys/unix"
 )
 
 type RosettaShare config.RosettaShare
@@ -221,17 +223,38 @@ func setRawMode(f *os.File) error {
 
 func (dev *VirtioSerial) toVz() (*vz.VirtioConsoleDeviceSerialPortConfiguration, error) {
 	var serialPortAttachment vz.SerialPortAttachment
-	var err error
-	if dev.UsesStdio {
+	var retErr error
+	switch {
+	case dev.UsesStdio:
 		if err := setRawMode(os.Stdin); err != nil {
 			return nil, err
 		}
-		serialPortAttachment, err = vz.NewFileHandleSerialPortAttachment(os.Stdin, os.Stdout)
-	} else {
-		serialPortAttachment, err = vz.NewFileSerialPortAttachment(dev.LogFile, false)
+		serialPortAttachment, retErr = vz.NewFileHandleSerialPortAttachment(os.Stdin, os.Stdout)
+	case dev.UsesPty:
+		master, slave, err := termios.Pty()
+		if err != nil {
+			return nil, err
+		}
+		// as far as I can tell, we have no use for the slave fd in the
+		// vfkit process, the user will open minicom/screen/... /dev/ttys00?
+		// when needed
+		defer slave.Close()
+
+		// the master fd must stay open for vfkit's lifetime
+		gocleanup.Register(func() { _ = master.Close() })
+
+		dev.PtyName = slave.Name()
+
+		if err := setRawMode(master); err != nil {
+			return nil, err
+		}
+		serialPortAttachment, retErr = vz.NewFileHandleSerialPortAttachment(master, master)
+
+	default:
+		serialPortAttachment, retErr = vz.NewFileSerialPortAttachment(dev.LogFile, false)
 	}
-	if err != nil {
-		return nil, err
+	if retErr != nil {
+		return nil, retErr
 	}
 
 	return vz.NewVirtioConsoleDeviceSerialPortConfiguration(serialPortAttachment)
@@ -244,11 +267,17 @@ func (dev *VirtioSerial) AddToVirtualMachineConfig(vmConfig *VirtualMachineConfi
 	if dev.UsesStdio {
 		log.Infof("Adding stdio console")
 	}
+	if dev.PtyName != "" {
+		return fmt.Errorf("VirtioSerial.PtyName must be empty (current value: %s)", dev.PtyName)
+	}
 
 	consoleConfig, err := dev.toVz()
 	if err != nil {
 		return err
 	}
+	if dev.UsesPty {
+		log.Infof("Using PTY (pty path: %s)", dev.PtyName)
+	}
 	vmConfig.serialPortsConfiguration = append(vmConfig.serialPortsConfiguration, consoleConfig)
 
 	return nil

test/vm_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/crc-org/vfkit/pkg/config"
 
 	log "github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -272,7 +273,7 @@ var pciidVersionedTests = map[int]map[string]pciidTest{
 	14: pciidMacOS14Tests,
 }
 
-func checkRestDevices(t *testing.T, vm *testVM) {
+func restInspect(t *testing.T, vm *testVM) *config.VirtualMachine {
 	tr := &http.Transport{
 		Dial: func(_, _ string) (conn net.Conn, err error) {
 			return net.Dial("unix", vm.restSocketPath)
@@ -287,7 +288,7 @@ func checkRestDevices(t *testing.T, vm *testVM) {
 	var unmarshalledVM config.VirtualMachine
 	err = json.Unmarshal(body, &unmarshalledVM)
 	require.NoError(t, err)
-	require.Equal(t, vm.config, &unmarshalledVM)
+	return &unmarshalledVM
 }
 
 func testPCIId(t *testing.T, test pciidTest, provider OsProvider) {
@@ -303,7 +304,9 @@ func testPCIId(t *testing.T, test pciidTest, provider OsProvider) {
 	vm.Start(t)
 	vm.WaitForSSH(t)
 	checkPCIDevice(t, vm, test.vendorID, test.deviceID)
-	checkRestDevices(t, vm)
+
+	unmarshalledVM := restInspect(t, vm)
+	require.Equal(t, vm.config, unmarshalledVM)
 
 	vm.Stop(t)
 }
@@ -334,6 +337,39 @@ func TestPCIIds(t *testing.T) {
 	}
 }
 
+func TestVirtioSerialPTY(t *testing.T) {
+	puipuiProvider := NewPuipuiProvider()
+	log.Info("fetching os image")
+	tempDir := t.TempDir()
+	err := puipuiProvider.Fetch(tempDir)
+	require.NoError(t, err)
+
+	vm := NewTestVM(t, puipuiProvider)
+	defer vm.Close(t)
+	require.NotNil(t, vm)
+
+	vm.AddSSH(t, "tcp")
+	dev, err := config.VirtioSerialNewPty()
+	require.NoError(t, err)
+	vm.AddDevice(t, dev)
+
+	vm.Start(t)
+	vm.WaitForSSH(t)
+	runtimeVM := restInspect(t, vm)
+	var foundVirtioSerial bool
+	for _, dev := range runtimeVM.Devices {
+		runtimeDev, ok := dev.(*config.VirtioSerial)
+		if ok {
+			assert.NotEmpty(t, runtimeDev.PtyName)
+			foundVirtioSerial = true
+			break
+		}
+	}
+	require.True(t, foundVirtioSerial)
+
+	vm.Stop(t)
+}
+
 func checkPCIDevice(t *testing.T, vm *testVM, vendorID, deviceID int) {
 	re := regexp.MustCompile(fmt.Sprintf("(?m)[[:blank:]]%04x:%04x\n", vendorID, deviceID))
 	lspci, err := vm.SSHCombinedOutput(t, "lspci")