[lxc-devel] [lxd/master] VM: Don't spin when Qemu QMP event channel is closed.

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 311 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201215/e581b79a/attachment.bin>
-------------- next part --------------
From 1f8d31f61d7f375b5fd8029d4d79d4ce6da8292e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 15 Dec 2020 12:41:08 +0000
Subject: [PATCH 1/2] lxd/instance/drivers/qmp/monitor: Handle closed event
 channel from qmp package in run

Fixes #8253

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

diff --git a/lxd/instance/drivers/qmp/monitor.go b/lxd/instance/drivers/qmp/monitor.go
index 20c6dd4d01..ea564e6080 100644
--- a/lxd/instance/drivers/qmp/monitor.go
+++ b/lxd/instance/drivers/qmp/monitor.go
@@ -12,6 +12,7 @@ import (
 	"github.com/digitalocean/go-qemu/qmp"
 
 	"github.com/lxc/lxd/shared"
+	"github.com/lxc/lxd/shared/logger"
 )
 
 var monitors = map[string]*Monitor{}
@@ -126,13 +127,22 @@ func (m *Monitor) run() error {
 			select {
 			case <-m.chDisconnect:
 				return
-			case e := <-chEvents:
-				if e.Event == "" {
-					continue
+			case e, more := <-chEvents:
+				// Deliver non-empty events to the event handler.
+				if m.eventHandler != nil && e.Event != "" {
+					go m.eventHandler(e.Event, e.Data)
 				}
 
-				if m.eventHandler != nil {
-					go m.eventHandler(e.Event, e.Data)
+				// Event channel is closed, lets disconnect.
+				if !more {
+					m.Disconnect()
+					return
+				}
+
+				if e.Event == "" {
+					logger.Warnf("Unexpected empty event received from qmp event channel")
+					time.Sleep(time.Second) // Don't busy wait if we receive a lot of these.
+					continue
 				}
 
 				// Check if the ringbuffer was updated (non-blocking).

From 2ab69cfc1fa49211b3fb3de0ed9ed29ac78c6e0a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 15 Dec 2020 12:41:56 +0000
Subject: [PATCH 2/2] lxd/instance/drivers/driver/qemu: Logs when instance is
 stopped in getMonitorEventHandler

And removes some references to the instance in the function returned from getMonitorEventHandler so they are not kept in memory.

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

diff --git a/lxd/instance/drivers/driver_qemu.go b/lxd/instance/drivers/driver_qemu.go
index d01f47a3f7..b6168a3f6e 100644
--- a/lxd/instance/drivers/driver_qemu.go
+++ b/lxd/instance/drivers/driver_qemu.go
@@ -343,9 +343,12 @@ func (d *qemu) getStoragePool() (storagePools.Pool, error) {
 }
 
 func (d *qemu) getMonitorEventHandler() func(event string, data map[string]interface{}) {
+	// Create local variables from device properties we need so as not to keep references to device around
+	// after we have returned the callback function.
 	projectName := d.Project()
 	instanceName := d.Name()
 	state := d.state
+	logger := d.logger
 
 	return func(event string, data map[string]interface{}) {
 		if !shared.StringInSlice(event, []string{"SHUTDOWN"}) {
@@ -354,11 +357,13 @@ func (d *qemu) getMonitorEventHandler() func(event string, data map[string]inter
 
 		inst, err := instance.LoadByProjectAndName(state, projectName, instanceName)
 		if err != nil {
-			d.logger.Error("Failed to load instance", log.Ctx{"err": err})
+			logger.Error("Failed to load instance", log.Ctx{"err": err})
 			return
 		}
 
 		if event == "SHUTDOWN" {
+			logger.Debug("Instance stopped")
+
 			target := "stop"
 			entry, ok := data["reason"]
 			if ok && entry == "guest-reset" {
@@ -367,7 +372,7 @@ func (d *qemu) getMonitorEventHandler() func(event string, data map[string]inter
 
 			err = inst.(*qemu).onStop(target)
 			if err != nil {
-				d.logger.Error("Failed to cleanly stop instance", log.Ctx{"err": err})
+				logger.Error("Failed to cleanly stop instance", log.Ctx{"err": err})
 				return
 			}
 		}