From 80da60e12d12e7c4b40cf80fbdd5260fa0c12785 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 30 May 2025 15:08:21 -0700 Subject: [PATCH 1/5] runc update: support per-device weight and iops This support was missing from runc, and thus the example from the podman-update wasn't working. To fix, introduce a function to either update or insert new weights and iops. Add integration tests. Signed-off-by: Kir Kolyshkin (cherry picked from commit 7696402dac66a1468f6269c1afc48c5d6b6e9115) Signed-off-by: Kir Kolyshkin --- tests/integration/cgroups.bats | 35 +++++++++++++++++---- tests/integration/helpers.bash | 16 ++++++++++ tests/integration/update.bats | 54 ++++++++++++++++++++++++++++++++ update.go | 57 ++++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 6 deletions(-) diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index 29cd09782..57aa898a2 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -174,17 +174,40 @@ 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 < Date: Thu, 12 Jun 2025 13:52:17 -0700 Subject: [PATCH 2/5] runc update: handle duplicated devs properly In case there's a duplicate in the device list, the latter entry overrides the former one. So, we need to modify the last entry, not the first one. To do that, use slices.Backward. Amend the test case to test the fix. Reported-by: lifubang Signed-off-by: Kir Kolyshkin (cherry picked from commit 0b01dccfbbadaf7780358cc4efed7fdc6ab7e466) Signed-off-by: Kir Kolyshkin --- tests/integration/update.bats | 8 ++++++++ update.go | 9 +++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/integration/update.bats b/tests/integration/update.bats index 2a360333a..7ed1d8d20 100644 --- a/tests/integration/update.bats +++ b/tests/integration/update.bats @@ -904,6 +904,14 @@ EOF echo "===" skip "can't get device major number from /proc/partitions (got $major)" fi + # 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: 0, rate: 485760 }, + { major: '"$major"', minor: 0, rate: 485760 } + ]' runc run -d --console-socket "$CONSOLE_SOCKET" test_update [ "$status" -eq 0 ] diff --git a/update.go b/update.go index a9acf27a3..d1697a6a7 100644 --- a/update.go +++ b/update.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os" + "slices" "strconv" "github.com/opencontainers/cgroups" @@ -406,7 +407,9 @@ other options are ignored. } func upsertWeightDevice(devices []*cgroups.WeightDevice, wd specs.LinuxWeightDevice) []*cgroups.WeightDevice { - for i, dev := range devices { + // 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 } @@ -433,7 +436,9 @@ func upsertWeightDevice(devices []*cgroups.WeightDevice, wd specs.LinuxWeightDev } func upsertThrottleDevice(devices []*cgroups.ThrottleDevice, td specs.LinuxThrottleDevice) []*cgroups.ThrottleDevice { - for i, dev := range devices { + // 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 From 178e03c7f3fc759dd2b27cd6ad0f33b70662f7f3 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 14 Jul 2025 17:05:20 -0700 Subject: [PATCH 3/5] tests/int/update: fix getting block major Apparently, having a minor of 0 does not always mean it's the whole device (not a partition): === /proc/partitions (using major: 259) === major minor #blocks name 8 16 78643200 sdb 8 17 77593583 sdb1 8 30 4096 sdb14 8 31 108544 sdb15 259 0 934912 sdb16 8 0 78643200 sda 8 1 78641152 sda1 Rewrite the test to not assume minor is 0, and use lsblk -d to find out whole devices. This fixes a test case which was added in commit 7696402da. Signed-off-by: Kir Kolyshkin (cherry picked from commit 46dac589c1ed4b61c04660249f9a764735489260) Signed-off-by: Kir Kolyshkin --- tests/integration/update.bats | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/tests/integration/update.bats b/tests/integration/update.bats index 7ed1d8d20..e3d1922ef 100644 --- a/tests/integration/update.bats +++ b/tests/integration/update.bats @@ -895,22 +895,23 @@ EOF @test "update per-device iops/bps values" { [ $EUID -ne 0 ] && requires rootless_cgroup - # We need a major number of any disk device. Usually those are partitioned, - # with the device itself having minor of 0, and partitions are 1, 2... - major=$(awk '$2 == 0 {print $1; exit}' /proc/partitions) - if [ "$major" = "0" ] || [ "$major" = "" ]; then - echo "=== /proc/partitions ===" - cat /proc/partitions - echo "===" - skip "can't get device major number from /proc/partitions (got $major)" + # 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: 0, rate: 485760 }, - { major: '"$major"', minor: 0, rate: 485760 } + { major: '"$major"', minor: '"$minor"', rate: 485760 }, + { major: '"$major"', minor: '"$minor"', rate: 485760 } ]' runc run -d --console-socket "$CONSOLE_SOCKET" test_update @@ -922,28 +923,28 @@ EOF "throttleReadBpsDevice": [ { "major": $major, - "minor": 0, + "minor": $minor, "rate": 10485760 } ], "throttleWriteBpsDevice": [ { "major": $major, - "minor": 0, + "minor": $minor, "rate": 9437184 } ], "throttleReadIOPSDevice": [ { "major": $major, - "minor": 0, + "minor": $minor, "rate": 1000 } ], "throttleWriteIOPSDevice": [ { "major": $major, - "minor": 0, + "minor": $minor, "rate": 900 } ] @@ -951,5 +952,5 @@ EOF } EOF [ "$status" -eq 0 ] - check_cgroup_dev_iops "$major:0" 10485760 9437184 1000 900 + check_cgroup_dev_iops "$dev" 10485760 9437184 1000 900 } From f1627a7ab75adc56c73fe7e7e2aa60907b9da2c2 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 31 Jul 2025 10:23:59 +1000 Subject: [PATCH 4/5] tests: clean up loopback devices properly If an error occurs during a test which sets up loopback devices, the loopback device is not freed. Since most systems have very conservative limits on the number of loopback devices, re-running a failing test locally to debug it often ends up erroring out due to loopback device exhaustion. So let's just move the "losetup -d" to teardown, where it belongs. Signed-off-by: Aleksa Sarai (cherry picked from commit ceef984fb3b2baa7ec54ff4e5092fe2db123bcf5) Signed-off-by: Kir Kolyshkin --- tests/integration/cgroups.bats | 20 ++++---------------- tests/integration/helpers.bash | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index 57aa898a2..cf1bf1aa7 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -4,6 +4,7 @@ load helpers function teardown() { teardown_bundle + teardown_loopdevs } function setup() { @@ -153,13 +154,11 @@ 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)" # 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 @@ -198,9 +197,6 @@ function setup() { EOF weights2=$(get_cgroup_value $file) - # The loop device itself is no longer needed. - losetup -d "$dev" - # Check original values. grep '^default 333$' <<<"$weights1" grep "^$major:$minor 444$" <<<"$weights1" @@ -213,12 +209,8 @@ EOF @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 @@ -233,10 +225,6 @@ EOF 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" diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash index e6fadd6b1..24035eb64 100755 --- a/tests/integration/helpers.bash +++ b/tests/integration/helpers.bash @@ -807,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" From 2e82e55edb31ecf3719a538f576af56c1e309d0c Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 31 Jul 2025 13:26:41 +1000 Subject: [PATCH 5/5] tests: bfq: skip tests on misbehaving udev systems openSUSE has an unfortunate default udev setup which forcefully sets all loop devices to use the "none" scheduler, even if you manually set it. As this is a property of the host configuration (and udev is monitoring from the host) we cannot really change this behaviour from inside our test container. So we should just skip the test in this (hopefully unusual) case. Ideally tools running the test suite should disable this behaviour when running our test suite. Signed-off-by: Aleksa Sarai (cherry picked from commit e6b4b5a12843ed5ea82fb2c24d1a1a028ea9a63b) Signed-off-by: Kir Kolyshkin --- tests/integration/cgroups.bats | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index cf1bf1aa7..b21c0efc5 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -156,12 +156,26 @@ function setup() { 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 + # . + 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 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")"