[lxc-devel] [lxd/master] Don't use FindProcess, just pass exec.Cmd

[stgraber on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 354 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170308/cbe90f77/attachment.bin>
-------------- next part --------------
From 212cfb3cc6cb23ce7dab7465cfb3df6411dd117b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Tue, 7 Mar 2017 19:59:47 -0500
Subject: [PATCH] Don't use FindProcess, just pass exec.Cmd
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/container.go      |  2 +-
 lxd/container_exec.go | 25 +++++++++----------------
 lxd/container_lxc.go  | 20 +++++++++++---------
 3 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/lxd/container.go b/lxd/container.go
index 1c5f8cd..ca4356d 100644
--- a/lxd/container.go
+++ b/lxd/container.go
@@ -441,7 +441,7 @@ type container interface {
 	         *      (the PID returned in the first return argument). It can however
 	         *      be used to e.g. forward signals.)
 	*/
-	Exec(command []string, env map[string]string, stdin *os.File, stdout *os.File, stderr *os.File, wait bool) (int, int, error)
+	Exec(command []string, env map[string]string, stdin *os.File, stdout *os.File, stderr *os.File, wait bool) (*exec.Cmd, int, int, error)
 
 	// Status
 	Render() (interface{}, interface{}, error)
diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index d4da9aa..cf3fed2 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"net/http"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -292,7 +293,7 @@ func (s *execWs) Do(op *operation) error {
 		return cmdErr
 	}
 
-	pid, attachedPid, err := s.container.Exec(s.command, s.env, stdin, stdout, stderr, false)
+	cmd, _, attachedPid, err := s.container.Exec(s.command, s.env, stdin, stdout, stderr, false)
 	if err != nil {
 		return err
 	}
@@ -301,23 +302,15 @@ func (s *execWs) Do(op *operation) error {
 		attachedChildIsBorn <- attachedPid
 	}
 
-	proc, err := os.FindProcess(pid)
-	if err != nil {
-		return finisher(-1, fmt.Errorf("Failed finding process: %q", err))
-	}
-
-	procState, err := proc.Wait()
-	if err != nil {
-		return finisher(-1, fmt.Errorf("Failed waiting on process %d: %q", pid, err))
-	}
-
-	if procState.Success() {
+	err = cmd.Wait()
+	if err == nil {
 		return finisher(0, nil)
 	}
 
-	status, ok := procState.Sys().(syscall.WaitStatus)
+	exitErr, ok := err.(*exec.ExitError)
 	if ok {
-		if status.Exited() {
+		status, ok := exitErr.Sys().(syscall.WaitStatus)
+		if ok {
 			return finisher(status.ExitStatus(), nil)
 		}
 
@@ -464,7 +457,7 @@ func containerExecPost(d *Daemon, r *http.Request) Response {
 			defer stderr.Close()
 
 			// Run the command
-			cmdResult, _, cmdErr = c.Exec(post.Command, env, nil, stdout, stderr, true)
+			_, cmdResult, _, cmdErr = c.Exec(post.Command, env, nil, stdout, stderr, true)
 
 			// Update metadata with the right URLs
 			metadata["return"] = cmdResult
@@ -473,7 +466,7 @@ func containerExecPost(d *Daemon, r *http.Request) Response {
 				"2": fmt.Sprintf("/%s/containers/%s/logs/%s", version.APIVersion, c.Name(), filepath.Base(stderr.Name())),
 			}
 		} else {
-			cmdResult, _, cmdErr = c.Exec(post.Command, env, nil, nil, nil, true)
+			_, cmdResult, _, cmdErr = c.Exec(post.Command, env, nil, nil, nil, true)
 			metadata["return"] = cmdResult
 		}
 
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 9fc36ea..4299395 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -4687,7 +4687,7 @@ func (c *containerLXC) FileRemove(path string) error {
 	return nil
 }
 
-func (c *containerLXC) Exec(command []string, env map[string]string, stdin *os.File, stdout *os.File, stderr *os.File, wait bool) (int, int, error) {
+func (c *containerLXC) Exec(command []string, env map[string]string, stdin *os.File, stdout *os.File, stderr *os.File, wait bool) (*exec.Cmd, int, int, error) {
 	envSlice := []string{}
 
 	for k, v := range env {
@@ -4715,25 +4715,26 @@ func (c *containerLXC) Exec(command []string, env map[string]string, stdin *os.F
 	defer r.Close()
 	if err != nil {
 		shared.LogErrorf("s", err)
-		return -1, -1, err
+		return nil, -1, -1, err
 	}
 
 	cmd.ExtraFiles = []*os.File{w}
 	err = cmd.Start()
 	if err != nil {
 		w.Close()
-		return -1, -1, err
+		return nil, -1, -1, err
 	}
 	w.Close()
+
 	attachedPid := -1
 	if err := json.NewDecoder(r).Decode(&attachedPid); err != nil {
 		shared.LogErrorf("Failed to retrieve PID of executing child process: %s", err)
-		return -1, -1, err
+		return nil, -1, -1, err
 	}
 
 	// It's the callers responsibility to wait or not wait.
 	if !wait {
-		return cmd.Process.Pid, attachedPid, nil
+		return &cmd, -1, attachedPid, nil
 	}
 
 	err = cmd.Wait()
@@ -4742,18 +4743,19 @@ func (c *containerLXC) Exec(command []string, env map[string]string, stdin *os.F
 		if ok {
 			status, ok := exitErr.Sys().(syscall.WaitStatus)
 			if ok {
-				return status.ExitStatus(), attachedPid, nil
+				return nil, status.ExitStatus(), attachedPid, nil
 			}
 
 			if status.Signaled() {
 				// 128 + n == Fatal error signal "n"
-				return 128 + int(status.Signal()), attachedPid, nil
+				return nil, 128 + int(status.Signal()), attachedPid, nil
 			}
 		}
-		return -1, -1, err
+
+		return nil, -1, -1, err
 	}
 
-	return 0, attachedPid, nil
+	return nil, 0, attachedPid, nil
 }
 
 func (c *containerLXC) cpuState() api.ContainerStateCPU {