From d72932998be48ff4eeb1b9d73d6f5d60560be031 Mon Sep 17 00:00:00 2001

Message-Id: <d72932998be48ff4eeb1b9d73d6f5d60560be031@dist-git>

From: Pavel Hrdina <phrdina@redhat.com>

Date: Mon, 1 Jul 2019 17:06:35 +0200

Subject: [PATCH] vircgroup: extract virCgroupV1MakeGroup

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

(cherry picked from commit 152c0f0bf52d4f727e09f388fef5c836c3c94db4)

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

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Message-Id: <b90114ff10174e747131ddc3dfeb4fe177d4bf21.1561993100.git.phrdina@redhat.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

---

 src/util/vircgroup.c        | 141 ++----------------------------------

 src/util/vircgroupbackend.h |  15 ++++

 src/util/vircgrouppriv.h    |  20 +++++

 src/util/vircgroupv1.c      | 132 +++++++++++++++++++++++++++++++++

 4 files changed, 174 insertions(+), 134 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c

index 438d9b4e70..8f0ea4de9a 100644

--- a/src/util/vircgroup.c

+++ b/src/util/vircgroup.c

@@ -80,14 +80,6 @@ VIR_ENUM_IMPL(virCgroupController, VIR_CGROUP_CONTROLLER_LAST,

               "freezer", "blkio", "net_cls", "perf_event",

               "name=systemd");

-typedef enum {

-    VIR_CGROUP_NONE = 0, /* create subdir under each cgroup if possible. */

-    VIR_CGROUP_MEM_HIERACHY = 1 << 0, /* call virCgroupSetMemoryUseHierarchy

-                                       * before creating subcgroups and

-                                       * attaching tasks

-                                       */

-} virCgroupFlags;

-

 /**

  * virCgroupGetDevicePermsString:

@@ -446,7 +438,7 @@ virCgroupGetBlockDevString(const char *path)

 }

-static int

+int

 virCgroupSetValueStr(virCgroupPtr group,

                      int controller,

                      const char *key,

@@ -476,7 +468,7 @@ virCgroupSetValueStr(virCgroupPtr group,

 }

-static int

+int

 virCgroupGetValueStr(virCgroupPtr group,

                      int controller,

                      const char *key,

@@ -537,7 +529,7 @@ virCgroupGetValueForBlkDev(virCgroupPtr group,

 }

-static int

+int

 virCgroupSetValueU64(virCgroupPtr group,

                      int controller,

                      const char *key,

@@ -589,7 +581,7 @@ virCgroupGetValueI64(virCgroupPtr group,

 }

-static int

+int

 virCgroupGetValueU64(virCgroupPtr group,

                      int controller,

                      const char *key,

@@ -611,137 +603,18 @@ virCgroupGetValueU64(virCgroupPtr group,

 }

-static int

-virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group)

-{

-    size_t i;

-    const char *inherit_values[] = {

-        "cpuset.cpus",

-        "cpuset.mems",

-        "cpuset.memory_migrate",

-    };

-

-    VIR_DEBUG("Setting up inheritance %s -> %s", parent->path, group->path);

-    for (i = 0; i < ARRAY_CARDINALITY(inherit_values); i++) {

-        VIR_AUTOFREE(char *) value = NULL;

-

-        if (virCgroupGetValueStr(parent,

-                                 VIR_CGROUP_CONTROLLER_CPUSET,

-                                 inherit_values[i],

-                                 &value) < 0)

-            return -1;

-

-        VIR_DEBUG("Inherit %s = %s", inherit_values[i], value);

-

-        if (virCgroupSetValueStr(group,

-                                 VIR_CGROUP_CONTROLLER_CPUSET,

-                                 inherit_values[i],

-                                 value) < 0)

-            return -1;

-    }

-

-    return 0;

-}

-

-

-static int

-virCgroupSetMemoryUseHierarchy(virCgroupPtr group)

-{

-    unsigned long long value;

-    const char *filename = "memory.use_hierarchy";

-

-    if (virCgroupGetValueU64(group,

-                             VIR_CGROUP_CONTROLLER_MEMORY,

-                             filename, &value) < 0)

-        return -1;

-

-    /* Setting twice causes error, so if already enabled, skip setting */

-    if (value == 1)

-        return 0;

-

-    VIR_DEBUG("Setting up %s/%s", group->path, filename);

-    if (virCgroupSetValueU64(group,

-                             VIR_CGROUP_CONTROLLER_MEMORY,

-                             filename, 1) < 0)

-        return -1;

-

-    return 0;

-}

-

-

 static int

 virCgroupMakeGroup(virCgroupPtr parent,

                    virCgroupPtr group,

                    bool create,

                    unsigned int flags)

 {

-    size_t i;

-

-    VIR_DEBUG("Make group %s", group->path);

-    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {

-        VIR_AUTOFREE(char *) path = NULL;

-

-        /* We must never mkdir() in systemd's hierarchy */

-        if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {

-            VIR_DEBUG("Not creating systemd controller group");

-            continue;

-        }

-

-        /* Skip over controllers that aren't mounted */

-        if (!group->controllers[i].mountPoint) {

-            VIR_DEBUG("Skipping unmounted controller %s",

-                      virCgroupControllerTypeToString(i));

-            continue;

-        }

-

-        if (virCgroupPathOfController(group, i, "", &path) < 0)

-            goto error;

-

-        VIR_DEBUG("Make controller %s", path);

-        if (!virFileExists(path)) {

-            if (!create ||

-                mkdir(path, 0755) < 0) {

-                if (errno == EEXIST)

-                    continue;

-                /* With a kernel that doesn't support multi-level directory

-                 * for blkio controller, libvirt will fail and disable all

-                 * other controllers even though they are available. So

-                 * treat blkio as unmounted if mkdir fails. */

-                if (i == VIR_CGROUP_CONTROLLER_BLKIO) {

-                    VIR_DEBUG("Ignoring mkdir failure with blkio controller. Kernel probably too old");

-                    VIR_FREE(group->controllers[i].mountPoint);

-                    continue;

-                } else {

-                    virReportSystemError(errno,

-                                         _("Failed to create controller %s for group"),

-                                         virCgroupControllerTypeToString(i));

-                    goto error;

-                }

-            }

-            if (i == VIR_CGROUP_CONTROLLER_CPUSET &&

-                group->controllers[i].mountPoint != NULL &&

-                virCgroupCpuSetInherit(parent, group) < 0) {

-                goto error;

-            }

-            /*

-             * Note that virCgroupSetMemoryUseHierarchy should always be

-             * called prior to creating subcgroups and attaching tasks.

-             */

-            if ((flags & VIR_CGROUP_MEM_HIERACHY) &&

-                i == VIR_CGROUP_CONTROLLER_MEMORY &&

-                group->controllers[i].mountPoint != NULL &&

-                virCgroupSetMemoryUseHierarchy(group) < 0) {

-                goto error;

-            }

-        }

+    if (group->backend->makeGroup(parent, group, create, flags) < 0) {

+        virCgroupRemove(group);

+        return -1;

     }

-    VIR_DEBUG("Done making controllers for group");

     return 0;

-

- error:

-    virCgroupRemove(group);

-    return -1;

 }

diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h

index 916227ba6a..b2848c2076 100644

--- a/src/util/vircgroupbackend.h

+++ b/src/util/vircgroupbackend.h

@@ -27,6 +27,14 @@

 # define CGROUP_MAX_VAL 512

+typedef enum {

+    VIR_CGROUP_NONE = 0, /* create subdir under each cgroup if possible. */

+    VIR_CGROUP_MEM_HIERACHY = 1 << 0, /* call virCgroupSetMemoryUseHierarchy

+                                       * before creating subcgroups and

+                                       * attaching tasks

+                                       */

+} virCgroupBackendFlags;

+

 typedef enum {

     VIR_CGROUP_BACKEND_TYPE_V1 = 0,

     VIR_CGROUP_BACKEND_TYPE_LAST,

@@ -86,6 +94,12 @@ typedef int

                                const char *key,

                                char **path);

+typedef int

+(*virCgroupMakeGroupCB)(virCgroupPtr parent,

+                        virCgroupPtr group,

+                        bool create,

+                        unsigned int flags);

+

 struct _virCgroupBackend {

     virCgroupBackendType type;

@@ -102,6 +116,7 @@ struct _virCgroupBackend {

     virCgroupHasControllerCB hasController;

     virCgroupGetAnyControllerCB getAnyController;

     virCgroupPathOfControllerCB pathOfController;

+    virCgroupMakeGroupCB makeGroup;

 };

 typedef struct _virCgroupBackend virCgroupBackend;

 typedef virCgroupBackend *virCgroupBackendPtr;

diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h

index e7f4a1f0fc..2e731458d5 100644

--- a/src/util/vircgrouppriv.h

+++ b/src/util/vircgrouppriv.h

@@ -53,6 +53,26 @@ struct _virCgroup {

     virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST];

 };

+int virCgroupSetValueStr(virCgroupPtr group,

+                         int controller,

+                         const char *key,

+                         const char *value);

+

+int virCgroupGetValueStr(virCgroupPtr group,

+                         int controller,

+                         const char *key,

+                         char **value);

+

+int virCgroupSetValueU64(virCgroupPtr group,

+                         int controller,

+                         const char *key,

+                         unsigned long long int value);

+

+int virCgroupGetValueU64(virCgroupPtr group,

+                         int controller,

+                         const char *key,

+                         unsigned long long int *value);

+

 int virCgroupPartitionEscape(char **path);

 int virCgroupNewPartition(const char *path,

diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c

index a6302d71b1..653e848a83 100644

--- a/src/util/vircgroupv1.c

+++ b/src/util/vircgroupv1.c

@@ -537,6 +537,137 @@ virCgroupV1PathOfController(virCgroupPtr group,

 }

+static int

+virCgroupV1CpuSetInherit(virCgroupPtr parent,

+                         virCgroupPtr group)

+{

+    size_t i;

+    const char *inherit_values[] = {

+        "cpuset.cpus",

+        "cpuset.mems",

+        "cpuset.memory_migrate",

+    };

+

+    VIR_DEBUG("Setting up inheritance %s -> %s", parent->path, group->path);

+    for (i = 0; i < ARRAY_CARDINALITY(inherit_values); i++) {

+        VIR_AUTOFREE(char *) value = NULL;

+

+        if (virCgroupGetValueStr(parent,

+                                 VIR_CGROUP_CONTROLLER_CPUSET,

+                                 inherit_values[i],

+                                 &value) < 0)

+            return -1;

+

+        VIR_DEBUG("Inherit %s = %s", inherit_values[i], value);

+

+        if (virCgroupSetValueStr(group,

+                                 VIR_CGROUP_CONTROLLER_CPUSET,

+                                 inherit_values[i],

+                                 value) < 0)

+            return -1;

+    }

+

+    return 0;

+}

+

+

+static int

+virCgroupV1SetMemoryUseHierarchy(virCgroupPtr group)

+{

+    unsigned long long value;

+    const char *filename = "memory.use_hierarchy";

+

+    if (virCgroupGetValueU64(group,

+                             VIR_CGROUP_CONTROLLER_MEMORY,

+                             filename, &value) < 0)

+        return -1;

+

+    /* Setting twice causes error, so if already enabled, skip setting */

+    if (value == 1)

+        return 0;

+

+    VIR_DEBUG("Setting up %s/%s", group->path, filename);

+    if (virCgroupSetValueU64(group,

+                             VIR_CGROUP_CONTROLLER_MEMORY,

+                             filename, 1) < 0)

+        return -1;

+

+    return 0;

+}

+

+

+static int

+virCgroupV1MakeGroup(virCgroupPtr parent,

+                     virCgroupPtr group,

+                     bool create,

+                     unsigned int flags)

+{

+    size_t i;

+

+    VIR_DEBUG("Make group %s", group->path);

+    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {

+        VIR_AUTOFREE(char *) path = NULL;

+

+        /* We must never mkdir() in systemd's hierarchy */

+        if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {

+            VIR_DEBUG("Not creating systemd controller group");

+            continue;

+        }

+

+        /* Skip over controllers that aren't mounted */

+        if (!group->controllers[i].mountPoint) {

+            VIR_DEBUG("Skipping unmounted controller %s",

+                      virCgroupV1ControllerTypeToString(i));

+            continue;

+        }

+

+        if (virCgroupV1PathOfController(group, i, "", &path) < 0)

+            return -1;

+

+        VIR_DEBUG("Make controller %s", path);

+        if (!virFileExists(path)) {

+            if (!create ||

+                mkdir(path, 0755) < 0) {

+                if (errno == EEXIST)

+                    continue;

+                /* With a kernel that doesn't support multi-level directory

+                 * for blkio controller, libvirt will fail and disable all

+                 * other controllers even though they are available. So

+                 * treat blkio as unmounted if mkdir fails. */

+                if (i == VIR_CGROUP_CONTROLLER_BLKIO) {

+                    VIR_DEBUG("Ignoring mkdir failure with blkio controller. Kernel probably too old");

+                    VIR_FREE(group->controllers[i].mountPoint);

+                    continue;

+                } else {

+                    virReportSystemError(errno,

+                                         _("Failed to create v1 controller %s for group"),

+                                         virCgroupV1ControllerTypeToString(i));

+                    return -1;

+                }

+            }

+            if (i == VIR_CGROUP_CONTROLLER_CPUSET &&

+                group->controllers[i].mountPoint != NULL &&

+                virCgroupV1CpuSetInherit(parent, group) < 0) {

+                return -1;

+            }

+            /*

+             * Note that virCgroupV1SetMemoryUseHierarchy should always be

+             * called prior to creating subcgroups and attaching tasks.

+             */

+            if ((flags & VIR_CGROUP_MEM_HIERACHY) &&

+                i == VIR_CGROUP_CONTROLLER_MEMORY &&

+                group->controllers[i].mountPoint != NULL &&

+                virCgroupV1SetMemoryUseHierarchy(group) < 0) {

+                return -1;

+            }

+        }

+    }

+

+    VIR_DEBUG("Done making controllers for group");

+    return 0;

+}

+

+

 virCgroupBackend virCgroupV1Backend = {

     .type = VIR_CGROUP_BACKEND_TYPE_V1,

@@ -552,6 +683,7 @@ virCgroupBackend virCgroupV1Backend = {

     .hasController = virCgroupV1HasController,

     .getAnyController = virCgroupV1GetAnyController,

     .pathOfController = virCgroupV1PathOfController,

+    .makeGroup = virCgroupV1MakeGroup,

 };

-- 

2.22.0