[lxc-devel] [lxd/master] Instance: Exec window resize via instance.Cmd interface

tomponline on Github lxc-bot at linuxcontainers.org
Tue Jan 21 11:43:28 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 433 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200121/0268c36d/attachment-0001.bin>
-------------- next part --------------
From 254bed8b320f9bb774c0b3b7742cdfc2673b9aa7 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 20 Jan 2020 14:47:17 +0000
Subject: [PATCH 01/12] lxd/container/exec: Removes duplication of env map now
 its being stored back into post data

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_exec.go | 44 +++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index 8bef4a1d4b..8107478c8f 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -391,52 +391,46 @@ func containerExecPost(d *Daemon, r *http.Request) response.Response {
 	}
 
 	// Process environment.
-	env := map[string]string{}
-	for k, v := range inst.ExpandedConfig() {
-		if strings.HasPrefix(k, "environment.") {
-			env[strings.TrimPrefix(k, "environment.")] = v
-		}
+	if post.Environment == nil {
+		post.Environment = map[string]string{}
 	}
 
-	if post.Environment != nil {
-		for k, v := range post.Environment {
-			env[k] = v
+	for k, v := range inst.ExpandedConfig() {
+		if strings.HasPrefix(k, "environment.") {
+			post.Environment[strings.TrimPrefix(k, "environment.")] = v
 		}
 	}
 
-	// Set default value for PATH
-	_, ok := env["PATH"]
+	// Set default value for PATH.
+	_, ok := post.Environment["PATH"]
 	if !ok {
-		env["PATH"] = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+		post.Environment["PATH"] = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
 		if inst.FileExists("/snap") == nil {
-			env["PATH"] = fmt.Sprintf("%s:/snap/bin", env["PATH"])
+			post.Environment["PATH"] = fmt.Sprintf("%s:/snap/bin", post.Environment["PATH"])
 		}
 	}
 
-	// If running as root, set some env variables
+	// If running as root, set some env variables.
 	if post.User == 0 {
-		// Set default value for HOME
-		_, ok = env["HOME"]
+		// Set default value for HOME.
+		_, ok = post.Environment["HOME"]
 		if !ok {
-			env["HOME"] = "/root"
+			post.Environment["HOME"] = "/root"
 		}
 
-		// Set default value for USER
-		_, ok = env["USER"]
+		// Set default value for USER.
+		_, ok = post.Environment["USER"]
 		if !ok {
-			env["USER"] = "root"
+			post.Environment["USER"] = "root"
 		}
 	}
 
-	// Set default value for LANG
-	_, ok = env["LANG"]
+	// Set default value for LANG.
+	_, ok = post.Environment["LANG"]
 	if !ok {
-		env["LANG"] = "C.UTF-8"
+		post.Environment["LANG"] = "C.UTF-8"
 	}
 
-	// Apply to request.
-	post.Environment = env
-
 	if post.WaitForWS {
 		ws := &execWs{}
 		ws.fds = map[int]string{}

From 540ee8a891d8bdd2e36bbc3369e4d18018535d28 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 10:02:26 +0000
Subject: [PATCH 02/12] Revert "lxd/exec: Forward control messages"

This reverts commit 86c70c80d5f6a0665087c54dd7552361efc77f33.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_exec.go                   | 18 ++-----
 lxd/container_lxc.go                    | 13 +++--
 lxd/instance/drivers/driver_qemu.go     | 72 ++++++++++++++++++-------
 lxd/instance/drivers/driver_qemu_cmd.go | 16 +++---
 lxd/instance/instance_interface.go      |  4 +-
 5 files changed, 72 insertions(+), 51 deletions(-)

diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index 8107478c8f..40c59bbd1b 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -40,7 +40,6 @@ type execWs struct {
 	allConnected     chan bool
 	controlConnected chan bool
 	fds              map[int]string
-	remoteControl    *websocket.Conn
 }
 
 func (s *execWs) Metadata() interface{} {
@@ -163,16 +162,6 @@ 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()
@@ -321,7 +310,7 @@ func (s *execWs) Do(op *operations.Operation) error {
 		return cmdErr
 	}
 
-	cmd, wsControl, err := s.instance.Exec(s.req, stdin, stdout, stderr)
+	cmd, err := s.instance.Exec(s.req, stdin, stdout, stderr)
 	if err != nil {
 		return err
 	}
@@ -329,7 +318,6 @@ 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()
@@ -495,7 +483,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
 			}
@@ -512,7 +500,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 4e8f3eb3ba..895ca69ee9 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -19,7 +19,6 @@ 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"
@@ -5687,7 +5686,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, *websocket.Conn, 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{}
 
@@ -5699,7 +5698,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, nil, err
+		return nil, err
 	}
 
 	// Prepare the subcommand
@@ -5752,21 +5751,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, nil, err
+		return nil, err
 	}
 
 	cmd.ExtraFiles = []*os.File{stdin, stdout, stderr, wStatus}
 	err = cmd.Start()
 	if err != nil {
 		wStatus.Close()
-		return nil, nil, err
+		return 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, nil, err
+		return nil, err
 	}
 
 	instCmd := &ContainerLXCCmd{
@@ -5774,7 +5773,7 @@ func (c *containerLXC) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os
 		attachedChildPid: attachedPid,
 	}
 
-	return instCmd, nil, nil
+	return instCmd, nil
 }
 
 func (c *containerLXC) cpuState() api.InstanceStateCPU {
diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index c20d3f784b..6bdd96f5f6 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -2,6 +2,7 @@ package drivers
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -2783,8 +2784,30 @@ 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, *websocket.Conn, 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
@@ -2809,13 +2832,13 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 
 	client, err := vm.getAgentClient()
 	if err != nil {
-		return nil, nil, err
+		return 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, nil, fmt.Errorf("Failed to connect to lxd-agent")
+		return nil, fmt.Errorf("Failed to connect to lxd-agent")
 	}
 	cleanupFuncs = append(cleanupFuncs, agent.Disconnect)
 
@@ -2824,7 +2847,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, nil, err
+			return nil, err
 		}
 		cleanupFuncs = append(cleanupFuncs, func() {
 			termios.Restore(int(stdin.Fd()), oldttystate)
@@ -2832,13 +2855,24 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 	}
 
 	dataDone := make(chan bool)
-
-	// 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)
+	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
+			}
+		}
 	}
 
 	args := lxdClient.InstanceExecArgs{
@@ -2851,19 +2885,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, nil, err
+		return nil, err
 	}
 
-	// Wait for the control websocket to be connected.
-	<-chControl
-
 	instCmd = &Cmd{
-		cmd:         op,
-		dataDone:    args.DataDone,
-		cleanupFunc: cleanupFunc,
+		cmd:              op,
+		attachedChildPid: -1, // Process is not running on LXD host.
+		dataDone:         args.DataDone,
+		cleanupFunc:      cleanupFunc,
+		signalSendCh:     signalSendCh,
+		signalResCh:      signalResCh,
 	}
 
-	return instCmd, wsControl, nil
+	return instCmd, nil
 }
 
 // Render returns info about the instance.
diff --git a/lxd/instance/drivers/driver_qemu_cmd.go b/lxd/instance/drivers/driver_qemu_cmd.go
index dc1c7d6c6c..2b117334f0 100644
--- a/lxd/instance/drivers/driver_qemu_cmd.go
+++ b/lxd/instance/drivers/driver_qemu_cmd.go
@@ -1,8 +1,6 @@
 package drivers
 
 import (
-	"fmt"
-
 	"golang.org/x/sys/unix"
 
 	lxdClient "github.com/lxc/lxd/client"
@@ -10,19 +8,23 @@ import (
 
 // Cmd represents a running command for an Qemu VM.
 type Cmd struct {
-	cmd         lxdClient.Operation
-	dataDone    chan bool
-	cleanupFunc func()
+	attachedChildPid int
+	cmd              lxdClient.Operation
+	dataDone         chan bool
+	signalSendCh     chan unix.Signal
+	signalResCh      chan error
+	cleanupFunc      func()
 }
 
 // PID returns the attached child's process ID.
 func (c *Cmd) PID() int {
-	return -1
+	return c.attachedChildPid
 }
 
 // Signal sends a signal to the command.
 func (c *Cmd) Signal(sig unix.Signal) error {
-	return fmt.Errorf("Not supported")
+	c.signalSendCh <- sig
+	return <-c.signalResCh
 }
 
 // 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 192420923b..59a39167ad 100644
--- a/lxd/instance/instance_interface.go
+++ b/lxd/instance/instance_interface.go
@@ -5,8 +5,6 @@ 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"
@@ -53,7 +51,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, *websocket.Conn, error)
+	Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File, stderr *os.File) (Cmd, error)
 
 	// Status
 	Render() (interface{}, interface{}, error)

From f36bc0ce605c7e222f091faec7b7d8963a0137e9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 10:15:14 +0000
Subject: [PATCH 03/12] lxd/instance/drivers/driver/qemu/cmd: Makes qemu cmd
 struct qemu specific

Now its in a shared drivers package.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu_cmd.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lxd/instance/drivers/driver_qemu_cmd.go b/lxd/instance/drivers/driver_qemu_cmd.go
index 2b117334f0..ff1db96133 100644
--- a/lxd/instance/drivers/driver_qemu_cmd.go
+++ b/lxd/instance/drivers/driver_qemu_cmd.go
@@ -7,7 +7,7 @@ import (
 )
 
 // Cmd represents a running command for an Qemu VM.
-type Cmd struct {
+type qemuCmd struct {
 	attachedChildPid int
 	cmd              lxdClient.Operation
 	dataDone         chan bool
@@ -17,18 +17,18 @@ type Cmd struct {
 }
 
 // PID returns the attached child's process ID.
-func (c *Cmd) PID() int {
+func (c *qemuCmd) PID() int {
 	return c.attachedChildPid
 }
 
 // Signal sends a signal to the command.
-func (c *Cmd) Signal(sig unix.Signal) error {
+func (c *qemuCmd) Signal(sig unix.Signal) error {
 	c.signalSendCh <- sig
 	return <-c.signalResCh
 }
 
 // Wait for the command to end and returns its exit code and any error.
-func (c *Cmd) Wait() (int, error) {
+func (c *qemuCmd) Wait() (int, error) {
 	if c.cleanupFunc != nil {
 		defer c.cleanupFunc()
 	}

From 697b54ed5821f64c7fa4eb62b4634fe8fb904cf9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 10:28:07 +0000
Subject: [PATCH 04/12] lxd/instance/drivers/driver/qemu: Simplifies Exec with
 revert

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu.go | 43 ++++++++---------------------
 1 file changed, 12 insertions(+), 31 deletions(-)

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 6bdd96f5f6..0443ff9b76 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -37,6 +37,7 @@ import (
 	"github.com/lxc/lxd/lxd/maas"
 	"github.com/lxc/lxd/lxd/operations"
 	"github.com/lxc/lxd/lxd/project"
+	"github.com/lxc/lxd/lxd/revert"
 	"github.com/lxc/lxd/lxd/state"
 	storagePools "github.com/lxc/lxd/lxd/storage"
 	storageDrivers "github.com/lxc/lxd/lxd/storage/drivers"
@@ -2808,27 +2809,8 @@ func (vm *qemu) forwardSignal(control *websocket.Conn, sig unix.Signal) error {
 
 // 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) {
-	var instCmd *Cmd
-
-	// Because this function will exit before the remote command has finished, we create a
-	// cleanup function that will be passed to the instance function if successfully started to
-	// perform any cleanup needed when finished.
-	cleanupFuncs := []func(){}
-	cleanupFunc := func() {
-		for _, f := range cleanupFuncs {
-			f()
-		}
-	}
-
-	defer func() {
-		// If no instance command has been been created it means something went wrong
-		// starting the remote command, so we should cleanup as this function ends.
-		// If the instance command is non-nil then we let the instance command itself run
-		// the cleanup functions when it is done.
-		if instCmd == nil {
-			cleanupFunc()
-		}
-	}()
+	revert := revert.New()
+	defer revert.Fail()
 
 	client, err := vm.getAgentClient()
 	if err != nil {
@@ -2840,7 +2822,7 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 		logger.Errorf("Failed to connect to lxd-agent on %s: %v", vm.Name(), err)
 		return nil, fmt.Errorf("Failed to connect to lxd-agent")
 	}
-	cleanupFuncs = append(cleanupFuncs, agent.Disconnect)
+	revert.Add(agent.Disconnect)
 
 	req.WaitForWS = true
 	if req.Interactive {
@@ -2849,18 +2831,16 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 		if err != nil {
 			return nil, err
 		}
-		cleanupFuncs = append(cleanupFuncs, func() {
-			termios.Restore(int(stdin.Fd()), oldttystate)
-		})
+
+		revert.Add(func() { termios.Restore(int(stdin.Fd()), oldttystate) })
 	}
 
 	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) {
+	// This is the signal control handler, it receives signals from lxc CLI and forwards them to the VM agent.
+	controlHandler := func(control *websocket.Conn) {
 		closeMsg := websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")
 		defer control.WriteMessage(websocket.CloseMessage, closeMsg)
 
@@ -2880,7 +2860,7 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 		Stdout:   stdout,
 		Stderr:   stderr,
 		DataDone: dataDone,
-		Control:  controlHander,
+		Control:  controlHandler,
 	}
 
 	op, err := agent.ExecInstance("", req, &args)
@@ -2888,15 +2868,16 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 		return nil, err
 	}
 
-	instCmd = &Cmd{
+	instCmd := &qemuCmd{
 		cmd:              op,
 		attachedChildPid: -1, // Process is not running on LXD host.
 		dataDone:         args.DataDone,
-		cleanupFunc:      cleanupFunc,
+		cleanupFunc:      revert.Clone().Fail, // Pass revert function clone as clean up function.
 		signalSendCh:     signalSendCh,
 		signalResCh:      signalResCh,
 	}
 
+	revert.Success()
 	return instCmd, nil
 }
 

From 4ffe1d15524235a480f6dad917aac8b4416ed0ad Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 11:25:34 +0000
Subject: [PATCH 05/12] lxd/container/exec: Cleaned up logging

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_exec.go | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index 40c59bbd1b..14a1ae9548 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -162,7 +162,7 @@ func (s *execWs) Do(op *operations.Operation) error {
 				return
 			}
 
-			logger.Debugf("Interactive child process handler started for child PID %d", attachedChild.PID())
+			logger.Debugf(`Interactive child process handler started for child PID "%d"`, attachedChild.PID())
 			for {
 				s.connsLock.Lock()
 				conn := s.conns[-1]
@@ -174,7 +174,7 @@ func (s *execWs) Do(op *operations.Operation) error {
 				}
 
 				if err != nil {
-					logger.Debugf("Got error getting next reader %s", err)
+					logger.Debugf("Got error getting next reader: %v", err)
 					er, ok := err.(*websocket.CloseError)
 					if !ok {
 						break
@@ -187,51 +187,50 @@ func (s *execWs) Do(op *operations.Operation) error {
 					// If an abnormal closure occurred, kill the attached child.
 					err := attachedChild.Signal(unix.SIGKILL)
 					if err != nil {
-						logger.Debugf("Failed to send SIGKILL to PID %d: %v", attachedChild.PID(), err)
+						logger.Debugf(`Failed to send SIGKILL to PID "%d": %v`, attachedChild.PID(), err)
 					} else {
-						logger.Debugf("Sent SIGKILL to PID %d", attachedChild.PID())
+						logger.Debugf(`Sent SIGKILL to PID "%d"`, attachedChild.PID())
 					}
 					return
 				}
 
 				buf, err := ioutil.ReadAll(r)
 				if err != nil {
-					logger.Debugf("Failed to read message %s", err)
+					logger.Debugf("Failed to read message: %v", err)
 					break
 				}
 
 				command := api.InstanceExecControl{}
 
 				if err := json.Unmarshal(buf, &command); err != nil {
-					logger.Debugf("Failed to unmarshal control socket command: %s", err)
+					logger.Debugf("Failed to unmarshal control socket command: %v", err)
 					continue
 				}
 
 				if command.Command == "window-resize" {
 					winchWidth, err := strconv.Atoi(command.Args["width"])
 					if err != nil {
-						logger.Debugf("Unable to extract window width: %s", err)
+						logger.Debugf("Unable to extract window width: %v", err)
 						continue
 					}
 
 					winchHeight, err := strconv.Atoi(command.Args["height"])
 					if err != nil {
-						logger.Debugf("Unable to extract window height: %s", err)
+						logger.Debugf("Unable to extract window height: %v", err)
 						continue
 					}
 
 					err = shared.SetSize(int(ptys[0].Fd()), winchWidth, winchHeight)
 					if err != nil {
-						logger.Debugf("Failed to set window size to: %dx%d", winchWidth, winchHeight)
+						logger.Debugf(`Failed to set window size to "%dx%d": %v`, winchWidth, winchHeight, err)
 						continue
 					}
 				} else if command.Command == "signal" {
 					err := attachedChild.Signal(unix.Signal(command.Signal))
 					if err != nil {
-						logger.Debugf("Failed forwarding signal '%d' to PID %d: %v", command.Signal, attachedChild.PID(), err)
+						logger.Debugf(`Failed forwarding signal "%d" to PID "%d": %v`, command.Signal, attachedChild.PID(), err)
 						continue
 					}
-					logger.Debugf("Forwarded signal '%d' to PID %d", command.Signal, attachedChild.PID())
 				}
 			}
 		}()

From e501531add0be6806384b5928aefbb4caf43042d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 11:25:46 +0000
Subject: [PATCH 06/12] lxd/container/exec: Switches to use instance command
 for resizing window

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_exec.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index 14a1ae9548..5313bc3e83 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -220,7 +220,7 @@ func (s *execWs) Do(op *operations.Operation) error {
 						continue
 					}
 
-					err = shared.SetSize(int(ptys[0].Fd()), winchWidth, winchHeight)
+					err = attachedChild.WindowResize(int(ptys[0].Fd()), winchWidth, winchHeight)
 					if err != nil {
 						logger.Debugf(`Failed to set window size to "%dx%d": %v`, winchWidth, winchHeight, err)
 						continue

From 3f85f1e3022e9918743b26f90aabe3463290f1c9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 11:26:17 +0000
Subject: [PATCH 07/12] lxd/container/lxc/exec/cmd: Adds WindowResize

And cleans up Signal function.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/container_lxc_exec_cmd.go | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/lxd/container_lxc_exec_cmd.go b/lxd/container_lxc_exec_cmd.go
index 9a1bb64a9b..ea9543022a 100644
--- a/lxd/container_lxc_exec_cmd.go
+++ b/lxd/container_lxc_exec_cmd.go
@@ -5,6 +5,9 @@ import (
 	"syscall"
 
 	"golang.org/x/sys/unix"
+
+	"github.com/lxc/lxd/shared"
+	"github.com/lxc/lxd/shared/logger"
 )
 
 // ContainerLXCCmd represents a running command for an LXC container.
@@ -19,12 +22,13 @@ func (c *ContainerLXCCmd) PID() int {
 }
 
 // Signal sends a signal to the command.
-func (c *ContainerLXCCmd) Signal(s unix.Signal) error {
-	err := unix.Kill(c.attachedChildPid, s)
+func (c *ContainerLXCCmd) Signal(sig unix.Signal) error {
+	err := unix.Kill(c.attachedChildPid, sig)
 	if err != nil {
 		return err
 	}
 
+	logger.Debugf(`Forwarded signal "%d" to PID "%d"`, sig, c.PID())
 	return nil
 }
 
@@ -50,3 +54,14 @@ func (c *ContainerLXCCmd) Wait() (int, error) {
 
 	return 0, nil
 }
+
+// WindowResize resizes the running command's window.
+func (c *ContainerLXCCmd) WindowResize(fd, winchWidth, winchHeight int) error {
+	err := shared.SetSize(fd, winchWidth, winchHeight)
+	if err != nil {
+		return err
+	}
+
+	logger.Debugf(`Set window size "%dx%d" of PID "%d"`, winchWidth, winchHeight, c.PID())
+	return nil
+}

From 79d6811a20fb20e4bd0bd2b5116113e85beb018e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 11:26:45 +0000
Subject: [PATCH 08/12] lxd/instance/instance/exec/cmd: Adds WindowResize
 function to signature

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/instance_exec_cmd.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lxd/instance/instance_exec_cmd.go b/lxd/instance/instance_exec_cmd.go
index b3ce642107..fe493a7964 100644
--- a/lxd/instance/instance_exec_cmd.go
+++ b/lxd/instance/instance_exec_cmd.go
@@ -9,4 +9,5 @@ type Cmd interface {
 	Wait() (int, error)
 	PID() int
 	Signal(s unix.Signal) error
+	WindowResize(fd, winchWidth, winchHeight int) error
 }

From 403bfa2b0a6375f550e09a22c4870a3a5ac095c9 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 11:27:08 +0000
Subject: [PATCH 09/12] lxd/instance/drivers/driver/qemu: Reworks command
 control

Switches to general control command forwarder to support resize window.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu.go | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 0443ff9b76..5a25c3c468 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -2785,19 +2785,13 @@ 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)
-
+func (vm *qemu) forwardControlCommand(control *websocket.Conn, cmd api.InstanceExecControl) error {
 	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)
+	buf, err := json.Marshal(cmd)
 	if err != nil {
 		return err
 	}
@@ -2836,8 +2830,8 @@ 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)
+	controlSendCh := make(chan api.InstanceExecControl)
+	controlResCh := make(chan error)
 
 	// This is the signal control handler, it receives signals from lxc CLI and forwards them to the VM agent.
 	controlHandler := func(control *websocket.Conn) {
@@ -2846,9 +2840,9 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 
 		for {
 			select {
-			case signal := <-signalSendCh:
-				err := vm.forwardSignal(control, signal)
-				signalResCh <- err
+			case cmd := <-controlSendCh:
+				err := vm.forwardControlCommand(control, cmd)
+				controlResCh <- err
 			case <-dataDone:
 				return
 			}
@@ -2873,8 +2867,8 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 		attachedChildPid: -1, // Process is not running on LXD host.
 		dataDone:         args.DataDone,
 		cleanupFunc:      revert.Clone().Fail, // Pass revert function clone as clean up function.
-		signalSendCh:     signalSendCh,
-		signalResCh:      signalResCh,
+		controlSendCh:    controlSendCh,
+		controlResCh:     controlResCh,
 	}
 
 	revert.Success()

From 3de080956973501361edbebe713284cfe17a20f5 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 11:27:43 +0000
Subject: [PATCH 10/12] lxd/instance/drivers/driver/qemu/cmd: Adds WindowResize
 support

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu_cmd.go | 42 ++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/lxd/instance/drivers/driver_qemu_cmd.go b/lxd/instance/drivers/driver_qemu_cmd.go
index ff1db96133..edc7e5ed25 100644
--- a/lxd/instance/drivers/driver_qemu_cmd.go
+++ b/lxd/instance/drivers/driver_qemu_cmd.go
@@ -1,9 +1,13 @@
 package drivers
 
 import (
+	"strconv"
+
 	"golang.org/x/sys/unix"
 
 	lxdClient "github.com/lxc/lxd/client"
+	"github.com/lxc/lxd/shared/api"
+	"github.com/lxc/lxd/shared/logger"
 )
 
 // Cmd represents a running command for an Qemu VM.
@@ -11,8 +15,8 @@ type qemuCmd struct {
 	attachedChildPid int
 	cmd              lxdClient.Operation
 	dataDone         chan bool
-	signalSendCh     chan unix.Signal
-	signalResCh      chan error
+	controlSendCh    chan api.InstanceExecControl
+	controlResCh     chan error
 	cleanupFunc      func()
 }
 
@@ -23,8 +27,19 @@ func (c *qemuCmd) PID() int {
 
 // Signal sends a signal to the command.
 func (c *qemuCmd) Signal(sig unix.Signal) error {
-	c.signalSendCh <- sig
-	return <-c.signalResCh
+	command := api.InstanceExecControl{
+		Command: "signal",
+		Signal:  int(sig),
+	}
+
+	c.controlSendCh <- command
+	err := <-c.controlResCh
+	if err != nil {
+		return err
+	}
+
+	logger.Debugf(`Forwarded signal "%d" to lxd-agent`, sig)
+	return nil
 }
 
 // Wait for the command to end and returns its exit code and any error.
@@ -44,3 +59,22 @@ func (c *qemuCmd) Wait() (int, error) {
 
 	return exitCode, nil
 }
+
+// WindowResize resizes the running command's window.
+func (c *qemuCmd) WindowResize(fd, winchWidth, winchHeight int) error {
+	command := api.InstanceExecControl{
+		Command: "window-resize",
+		Args: map[string]string{
+			"width":  strconv.Itoa(winchWidth),
+			"height": strconv.Itoa(winchHeight),
+		},
+	}
+
+	c.controlSendCh <- command
+	err := <-c.controlResCh
+	if err != nil {
+		return err
+	}
+	logger.Debugf(`Forwarded window resize "%dx%d" to lxd-agent`, winchWidth, winchHeight)
+	return nil
+}

From 6b2627ef408b28243b0d7fbf3abeca6720d8be5e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 11:39:10 +0000
Subject: [PATCH 11/12] lxd/instance/drivers/driver/qemu: Sets PID to 0 for VM
 commands

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 5a25c3c468..036c161dc1 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -2864,7 +2864,7 @@ func (vm *qemu) Exec(req api.InstanceExecPost, stdin *os.File, stdout *os.File,
 
 	instCmd := &qemuCmd{
 		cmd:              op,
-		attachedChildPid: -1, // Process is not running on LXD host.
+		attachedChildPid: 0, // Process is not running on LXD host.
 		dataDone:         args.DataDone,
 		cleanupFunc:      revert.Clone().Fail, // Pass revert function clone as clean up function.
 		controlSendCh:    controlSendCh,

From b3b896725d800ef9d856627be6c86bbe8fd536b2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jan 2020 11:40:59 +0000
Subject: [PATCH 12/12] lxd/instance/drivers/driver/qemu: comment on
 forwardControlCommand

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/instance/drivers/driver_qemu.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index 036c161dc1..0639b907b2 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -2785,6 +2785,7 @@ func (vm *qemu) Console() (*os.File, chan error, error) {
 	return console, chDisconnect, nil
 }
 
+// forwardControlCommand is used to send command control messages to the lxd-agent.
 func (vm *qemu) forwardControlCommand(control *websocket.Conn, cmd api.InstanceExecControl) error {
 	w, err := control.NextWriter(websocket.TextMessage)
 	if err != nil {


More information about the lxc-devel mailing list