Merge pull request #1455 from dqminh/epoll-io

tty: move IO of master pty to be done with epoll

Author: Mrunal Patel

contrib/cmd/recvtty/recvtty.go

@@ -24,7 +24,7 @@ import (
 	"os"
 	"strings"
 
-	"github.com/opencontainers/runc/libcontainer"
+	"github.com/containerd/console"
 	"github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/urfave/cli"
 )
@@ -101,24 +101,25 @@ func handleSingle(path string) error {
 	if err != nil {
 		return err
 	}
-	if err = libcontainer.SaneTerminal(master); err != nil {
+	console, err := console.ConsoleFromFile(master)
+	if err != nil {
 		return err
 	}
 
 	// Copy from our stdio to the master fd.
 	quitChan := make(chan struct{})
 	go func() {
-		io.Copy(os.Stdout, master)
+		io.Copy(os.Stdout, console)
 		quitChan <- struct{}{}
 	}()
 	go func() {
-		io.Copy(master, os.Stdin)
+		io.Copy(console, os.Stdin)
 		quitChan <- struct{}{}
 	}()
 
 	// Only close the master fd once we've stopped copying.
 	<-quitChan
-	master.Close()
+	console.Close()
 	return nil
 }
 

libcontainer/console.go

@@ -1,71 +1,14 @@
 package libcontainer
 
 import (
-	"fmt"
 	"os"
-	"unsafe"
 
 	"golang.org/x/sys/unix"
 )
 
-func ConsoleFromFile(f *os.File) Console {
-	return &linuxConsole{
-		master: f,
-	}
-}
-
-// newConsole returns an initialized console that can be used within a container by copying bytes
-// from the master side to the slave that is attached as the tty for the container's init process.
-func newConsole() (Console, error) {
-	master, err := os.OpenFile("/dev/ptmx", unix.O_RDWR|unix.O_NOCTTY|unix.O_CLOEXEC, 0)
-	if err != nil {
-		return nil, err
-	}
-	console, err := ptsname(master)
-	if err != nil {
-		return nil, err
-	}
-	if err := unlockpt(master); err != nil {
-		return nil, err
-	}
-	return &linuxConsole{
-		slavePath: console,
-		master:    master,
-	}, nil
-}
-
-// linuxConsole is a linux pseudo TTY for use within a container.
-type linuxConsole struct {
-	master    *os.File
-	slavePath string
-}
-
-func (c *linuxConsole) File() *os.File {
-	return c.master
-}
-
-func (c *linuxConsole) Path() string {
-	return c.slavePath
-}
-
-func (c *linuxConsole) Read(b []byte) (int, error) {
-	return c.master.Read(b)
-}
-
-func (c *linuxConsole) Write(b []byte) (int, error) {
-	return c.master.Write(b)
-}
-
-func (c *linuxConsole) Close() error {
-	if m := c.master; m != nil {
-		return m.Close()
-	}
-	return nil
-}
-
 // mount initializes the console inside the rootfs mounting with the specified mount label
 // and applying the correct ownership of the console.
-func (c *linuxConsole) mount() error {
+func mountConsole(slavePath string) error {
 	oldMask := unix.Umask(0000)
 	defer unix.Umask(oldMask)
 	f, err := os.Create("/dev/console")
@@ -75,78 +18,24 @@ func (c *linuxConsole) mount() error {
 	if f != nil {
 		f.Close()
 	}
-	return unix.Mount(c.slavePath, "/dev/console", "bind", unix.MS_BIND, "")
+	return unix.Mount(slavePath, "/dev/console", "bind", unix.MS_BIND, "")
 }
 
 // dupStdio opens the slavePath for the console and dups the fds to the current
 // processes stdio, fd 0,1,2.
-func (c *linuxConsole) dupStdio() error {
-	slave, err := c.open(unix.O_RDWR)
+func dupStdio(slavePath string) error {
+	fd, err := unix.Open(slavePath, unix.O_RDWR, 0)
 	if err != nil {
-		return err
+		return &os.PathError{
+			Op:   "open",
+			Path: slavePath,
+			Err:  err,
+		}
 	}
-	fd := int(slave.Fd())
 	for _, i := range []int{0, 1, 2} {
 		if err := unix.Dup3(fd, i, 0); err != nil {
 			return err
 		}
 	}
 	return nil
 }
-
-// open is a clone of os.OpenFile without the O_CLOEXEC used to open the pty slave.
-func (c *linuxConsole) open(flag int) (*os.File, error) {
-	r, e := unix.Open(c.slavePath, flag, 0)
-	if e != nil {
-		return nil, &os.PathError{
-			Op:   "open",
-			Path: c.slavePath,
-			Err:  e,
-		}
-	}
-	return os.NewFile(uintptr(r), c.slavePath), nil
-}
-
-func ioctl(fd uintptr, flag, data uintptr) error {
-	if _, _, err := unix.Syscall(unix.SYS_IOCTL, fd, flag, data); err != 0 {
-		return err
-	}
-	return nil
-}
-
-// unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f.
-// unlockpt should be called before opening the slave side of a pty.
-func unlockpt(f *os.File) error {
-	var u int32
-	return ioctl(f.Fd(), unix.TIOCSPTLCK, uintptr(unsafe.Pointer(&u)))
-}
-
-// ptsname retrieves the name of the first available pts for the given master.
-func ptsname(f *os.File) (string, error) {
-	n, err := unix.IoctlGetInt(int(f.Fd()), unix.TIOCGPTN)
-	if err != nil {
-		return "", err
-	}
-	return fmt.Sprintf("/dev/pts/%d", n), nil
-}
-
-// SaneTerminal sets the necessary tty_ioctl(4)s to ensure that a pty pair
-// created by us acts normally. In particular, a not-very-well-known default of
-// Linux unix98 ptys is that they have +onlcr by default. While this isn't a
-// problem for terminal emulators, because we relay data from the terminal we
-// also relay that funky line discipline.
-func SaneTerminal(terminal *os.File) error {
-	termios, err := unix.IoctlGetTermios(int(terminal.Fd()), unix.TCGETS)
-	if err != nil {
-		return fmt.Errorf("ioctl(tty, tcgets): %s", err.Error())
-	}
-
-	// Set -onlcr so we don't have to deal with \r.
-	termios.Oflag &^= unix.ONLCR
-
-	if err := unix.IoctlSetTermios(int(terminal.Fd()), unix.TCSETS, termios); err != nil {
-		return fmt.Errorf("ioctl(tty, tcsets): %s", err.Error())
-	}
-
-	return nil
-}

libcontainer/console_freebsd.go

@@ -1481,7 +1481,7 @@ func (c *linuxContainer) criuNotifications(resp *criurpc.CriuResp, process *Proc
 		defer master.Close()
 
 		// While we can access console.master, using the API is a good idea.
-		if err := utils.SendFd(process.ConsoleSocket, master); err != nil {
+		if err := utils.SendFd(process.ConsoleSocket, master.Name(), master.Fd()); err != nil {
 			return err
 		}
 	}

libcontainer/console_linux.go

@@ -12,15 +12,16 @@ import (
 	"syscall" // only for Errno
 	"unsafe"
 
+	"golang.org/x/sys/unix"
+
+	"github.com/containerd/console"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/system"
 	"github.com/opencontainers/runc/libcontainer/user"
 	"github.com/opencontainers/runc/libcontainer/utils"
-
 	"github.com/sirupsen/logrus"
 	"github.com/vishvananda/netlink"
-	"golang.org/x/sys/unix"
 )
 
 type initType string
@@ -170,29 +171,25 @@ func setupConsole(socket *os.File, config *initConfig, mount bool) error {
 	// however, that setupUser (specifically fixStdioPermissions) *will* change
 	// the UID owner of the console to be the user the process will run as (so
 	// they can actually control their console).
-	console, err := newConsole()
+	console, slavePath, err := console.NewPty()
 	if err != nil {
 		return err
 	}
 	// After we return from here, we don't need the console anymore.
 	defer console.Close()
 
-	linuxConsole, ok := console.(*linuxConsole)
-	if !ok {
-		return fmt.Errorf("failed to cast console to *linuxConsole")
-	}
 	// Mount the console inside our rootfs.
 	if mount {
-		if err := linuxConsole.mount(); err != nil {
+		if err := mountConsole(slavePath); err != nil {
 			return err
 		}
 	}
 	// While we can access console.master, using the API is a good idea.
-	if err := utils.SendFd(socket, linuxConsole.File()); err != nil {
+	if err := utils.SendFd(socket, console.Name(), console.Fd()); err != nil {
 		return err
 	}
 	// Now, dup over all the things.
-	return linuxConsole.dupStdio()
+	return dupStdio(slavePath)
 }
 
 // syncParentReady sends to the given pipe a JSON payload which indicates that

libcontainer/console_solaris.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/containerd/console"
 	"github.com/opencontainers/runc/libcontainer"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/utils"
@@ -289,7 +290,7 @@ func TestExecInTTY(t *testing.T) {
 	defer child.Close()
 	ps.ConsoleSocket = child
 	type cdata struct {
-		c   libcontainer.Console
+		c   console.Console
 		err error
 	}
 	dc := make(chan *cdata, 1)
@@ -299,10 +300,18 @@ func TestExecInTTY(t *testing.T) {
 			dc <- &cdata{
 				err: err,
 			}
+			return
 		}
-		libcontainer.SaneTerminal(f)
+		c, err := console.ConsoleFromFile(f)
+		if err != nil {
+			dc <- &cdata{
+				err: err,
+			}
+			return
+		}
+		console.SaneTerminal(f)
 		dc <- &cdata{
-			c: libcontainer.ConsoleFromFile(f),
+			c: c,
 		}
 	}()
 	err = container.Run(ps)

libcontainer/console_windows.go

@@ -84,12 +84,10 @@ func RecvFd(socket *os.File) (*os.File, error) {
 // addition, the file.Name() of the given file will also be sent as
 // non-auxiliary data in the same payload (allowing to send contextual
 // information for a file descriptor).
-func SendFd(socket, file *os.File) error {
-	name := []byte(file.Name())
+func SendFd(socket *os.File, name string, fd uintptr) error {
 	if len(name) >= MaxNameLen {
-		return fmt.Errorf("sendfd: filename too long: %s", file.Name())
+		return fmt.Errorf("sendfd: filename too long: %s", name)
 	}
-	oob := unix.UnixRights(int(file.Fd()))
-
-	return unix.Sendmsg(int(socket.Fd()), name, oob, nil, 0)
+	oob := unix.UnixRights(int(fd))
+	return unix.Sendmsg(int(socket.Fd()), []byte(name), oob, nil, 0)
 }