From d72932998be48ff4eeb1b9d73d6f5d60560be031 Mon Sep 17 00:00:00 2001 Message-Id: From: Pavel Hrdina 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 Reviewed-by: Ján Tomko Signed-off-by: Pavel Hrdina (cherry picked from commit 152c0f0bf52d4f727e09f388fef5c836c3c94db4) Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1689297 Signed-off-by: Pavel Hrdina Message-Id: Reviewed-by: Ján Tomko --- 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