[lxc-devel] [lxd/master] lxd/device/disk: Improvements in disk limits

[tomponline on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 598 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20191008/1c2f1e1e/attachment-0001.bin>
-------------- next part --------------
From e17665f8b463115fb091fa4d3049929fb81daf73 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 8 Oct 2019 10:20:45 +0100
Subject: [PATCH] lxd/device/disk: Improvements in disk limits

- Removes duplicated if diskPriority != "" check.
- Don't try and apply limits on stopped container during update.
- Fail if limits on a device are specified but device isn't mounted yet (this avoids silently not setting limits).

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/disk.go | 57 +++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/lxd/device/disk.go b/lxd/device/disk.go
index 54157504cd..6ed484147c 100644
--- a/lxd/device/disk.go
+++ b/lxd/device/disk.go
@@ -384,16 +384,18 @@ func (d *disk) Update(oldDevices deviceConfig.Devices, isRunning bool) error {
 		}
 	}
 
-	runConf := RunConfig{}
-
-	err := d.generateLimits(&runConf)
-	if err != nil {
-		return err
-	}
+	// Only apply IO limits if instance is running.
+	if isRunning {
+		runConf := RunConfig{}
+		err := d.generateLimits(&runConf)
+		if err != nil {
+			return err
+		}
 
-	err = d.instance.DeviceEventHandler(&runConf)
-	if err != nil {
-		return err
+		err = d.instance.DeviceEventHandler(&runConf)
+		if err != nil {
+			return err
+		}
 	}
 
 	return nil
@@ -419,24 +421,22 @@ func (d *disk) generateLimits(runConf *RunConfig) error {
 	diskPriority := d.instance.ExpandedConfig()["limits.disk.priority"]
 	if diskPriority != "" {
 		if d.state.OS.CGroupBlkioWeightController {
-			if diskPriority != "" {
-				priorityInt, err := strconv.Atoi(diskPriority)
-				if err != nil {
-					return err
-				}
-
-				priority := priorityInt * 100
+			priorityInt, err := strconv.Atoi(diskPriority)
+			if err != nil {
+				return err
+			}
 
-				// Minimum valid value is 10
-				if priority == 0 {
-					priority = 10
-				}
+			priority := priorityInt * 100
 
-				runConf.CGroups = append(runConf.CGroups, RunConfigItem{
-					Key:   "blkio.weight",
-					Value: fmt.Sprintf("%d", priority),
-				})
+			// Minimum valid value is 10
+			if priority == 0 {
+				priority = 10
 			}
+
+			runConf.CGroups = append(runConf.CGroups, RunConfigItem{
+				Key:   "blkio.weight",
+				Value: fmt.Sprintf("%d", priority),
+			})
 		} else {
 			return fmt.Errorf("Cannot apply limits.disk.priority as blkio.weight cgroup controller is missing")
 		}
@@ -718,9 +718,14 @@ func (d *disk) getDiskLimits() (map[string]diskBlockLimit, error) {
 			source = d.instance.RootfsPath()
 		}
 
-		// Don't try to resolve the block device behind a non-existing path
+		// Require that device is mounted before resolving block device.
 		if !shared.PathExists(source) {
-			continue
+			// Skip devices that don't have a valid mount if there are no limits set.
+			if readBps == 0 && readIops == 0 && writeBps == 0 && writeIops == 0 {
+				continue
+			}
+
+			return nil, fmt.Errorf("Block device path doesn't exist: %s", source)
 		}
 
 		// Get the backing block devices (major:minor)