From 8c784cd15bbac88334271f95d14a9770c1b2d864 Mon Sep 17 00:00:00 2001
Message-Id: <8c784cd15bbac88334271f95d14a9770c1b2d864@dist-git>
From: Pavel Hrdina <phrdina@redhat.com>
Date: Mon, 1 Jul 2019 17:08:17 +0200
Subject: [PATCH] util: vircgroup: improve controller detection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This affects only cgroups v2 where enabled controllers are not based on
available mount points but on the list provided in cgroup.controllers
file.  However, moving it will fill in placement as well, so it needs
to be freed together with mount point if we don't need that controller.

Before this patch we were assuming that all controllers available in
root cgroup where available in all other sub-cgroups which was wrong.

In order to fix it we need to move the cgroup controllers detection
after cgroup placement was prepared in order to build correct path for
cgroup.controllers file.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
(cherry picked from commit d3007c844d8aa0caa0328ab18275be375f597704)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1689297

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Message-Id: <cec99e4c53912c3ee6f45012fc4fd760f6e8445b.1561993100.git.phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/util/vircgroup.c   | 32 ++++++++++++++++----------------
 src/util/vircgroupv1.c |  1 +
 src/util/vircgroupv2.c |  5 +++--
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 4f71a1d197..ff2a0b75b5 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -386,22 +386,6 @@ virCgroupDetect(virCgroupPtr group,
             return -1;
     }
 
-    for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
-        if (group->backends[i]) {
-            int rc = group->backends[i]->detectControllers(group, controllers, parent);
-            if (rc < 0)
-                return -1;
-            controllersAvailable |= rc;
-        }
-    }
-
-    /* Check that at least 1 controller is available */
-    if (controllersAvailable == 0) {
-        virReportSystemError(ENXIO, "%s",
-                             _("At least one cgroup controller is required"));
-        return -1;
-    }
-
     /* In some cases we can copy part of the placement info
      * based on the parent cgroup...
      */
@@ -426,6 +410,22 @@ virCgroupDetect(virCgroupPtr group,
         }
     }
 
+    for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
+        if (group->backends[i]) {
+            int rc = group->backends[i]->detectControllers(group, controllers, parent);
+            if (rc < 0)
+                return -1;
+            controllersAvailable |= rc;
+        }
+    }
+
+    /* Check that at least 1 controller is available */
+    if (controllersAvailable == 0) {
+        virReportSystemError(ENXIO, "%s",
+                             _("At least one cgroup controller is required"));
+        return -1;
+    }
+
     return 0;
 }
 
diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index 784a963b79..5b218c7f78 100644
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -463,6 +463,7 @@ virCgroupV1DetectControllers(virCgroupPtr group,
                     }
                 }
                 VIR_FREE(group->legacy[i].mountPoint);
+                VIR_FREE(group->legacy[i].placement);
             }
         }
     } else {
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index 60598712c8..10f8a9bbdf 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -249,8 +249,9 @@ virCgroupV2ParseControllersFile(virCgroupPtr group)
     char **contList = NULL;
     char **tmp;
 
-    if (virAsprintf(&contFile, "%s/cgroup.controllers",
-                    group->unified.mountPoint) < 0)
+    if (virAsprintf(&contFile, "%s%s/cgroup.controllers",
+                    group->unified.mountPoint,
+                    NULLSTR_EMPTY(group->unified.placement)) < 0)
         return -1;
 
     rc = virFileReadAll(contFile, 1024 * 1024, &contStr);
-- 
2.22.0