[1.3] runc update: support per-device weight and iops

tests/integration/cgroups.bats

@@ -4,6 +4,7 @@ load helpers
 
 function teardown() {
 	teardown_bundle
+	teardown_loopdevs
 }
 
 function setup() {
@@ -153,16 +154,28 @@ function setup() {
 @test "runc run (per-device io weight for bfq)" {
 	requires root # to create a loop device
 
-	dd if=/dev/zero of=backing.img bs=4096 count=1
-	dev=$(losetup --find --show backing.img) || skip "unable to create a loop device"
+	dev="$(setup_loopdev)"
+
+	# Some distributions (like openSUSE) have udev configured to forcefully
+	# reset the scheduler for loopN devices to be "none", which breaks this
+	# test. We cannot modify the udev behaviour of the host, but since this is
+	# usually triggered by the "change" event from losetup, we can wait for a
+	# little bit before continuing the test. For more details, see
+	# <https://github.com/opencontainers/runc/issues/4781>.
+	sleep 2s
 
 	# See if BFQ scheduler is available.
 	if ! { grep -qw bfq "/sys/block/${dev#/dev/}/queue/scheduler" &&
 		echo bfq >"/sys/block/${dev#/dev/}/queue/scheduler"; }; then
-		losetup -d "$dev"
 		skip "BFQ scheduler not available"
 	fi
 
+	# Check that the device still has the right scheduler, in case we lost the
+	# race above...
+	if ! grep -qw '\[bfq\]' "/sys/block/${dev#/dev/}/queue/scheduler"; then
+		skip "udev is configured to reset loop device io scheduler"
+	fi
+
 	set_cgroups_path
 
 	IFS=$' \t:' read -r major minor <<<"$(lsblk -nd -o MAJ:MIN "$dev")"
@@ -174,28 +187,44 @@ function setup() {
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_dev_weight
 	[ "$status" -eq 0 ]
 
-	# The loop device itself is no longer needed.
-	losetup -d "$dev"
-
 	if [ -v CGROUP_V2 ]; then
 		file="io.bfq.weight"
 	else
 		file="blkio.bfq.weight_device"
 	fi
-	weights=$(get_cgroup_value $file)
-	[[ "$weights" == *"default 333"* ]]
-	[[ "$weights" == *"$major:$minor 444"* ]]
+	weights1=$(get_cgroup_value $file)
+
+	# Check that runc update works.
+	runc update -r - test_dev_weight <<EOF
+{
+  "blockIO": {
+    "weight": 111,
+    "weightDevice": [
+      {
+        "major": $major,
+        "minor": $minor,
+        "weight": 222
+      }
+    ]
+  }
+}
+EOF
+	weights2=$(get_cgroup_value $file)
+
+	# Check original values.
+	grep '^default 333$' <<<"$weights1"
+	grep "^$major:$minor 444$" <<<"$weights1"
+	# Check updated values.
+	grep '^default 111$' <<<"$weights2"
+	grep "^$major:$minor 222$" <<<"$weights2"
+
 }
 
 @test "runc run (per-device multiple iops via unified)" {
 	requires root cgroups_v2
 
-	dd if=/dev/zero of=backing1.img bs=4096 count=1
-	dev1=$(losetup --find --show backing1.img) || skip "unable to create a loop device"
-
-	# Second device.
-	dd if=/dev/zero of=backing2.img bs=4096 count=1
-	dev2=$(losetup --find --show backing2.img) || skip "unable to create a loop device"
+	dev1="$(setup_loopdev)"
+	dev2="$(setup_loopdev)"
 
 	set_cgroups_path
 
@@ -210,10 +239,6 @@ function setup() {
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_dev_weight
 	[ "$status" -eq 0 ]
 
-	# The loop devices are no longer needed.
-	losetup -d "$dev1"
-	losetup -d "$dev2"
-
 	weights=$(get_cgroup_value "io.max")
 	grep "^$major1:$minor1 .* riops=333 wiops=444$" <<<"$weights"
 	grep "^$major2:$minor2 .* riops=555 wiops=666$" <<<"$weights"

tests/integration/helpers.bash

@@ -395,6 +395,22 @@ function check_cpu_weight() {
 	check_systemd_value "CPUWeight" "$weight"
 }
 
+function check_cgroup_dev_iops() {
+	local dev=$1 rbps=$2 wbps=$3 riops=$4 wiops=$5
+
+	if [ -v CGROUP_V2 ]; then
+		iops=$(get_cgroup_value "io.max")
+		printf "== io.max ==\n%s\n" "$iops"
+		grep "^$dev rbps=$rbps wbps=$wbps riops=$riops wiops=$wiops$" <<<"$iops"
+		return
+	fi
+
+	grep "^$dev ${rbps}$" <<<"$(get_cgroup_value blkio.throttle.read_bps_device)"
+	grep "^$dev ${wbps}$" <<<"$(get_cgroup_value blkio.throttle.write_bps_device)"
+	grep "^$dev ${riops}$" <<<"$(get_cgroup_value blkio.throttle.read_iops_device)"
+	grep "^$dev ${wiops}$" <<<"$(get_cgroup_value blkio.throttle.write_iops_device)"
+}
+
 # Helper function to set a resources limit
 function set_resources_limit() {
 	update_config '.linux.resources.pids.limit |= 100'
@@ -791,6 +807,29 @@ function teardown_seccompagent() {
 	rm -f "$SECCCOMP_AGENT_SOCKET"
 }
 
+LOOPBACK_DEVICE_LIST="$(mktemp "$BATS_TMPDIR/losetup.XXXXXX")"
+
+function setup_loopdev() {
+	local backing dev
+	backing="$(mktemp "$BATS_RUN_TMPDIR/backing.img.XXXXXX")"
+	truncate --size=4K "$backing"
+
+	dev="$(losetup --find --show "$backing")" || skip "unable to create a loop device"
+	echo "$dev" >>"$LOOPBACK_DEVICE_LIST"
+
+	unlink "$backing"
+	echo "$dev"
+}
+
+function teardown_loopdevs() {
+	[ -s "$LOOPBACK_DEVICE_LIST" ] || return 0
+	while IFS= read -r dev; do
+		echo "losetup -d '$dev'" >&2
+		losetup -d "$dev"
+	done <"$LOOPBACK_DEVICE_LIST"
+	truncate --size=0 "$LOOPBACK_DEVICE_LIST"
+}
+
 function setup_bundle() {
 	local image="$1"
 

tests/integration/update.bats

@@ -891,3 +891,66 @@ EOF
 	runc update test_update --memory 1024
 	wait_for_container 10 1 test_update stopped
 }
+
+@test "update per-device iops/bps values" {
+	[ $EUID -ne 0 ] && requires rootless_cgroup
+
+	# Need major:minor for any block device (but not a partition).
+	dev=$(lsblk -dno MAJ:MIN | head -1 | tr -d ' \t\n')
+	if [ -z "$dev" ]; then
+		echo "=== lsblk -d ===" >&2
+		lsblk -d >&2
+		echo "===" >&2
+		fail "can't get device from lsblk"
+	fi
+	IFS=':' read -r major minor <<<"$dev"
+
+	# Add an entry to check that
+	#   - existing devices can be updated;
+	#   - duplicates are handled properly;
+	# (see func upsert* in update.go).
+	update_config '	  .linux.resources.blockIO.throttleReadBpsDevice |= [
+				{ major: '"$major"', minor: '"$minor"', rate: 485760 },
+				{ major: '"$major"', minor: '"$minor"', rate: 485760 }
+			]'
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_update
+	[ "$status" -eq 0 ]
+
+	runc update -r - test_update <<EOF
+{
+  "blockIO": {
+    "throttleReadBpsDevice": [
+      {
+        "major": $major,
+        "minor": $minor,
+        "rate": 10485760
+      }
+    ],
+    "throttleWriteBpsDevice": [
+      {
+        "major": $major,
+        "minor": $minor,
+        "rate": 9437184
+      }
+    ],
+    "throttleReadIOPSDevice": [
+      {
+        "major": $major,
+        "minor": $minor,
+        "rate": 1000
+      }
+    ],
+    "throttleWriteIOPSDevice": [
+      {
+        "major": $major,
+        "minor": $minor,
+        "rate": 900
+      }
+    ]
+  }
+}
+EOF
+	[ "$status" -eq 0 ]
+	check_cgroup_dev_iops "$dev" 10485760 9437184 1000 900
+}

update.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"slices"
 	"strconv"
 
 	"github.com/opencontainers/cgroups"
@@ -273,6 +274,25 @@ other options are ignored.
 		if r.BlockIO.Weight != nil {
 			config.Cgroups.Resources.BlkioWeight = *r.BlockIO.Weight
 		}
+		if r.BlockIO.LeafWeight != nil {
+			config.Cgroups.Resources.BlkioLeafWeight = *r.BlockIO.LeafWeight
+		}
+		// For devices, we either update an existing one, or insert a new one.
+		for _, wd := range r.BlockIO.WeightDevice {
+			config.Cgroups.Resources.BlkioWeightDevice = upsertWeightDevice(config.Cgroups.Resources.BlkioWeightDevice, wd)
+		}
+		for _, td := range r.BlockIO.ThrottleReadBpsDevice {
+			config.Cgroups.Resources.BlkioThrottleReadBpsDevice = upsertThrottleDevice(config.Cgroups.Resources.BlkioThrottleReadBpsDevice, td)
+		}
+		for _, td := range r.BlockIO.ThrottleWriteBpsDevice {
+			config.Cgroups.Resources.BlkioThrottleWriteBpsDevice = upsertThrottleDevice(config.Cgroups.Resources.BlkioThrottleWriteBpsDevice, td)
+		}
+		for _, td := range r.BlockIO.ThrottleReadIOPSDevice {
+			config.Cgroups.Resources.BlkioThrottleReadIOPSDevice = upsertThrottleDevice(config.Cgroups.Resources.BlkioThrottleReadIOPSDevice, td)
+		}
+		for _, td := range r.BlockIO.ThrottleWriteIOPSDevice {
+			config.Cgroups.Resources.BlkioThrottleWriteIOPSDevice = upsertThrottleDevice(config.Cgroups.Resources.BlkioThrottleWriteIOPSDevice, td)
+		}
 
 		// Setting CPU quota and period independently does not make much sense,
 		// but historically runc allowed it and this needs to be supported
@@ -385,3 +405,45 @@ other options are ignored.
 		return container.Set(config)
 	},
 }
+
+func upsertWeightDevice(devices []*cgroups.WeightDevice, wd specs.LinuxWeightDevice) []*cgroups.WeightDevice {
+	// Iterate backwards because in case of a duplicate
+	// the last one will be used.
+	for i, dev := range slices.Backward(devices) {
+		if dev.Major != wd.Major || dev.Minor != wd.Minor {
+			continue
+		}
+		// Update weights for existing device.
+		if wd.Weight != nil {
+			devices[i].Weight = *wd.Weight
+		}
+		if wd.LeafWeight != nil {
+			devices[i].LeafWeight = *wd.LeafWeight
+		}
+		return devices
+	}
+
+	// New device -- append it.
+	var weight, leafWeight uint16
+	if wd.Weight != nil {
+		weight = *wd.Weight
+	}
+	if wd.LeafWeight != nil {
+		leafWeight = *wd.LeafWeight
+	}
+
+	return append(devices, cgroups.NewWeightDevice(wd.Major, wd.Minor, weight, leafWeight))
+}
+
+func upsertThrottleDevice(devices []*cgroups.ThrottleDevice, td specs.LinuxThrottleDevice) []*cgroups.ThrottleDevice {
+	// Iterate backwards because in case of a duplicate
+	// the last one will be used.
+	for i, dev := range slices.Backward(devices) {
+		if dev.Major == td.Major && dev.Minor == td.Minor {
+			devices[i].Rate = td.Rate
+			return devices
+		}
+	}
+
+	return append(devices, cgroups.NewThrottleDevice(td.Major, td.Minor, td.Rate))
+}