8:11 a.m.
On Thu, Oct 04, 2018 at 02:43:18PM +0200, Fabiano Fidêncio wrote:
On Tue, 2018-10-02 at 10:43 +0200, Pavel Hrdina wrote:
> When creating cgroup hierarchy we need to enable controllers in the
> parent cgroup in order to be usable. That means writing
> "+{controller}"
> into cgroup.subtree_control file. We can enable only controllers
> that
> are enabled for parent cgroup, that means we need to do that for the
> whole cgroup tree.
>
> Cgroups for threads needs to be handled differently in cgroup
> v2. There
> are two types of controllers:
>
> - domain controllers: these cannot be enabled for threads
> - threaded controllers: these can be enabled for threads
>
> In addition there are multiple types of cgroups:
>
> - domain: normal cgroup
> - domain threaded: a domain cgroup that serves as root for
> threaded
> cgroups
> - domain invalid: invalid cgroup, can be changed into threaded,
> this
> is the default state if you create subgroup
> inside
> domain threaded group or threaded group
> - threaded: threaded cgroup which can have domain threaded or
> threaded as parent group
>
> In order to create threaded cgroup it's sufficient to write
> "threaded"
> into cgroup.type file, it will automatically make parent cgroup
> "domain threaded" if it was only "domain". In case the parent
cgroup
> is already "domain threaded" or "threaded" it will modify only
the
> type
> of current cgroup. After that we can enable threaded controllers.
>
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> src/util/vircgroup.c | 2 +-
> src/util/vircgroupbackend.h | 1 +
> src/util/vircgroupv2.c | 78
> +++++++++++++++++++++++++++++++++++++
> 3 files changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 1097b1f998..dc249bfe33 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -938,7 +938,7 @@ virCgroupNewThread(virCgroupPtr domain,
> if (virCgroupNew(-1, name, domain, controllers, group) < 0)
> return -1;
>
> - if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE)
> < 0) {
> + if (virCgroupMakeGroup(domain, *group, create,
> VIR_CGROUP_THREAD) < 0) {
> virCgroupFree(group);
> return -1;
> }
> diff --git a/src/util/vircgroupbackend.h
> b/src/util/vircgroupbackend.h
> index b1f19233e4..86d1539e07 100644
> --- a/src/util/vircgroupbackend.h
> +++ b/src/util/vircgroupbackend.h
> @@ -33,6 +33,7 @@ typedef enum {
> * before creating subcgroups
> and
> * attaching tasks
> */
> + VIR_CGROUP_THREAD = 1 << 1, /* cgroup v2 handles threads
> differently */
> } virCgroupBackendFlags;
>
> typedef enum {
> diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> index 3ca463e4c2..8fb9ace474 100644
> --- a/src/util/vircgroupv2.c
> +++ b/src/util/vircgroupv2.c
> @@ -336,6 +336,83 @@ virCgroupV2PathOfController(virCgroupPtr group,
> }
>
>
> +static int
> +virCgroupV2EnableController(virCgroupPtr parent,
> + int controller)
> +{
> + VIR_AUTOFREE(char *) val = NULL;
> +
> + if (virAsprintf(&val, "+%s",
> + virCgroupV2ControllerTypeToString(controller)) <
> 0) {
> + return -1;
> + }
> +
> + if (virCgroupSetValueStr(parent, controller,
> + "cgroup.subtree_control", val) < 0) {
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +static int
> +virCgroupV2MakeGroup(virCgroupPtr parent ATTRIBUTE_UNUSED,
> + virCgroupPtr group,
> + bool create,
> + unsigned int flags)
> +{
> + VIR_AUTOFREE(char *) path = NULL;
> + int controller;
> +
> + VIR_DEBUG("Make group %s", group->path);
> +
> + controller = virCgroupV2GetAnyController(group);
> + if (virCgroupV2PathOfController(group, controller, "", &path)
<
> 0)
> + return -1;
> +
> + VIR_DEBUG("Make controller %s", path);
> +
> + if (!virFileExists(path)) {
> + if (!create || mkdir(path, 0755 || errno == EEXIST) < 0) {
The `mdkir(path, 0755 || errno == EEXIST) < 0` part of the check
doesn't look right.
Nice catch, I'll fix that, thanks.
Pavel