The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/6867
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) ===
- Adds ability to switch `WebsocketExecMirror` from using custom `poll` to using standard `read`.
- Closes PTYs earlier to prevent `WebsocketExecMirror` from blocking on reading of file handle.
From 0d76248959f4b96ecbff4e8740d616455cb6a061 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 11 Feb 2020 12:13:32 +0000
Subject: [PATCH 1/3] shared/network: Don't log contents of file handle in
WebsocketRecvStream
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
shared/network.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/shared/network.go b/shared/network.go
index 71e5f09551..3a49052850 100644
--- a/shared/network.go
+++ b/shared/network.go
@@ -211,7 +211,7 @@ func WebsocketRecvStream(w io.Writer, conn *websocket.Conn)
chan bool {
}
if err != nil {
- logger.Debugf("Got error getting next reader
%s, %s", err, w)
+ logger.Debugf("Got error getting next reader
%s", err)
break
}
From 5106f95ceb32b8a0603326906279f30e2d66deea Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 11 Feb 2020 12:13:57 +0000
Subject: [PATCH 2/3] shared/netutils/network/linux: Adds ability to switch
WebsocketExecMirror to use ReaderToChannel
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
shared/netutils/network_linux.go | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/shared/netutils/network_linux.go b/shared/netutils/network_linux.go
index cbea0423b3..93f0846768 100644
--- a/shared/netutils/network_linux.go
+++ b/shared/netutils/network_linux.go
@@ -171,14 +171,22 @@ func NetnsGetifaddrs(initPID int32)
(map[string]api.InstanceStateNetwork, error)
return networks, nil
}
-func WebsocketExecMirror(conn *websocket.Conn, w io.WriteCloser, r
io.ReadCloser, exited chan bool, fd int) (chan bool, chan bool) {
+// WebsocketExecMirror if supplied with pollFd then it uses polling on that
descriptor.
+func WebsocketExecMirror(conn *websocket.Conn, w io.WriteCloser, r
io.ReadCloser, exited chan bool, pollFd int) (chan bool, chan bool) {
readDone := make(chan bool, 1)
writeDone := make(chan bool, 1)
go shared.DefaultWriter(conn, w, writeDone)
go func(conn *websocket.Conn, r io.ReadCloser) {
- in := shared.ExecReaderToChannel(r, -1, exited, fd)
+ var in <-chan []byte
+ if pollFd > 0 {
+ in = shared.ExecReaderToChannel(r, -1, exited, pollFd)
+
+ } else {
+ in = shared.ReaderToChannel(r, -1)
+ }
+
for {
buf, ok := <-in
if !ok {
From 28c0f927e295e3bb5873ef780b478a86338c8fce Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parr...@canonical.com>
Date: Tue, 11 Feb 2020 12:14:41 +0000
Subject: [PATCH 3/3] lxd/container/exec: Switch WebsocketMirror to use
standard read mechanism for VMs
Closes PTY earlier so it doesn't block the mirror function.
Signed-off-by: Thomas Parrott <thomas.parr...@canonical.com>
---
lxd-agent/exec.go | 15 ++++++++-------
lxd/container_exec.go | 22 ++++++++++++++--------
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/lxd-agent/exec.go b/lxd-agent/exec.go
index 1feddb8325..7bbc84643a 100644
--- a/lxd-agent/exec.go
+++ b/lxd-agent/exec.go
@@ -349,8 +349,9 @@ func (s *execWs) Do(op *operations.Operation) error {
conn := s.conns[0]
s.connsLock.Unlock()
- logger.Info("Started mirroring websocket")
- readDone, writeDone :=
netutils.WebsocketExecMirror(conn, ptys[0], ptys[0], attachedChildIsDead,
int(ptys[0].Fd()))
+ pollFD := int(ptys[0].Fd())
+ logger.Infof("Started mirroring websocket with FD %v",
pollFD)
+ readDone, writeDone :=
netutils.WebsocketExecMirror(conn, ptys[0], ptys[0], attachedChildIsDead,
pollFD)
<-readDone
<-writeDone
@@ -388,6 +389,11 @@ func (s *execWs) Do(op *operations.Operation) error {
tty.Close()
}
+ // Ensure PTYs are closed before websocket to end wgEOF.Wait().
+ for _, pty := range ptys {
+ pty.Close()
+ }
+
s.connsLock.Lock()
conn := s.conns[-1]
s.connsLock.Unlock()
@@ -401,13 +407,8 @@ func (s *execWs) Do(op *operations.Operation) error {
}
attachedChildIsDead <- true
-
wgEOF.Wait()
- for _, pty := range ptys {
- pty.Close()
- }
-
metadata := shared.Jmap{"return": cmdResult}
err = op.UpdateMetadata(metadata)
if err != nil {
diff --git a/lxd/container_exec.go b/lxd/container_exec.go
index 2302ee54b3..0853b8b923 100644
--- a/lxd/container_exec.go
+++ b/lxd/container_exec.go
@@ -240,10 +240,16 @@ func (s *execWs) Do(op *operations.Operation) error {
conn := s.conns[0]
s.connsLock.Unlock()
- logger.Debugf("Started mirroring websocket")
- defer logger.Debugf("Finished mirroring websocket")
- readDone, writeDone :=
netutils.WebsocketExecMirror(conn, ptys[0], ptys[0], attachedChildIsDead,
int(ptys[0].Fd()))
+ pollFD := 0
+
+ // Use the polling mechanism for websocket mirror if
local container, otherwise use reads.
+ if s.instance.Type() == instancetype.Container {
+ pollFD = int(ptys[0].Fd())
+ }
+ logger.Debugf("Started mirroring websocket with FD %v",
pollFD)
+ defer logger.Debugf("Finished mirroring websocket")
+ readDone, writeDone :=
netutils.WebsocketExecMirror(conn, ptys[0], ptys[0], attachedChildIsDead,
pollFD)
<-readDone
<-writeDone
conn.Close()
@@ -279,6 +285,11 @@ func (s *execWs) Do(op *operations.Operation) error {
tty.Close()
}
+ // Ensure PTYs are closed before websocket to end wgEOF.Wait().
+ for _, pty := range ptys {
+ pty.Close()
+ }
+
s.connsLock.Lock()
conn := s.conns[-1]
s.connsLock.Unlock()
@@ -292,13 +303,8 @@ func (s *execWs) Do(op *operations.Operation) error {
}
attachedChildIsDead <- true
-
wgEOF.Wait()
- for _, pty := range ptys {
- pty.Close()
- }
-
metadata := shared.Jmap{"return": cmdResult}
err = op.UpdateMetadata(metadata)
if err != nil {
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
