[lxc-devel] [lxd/master] Use fork for command execution

stgraber on Github lxc-bot at linuxcontainers.org
Thu Mar 31 06:28:36 UTC 2016


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 627 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160331/dd6226d8/attachment.bin>
-------------- next part --------------
From b319775481441e845e5490bd2c3ba20ffdad90e2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Thu, 31 Mar 2016 02:16:32 -0400
Subject: [PATCH] Use fork for command execution
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This introduces a new "forkexec" sub-command which is used for all exec
calls. The thought is that the random lockups we were seeing before this
were caused by a race between go-lxc calling liblxc which called fork()
and some of the other goroutines.

Closes #1803

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/container.go      |  4 ++-
 lxd/container_exec.go | 65 ++++++++++++++-----------------------
 lxd/container_lxc.go  | 52 +++++++++++++++++++++++-------
 lxd/containers.go     | 88 +++++++++++++++++++++++++++++++++++++++++++++++++--
 lxd/main.go           |  8 +++++
 5 files changed, 161 insertions(+), 56 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index 5e216a6..b3d8ac5 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -346,6 +346,9 @@ type container interface {
 	FilePull(srcpath string, dstpath string) error
 	FilePush(srcpath string, dstpath string, uid int, gid int, mode os.FileMode) error
 
+	// Command execution
+	Exec(command []string, env map[string]string, stdin *os.File, stdout *os.File, stderr *os.File) (int, error)
+
 	// Status
 	Render() (interface{}, error)
 	RenderState() (*shared.ContainerState, error)
@@ -383,7 +386,6 @@ type container interface {
 	LogPath() string
 
 	// FIXME: Those should be internal functions
-	LXContainerGet() *lxc.Container
 	StorageStart() error
 	StorageStop() error
 	Storage() storage
diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index 6fedf9e..2e70605 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -12,7 +12,6 @@ import (
 
 	"github.com/gorilla/mux"
 	"github.com/gorilla/websocket"
-	"gopkg.in/lxc/go-lxc.v2"
 
 	"github.com/lxc/lxd/shared"
 )
@@ -26,22 +25,13 @@ type commandPostContent struct {
 	Height      int               `json:"height"`
 }
 
-func runCommand(container *lxc.Container, command []string, options lxc.AttachOptions) (int, error) {
-	status, err := container.RunCommandStatus(command, options)
-	if err != nil {
-		shared.Debugf("Failed running command: %q", err.Error())
-		return 0, err
-	}
-
-	return status, nil
-}
-
 type execWs struct {
-	command          []string
-	container        *lxc.Container
+	command   []string
+	container container
+	env       map[string]string
+
 	rootUid          int
 	rootGid          int
-	options          lxc.AttachOptions
 	conns            map[int]*websocket.Conn
 	connsLock        sync.Mutex
 	allConnected     chan bool
@@ -109,13 +99,18 @@ func (s *execWs) Do(op *operation) error {
 	var ttys []*os.File
 	var ptys []*os.File
 
+	var stdin *os.File
+	var stdout *os.File
+	var stderr *os.File
+
 	if s.interactive {
 		ttys = make([]*os.File, 1)
 		ptys = make([]*os.File, 1)
 		ptys[0], ttys[0], err = shared.OpenPty(s.rootUid, s.rootGid)
-		s.options.StdinFd = ttys[0].Fd()
-		s.options.StdoutFd = ttys[0].Fd()
-		s.options.StderrFd = ttys[0].Fd()
+
+		stdin = ttys[0]
+		stdout = ttys[0]
+		stderr = ttys[0]
 
 		if s.width > 0 && s.height > 0 {
 			shared.SetSize(int(ptys[0].Fd()), s.width, s.height)
@@ -129,9 +124,10 @@ func (s *execWs) Do(op *operation) error {
 				return err
 			}
 		}
-		s.options.StdinFd = ptys[0].Fd()
-		s.options.StdoutFd = ttys[1].Fd()
-		s.options.StderrFd = ttys[2].Fd()
+
+		stdin = ptys[0]
+		stdout = ttys[1]
+		stderr = ttys[2]
 	}
 
 	controlExit := make(chan bool)
@@ -216,11 +212,7 @@ func (s *execWs) Do(op *operation) error {
 		}
 	}
 
-	cmdResult, cmdErr := runCommand(
-		s.container,
-		s.command,
-		s.options,
-	)
+	cmdResult, cmdErr := s.container.Exec(s.command, s.env, stdin, stdout, stderr)
 
 	for _, tty := range ttys {
 		tty.Close()
@@ -274,22 +266,17 @@ func containerExecPost(d *Daemon, r *http.Request) Response {
 		return BadRequest(err)
 	}
 
-	opts := lxc.DefaultAttachOptions
-	opts.ClearEnv = true
-	opts.Env = []string{}
+	env := map[string]string{}
 
 	for k, v := range c.ExpandedConfig() {
 		if strings.HasPrefix(k, "environment.") {
-			opts.Env = append(opts.Env, fmt.Sprintf("%s=%s", strings.TrimPrefix(k, "environment."), v))
+			env[strings.TrimPrefix(k, "environment.")] = v
 		}
 	}
 
 	if post.Environment != nil {
 		for k, v := range post.Environment {
-			if k == "HOME" {
-				opts.Cwd = v
-			}
-			opts.Env = append(opts.Env, fmt.Sprintf("%s=%s", k, v))
+			env[k] = v
 		}
 	}
 
@@ -310,7 +297,6 @@ func containerExecPost(d *Daemon, r *http.Request) Response {
 		ws.allConnected = make(chan bool, 1)
 		ws.controlConnected = make(chan bool, 1)
 		ws.interactive = post.Interactive
-		ws.options = opts
 		for i := -1; i < len(ws.conns)-1; i++ {
 			ws.fds[i], err = shared.RandomCryptoString()
 			if err != nil {
@@ -319,7 +305,9 @@ func containerExecPost(d *Daemon, r *http.Request) Response {
 		}
 
 		ws.command = post.Command
-		ws.container = c.LXContainerGet()
+		ws.container = c
+		ws.env = env
+
 		ws.width = post.Width
 		ws.height = post.Height
 
@@ -340,13 +328,8 @@ func containerExecPost(d *Daemon, r *http.Request) Response {
 			return err
 		}
 		defer nullDev.Close()
-		nullfd := nullDev.Fd()
-
-		opts.StdinFd = nullfd
-		opts.StdoutFd = nullfd
-		opts.StderrFd = nullfd
 
-		_, cmdErr := runCommand(c.LXContainerGet(), post.Command, opts)
+		_, cmdErr := c.Exec(post.Command, env, nil, nil, nil)
 		return cmdErr
 	}
 
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index b78a77a..8918160 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -2921,6 +2921,46 @@ func (c *containerLXC) FilePush(srcpath string, dstpath string, uid int, gid int
 	return nil
 }
 
+func (c *containerLXC) Exec(command []string, env map[string]string, stdin *os.File, stdout *os.File, stderr *os.File) (int, error) {
+	envSlice := []string{}
+
+	for k, v := range env {
+		envSlice = append(envSlice, fmt.Sprintf("%s=%s", k, v))
+	}
+
+	args := []string{c.daemon.execPath, "forkexec", c.name, c.daemon.lxcpath, filepath.Join(c.LogPath(), "lxc.conf")}
+
+	args = append(args, "--")
+	args = append(args, "env")
+	args = append(args, envSlice...)
+
+	args = append(args, "--")
+	args = append(args, "cmd")
+	args = append(args, command...)
+
+	cmd := exec.Cmd{}
+	cmd.Path = c.daemon.execPath
+	cmd.Args = args
+	cmd.Stdin = stdin
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
+
+	err := cmd.Run()
+	if err != nil {
+		exitErr, ok := err.(*exec.ExitError)
+		if ok {
+			status, ok := exitErr.Sys().(syscall.WaitStatus)
+			if ok {
+				return status.ExitStatus(), nil
+			}
+		}
+
+		return -1, err
+	}
+
+	return 0, nil
+}
+
 func (c *containerLXC) diskState() map[string]shared.ContainerStateDisk {
 	disk := map[string]shared.ContainerStateDisk{}
 
@@ -4332,18 +4372,6 @@ func (c *containerLXC) LastIdmapSet() (*shared.IdmapSet, error) {
 	return lastIdmap, nil
 }
 
-func (c *containerLXC) LXContainerGet() *lxc.Container {
-	// FIXME: This function should go away
-
-	// Load the go-lxc struct
-	err := c.initLXC()
-	if err != nil {
-		return nil
-	}
-
-	return c.c
-}
-
 func (c *containerLXC) Daemon() *Daemon {
 	// FIXME: This function should go away
 	return c.daemon
diff --git a/lxd/containers.go b/lxd/containers.go
index a7a8351..ac191d0 100644
--- a/lxd/containers.go
+++ b/lxd/containers.go
@@ -5,6 +5,7 @@ import (
 	"os"
 	"sort"
 	"strconv"
+	"strings"
 	"sync"
 	"syscall"
 	"time"
@@ -198,8 +199,6 @@ func containerDeleteSnapshots(d *Daemon, cname string) error {
  * This is called by lxd when called as "lxd forkstart <container>"
  * 'forkstart' is used instead of just 'start' in the hopes that people
  * do not accidentally type 'lxd start' instead of 'lxc start'
- *
- * We expect to read the lxcconfig over fd 3.
  */
 func startContainer(args []string) error {
 	if len(args) != 4 {
@@ -246,3 +245,88 @@ func startContainer(args []string) error {
 
 	return c.Start()
 }
+
+/*
+ * This is called by lxd when called as "lxd forkexec <container>"
+ */
+func execContainer(args []string) (int, error) {
+	if len(args) < 6 {
+		return -1, fmt.Errorf("Bad arguments: %q", args)
+	}
+
+	name := args[1]
+	lxcpath := args[2]
+	configPath := args[3]
+
+	c, err := lxc.NewContainer(name, lxcpath)
+	if err != nil {
+		return -1, fmt.Errorf("Error initializing container for start: %q", err)
+	}
+
+	err = c.LoadConfigFile(configPath)
+	if err != nil {
+		return -1, fmt.Errorf("Error opening startup config file: %q", err)
+	}
+
+	syscall.Dup3(int(os.Stdin.Fd()), 200, 0)
+	syscall.Dup3(int(os.Stdout.Fd()), 201, 0)
+	syscall.Dup3(int(os.Stderr.Fd()), 202, 0)
+
+	syscall.Close(int(os.Stdin.Fd()))
+	syscall.Close(int(os.Stdout.Fd()))
+	syscall.Close(int(os.Stderr.Fd()))
+
+	opts := lxc.DefaultAttachOptions
+	opts.ClearEnv = true
+	opts.StdinFd = 200
+	opts.StdoutFd = 201
+	opts.StderrFd = 202
+
+	logPath := shared.LogPath(name, "forkexec.log")
+	if shared.PathExists(logPath) {
+		os.Remove(logPath)
+	}
+
+	logFile, err := os.OpenFile(logPath, os.O_WRONLY|os.O_CREATE|os.O_SYNC, 0644)
+	if err == nil {
+		syscall.Dup3(int(logFile.Fd()), 1, 0)
+		syscall.Dup3(int(logFile.Fd()), 2, 0)
+	}
+
+	env := []string{}
+	cmd := []string{}
+
+	section := ""
+	for _, arg := range args[5:len(args)] {
+		if arg == "--" {
+			section = ""
+			continue
+		}
+
+		if section == "" {
+			section = arg
+			continue
+		}
+
+		if section == "env" {
+			fields := strings.SplitN(arg, "=", 2)
+			if len(fields) == 2 && fields[0] == "HOME" {
+				opts.Cwd = fields[1]
+			}
+			env = append(env, arg)
+		} else if section == "cmd" {
+			cmd = append(cmd, arg)
+		} else {
+			return -1, fmt.Errorf("Invalid exec section: %s", section)
+		}
+	}
+
+	opts.Env = env
+
+	status, err := c.RunCommandStatus(cmd, opts)
+	if err != nil {
+		return -1, fmt.Errorf("Failed running command: %q", err)
+	}
+
+	return status, nil
+}
diff --git a/lxd/main.go b/lxd/main.go
index 0a4bbd4..ca1245c 100644
--- a/lxd/main.go
+++ b/lxd/main.go
@@ -138,6 +138,8 @@ func run() error {
 		fmt.Printf("        How long to wait before failing\n")
 
 		fmt.Printf("\n\nInternal commands (don't call these directly):\n")
+		fmt.Printf("    forkexec\n")
+		fmt.Printf("        Execute a command in a container\n")
 		fmt.Printf("    forkgetnet\n")
 		fmt.Printf("        Get container network information\n")
 		fmt.Printf("    forkgetfile\n")
@@ -219,6 +221,12 @@ func run() error {
 			return MigrateContainer(os.Args[1:])
 		case "forkstart":
 			return startContainer(os.Args[1:])
+		case "forkexec":
+			ret, err := execContainer(os.Args[1:])
+			if err != nil {
+				fmt.Fprintf(os.Stderr, "error: %v\n", err)
+			}
+			os.Exit(ret)
 		}
 	}
 


More information about the lxc-devel mailing list