From e0d39536c4c8d1a870307f98cff373a43cef8c6e Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Mon, 22 Jul 2024 12:41:24 -0700
Subject: [PATCH] runc update: fix updating swap for cgroup v2

This allows to do

	runc update $ID --memory=-1 --memory-swap=$VAL

for cgroup v2, i.e. set memory to unlimited and swap to a specific
value.

This was not possible because ConvertMemorySwapToCgroupV2Value rejected
memory=-1 ("unlimited"). In a hindsight, it was a mistake, because if
memory limit is unlimited, we should treat memory+swap limit as just swap
limit.

Revise the unit test; add description to each case.

Fixes: c86be8a2 ("cgroupv2: fix setting MemorySwap")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 732806e24c3210993f1649084ffbddbfd9cfd4de)
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
---
 libcontainer/cgroups/utils.go      |  7 ++--
 libcontainer/cgroups/utils_test.go | 51 +++++++++++++++++++-----------
 2 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go
index ec3e0e71e09..a05945cba6c 100644
--- a/libcontainer/cgroups/utils.go
+++ b/libcontainer/cgroups/utils.go
@@ -427,8 +427,11 @@ func ConvertMemorySwapToCgroupV2Value(memorySwap, memory int64) (int64, error) {
 	case memorySwap == -1, memorySwap == 0:
 		// Treat -1 ("max") and 0 ("unset") swap as is.
 		return memorySwap, nil
-	case memory == 0, memory == -1:
-		// Unset or unlimited memory, can't calculate swap.
+	case memory == -1:
+		// Unlimited memory, so treat swap as is.
+		return memorySwap, nil
+	case memory == 0:
+		// Unset or unknown memory, can't calculate swap.
 		return 0, errors.New("unable to set swap limit without memory limit")
 	case memory < 0:
 		// Does not make sense to subtract a negative value.
diff --git a/libcontainer/cgroups/utils_test.go b/libcontainer/cgroups/utils_test.go
index c9feb84484c..fc81992f0b0 100644
--- a/libcontainer/cgroups/utils_test.go
+++ b/libcontainer/cgroups/utils_test.go
@@ -554,82 +554,97 @@ func TestConvertCPUSharesToCgroupV2Value(t *testing.T) {
 
 func TestConvertMemorySwapToCgroupV2Value(t *testing.T) {
 	cases := []struct {
+		descr           string
 		memswap, memory int64
 		expected        int64
 		expErr          bool
 	}{
 		{
+			descr:    "all unset",
 			memswap:  0,
 			memory:   0,
 			expected: 0,
 		},
 		{
+			descr:    "unlimited memory+swap, unset memory",
 			memswap:  -1,
 			memory:   0,
 			expected: -1,
 		},
 		{
+			descr:    "unlimited memory",
+			memswap:  300,
+			memory:   -1,
+			expected: 300,
+		},
+		{
+			descr:    "all unlimited",
 			memswap:  -1,
 			memory:   -1,
 			expected: -1,
 		},
 		{
+			descr:   "negative memory+swap",
 			memswap: -2,
 			memory:  0,
 			expErr:  true,
 		},
 		{
+			descr:    "unlimited memory+swap, set memory",
 			memswap:  -1,
 			memory:   1000,
 			expected: -1,
 		},
 		{
+			descr:    "memory+swap == memory",
 			memswap:  1000,
 			memory:   1000,
 			expected: 0,
 		},
 		{
+			descr:    "memory+swap > memory",
 			memswap:  500,
 			memory:   200,
 			expected: 300,
 		},
 		{
+			descr:   "memory+swap < memory",
 			memswap: 300,
 			memory:  400,
 			expErr:  true,
 		},
 		{
+			descr:   "unset memory",
 			memswap: 300,
 			memory:  0,
 			expErr:  true,
 		},
 		{
+			descr:   "negative memory",
 			memswap: 300,
 			memory:  -300,
 			expErr:  true,
 		},
-		{
-			memswap: 300,
-			memory:  -1,
-			expErr:  true,
-		},
 	}
 
 	for _, c := range cases {
-		swap, err := ConvertMemorySwapToCgroupV2Value(c.memswap, c.memory)
-		if c.expErr {
-			if err == nil {
-				t.Errorf("memswap: %d, memory %d, expected error, got %d, nil", c.memswap, c.memory, swap)
+		c := c
+		t.Run(c.descr, func(t *testing.T) {
+			swap, err := ConvertMemorySwapToCgroupV2Value(c.memswap, c.memory)
+			if c.expErr {
+				if err == nil {
+					t.Errorf("memswap: %d, memory %d, expected error, got %d, nil", c.memswap, c.memory, swap)
+				}
+				// No more checks.
+				return
 			}
-			// no more checks
-			continue
-		}
-		if err != nil {
-			t.Errorf("memswap: %d, memory %d, expected success, got error %s", c.memswap, c.memory, err)
-		}
-		if swap != c.expected {
-			t.Errorf("memswap: %d, memory %d, expected %d, got %d", c.memswap, c.memory, c.expected, swap)
-		}
+			if err != nil {
+				t.Errorf("memswap: %d, memory %d, expected success, got error %s", c.memswap, c.memory, err)
+			}
+			if swap != c.expected {
+				t.Errorf("memswap: %d, memory %d, expected %d, got %d", c.memswap, c.memory, c.expected, swap)
+			}
+		})
 	}
 }