The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6728

This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.

=== Description (from pull-request) ===

From dd208c34185c680ab9baa48ad5747be926470582 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@ubuntu.com>
Date: Sat, 18 Jan 2020 08:42:30 +0200
Subject: [PATCH 1/5] lxd/exec: Pass full req through
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgra...@ubuntu.com>
---
 lxd/container_exec.go              | 51 +++++++++++-------------------
 lxd/container_lxc.go               | 12 +++----
 lxd/instance/drivers/vm_qemu.go    | 17 +++-------
 lxd/instance/instance_interface.go |  2 +-
 4 files changed, 30 insertions(+), 52 deletions(-)

diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index cb6f224c99..e8da9b5036 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -30,23 +30,16 @@ import (
 )
 
 type execWs struct {
-       command  []string
-       instance instance.Instance
-       env      map[string]string
+       req api.InstanceExecPost
 
+       instance         instance.Instance
        rootUid          int64
        rootGid          int64
        conns            map[int]*websocket.Conn
        connsLock        sync.Mutex
        allConnected     chan bool
        controlConnected chan bool
-       interactive      bool
        fds              map[int]string
-       width            int
-       height           int
-       uid              uint32
-       gid              uint32
-       cwd              string
 }
 
 func (s *execWs) Metadata() interface{} {
@@ -61,9 +54,9 @@ func (s *execWs) Metadata() interface{} {
 
        return shared.Jmap{
                "fds":         fds,
-               "command":     s.command,
-               "environment": s.env,
-               "interactive": s.interactive,
+               "command":     s.req.Command,
+               "environment": s.req.Environment,
+               "interactive": s.req.Interactive,
        }
 }
 
@@ -119,7 +112,7 @@ func (s *execWs) Do(op *operations.Operation) error {
        var stdout *os.File
        var stderr *os.File
 
-       if s.interactive {
+       if s.req.Interactive {
                ttys = make([]*os.File, 1)
                ptys = make([]*os.File, 1)
                ptys[0], ttys[0], err = shared.OpenPty(s.rootUid, s.rootGid)
@@ -131,8 +124,8 @@ func (s *execWs) Do(op *operations.Operation) error {
                stdout = ttys[0]
                stderr = ttys[0]
 
-               if s.width > 0 && s.height > 0 {
-                       shared.SetSize(int(ptys[0].Fd()), s.width, s.height)
+               if s.req.Width > 0 && s.req.Height > 0 {
+                       shared.SetSize(int(ptys[0].Fd()), s.req.Width, 
s.req.Height)
                }
        } else {
                ttys = make([]*os.File, 3)
@@ -154,7 +147,7 @@ func (s *execWs) Do(op *operations.Operation) error {
        attachedChildIsDead := make(chan bool, 1)
        var wgEOF sync.WaitGroup
 
-       if s.interactive {
+       if s.req.Interactive {
                wgEOF.Add(1)
                go func() {
                        logger.Debugf("Interactive child process handler 
waiting")
@@ -293,7 +286,7 @@ func (s *execWs) Do(op *operations.Operation) error {
                s.connsLock.Unlock()
 
                if conn == nil {
-                       if s.interactive {
+                       if s.req.Interactive {
                                controlExit <- true
                        }
                } else {
@@ -317,12 +310,12 @@ func (s *execWs) Do(op *operations.Operation) error {
                return cmdErr
        }
 
-       cmd, err := s.instance.Exec(s.command, s.env, stdin, stdout, stderr, 
s.cwd, s.uid, s.gid)
+       cmd, err := s.instance.Exec(s.req, stdin, stdout, stderr)
        if err != nil {
                return err
        }
 
-       if s.interactive {
+       if s.req.Interactive {
                // Start the interactive process handler.
                attachedChildIsBorn <- cmd
        }
@@ -385,8 +378,8 @@ func containerExecPost(d *Daemon, r *http.Request) 
response.Response {
                return response.BadRequest(fmt.Errorf("Container is frozen"))
        }
 
+       // Process environment.
        env := map[string]string{}
-
        for k, v := range inst.ExpandedConfig() {
                if strings.HasPrefix(k, "environment.") {
                        env[strings.TrimPrefix(k, "environment.")] = v
@@ -429,6 +422,9 @@ func containerExecPost(d *Daemon, r *http.Request) 
response.Response {
                env["LANG"] = "C.UTF-8"
        }
 
+       // Apply to request.
+       post.Environment = env
+
        if post.WaitForWS {
                ws := &execWs{}
                ws.fds = map[int]string{}
@@ -454,7 +450,6 @@ func containerExecPost(d *Daemon, r *http.Request) 
response.Response {
                }
                ws.allConnected = make(chan bool, 1)
                ws.controlConnected = make(chan bool, 1)
-               ws.interactive = post.Interactive
                for i := -1; i < len(ws.conns)-1; i++ {
                        ws.fds[i], err = shared.RandomCryptoString()
                        if err != nil {
@@ -462,16 +457,8 @@ func containerExecPost(d *Daemon, r *http.Request) 
response.Response {
                        }
                }
 
-               ws.command = post.Command
                ws.instance = inst
-               ws.env = env
-
-               ws.width = post.Width
-               ws.height = post.Height
-
-               ws.cwd = post.Cwd
-               ws.uid = post.User
-               ws.gid = post.Group
+               ws.req = post
 
                resources := map[string][]string{}
                resources["containers"] = []string{ws.instance.Name()}
@@ -502,7 +489,7 @@ func containerExecPost(d *Daemon, r *http.Request) 
response.Response {
                        defer stderr.Close()
 
                        // Run the command
-                       cmd, err := inst.Exec(post.Command, env, nil, stdout, 
stderr, post.Cwd, post.User, post.Group)
+                       cmd, err := inst.Exec(post, nil, stdout, stderr)
                        if err != nil {
                                return err
                        }
@@ -519,7 +506,7 @@ func containerExecPost(d *Daemon, r *http.Request) 
response.Response {
                                "2": fmt.Sprintf("/%s/containers/%s/logs/%s", 
version.APIVersion, inst.Name(), filepath.Base(stderr.Name())),
                        }
                } else {
-                       cmd, err := inst.Exec(post.Command, env, nil, nil, nil, 
post.Cwd, post.User, post.Group)
+                       cmd, err := inst.Exec(post, nil, nil, nil)
                        if err != nil {
                                return err
                        }
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index ba32e1ed86..fdb21fc928 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -5681,11 +5681,11 @@ func (c *containerLXC) ConsoleLog(opts 
lxc.ConsoleLogOptions) (string, error) {
        return string(msg), nil
 }
 
-func (c *containerLXC) Exec(command []string, env map[string]string, stdin 
*os.File, stdout *os.File, stderr *os.File, cwd string, uid uint32, gid uint32) 
(instance.Cmd, error) {
+func (c *containerLXC) Exec(req api.InstanceExecPost, stdin *os.File, stdout 
*os.File, stderr *os.File) (instance.Cmd, error) {
        // Prepare the environment
        envSlice := []string{}
 
-       for k, v := range env {
+       for k, v := range req.Environment {
                envSlice = append(envSlice, fmt.Sprintf("%s=%s", k, v))
        }
 
@@ -5704,9 +5704,9 @@ func (c *containerLXC) Exec(command []string, env 
map[string]string, stdin *os.F
                cname,
                c.state.OS.LxcPath,
                filepath.Join(c.LogPath(), "lxc.conf"),
-               cwd,
-               fmt.Sprintf("%d", uid),
-               fmt.Sprintf("%d", gid),
+               req.Cwd,
+               fmt.Sprintf("%d", req.User),
+               fmt.Sprintf("%d", req.Group),
        }
 
        args = append(args, "--")
@@ -5715,7 +5715,7 @@ func (c *containerLXC) Exec(command []string, env 
map[string]string, stdin *os.F
 
        args = append(args, "--")
        args = append(args, "cmd")
-       args = append(args, command...)
+       args = append(args, req.Command...)
 
        cmd := exec.Cmd{}
        cmd.Path = c.state.OS.ExecPath
diff --git a/lxd/instance/drivers/vm_qemu.go b/lxd/instance/drivers/vm_qemu.go
index a2eb2968dd..29bf11973f 100644
--- a/lxd/instance/drivers/vm_qemu.go
+++ b/lxd/instance/drivers/vm_qemu.go
@@ -2493,7 +2493,7 @@ func (vm *qemu) forwardSignal(control *websocket.Conn, 
sig unix.Signal) error {
 }
 
 // Exec a command inside the instance.
-func (vm *qemu) Exec(command []string, env map[string]string, stdin *os.File, 
stdout *os.File, stderr *os.File, cwd string, uid uint32, gid uint32) 
(instance.Cmd, error) {
+func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout 
*os.File, stderr *os.File) (instance.Cmd, error) {
        var instCmd *Cmd
 
        // Because this function will exit before the remote command has 
finished, we create a
@@ -2528,17 +2528,8 @@ func (vm *qemu) Exec(command []string, env 
map[string]string, stdin *os.File, st
        }
        cleanupFuncs = append(cleanupFuncs, agent.Disconnect)
 
-       post := api.InstanceExecPost{
-               Command:     command,
-               WaitForWS:   true,
-               Interactive: stdin == stdout,
-               Environment: env,
-               User:        uid,
-               Group:       gid,
-               Cwd:         cwd,
-       }
-
-       if post.Interactive {
+       req.WaitForWS = true
+       if req.Interactive {
                // Set console to raw.
                oldttystate, err := termios.MakeRaw(int(stdin.Fd()))
                if err != nil {
@@ -2578,7 +2569,7 @@ func (vm *qemu) Exec(command []string, env 
map[string]string, stdin *os.File, st
                Control:  controlHander,
        }
 
-       op, err := agent.ExecInstance("", post, &args)
+       op, err := agent.ExecInstance("", req, &args)
        if err != nil {
                return nil, err
        }
diff --git a/lxd/instance/instance_interface.go 
b/lxd/instance/instance_interface.go
index 8c9e284e47..59a39167ad 100644
--- a/lxd/instance/instance_interface.go
+++ b/lxd/instance/instance_interface.go
@@ -51,7 +51,7 @@ type Instance interface {
 
        // Console - Allocate and run a console tty.
        Console() (*os.File, chan error, error)
-       Exec(command []string, env map[string]string, stdin *os.File, stdout 
*os.File, stderr *os.File, cwd string, uid uint32, gid uint32) (Cmd, error)
+       Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File, stderr 
*os.File) (Cmd, error)
 
        // Status
        Render() (interface{}, interface{}, error)

From 86c70c80d5f6a0665087c54dd7552361efc77f33 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@ubuntu.com>
Date: Sat, 18 Jan 2020 09:13:52 +0200
Subject: [PATCH 2/5] lxd/exec: Forward control messages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgra...@ubuntu.com>
---
 lxd/container_exec.go               | 18 ++++++--
 lxd/container_lxc.go                | 13 +++---
 lxd/instance/drivers/vm_qemu.go     | 72 ++++++++---------------------
 lxd/instance/drivers/vm_qemu_cmd.go | 16 +++----
 lxd/instance/instance_interface.go  |  4 +-
 5 files changed, 51 insertions(+), 72 deletions(-)

diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index e8da9b5036..8bef4a1d4b 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -40,6 +40,7 @@ type execWs struct {
        allConnected     chan bool
        controlConnected chan bool
        fds              map[int]string
+       remoteControl    *websocket.Conn
 }
 
 func (s *execWs) Metadata() interface{} {
@@ -162,6 +163,16 @@ func (s *execWs) Do(op *operations.Operation) error {
                                return
                        }
 
+                       // Handle cases where the instance provides us a 
control websocket.
+                       if s.remoteControl != nil {
+                               s.connsLock.Lock()
+                               conn := s.conns[-1]
+                               s.connsLock.Unlock()
+
+                               <-shared.WebsocketProxy(conn, s.remoteControl)
+                               return
+                       }
+
                        logger.Debugf("Interactive child process handler 
started for child PID %d", attachedChild.PID())
                        for {
                                s.connsLock.Lock()
@@ -310,7 +321,7 @@ func (s *execWs) Do(op *operations.Operation) error {
                return cmdErr
        }
 
-       cmd, err := s.instance.Exec(s.req, stdin, stdout, stderr)
+       cmd, wsControl, err := s.instance.Exec(s.req, stdin, stdout, stderr)
        if err != nil {
                return err
        }
@@ -318,6 +329,7 @@ func (s *execWs) Do(op *operations.Operation) error {
        if s.req.Interactive {
                // Start the interactive process handler.
                attachedChildIsBorn <- cmd
+               s.remoteControl = wsControl
        }
 
        exitCode, err := cmd.Wait()
@@ -489,7 +501,7 @@ func containerExecPost(d *Daemon, r *http.Request) 
response.Response {
                        defer stderr.Close()
 
                        // Run the command
-                       cmd, err := inst.Exec(post, nil, stdout, stderr)
+                       cmd, _, err := inst.Exec(post, nil, stdout, stderr)
                        if err != nil {
                                return err
                        }
@@ -506,7 +518,7 @@ func containerExecPost(d *Daemon, r *http.Request) 
response.Response {
                                "2": fmt.Sprintf("/%s/containers/%s/logs/%s", 
version.APIVersion, inst.Name(), filepath.Base(stderr.Name())),
                        }
                } else {
-                       cmd, err := inst.Exec(post, nil, nil, nil)
+                       cmd, _, err := inst.Exec(post, nil, nil, nil)
                        if err != nil {
                                return err
                        }
diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index fdb21fc928..3d670c00d3 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -19,6 +19,7 @@ import (
        "time"
 
        "github.com/flosch/pongo2"
+       "github.com/gorilla/websocket"
        "github.com/pkg/errors"
        "golang.org/x/sys/unix"
        lxc "gopkg.in/lxc/go-lxc.v2"
@@ -5681,7 +5682,7 @@ func (c *containerLXC) ConsoleLog(opts 
lxc.ConsoleLogOptions) (string, error) {
        return string(msg), nil
 }
 
-func (c *containerLXC) Exec(req api.InstanceExecPost, stdin *os.File, stdout 
*os.File, stderr *os.File) (instance.Cmd, error) {
+func (c *containerLXC) Exec(req api.InstanceExecPost, stdin *os.File, stdout 
*os.File, stderr *os.File) (instance.Cmd, *websocket.Conn, error) {
        // Prepare the environment
        envSlice := []string{}
 
@@ -5693,7 +5694,7 @@ func (c *containerLXC) Exec(req api.InstanceExecPost, 
stdin *os.File, stdout *os
        logPath := filepath.Join(c.LogPath(), "forkexec.log")
        logFile, err := os.OpenFile(logPath, os.O_WRONLY|os.O_CREATE|os.O_SYNC, 
0644)
        if err != nil {
-               return nil, err
+               return nil, nil, err
        }
 
        // Prepare the subcommand
@@ -5746,21 +5747,21 @@ func (c *containerLXC) Exec(req api.InstanceExecPost, 
stdin *os.File, stdout *os
        rStatus, wStatus, err := shared.Pipe()
        defer rStatus.Close()
        if err != nil {
-               return nil, err
+               return nil, nil, err
        }
 
        cmd.ExtraFiles = []*os.File{stdin, stdout, stderr, wStatus}
        err = cmd.Start()
        if err != nil {
                wStatus.Close()
-               return nil, err
+               return nil, nil, err
        }
        wStatus.Close()
 
        attachedPid := -1
        if err := json.NewDecoder(rStatus).Decode(&attachedPid); err != nil {
                logger.Errorf("Failed to retrieve PID of executing child 
process: %s", err)
-               return nil, err
+               return nil, nil, err
        }
 
        instCmd := &ContainerLXCCmd{
@@ -5768,7 +5769,7 @@ func (c *containerLXC) Exec(req api.InstanceExecPost, 
stdin *os.File, stdout *os
                attachedChildPid: attachedPid,
        }
 
-       return instCmd, nil
+       return instCmd, nil, nil
 }
 
 func (c *containerLXC) cpuState() api.InstanceStateCPU {
diff --git a/lxd/instance/drivers/vm_qemu.go b/lxd/instance/drivers/vm_qemu.go
index 29bf11973f..a9156481ab 100644
--- a/lxd/instance/drivers/vm_qemu.go
+++ b/lxd/instance/drivers/vm_qemu.go
@@ -2,7 +2,6 @@ package drivers
 
 import (
        "bytes"
-       "encoding/json"
        "fmt"
        "io"
        "io/ioutil"
@@ -2470,30 +2469,8 @@ func (vm *qemu) Console() (*os.File, chan error, error) {
        return console, chDisconnect, nil
 }
 
-func (vm *qemu) forwardSignal(control *websocket.Conn, sig unix.Signal) error {
-       logger.Debugf("Forwarding signal to lxd-agent: %s", sig)
-
-       w, err := control.NextWriter(websocket.TextMessage)
-       if err != nil {
-               return err
-       }
-
-       msg := api.InstanceExecControl{}
-       msg.Command = "signal"
-       msg.Signal = int(sig)
-
-       buf, err := json.Marshal(msg)
-       if err != nil {
-               return err
-       }
-       _, err = w.Write(buf)
-
-       w.Close()
-       return err
-}
-
 // Exec a command inside the instance.
-func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout 
*os.File, stderr *os.File) (instance.Cmd, error) {
+func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout 
*os.File, stderr *os.File) (instance.Cmd, *websocket.Conn, error) {
        var instCmd *Cmd
 
        // Because this function will exit before the remote command has 
finished, we create a
@@ -2518,13 +2495,13 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin 
*os.File, stdout *os.File,
 
        client, err := vm.getAgentClient()
        if err != nil {
-               return nil, err
+               return nil, nil, err
        }
 
        agent, err := lxdClient.ConnectLXDHTTP(nil, client)
        if err != nil {
                logger.Errorf("Failed to connect to lxd-agent on %s: %v", 
vm.Name(), err)
-               return nil, fmt.Errorf("Failed to connect to lxd-agent")
+               return nil, nil, fmt.Errorf("Failed to connect to lxd-agent")
        }
        cleanupFuncs = append(cleanupFuncs, agent.Disconnect)
 
@@ -2533,7 +2510,7 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin 
*os.File, stdout *os.File,
                // Set console to raw.
                oldttystate, err := termios.MakeRaw(int(stdin.Fd()))
                if err != nil {
-                       return nil, err
+                       return nil, nil, err
                }
                cleanupFuncs = append(cleanupFuncs, func() {
                        termios.Restore(int(stdin.Fd()), oldttystate)
@@ -2541,24 +2518,13 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin 
*os.File, stdout *os.File,
        }
 
        dataDone := make(chan bool)
-       signalSendCh := make(chan unix.Signal)
-       signalResCh := make(chan error)
-
-       // This is the signal control handler, it receives signals from lxc CLI 
and forwards them
-       // to the VM agent.
-       controlHander := func(control *websocket.Conn) {
-               closeMsg := 
websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")
-               defer control.WriteMessage(websocket.CloseMessage, closeMsg)
-
-               for {
-                       select {
-                       case signal := <-signalSendCh:
-                               err := vm.forwardSignal(control, signal)
-                               signalResCh <- err
-                       case <-dataDone:
-                               return
-                       }
-               }
+
+       // Retrieve the raw control websocket and pass it to the generic exec 
handler.
+       var wsControl *websocket.Conn
+       chControl := make(chan struct{})
+       controlHander := func(conn *websocket.Conn) {
+               wsControl = conn
+               close(chControl)
        }
 
        args := lxdClient.InstanceExecArgs{
@@ -2571,19 +2537,19 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin 
*os.File, stdout *os.File,
 
        op, err := agent.ExecInstance("", req, &args)
        if err != nil {
-               return nil, err
+               return nil, nil, err
        }
 
+       // Wait for the control websocket to be connected.
+       <-chControl
+
        instCmd = &Cmd{
-               cmd:              op,
-               attachedChildPid: -1, // Process is not running on LXD host.
-               dataDone:         args.DataDone,
-               cleanupFunc:      cleanupFunc,
-               signalSendCh:     signalSendCh,
-               signalResCh:      signalResCh,
+               cmd:         op,
+               dataDone:    args.DataDone,
+               cleanupFunc: cleanupFunc,
        }
 
-       return instCmd, nil
+       return instCmd, wsControl, nil
 }
 
 // Render returns info about the instance.
diff --git a/lxd/instance/drivers/vm_qemu_cmd.go 
b/lxd/instance/drivers/vm_qemu_cmd.go
index 2b117334f0..dc1c7d6c6c 100644
--- a/lxd/instance/drivers/vm_qemu_cmd.go
+++ b/lxd/instance/drivers/vm_qemu_cmd.go
@@ -1,6 +1,8 @@
 package drivers
 
 import (
+       "fmt"
+
        "golang.org/x/sys/unix"
 
        lxdClient "github.com/lxc/lxd/client"
@@ -8,23 +10,19 @@ import (
 
 // Cmd represents a running command for an Qemu VM.
 type Cmd struct {
-       attachedChildPid int
-       cmd              lxdClient.Operation
-       dataDone         chan bool
-       signalSendCh     chan unix.Signal
-       signalResCh      chan error
-       cleanupFunc      func()
+       cmd         lxdClient.Operation
+       dataDone    chan bool
+       cleanupFunc func()
 }
 
 // PID returns the attached child's process ID.
 func (c *Cmd) PID() int {
-       return c.attachedChildPid
+       return -1
 }
 
 // Signal sends a signal to the command.
 func (c *Cmd) Signal(sig unix.Signal) error {
-       c.signalSendCh <- sig
-       return <-c.signalResCh
+       return fmt.Errorf("Not supported")
 }
 
 // Wait for the command to end and returns its exit code and any error.
diff --git a/lxd/instance/instance_interface.go 
b/lxd/instance/instance_interface.go
index 59a39167ad..192420923b 100644
--- a/lxd/instance/instance_interface.go
+++ b/lxd/instance/instance_interface.go
@@ -5,6 +5,8 @@ import (
        "os"
        "time"
 
+       "github.com/gorilla/websocket"
+
        "github.com/lxc/lxd/lxd/backup"
        "github.com/lxc/lxd/lxd/db"
        deviceConfig "github.com/lxc/lxd/lxd/device/config"
@@ -51,7 +53,7 @@ type Instance interface {
 
        // Console - Allocate and run a console tty.
        Console() (*os.File, chan error, error)
-       Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File, stderr 
*os.File) (Cmd, error)
+       Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File, stderr 
*os.File) (Cmd, *websocket.Conn, error)
 
        // Status
        Render() (interface{}, interface{}, error)

From 565dcb3abd5f26178e3ed5547676d8898718c411 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@ubuntu.com>
Date: Sat, 18 Jan 2020 09:28:55 +0200
Subject: [PATCH 3/5] lxd/containers: Fix error handling on stop
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgra...@ubuntu.com>
---
 lxd/container_lxc.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 3d670c00d3..ca6e59c495 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -2625,6 +2625,7 @@ func (c *containerLXC) Stop(stateful bool) error {
        // Load cgroup abstraction
        cg, err := c.cgroup(nil)
        if err != nil {
+               op.Done(err)
                return err
        }
 

From 19f3e9b0559f2d37c0e400c334ae061c7793fb0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@ubuntu.com>
Date: Sat, 18 Jan 2020 09:29:17 +0200
Subject: [PATCH 4/5] lxd/vm: Fix stop race condition
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgra...@ubuntu.com>
---
 lxd/instance/drivers/vm_qemu.go | 38 ++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/lxd/instance/drivers/vm_qemu.go b/lxd/instance/drivers/vm_qemu.go
index a9156481ab..4f78d69dce 100644
--- a/lxd/instance/drivers/vm_qemu.go
+++ b/lxd/instance/drivers/vm_qemu.go
@@ -30,6 +30,7 @@ import (
        "github.com/lxc/lxd/lxd/instance"
        "github.com/lxc/lxd/lxd/instance/drivers/qmp"
        "github.com/lxc/lxd/lxd/instance/instancetype"
+       "github.com/lxc/lxd/lxd/instance/operationlock"
        "github.com/lxc/lxd/lxd/maas"
        "github.com/lxc/lxd/lxd/operations"
        "github.com/lxc/lxd/lxd/project"
@@ -456,17 +457,28 @@ func (vm *qemu) Freeze() error {
 
 // OnStop is run when the instance stops.
 func (vm *qemu) OnStop(target string) error {
+       // Pick up the existing stop operation lock created in Stop() function.
+       op := operationlock.Get(vm.id)
+       if op != nil && op.Action() != "stop" {
+               return fmt.Errorf("Instance is already running a %s operation", 
op.Action())
+       }
+
+       // Cleanup.
        vm.cleanupDevices()
        os.Remove(vm.pidFilePath())
        os.Remove(vm.getMonitorPath())
        vm.unmount()
 
-       // Record power state
+       // Record power state.
        err := vm.state.Cluster.ContainerSetState(vm.id, "STOPPED")
        if err != nil {
+               op.Done(err)
                return err
        }
 
+       // Done after this.
+       defer op.Done(nil)
+
        if target == "reboot" {
                return vm.Start(false)
        }
@@ -1469,18 +1481,27 @@ func (vm *qemu) pid() (int, error) {
 
 // Stop stops the VM.
 func (vm *qemu) Stop(stateful bool) error {
+       // Check that we're not already stopped.
+       if !vm.IsRunning() {
+               return fmt.Errorf("The instance is already stopped")
+       }
+
+       // Check that no stateful stop was requested.
        if stateful {
                return fmt.Errorf("Stateful stop isn't supported for VMs at 
this time")
        }
 
-       if !vm.IsRunning() {
-               return fmt.Errorf("Instance is not running")
+       // Setup a new operation.
+       op, err := operationlock.Create(vm.id, "stop", false, true)
+       if err != nil {
+               return err
        }
 
        // Connect to the monitor.
        monitor, err := qmp.Connect(vm.getMonitorPath(), 
vm.getMonitorEventHandler())
        if err != nil {
                // If we fail to connect, it's most likely because the VM is 
already off.
+               op.Done(nil)
                return nil
        }
 
@@ -1488,9 +1509,11 @@ func (vm *qemu) Stop(stateful bool) error {
        chDisconnect, err := monitor.Wait()
        if err != nil {
                if err == qmp.ErrMonitorDisconnect {
+                       op.Done(nil)
                        return nil
                }
 
+               op.Done(err)
                return err
        }
 
@@ -1498,15 +1521,24 @@ func (vm *qemu) Stop(stateful bool) error {
        err = monitor.Quit()
        if err != nil {
                if err == qmp.ErrMonitorDisconnect {
+                       op.Done(nil)
                        return nil
                }
 
+               op.Done(err)
                return err
        }
 
        // Wait for QEMU to exit (can take a while if pending I/O).
        <-chDisconnect
 
+       // Wait for OnStop.
+       err = op.Wait()
+       if err != nil && vm.IsRunning() {
+               return err
+       }
+
+       vm.state.Events.SendLifecycle(vm.project, "virtual-machine-stopped", 
fmt.Sprintf("/1.0/virtual-machines/%s", vm.name), nil)
        return nil
 }
 

From 7d5e22c0cf34f230413c107a751683814f09c63e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgra...@ubuntu.com>
Date: Sat, 18 Jan 2020 09:36:24 +0200
Subject: [PATCH 5/5] lxd/vm: Add locking for stop and shutdown
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgra...@ubuntu.com>
---
 lxd/instance/drivers/vm_qemu.go | 40 +++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/lxd/instance/drivers/vm_qemu.go b/lxd/instance/drivers/vm_qemu.go
index 4f78d69dce..e283e2fe78 100644
--- a/lxd/instance/drivers/vm_qemu.go
+++ b/lxd/instance/drivers/vm_qemu.go
@@ -492,9 +492,16 @@ func (vm *qemu) Shutdown(timeout time.Duration) error {
                return fmt.Errorf("The instance is already stopped")
        }
 
+       // Setup a new operation
+       op, err := operationlock.Create(vm.id, "stop", true, true)
+       if err != nil {
+               return err
+       }
+
        // Connect to the monitor.
        monitor, err := qmp.Connect(vm.getMonitorPath(), 
vm.getMonitorEventHandler())
        if err != nil {
+               op.Done(err)
                return err
        }
 
@@ -502,9 +509,11 @@ func (vm *qemu) Shutdown(timeout time.Duration) error {
        chDisconnect, err := monitor.Wait()
        if err != nil {
                if err == qmp.ErrMonitorDisconnect {
+                       op.Done(nil)
                        return nil
                }
 
+               op.Done(err)
                return err
        }
 
@@ -512,9 +521,11 @@ func (vm *qemu) Shutdown(timeout time.Duration) error {
        err = monitor.Powerdown()
        if err != nil {
                if err == qmp.ErrMonitorDisconnect {
+                       op.Done(nil)
                        return nil
                }
 
+               op.Done(err)
                return err
        }
 
@@ -522,14 +533,19 @@ func (vm *qemu) Shutdown(timeout time.Duration) error {
        if timeout > 0 {
                select {
                case <-chDisconnect:
+                       op.Done(nil)
+                       vm.state.Events.SendLifecycle(vm.project, 
"instance-shutdown", fmt.Sprintf("/1.0/virtual-machines/%s", vm.name), nil)
                        return nil
                case <-time.After(timeout):
+                       op.Done(fmt.Errorf("Instance was not shutdown after 
timeout"))
                        return fmt.Errorf("Instance was not shutdown after 
timeout")
                }
        } else {
                <-chDisconnect // Block until VM is not running if no timeout 
provided.
        }
 
+       op.Done(nil)
+       vm.state.Events.SendLifecycle(vm.project, "instance-shutdown", 
fmt.Sprintf("/1.0/virtual-machines/%s", vm.name), nil)
        return nil
 }
 
@@ -553,29 +569,41 @@ func (vm *qemu) Start(stateful bool) error {
                return fmt.Errorf("The instance is already running")
        }
 
+       // Setup a new operation
+       op, err := operationlock.Create(vm.id, "start", false, false)
+       if err != nil {
+               return errors.Wrap(err, "Create instance start operation")
+       }
+       defer op.Done(nil)
+
        // Mount the instance's config volume.
        _, err = vm.mount()
        if err != nil {
+               op.Done(err)
                return err
        }
 
        err = vm.generateConfigShare()
        if err != nil {
+               op.Done(err)
                return err
        }
 
        err = os.MkdirAll(vm.LogPath(), 0700)
        if err != nil {
+               op.Done(err)
                return err
        }
 
        err = os.MkdirAll(vm.DevicesPath(), 0711)
        if err != nil {
+               op.Done(err)
                return err
        }
 
        err = os.MkdirAll(vm.ShmountsPath(), 0711)
        if err != nil {
+               op.Done(err)
                return err
        }
 
@@ -591,6 +619,7 @@ func (vm *qemu) Start(stateful bool) error {
        if !shared.PathExists(vm.getNvramPath()) {
                err = vm.setupNvram()
                if err != nil {
+                       op.Done(err)
                        return err
                }
        }
@@ -602,6 +631,7 @@ func (vm *qemu) Start(stateful bool) error {
                // Start the device.
                runConf, err := vm.deviceStart(dev.Name, dev.Config, false)
                if err != nil {
+                       op.Done(err)
                        return errors.Wrapf(err, "Failed to start device '%s'", 
dev.Name)
                }
 
@@ -615,17 +645,20 @@ func (vm *qemu) Start(stateful bool) error {
        // Get qemu configuration
        qemuBinary, qemuType, qemuConfig, err := vm.qemuArchConfig()
        if err != nil {
+               op.Done(err)
                return err
        }
 
        confFile, err := vm.generateQemuConfigFile(qemuType, qemuConfig, 
devConfs)
        if err != nil {
+               op.Done(err)
                return err
        }
 
        // Check qemu is installed.
        _, err = exec.LookPath(qemuBinary)
        if err != nil {
+               op.Done(err)
                return err
        }
 
@@ -670,6 +703,7 @@ func (vm *qemu) Start(stateful bool) error {
                                return nil
                        })
                if err != nil {
+                       op.Done(err)
                        return err
                }
        }
@@ -685,18 +719,21 @@ func (vm *qemu) Start(stateful bool) error {
 
        _, err = shared.RunCommand(qemuBinary, args...)
        if err != nil {
+               op.Done(err)
                return err
        }
 
        // Start QMP monitoring.
        monitor, err := qmp.Connect(vm.getMonitorPath(), 
vm.getMonitorEventHandler())
        if err != nil {
+               op.Done(err)
                return err
        }
 
        // Start the VM.
        err = monitor.Start()
        if err != nil {
+               op.Done(err)
                return err
        }
 
@@ -717,9 +754,12 @@ func (vm *qemu) Start(stateful bool) error {
                return nil
        })
        if err != nil {
+               op.Done(err)
                return err
        }
 
+       vm.state.Events.SendLifecycle(vm.project, "virtual-machine-started", 
fmt.Sprintf("/1.0/virtual-machines/%s", vm.name), nil)
+
        return nil
 }
 
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to