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

doc/usage.md

@@ -279,11 +279,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` and `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
 
@@ -298,6 +299,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

@@ -8,6 +8,7 @@ require (
 	github.com/containers/common v0.60.4
 	github.com/crc-org/crc/v2 v2.44.0
 	github.com/gin-gonic/gin v1.10.0
+	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.1

go.sum

@@ -87,6 +87,8 @@ github.com/pelletier/go-toml/v2 v2.2.2 h1:aYUidT7k73Pcl9nb2gScu7NSrKCSHIDE89b3+6
 github.com/pelletier/go-toml/v2 v2.2.2/go.mod h1:1t835xjRzz80PqgE6HHgN2JOsmgYu/h4qDAS4n929Rs=
 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=
@@ -145,6 +147,7 @@ golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
 golang.org/x/net v0.31.0 h1:68CPQngjLL0r2AlUKiSxtQFKvzRVbnzLwMUn5SzcLHo=
 golang.org/x/net v0.31.0/go.mod h1:P4fl1q7dY2hnZFxEk4pPSkDHF+QqjitcnDjUQyMM+pM=
 golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200909081042-eff7692f9009/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

pkg/config/json_test.go

@@ -246,7 +246,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

@@ -107,6 +107,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"`
 }
 
 type NBDSynchronizationMode string
@@ -214,12 +218,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
@@ -229,11 +245,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 {
@@ -246,6 +267,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

@@ -128,6 +128,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/rest/rest.go

@@ -4,10 +4,12 @@ import (
 	"errors"
 	"fmt"
 	"net/url"
+	"os"
 	"strings"
 	"syscall"
 
 	"github.com/crc-org/vfkit/pkg/cmdline"
+	"github.com/crc-org/vfkit/pkg/util"
 	"github.com/gin-gonic/gin"
 	"github.com/sirupsen/logrus"
 )
@@ -73,6 +75,7 @@ func (v *VFKitService) Start() {
 		case TCP:
 			err = v.router.Run(v.Host)
 		case Unix:
+			util.RegisterExitHandler(func() { os.Remove(v.Path) })
 			err = v.router.RunUnix(v.Path)
 		}
 		logrus.Fatal(err)

pkg/vf/virtio.go

@@ -6,13 +6,14 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"syscall"
 
 	"github.com/crc-org/vfkit/pkg/config"
+	"github.com/crc-org/vfkit/pkg/util"
+	"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"
 )
 
 // vf will define toVZ() and AddToVirtualMachineConfig() methods on these types
@@ -230,39 +231,63 @@ func unixFd(fd uintptr) int {
 // https://developer.apple.com/documentation/virtualization/running_linux_in_a_virtual_machine#3880009
 func setRawMode(f *os.File) error {
 	// Get settings for terminal
-	attr, _ := unix.IoctlGetTermios(unixFd(f.Fd()), unix.TIOCGETA)
+	var attr unix.Termios
+	if err := termios.Tcgetattr(f.Fd(), &attr); err != nil {
+		return err
+	}
 
 	// Put stdin into raw mode, disabling local echo, input canonicalization,
 	// and CR-NL mapping.
-	attr.Iflag &^= syscall.ICRNL
-	attr.Lflag &^= syscall.ICANON | syscall.ECHO
-
-	// Set minimum characters when reading = 1 char
-	attr.Cc[syscall.VMIN] = 1
-
-	// set timeout when reading as non-canonical mode
-	attr.Cc[syscall.VTIME] = 0
+	attr.Iflag &^= unix.ICRNL
+	attr.Lflag &^= unix.ICANON | unix.ECHO
 
 	// reflects the changed settings
-	return unix.IoctlSetTermios(unixFd(f.Fd()), unix.TIOCSETA, attr)
+	return termios.Tcsetattr(f.Fd(), termios.TCSANOW, &attr)
 }
 
 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)
+	default:
+		serialPortAttachment, retErr = vz.NewFileSerialPortAttachment(dev.LogFile, false)
 	}
+	if retErr != nil {
+		return nil, retErr
+	}
+
+	return vz.NewVirtioConsoleDeviceSerialPortConfiguration(serialPortAttachment)
+}
+
+func (dev *VirtioSerial) toVzConsole() (*vz.VirtioConsolePortConfiguration, error) {
+	master, slave, err := termios.Pty()
 	if err != nil {
 		return nil, err
 	}
 
-	return vz.NewVirtioConsoleDeviceSerialPortConfiguration(serialPortAttachment)
+	// the master fd and slave fd must stay open for vfkit's lifetime
+	util.RegisterExitHandler(func() {
+		_ = master.Close()
+		_ = slave.Close()
+	})
+
+	dev.PtyName = slave.Name()
+
+	if err := setRawMode(master); err != nil {
+		return nil, err
+	}
+	serialPortAttachment, retErr := vz.NewFileHandleSerialPortAttachment(master, master)
+	if retErr != nil {
+		return nil, retErr
+	}
+	return vz.NewVirtioConsolePortConfiguration(
+		vz.WithVirtioConsolePortConfigurationAttachment(serialPortAttachment),
+		vz.WithVirtioConsolePortConfigurationIsConsole(true))
 }
 
 func (dev *VirtioSerial) AddToVirtualMachineConfig(vmConfig *VirtualMachineConfiguration) error {
@@ -272,12 +297,24 @@ 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 {
+		consolePortConfig, err := dev.toVzConsole()
+		if err != nil {
+			return err
+		}
+		vmConfig.consolePortsConfiguration = append(vmConfig.consolePortsConfiguration, consolePortConfig)
+		log.Infof("Using PTY (pty path: %s)", dev.PtyName)
+	} else {
+		consoleConfig, err := dev.toVz()
+		if err != nil {
+			return err
+		}
+		vmConfig.serialPortsConfiguration = append(vmConfig.serialPortsConfiguration, consoleConfig)
 	}
-	vmConfig.serialPortsConfiguration = append(vmConfig.serialPortsConfiguration, consoleConfig)
 
 	return nil
 }

pkg/vf/vm.go

@@ -85,6 +85,7 @@ type VirtualMachineConfiguration struct {
 	entropyDevicesConfiguration          []*vz.VirtioEntropyDeviceConfiguration
 	serialPortsConfiguration             []*vz.VirtioConsoleDeviceSerialPortConfiguration
 	socketDevicesConfiguration           []vz.SocketDeviceConfiguration
+	consolePortsConfiguration            []*vz.VirtioConsolePortConfiguration
 }
 
 func NewVirtualMachineConfiguration(vmConfig *config.VirtualMachine) (*VirtualMachineConfiguration, error) {
@@ -129,6 +130,18 @@ func (cfg *VirtualMachineConfiguration) toVz() (*vz.VirtualMachineConfiguration,
 	cfg.SetNetworkDevicesVirtualMachineConfiguration(cfg.networkDevicesConfiguration)
 	cfg.SetEntropyDevicesVirtualMachineConfiguration(cfg.entropyDevicesConfiguration)
 	cfg.SetSerialPortsVirtualMachineConfiguration(cfg.serialPortsConfiguration)
+
+	if len(cfg.consolePortsConfiguration) > 0 {
+		consoleDeviceConfiguration, err := vz.NewVirtioConsoleDeviceConfiguration()
+		if err != nil {
+			return nil, err
+		}
+		for i, portCfg := range cfg.consolePortsConfiguration {
+			consoleDeviceConfiguration.SetVirtioConsolePortConfiguration(i, portCfg)
+		}
+		cfg.SetConsoleDevicesVirtualMachineConfiguration([]vz.ConsoleDeviceConfiguration{consoleDeviceConfiguration})
+	}
+
 	// len(cfg.socketDevicesConfiguration should be 0 or 1
 	// https://developer.apple.com/documentation/virtualization/vzvirtiosocketdeviceconfiguration?language=objc
 	cfg.SetSocketDevicesVirtualMachineConfiguration(cfg.socketDevicesConfiguration)

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")