From 570eed473b8197e195d5e6f69282b7ab1488b4b4 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Wed, 8 Apr 2015 14:14:51 -0700 Subject: [PATCH] Move childPipe to the end of FD set This adds a new env var for identifying the internal sync pipe that libcontainer uses to sync with the container and parent process. This replaces #496 to allow the user to add additional files to the processes and not take over fd 3 for all containers. Closes #496 Signed-off-by: Michael Crosby --- container_linux.go | 7 ++++--- factory.go | 10 ++++------ factory_linux.go | 7 ++++++- init_linux.go | 12 ------------ integration/init_test.go | 2 +- nsinit/init.go | 2 +- 6 files changed, 16 insertions(+), 24 deletions(-) diff --git a/container_linux.go b/container_linux.go index f2c2ab07..1ffd7d9c 100644 --- a/container_linux.go +++ b/container_linux.go @@ -16,6 +16,8 @@ import ( "github.com/docker/libcontainer/configs" ) +const stdioFdCount = 3 + type linuxContainer struct { id string root string @@ -139,7 +141,8 @@ func (c *linuxContainer) commandTemplate(p *Process, childPipe *os.File) (*exec. if cmd.SysProcAttr == nil { cmd.SysProcAttr = &syscall.SysProcAttr{} } - cmd.ExtraFiles = append([]*os.File{childPipe}, p.ExtraFiles...) + cmd.ExtraFiles = append(p.ExtraFiles, childPipe) + cmd.Env = append(cmd.Env, fmt.Sprintf("_LIBCONTAINER_INITPIPE=%d", stdioFdCount+len(cmd.ExtraFiles)-1)) // NOTE: when running a container with no PID namespace and the parent process spawning the container is // PID1 the pdeathsig is being delivered to the container's init process by the kernel for some reason // even with the parent still running. @@ -178,11 +181,9 @@ func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, fmt.Sprintf("_LIBCONTAINER_INITPID=%d", c.initProcess.pid()), "_LIBCONTAINER_INITTYPE=setns", ) - if p.consolePath != "" { cmd.Env = append(cmd.Env, "_LIBCONTAINER_CONSOLE_PATH="+p.consolePath) } - // TODO: set on container for process management return &setnsProcess{ cmd: cmd, diff --git a/factory.go b/factory.go index 0c9fa63a..6a3706fd 100644 --- a/factory.go +++ b/factory.go @@ -33,14 +33,12 @@ type Factory interface { Load(id string) (Container, error) // StartInitialization is an internal API to libcontainer used during the rexec of the - // container. pipefd is the fd to the child end of the pipe used to syncronize the - // parent and child process providing state and configuration to the child process and - // returning any errors during the init of the container + // container. // // Errors: - // pipe connection error - // system error - StartInitialization(pipefd uintptr) error + // Pipe connection error + // System error + StartInitialization() error // Type returns info string about factory type (e.g. lxc, libcontainer...) Type() string diff --git a/factory_linux.go b/factory_linux.go index a2d3bec7..3cf1c3d2 100644 --- a/factory_linux.go +++ b/factory_linux.go @@ -10,6 +10,7 @@ import ( "os/exec" "path/filepath" "regexp" + "strconv" "syscall" "github.com/docker/docker/pkg/mount" @@ -194,7 +195,11 @@ func (l *LinuxFactory) Type() string { // StartInitialization loads a container by opening the pipe fd from the parent to read the configuration and state // This is a low level implementation detail of the reexec and should not be consumed externally -func (l *LinuxFactory) StartInitialization(pipefd uintptr) (err error) { +func (l *LinuxFactory) StartInitialization() (err error) { + pipefd, err := strconv.Atoi(os.Getenv("_LIBCONTAINER_INITPIPE")) + if err != nil { + return err + } var ( pipe = os.NewFile(uintptr(pipefd), "pipe") it = initType(os.Getenv("_LIBCONTAINER_INITTYPE")) diff --git a/init_linux.go b/init_linux.go index 66e46f16..4bbb713d 100644 --- a/init_linux.go +++ b/init_linux.go @@ -96,21 +96,9 @@ func populateProcessEnvironment(env []string) error { // and working dir, and closes any leaked file descriptors // before executing the command inside the namespace func finalizeNamespace(config *initConfig) error { - // FD 3 is the child pipe, which needs to be closed. - // Additional file descriptors starts from 3 to (3 + n) - // To fix the order all additional file descriptors - // are shiftet by one right - for fd := 3; fd < (config.PassedFilesCount + 3); fd++ { - err := syscall.Dup2(fd+1, fd) - if err != nil { - return err - } - } - // Ensure that all unwanted fds we may have accidentally // inherited are marked close-on-exec so they stay out of the // container - if err := utils.CloseExecFrom(config.PassedFilesCount + 3); err != nil { return err } diff --git a/integration/init_test.go b/integration/init_test.go index f11834de..1f75ef52 100644 --- a/integration/init_test.go +++ b/integration/init_test.go @@ -21,7 +21,7 @@ func init() { if err != nil { log.Fatalf("unable to initialize for container: %s", err) } - if err := factory.StartInitialization(3); err != nil { + if err := factory.StartInitialization(); err != nil { log.Fatal(err) } } diff --git a/nsinit/init.go b/nsinit/init.go index 7b2cf193..c7506a0e 100644 --- a/nsinit/init.go +++ b/nsinit/init.go @@ -20,7 +20,7 @@ var initCommand = cli.Command{ if err != nil { fatal(err) } - if err := factory.StartInitialization(3); err != nil { + if err := factory.StartInitialization(); err != nil { fatal(err) } panic("This line should never been executed")