Place cgroup v2 backend type before cgroup v1 to make it obvious that cgroup v2 is preferred implementation. Following patches will introduce support for hybrid configuration which will allow us to use both at the same time, but we should prefer cgroup v2 regardless. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/Makefile.am | 1 + src/libvirt_private.syms | 3 ++ src/util/Makefile.inc.am | 2 ++ src/util/vircgroup.c | 3 ++ src/util/vircgroupbackend.c | 2 ++ src/util/vircgroupbackend.h | 3 +- src/util/vircgrouppriv.h | 9 ++++++ src/util/vircgroupv2.c | 63 +++++++++++++++++++++++++++++++++++++ src/util/vircgroupv2.h | 27 ++++++++++++++++ 9 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 src/util/vircgroupv2.c create mode 100644 src/util/vircgroupv2.h diff --git a/src/Makefile.am b/src/Makefile.am index f515569fd5..33ff280d78 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -675,6 +675,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ util/vircgroup.c \ util/vircgroupbackend.c \ util/vircgroupv1.c \ + util/vircgroupv2.c \ util/vircommand.c \ util/virconf.c \ util/virdbus.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 92363913e3..335210c31d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1583,6 +1583,9 @@ virCgroupBackendRegister; # util/vircgroupv1.h virCgroupV1Register; +# util/vircgroupv2.h +virCgroupV2Register; + # util/virclosecallbacks.h virCloseCallbacksGet; virCloseCallbacksGetConn; diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index ad3be91867..cffbb357bc 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -27,6 +27,8 @@ UTIL_SOURCES = \ util/vircgroupbackend.h \ util/vircgroupv1.c \ util/vircgroupv1.h \ + util/vircgroupv2.c \ + util/vircgroupv2.h \ util/virclosecallbacks.c \ util/virclosecallbacks.h \ util/vircommand.c \ diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 548c873da8..1097b1f998 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1236,6 +1236,9 @@ virCgroupFree(virCgroupPtr *group) VIR_FREE((*group)->legacy[i].placement); } + VIR_FREE((*group)->unified.mountPoint); + VIR_FREE((*group)->unified.placement); + VIR_FREE((*group)->path); VIR_FREE(*group); } diff --git a/src/util/vircgroupbackend.c b/src/util/vircgroupbackend.c index d854c9711d..79fe6cb73d 100644 --- a/src/util/vircgroupbackend.c +++ b/src/util/vircgroupbackend.c @@ -21,6 +21,7 @@ #include "vircgroupbackend.h" #include "vircgroupv1.h" +#include "vircgroupv2.h" #include "virerror.h" #include "virthread.h" @@ -28,6 +29,7 @@ VIR_ENUM_DECL(virCgroupBackend); VIR_ENUM_IMPL(virCgroupBackend, VIR_CGROUP_BACKEND_TYPE_LAST, + "cgroup V2", "cgroup V1"); static virOnceControl virCgroupBackendOnce = VIR_ONCE_CONTROL_INITIALIZER; diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 1c5744ef76..b1f19233e4 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -50,7 +50,8 @@ typedef enum { } virCgroupBackendTaskFlags; typedef enum { - VIR_CGROUP_BACKEND_TYPE_V1 = 0, + VIR_CGROUP_BACKEND_TYPE_V2 = 0, + VIR_CGROUP_BACKEND_TYPE_V1, VIR_CGROUP_BACKEND_TYPE_LAST, } virCgroupBackendType; diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index c50a25f195..4a0d75ddbc 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -45,12 +45,21 @@ struct _virCgroupV1Controller { typedef struct _virCgroupV1Controller virCgroupV1Controller; typedef virCgroupV1Controller *virCgroupV1ControllerPtr; +struct _virCgroupV2Controller { + int controllers; + char *mountPoint; + char *placement; +}; +typedef struct _virCgroupV2Controller virCgroupV2Controller; +typedef virCgroupV2Controller *virCgroupV2ControllerPtr; + struct _virCgroup { char *path; virCgroupBackendPtr backend; virCgroupV1Controller legacy[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupV2Controller unified; }; int virCgroupSetValueStr(virCgroupPtr group, diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c new file mode 100644 index 0000000000..23bf81dae2 --- /dev/null +++ b/src/util/vircgroupv2.c @@ -0,0 +1,63 @@ +/* + * vircgroupv2.c: methods for cgroups v2 backend + * + * Copyright (C) 2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ +#include <config.h> + +#include "internal.h" + +#define __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__ +#include "vircgrouppriv.h" +#undef __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__ + +#include "vircgroup.h" +#include "vircgroupbackend.h" +#include "vircgroupv2.h" +#include "virlog.h" + +VIR_LOG_INIT("util.cgroup"); + +#define VIR_FROM_THIS VIR_FROM_CGROUP + +VIR_ENUM_DECL(virCgroupV2Controller); +VIR_ENUM_IMPL(virCgroupV2Controller, VIR_CGROUP_CONTROLLER_LAST, + "cpu", "cpuacct", "cpuset", "memory", "devices", + "freezer", "io", "net_cls", "perf_event", "name=systemd"); + +#ifdef __linux__ + +virCgroupBackend virCgroupV2Backend = { + .type = VIR_CGROUP_BACKEND_TYPE_V2, +}; + + +void +virCgroupV2Register(void) +{ + virCgroupBackendRegister(&virCgroupV2Backend); +} + +#else /* !__linux__ */ + +void +virCgroupV2Register(void) +{ + VIR_INFO("Control groups not supported on this platform"); +} + +#endif /* !__linux__ */ diff --git a/src/util/vircgroupv2.h b/src/util/vircgroupv2.h new file mode 100644 index 0000000000..a5d0bd0978 --- /dev/null +++ b/src/util/vircgroupv2.h @@ -0,0 +1,27 @@ +/* + * vircgroupv2.h: methods for cgroups v2 backend + * + * Copyright (C) 2018 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#ifndef __VIR_CGROUP_V2_H__ +# define __VIR_CGROUP_V2_H__ + +void +virCgroupV2Register(void); + +#endif /* __VIR_CGROUP_V2_H__ */ -- 2.17.1

We cannot detect only mount points to figure out whether cgroup v2 is available because systemd uses cgroup v2 for process tracking and all controllers are mounted as cgroup v1 controllers. To make sure that this is no the situation we need to check 'cgroup.controllers' file if it's not empty to make sure that cgroup v2 is not mounted only for process tracking. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 52 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 23bf81dae2..78497bd31d 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -19,6 +19,10 @@ */ #include <config.h> +#ifdef __linux__ +# include <mntent.h> +#endif /* __linux__ */ + #include "internal.h" #define __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__ @@ -28,7 +32,9 @@ #include "vircgroup.h" #include "vircgroupbackend.h" #include "vircgroupv2.h" +#include "virfile.h" #include "virlog.h" +#include "virstring.h" VIR_LOG_INIT("util.cgroup"); @@ -41,8 +47,54 @@ VIR_ENUM_IMPL(virCgroupV2Controller, VIR_CGROUP_CONTROLLER_LAST, #ifdef __linux__ +/* We're looking for one 'cgroup2' fs mount which has some + * controllers enabled. */ +static bool +virCgroupV2Available(void) +{ + bool ret = false; + FILE *mounts = NULL; + struct mntent entry; + char buf[CGROUP_MAX_VAL]; + + if (!(mounts = fopen("/proc/mounts", "r"))) + return false; + + while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { + if (STREQ(entry.mnt_type, "cgroup2")) { + ret = true; + break; + } + } + + /* Systemd uses cgroup v2 for process tracking but no controller is + * available. We should consider this configuration as cgroup v2 is + * not available. */ + if (ret) { + int rc; + VIR_AUTOFREE(char *) contFile = NULL; + VIR_AUTOFREE(char *) contStr = NULL; + + if (virAsprintf(&contFile, "%s/cgroup.controllers", entry.mnt_dir) < 0) + return false; + + rc = virFileReadAll(contFile, 1024 * 1024, &contStr); + if (rc < 0) + return false; + + if (STREQ(contStr, "")) + return false; + } + + VIR_FORCE_FCLOSE(mounts); + return ret; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, + + .available = virCgroupV2Available, }; -- 2.17.1

On Wed, Oct 03, 2018 at 04:49:35PM +0200, Michal Privoznik wrote: /me hides under table Right, I totally missed that with all the rebases and code movement. I'll fix that. Thanks Pavel

On 10/02/2018 10:43 AM, Pavel Hrdina wrote: Maybe it's the lack of morning coffee, but I had some difficulties parsing this. s/\+/&&/ so that it looks like a C condition. Maybe if you got rid of the ternary operator it would be more readable. But now that I finally understood this, I don't care that much :-) ACK Michal

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 11d9876d36..eaf07397d5 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -167,6 +167,21 @@ virCgroupV2CopyPlacement(virCgroupPtr group, } +static int +virCgroupV2DetectMounts(virCgroupPtr group, + const char *mntType, + const char *mntOpts ATTRIBUTE_UNUSED, + const char *mntDir) +{ + if (STRNEQ(mntType, "cgroup2")) + return 0; + + VIR_FREE(group->unified.mountPoint); + + return VIR_STRDUP(group->unified.mountPoint, mntDir); +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -174,6 +189,7 @@ virCgroupBackend virCgroupV2Backend = { .validateMachineGroup = virCgroupV2ValidateMachineGroup, .copyMounts = virCgroupV2CopyMounts, .copyPlacement = virCgroupV2CopyPlacement, + .detectMounts = virCgroupV2DetectMounts, }; -- 2.17.1

On Thu, Oct 04, 2018 at 01:18:21PM +0200, Michal Privoznik wrote: So I should have probably used the same comment from cgroup v1 backend. We do it there as well because of bind mounts and we are taking into account only the last entry from /proc/mounts. See commit <dacd160d747> for more information. I don't know whether it can/will happen for cgroup v2.

On 10/02/2018 10:43 AM, Pavel Hrdina wrote: Same comment here as in 05/53. ACK Michal

On 10/02/2018 10:43 AM, Pavel Hrdina wrote: I'd put a comment here describing what you do in the commit message. Sure I can go to commit message if I want to, but a comment is more handy. ACK Michal

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 3200338fe3..5cd8e86b97 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -298,6 +298,14 @@ virCgroupV2DetectControllers(virCgroupPtr group, } +static bool +virCgroupV2HasController(virCgroupPtr group, + int controller) +{ + return group->unified.controllers & (1 << controller); +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -310,6 +318,7 @@ virCgroupBackend virCgroupV2Backend = { .validatePlacement = virCgroupV2ValidatePlacement, .stealPlacement = virCgroupV2StealPlacement, .detectControllers = virCgroupV2DetectControllers, + .hasController = virCgroupV2HasController, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 5cd8e86b97..57be0b92c9 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -306,6 +306,13 @@ virCgroupV2HasController(virCgroupPtr group, } +static int +virCgroupV2GetAnyController(virCgroupPtr group) +{ + return ffs(group->unified.controllers) - 1; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -319,6 +326,7 @@ virCgroupBackend virCgroupV2Backend = { .stealPlacement = virCgroupV2StealPlacement, .detectControllers = virCgroupV2DetectControllers, .hasController = virCgroupV2HasController, + .getAnyController = virCgroupV2GetAnyController, }; -- 2.17.1

On Thu, Oct 04, 2018 at 01:18:14PM +0200, Michal Privoznik wrote: I figured that out the hard way :) going through debug logs and investigating why the code is not working how it should. Reading man page would probably help me to realize that sooner.

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 57be0b92c9..3ca463e4c2 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -313,6 +313,29 @@ virCgroupV2GetAnyController(virCgroupPtr group) } +static int +virCgroupV2PathOfController(virCgroupPtr group, + int controller, + const char *key, + char **path) +{ + if (!virCgroupV2HasController(group, controller)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("v2 controller '%s' is not available"), + virCgroupV2ControllerTypeToString(controller)); + return -1; + } + + if (virAsprintf(path, "%s%s/%s", + group->unified.mountPoint, + group->unified.placement, + key ? key : "") < 0) + return -1; + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -327,6 +350,7 @@ virCgroupBackend virCgroupV2Backend = { .detectControllers = virCgroupV2DetectControllers, .hasController = virCgroupV2HasController, .getAnyController = virCgroupV2GetAnyController, + .pathOfController = virCgroupV2PathOfController, }; -- 2.17.1

On Thu, Oct 04, 2018 at 01:18:12PM +0200, Michal Privoznik wrote: Technically yes, but once hybrid support is enabled using the backend specific function in each backend will save some cycles to figure out which backend to use. Originally I had it like that but I realized that moving it to backend I can use the backend specific *HasController.

On Tue, 2018-10-02 at 10:43 +0200, Pavel Hrdina wrote: The `mdkir(path, 0755 || errno == EEXIST) < 0` part of the check doesn't look right.

On 10/02/2018 10:43 AM, Pavel Hrdina wrote: So unsupported flags are ignored? This should have been virFileIsDir() because EEXIST doesn't mean the @path is a directory. It could be a regular file. Ooops, misplaced ')'. mkdir() || errno ;-) This is why I tend to write each condition on a separate line. But if you want to ignore EEXIST then you need to rewrite this check as follows: if (!create || (mkdir() < 0 && errno != EEXIST)) However, I don't think you want to ignore any errno. Also, any reason these two if()-s should not be merged into one? Michal

On Thu, Oct 04, 2018 at 03:20:18PM +0200, Pavel Hrdina wrote: Actually it is guaranteed that the path will be only DIR, cgroup fs doesn't allow to create regular files and when creating partitions we check for collisions with cgroup interface files so virFileExists is good enough. Pavel

On Tue, 2018-10-02 at 10:44 +0200, Pavel Hrdina wrote: Just a nitpick here. I've noticed that in the v1 you're actually checking whether cgroup is NULL or not. Shouldn't you consider doing the same here as well?

On 10/02/2018 10:44 AM, Pavel Hrdina wrote: Useless empty line; Or virFileChownFiles(). ACK Michal

On 10/02/2018 10:44 AM, Pavel Hrdina wrote: Well, the callbacks are implemented, but that's about it. No API that involves reading/setting a value from CGroup will work at this point, oui? For instance: virsh blkiotune fedora error: Operation not supported: operation 'getBlkioWeight' not supported ACK if you move this to be the very last patch (or somewhere before the tests - I haven't gotten that far yet, so I don't know how tests are implemented, whether they need the v2 backend to be registered or not). Michal

On 10/04/2018 06:10 PM, Pavel Hrdina wrote: Sure, but some of them will. Just like I demonstrated above. Hold on, so how are we supposed to limit what devices qemu has access to if there is no devices cgroup? If not adding new layers of security we should not remove them at least. I think there is. Because right now, around these patches none of the APIs that involve CGroup work. It's okay to add code for new feature and enable it only at the very last moment. I think it's even in our coding guidelines. Michal

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index fec5d28835..3e2cd16335 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -546,6 +546,52 @@ virCgroupV2SetOwner(virCgroupPtr cgroup, } +static int +virCgroupV2SetBlkioWeight(virCgroupPtr group, + unsigned int weight) +{ + VIR_AUTOFREE(char *) value = NULL; + + if (virAsprintf(&value, "default %u", weight) < 0) + return -1; + + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.weight", + value); +} + + +static int +virCgroupV2GetBlkioWeight(virCgroupPtr group, + unsigned int *weight) +{ + VIR_AUTOFREE(char *) value = NULL; + char *tmp; + + if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_BLKIO, + "io.weight", &value) < 0) { + return -1; + } + + if (!(tmp = strstr(value, "default "))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot find default io weight.")); + return -1; + } + tmp += strlen("default "); + + if (virStrToLong_ui(tmp, NULL, 10, weight) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + tmp); + return -1; + } + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -567,6 +613,9 @@ virCgroupBackend virCgroupV2Backend = { .hasEmptyTasks = virCgroupV2HasEmptyTasks, .bindMount = virCgroupV2BindMount, .setOwner = virCgroupV2SetOwner, + + .setBlkioWeight = virCgroupV2SetBlkioWeight, + .getBlkioWeight = virCgroupV2GetBlkioWeight, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 66 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 3e2cd16335..30c400f129 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -592,6 +592,71 @@ virCgroupV2GetBlkioWeight(virCgroupPtr group, } +static int +virCgroupV2GetBlkioIoServiced(virCgroupPtr group, + long long *bytes_read, + long long *bytes_write, + long long *requests_read, + long long *requests_write) +{ + long long stats_val; + VIR_AUTOFREE(char *) str1 = NULL; + char *p1; + size_t i; + + const char *value_names[] = { + "rbytes=", + "wbytes=", + "rios=", + "wios=", + }; + long long *value_ptrs[] = { + bytes_read, + bytes_write, + requests_read, + requests_write + }; + + *bytes_read = 0; + *bytes_write = 0; + *requests_read = 0; + *requests_write = 0; + + if (virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.stat", &str1) < 0) { + return -1; + } + + /* sum up all entries of the same kind, from all devices */ + for (i = 0; i < ARRAY_CARDINALITY(value_names); i++) { + p1 = str1; + + while ((p1 = strstr(p1, value_names[i]))) { + p1 += strlen(value_names[i]); + if (virStrToLong_ll(p1, &p1, 10, &stats_val) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot parse byte %sstat '%s'"), + value_names[i], + p1); + return -1; + } + + if (stats_val < 0 || + (stats_val > 0 && *value_ptrs[i] > (LLONG_MAX - stats_val))) { + virReportError(VIR_ERR_OVERFLOW, + _("Sum of byte %sstat overflows"), + value_names[i]); + return -1; + } + *value_ptrs[i] += stats_val; + } + } + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -616,6 +681,7 @@ virCgroupBackend virCgroupV2Backend = { .setBlkioWeight = virCgroupV2SetBlkioWeight, .getBlkioWeight = virCgroupV2GetBlkioWeight, + .getBlkioIoServiced = virCgroupV2GetBlkioIoServiced, }; -- 2.17.1

On Thu, Oct 04, 2018 at 05:26:30PM +0200, Michal Privoznik wrote: Actually there is a lot of other uses like this throughout the code so I would not say it's unusual. Yes, we can use different pointer for the endptr and assign to the startptr, which would end up with the same result. With two pointer it would look like this: p1 += strlen(value_names[i]); if (virStrToLong_ll(p1, &p, 10, &stats_val) < 0) { ... } p1 = p; That's a good question :) ideally we should print only the part that we cannot parse as number, however, that might be tricky to do with this approach and it's not probably worth of handling for the error case which will most likely never happen. I'll fix the format strings. Pavel

On 10/02/2018 10:44 AM, Pavel Hrdina wrote: Same comment here as in previous patch. Otherwise looking good. Michal

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index b7e4db9f41..408f7e3eeb 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -769,6 +769,68 @@ virCgroupV2GetBlkioDeviceWeight(virCgroupPtr group, } +static int +virCgroupV2SetBlkioDeviceReadIops(virCgroupPtr group, + const char *path, + unsigned int riops) +{ + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; + + if (!(blkstr = virCgroupGetBlockDevString(path))) + return -1; + + if (riops == 0) { + if (virAsprintf(&str, "%sriops=max", blkstr) < 0) + return -1; + } else { + if (virAsprintf(&str, "%sriops=%u", blkstr, riops) < 0) + return -1; + } + + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.max", + str); +} + + +static int +virCgroupV2GetBlkioDeviceReadIops(virCgroupPtr group, + const char *path, + unsigned int *riops) +{ + VIR_AUTOFREE(char *) str = NULL; + char *tmp; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.max", + path, + &str) < 0) { + return -1; + } + + if (!str) { + *riops = 0; + } else { + tmp = strstr(str, "riops="); + tmp += strlen("riops="); + + if (STREQLEN(tmp, "max", 3)) { + *riops = 0; + } else if (virStrToLong_ui(tmp, NULL, 10, riops) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + return -1; + } + } + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -797,6 +859,8 @@ virCgroupBackend virCgroupV2Backend = { .getBlkioIoDeviceServiced = virCgroupV2GetBlkioIoDeviceServiced, .setBlkioDeviceWeight = virCgroupV2SetBlkioDeviceWeight, .getBlkioDeviceWeight = virCgroupV2GetBlkioDeviceWeight, + .setBlkioDeviceReadIops = virCgroupV2SetBlkioDeviceReadIops, + .getBlkioDeviceReadIops = virCgroupV2GetBlkioDeviceReadIops, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 408f7e3eeb..6279072aab 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -831,6 +831,68 @@ virCgroupV2GetBlkioDeviceReadIops(virCgroupPtr group, } +static int +virCgroupV2SetBlkioDeviceWriteIops(virCgroupPtr group, + const char *path, + unsigned int wiops) +{ + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; + + if (!(blkstr = virCgroupGetBlockDevString(path))) + return -1; + + if (wiops == 0) { + if (virAsprintf(&str, "%swiops=max", blkstr) < 0) + return -1; + } else { + if (virAsprintf(&str, "%swiops=%u", blkstr, wiops) < 0) + return -1; + } + + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.max", + str); +} + + +static int +virCgroupV2GetBlkioDeviceWriteIops(virCgroupPtr group, + const char *path, + unsigned int *wiops) +{ + VIR_AUTOFREE(char *) str = NULL; + char *tmp; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.max", + path, + &str) < 0) { + return -1; + } + + if (!str) { + *wiops = 0; + } else { + tmp = strstr(str, "wiops="); + tmp += strlen("wiops="); + + if (STREQLEN(tmp, "max", 3)) { + *wiops = 0; + } else if (virStrToLong_ui(tmp, NULL, 10, wiops) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + return -1; + } + } + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -861,6 +923,8 @@ virCgroupBackend virCgroupV2Backend = { .getBlkioDeviceWeight = virCgroupV2GetBlkioDeviceWeight, .setBlkioDeviceReadIops = virCgroupV2SetBlkioDeviceReadIops, .getBlkioDeviceReadIops = virCgroupV2GetBlkioDeviceReadIops, + .setBlkioDeviceWriteIops = virCgroupV2SetBlkioDeviceWriteIops, + .getBlkioDeviceWriteIops = virCgroupV2GetBlkioDeviceWriteIops, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 6279072aab..88bd9fb37c 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -893,6 +893,68 @@ virCgroupV2GetBlkioDeviceWriteIops(virCgroupPtr group, } +static int +virCgroupV2SetBlkioDeviceReadBps(virCgroupPtr group, + const char *path, + unsigned long long rbps) +{ + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; + + if (!(blkstr = virCgroupGetBlockDevString(path))) + return -1; + + if (rbps == 0) { + if (virAsprintf(&str, "%srbps=max", blkstr) < 0) + return -1; + } else { + if (virAsprintf(&str, "%srbps=%llu", blkstr, rbps) < 0) + return -1; + } + + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.max", + str); +} + + +static int +virCgroupV2GetBlkioDeviceReadBps(virCgroupPtr group, + const char *path, + unsigned long long *rbps) +{ + VIR_AUTOFREE(char *) str = NULL; + char *tmp; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.max", + path, + &str) < 0) { + return -1; + } + + if (!str) { + *rbps = 0; + } else { + tmp = strstr(str, "rbps="); + tmp += strlen("rbps="); + + if (STREQLEN(tmp, "max", 3)) { + *rbps = 0; + } else if (virStrToLong_ull(tmp, NULL, 10, rbps) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + return -1; + } + } + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -925,6 +987,8 @@ virCgroupBackend virCgroupV2Backend = { .getBlkioDeviceReadIops = virCgroupV2GetBlkioDeviceReadIops, .setBlkioDeviceWriteIops = virCgroupV2SetBlkioDeviceWriteIops, .getBlkioDeviceWriteIops = virCgroupV2GetBlkioDeviceWriteIops, + .setBlkioDeviceReadBps = virCgroupV2SetBlkioDeviceReadBps, + .getBlkioDeviceReadBps = virCgroupV2GetBlkioDeviceReadBps, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 64 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 88bd9fb37c..d48e9db559 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -955,6 +955,68 @@ virCgroupV2GetBlkioDeviceReadBps(virCgroupPtr group, } +static int +virCgroupV2SetBlkioDeviceWriteBps(virCgroupPtr group, + const char *path, + unsigned long long wbps) +{ + VIR_AUTOFREE(char *) str = NULL; + VIR_AUTOFREE(char *) blkstr = NULL; + + if (!(blkstr = virCgroupGetBlockDevString(path))) + return -1; + + if (wbps == 0) { + if (virAsprintf(&str, "%swbps=max", blkstr) < 0) + return -1; + } else { + if (virAsprintf(&str, "%swbps=%llu", blkstr, wbps) < 0) + return -1; + } + + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.max", + str); +} + + +static int +virCgroupV2GetBlkioDeviceWriteBps(virCgroupPtr group, + const char *path, + unsigned long long *wbps) +{ + VIR_AUTOFREE(char *) str = NULL; + char *tmp; + + if (virCgroupGetValueForBlkDev(group, + VIR_CGROUP_CONTROLLER_BLKIO, + "io.max", + path, + &str) < 0) { + return -1; + } + + if (!str) { + *wbps = 0; + } else { + tmp = strstr(str, "wbps="); + tmp += strlen("wbps="); + + if (STREQLEN(tmp, "max", 3)) { + *wbps = 0; + } else if (virStrToLong_ull(tmp, NULL, 10, wbps) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + str); + return -1; + } + } + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -989,6 +1051,8 @@ virCgroupBackend virCgroupV2Backend = { .getBlkioDeviceWriteIops = virCgroupV2GetBlkioDeviceWriteIops, .setBlkioDeviceReadBps = virCgroupV2SetBlkioDeviceReadBps, .getBlkioDeviceReadBps = virCgroupV2GetBlkioDeviceReadBps, + .setBlkioDeviceWriteBps = virCgroupV2SetBlkioDeviceWriteBps, + .getBlkioDeviceWriteBps = virCgroupV2GetBlkioDeviceWriteBps, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index d48e9db559..32d20f2ff6 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1017,6 +1017,33 @@ virCgroupV2GetBlkioDeviceWriteBps(virCgroupPtr group, } +static int +virCgroupV2SetMemory(virCgroupPtr group, + unsigned long long kb) +{ + unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + + if (kb > maxkb) { + virReportError(VIR_ERR_INVALID_ARG, + _("Memory '%llu' must be less than %llu"), + kb, maxkb); + return -1; + } + + if (kb == maxkb) { + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.max", + "max"); + } else { + return virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.max", + kb << 10); + } +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1053,6 +1080,8 @@ virCgroupBackend virCgroupV2Backend = { .getBlkioDeviceReadBps = virCgroupV2GetBlkioDeviceReadBps, .setBlkioDeviceWriteBps = virCgroupV2SetBlkioDeviceWriteBps, .getBlkioDeviceWriteBps = virCgroupV2GetBlkioDeviceWriteBps, + + .setMemory = virCgroupV2SetMemory, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 75 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 32d20f2ff6..da3b3a984c 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1044,6 +1044,80 @@ virCgroupV2SetMemory(virCgroupPtr group, } +static int +virCgroupV2GetMemoryStat(virCgroupPtr group, + unsigned long long *cache, + unsigned long long *activeAnon, + unsigned long long *inactiveAnon, + unsigned long long *activeFile, + unsigned long long *inactiveFile, + unsigned long long *unevictable) +{ + VIR_AUTOFREE(char *) stat = NULL; + char *line = NULL; + unsigned long long cacheVal = 0; + unsigned long long activeAnonVal = 0; + unsigned long long inactiveAnonVal = 0; + unsigned long long activeFileVal = 0; + unsigned long long inactiveFileVal = 0; + unsigned long long unevictableVal = 0; + + if (virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.stat", + &stat) < 0) { + return -1; + } + + line = stat; + + while (line) { + char *newLine = strchr(line, '\n'); + char *valueStr = strchr(line, ' '); + unsigned long long value; + + if (newLine) + *newLine = '\0'; + + if (!valueStr) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse 'memory.stat' cgroup file.")); + return -1; + } + *valueStr = '\0'; + + if (virStrToLong_ull(valueStr + 1, NULL, 10, &value) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse '%s' as an integer"), + valueStr + 1); + return -1; + } + + if (STREQ(line, "file")) + cacheVal = value >> 10; + else if (STREQ(line, "active_anon")) + activeAnonVal = value >> 10; + else if (STREQ(line, "inactive_anon")) + inactiveAnonVal = value >> 10; + else if (STREQ(line, "active_file")) + activeFileVal = value >> 10; + else if (STREQ(line, "inactive_file")) + inactiveFileVal = value >> 10; + else if (STREQ(line, "unevictable")) + unevictableVal = value >> 10; + } + + *cache = cacheVal; + *activeAnon = activeAnonVal; + *inactiveAnon = inactiveAnonVal; + *activeFile = activeFileVal; + *inactiveFile = inactiveFileVal; + *unevictable = unevictableVal; + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1082,6 +1156,7 @@ virCgroupBackend virCgroupV2Backend = { .getBlkioDeviceWriteBps = virCgroupV2GetBlkioDeviceWriteBps, .setMemory = virCgroupV2SetMemory, + .getMemoryStat = virCgroupV2GetMemoryStat, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index da3b3a984c..9f37ff5be5 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1118,6 +1118,20 @@ virCgroupV2GetMemoryStat(virCgroupPtr group, } +static int +virCgroupV2GetMemoryUsage(virCgroupPtr group, + unsigned long *kb) +{ + long long unsigned int usage_in_bytes; + int ret = virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.current", &usage_in_bytes); + if (ret == 0) + *kb = (unsigned long) usage_in_bytes >> 10; + return ret; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1157,6 +1171,7 @@ virCgroupBackend virCgroupV2Backend = { .setMemory = virCgroupV2SetMemory, .getMemoryStat = virCgroupV2GetMemoryStat, + .getMemoryUsage = virCgroupV2GetMemoryUsage, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 43 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 9f37ff5be5..06522498f2 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1132,6 +1132,47 @@ virCgroupV2GetMemoryUsage(virCgroupPtr group, } +static int +virCgroupV2SetMemoryHardLimit(virCgroupPtr group, + unsigned long long kb) +{ + return virCgroupV2SetMemory(group, kb); +} + + +static int +virCgroupV2GetMemoryHardLimit(virCgroupPtr group, + unsigned long long *kb) +{ + VIR_AUTOFREE(char *) value = NULL; + unsigned long long max; + + if (virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.max", &value) < 0) { + return -1; + } + + if (STREQ(value, "max")) { + *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + return 0; + } + + if (virStrToLong_ull(value, NULL, 10, &max) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse value '%s' as number."), + value); + return -1; + } + + *kb = max >> 10; + if (*kb >= VIR_DOMAIN_MEMORY_PARAM_UNLIMITED) + *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1172,6 +1213,8 @@ virCgroupBackend virCgroupV2Backend = { .setMemory = virCgroupV2SetMemory, .getMemoryStat = virCgroupV2GetMemoryStat, .getMemoryUsage = virCgroupV2GetMemoryUsage, + .setMemoryHardLimit = virCgroupV2SetMemoryHardLimit, + .getMemoryHardLimit = virCgroupV2GetMemoryHardLimit, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 61 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 06522498f2..b5cc5f8f2e 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1173,6 +1173,65 @@ virCgroupV2GetMemoryHardLimit(virCgroupPtr group, } +static int +virCgroupV2SetMemorySoftLimit(virCgroupPtr group, + unsigned long long kb) +{ + unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + + if (kb > maxkb) { + virReportError(VIR_ERR_INVALID_ARG, + _("Memory '%llu' must be less than %llu"), + kb, maxkb); + return -1; + } + + if (kb == maxkb) { + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.high", + "max"); + } else { + return virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.high", + kb << 10); + } +} + + +static int +virCgroupV2GetMemorySoftLimit(virCgroupPtr group, + unsigned long long *kb) +{ + VIR_AUTOFREE(char *) value = NULL; + unsigned long long high; + + if (virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.high", &value) < 0) + return -1; + + if (STREQ(value, "max")) { + *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + return 0; + } + + if (virStrToLong_ull(value, NULL, 10, &high) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse value '%s' as number."), + value); + return -1; + } + + *kb = high >> 10; + if (*kb >= VIR_DOMAIN_MEMORY_PARAM_UNLIMITED) + *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1215,6 +1274,8 @@ virCgroupBackend virCgroupV2Backend = { .getMemoryUsage = virCgroupV2GetMemoryUsage, .setMemoryHardLimit = virCgroupV2SetMemoryHardLimit, .getMemoryHardLimit = virCgroupV2GetMemoryHardLimit, + .setMemorySoftLimit = virCgroupV2SetMemorySoftLimit, + .getMemorySoftLimit = virCgroupV2GetMemorySoftLimit, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 62 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index b5cc5f8f2e..ae352f0212 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1232,6 +1232,66 @@ virCgroupV2GetMemorySoftLimit(virCgroupPtr group, } +static int +virCgroupV2SetMemSwapHardLimit(virCgroupPtr group, + unsigned long long kb) +{ + unsigned long long maxkb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + + if (kb > maxkb) { + virReportError(VIR_ERR_INVALID_ARG, + _("Memory '%llu' must be less than %llu"), + kb, maxkb); + return -1; + } + + if (kb == maxkb) { + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.swap.max", + "max"); + } else { + return virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.swap.max", + kb << 10); + } +} + + +static int +virCgroupV2GetMemSwapHardLimit(virCgroupPtr group, + unsigned long long *kb) +{ + VIR_AUTOFREE(char *) value = NULL; + unsigned long long max; + + if (virCgroupGetValueStr(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.swap.max", &value) < 0) { + return -1; + } + + if (STREQ(value, "max")) { + *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + return 0; + } + + if (virStrToLong_ull(value, NULL, 10, &max) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse value '%s' as number."), + value); + return -1; + } + + *kb = max >> 10; + if (*kb >= VIR_DOMAIN_MEMORY_PARAM_UNLIMITED) + *kb = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED; + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1276,6 +1336,8 @@ virCgroupBackend virCgroupV2Backend = { .getMemoryHardLimit = virCgroupV2GetMemoryHardLimit, .setMemorySoftLimit = virCgroupV2SetMemorySoftLimit, .getMemorySoftLimit = virCgroupV2GetMemorySoftLimit, + .setMemSwapHardLimit = virCgroupV2SetMemSwapHardLimit, + .getMemSwapHardLimit = virCgroupV2GetMemSwapHardLimit, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index ae352f0212..3d1d68ded5 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1292,6 +1292,21 @@ virCgroupV2GetMemSwapHardLimit(virCgroupPtr group, } +static int +virCgroupV2GetMemSwapUsage(virCgroupPtr group, + unsigned long long *kb) +{ + long long unsigned int usage_in_bytes; + int ret; + ret = virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_MEMORY, + "memory.swap.current", &usage_in_bytes); + if (ret == 0) + *kb = (unsigned long) usage_in_bytes >> 10; + return ret; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1338,6 +1353,7 @@ virCgroupBackend virCgroupV2Backend = { .getMemorySoftLimit = virCgroupV2GetMemorySoftLimit, .setMemSwapHardLimit = virCgroupV2SetMemSwapHardLimit, .getMemSwapHardLimit = virCgroupV2GetMemSwapHardLimit, + .getMemSwapUsage = virCgroupV2GetMemSwapUsage, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 3d1d68ded5..6cecfb9741 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1307,6 +1307,26 @@ virCgroupV2GetMemSwapUsage(virCgroupPtr group, } +static int +virCgroupV2SetCpuShares(virCgroupPtr group, + unsigned long long shares) +{ + return virCgroupSetValueU64(group, + VIR_CGROUP_CONTROLLER_CPU, + "cpu.weight", shares); +} + + +static int +virCgroupV2GetCpuShares(virCgroupPtr group, + unsigned long long *shares) +{ + return virCgroupGetValueU64(group, + VIR_CGROUP_CONTROLLER_CPU, + "cpu.weight", shares); +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1354,6 +1374,9 @@ virCgroupBackend virCgroupV2Backend = { .setMemSwapHardLimit = virCgroupV2SetMemSwapHardLimit, .getMemSwapHardLimit = virCgroupV2GetMemSwapHardLimit, .getMemSwapUsage = virCgroupV2GetMemSwapUsage, + + .setCpuShares = virCgroupV2SetCpuShares, + .getCpuShares = virCgroupV2GetCpuShares, }; -- 2.17.1

In order to set CPU cfs period using cgroup v2 'cpu.max' interface we need to load the current value of CPU cfs quota first because format of 'cpu.max' interface is '$quota $period' and in order to change 'period' we need to write 'quota' as well. Writing only one number changes only 'quota'. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 68 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 6cecfb9741..92fe825795 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1327,6 +1327,72 @@ virCgroupV2GetCpuShares(virCgroupPtr group, } +static int +virCgroupV2SetCpuCfsPeriod(virCgroupPtr group, + unsigned long long cfs_period) +{ + VIR_AUTOFREE(char *) value = NULL; + VIR_AUTOFREE(char *) str = NULL; + char *tmp; + + /* The cfs_period should be greater or equal than 1ms, and less or equal + * than 1s. + */ + if (cfs_period < 1000 || cfs_period > 1000000) { + virReportError(VIR_ERR_INVALID_ARG, + _("cfs_period '%llu' must be in range (1000, 1000000)"), + cfs_period); + return -1; + } + + if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPU, + "cpu.max", &str) < 0) { + return -1; + } + + if (!(tmp = strchr(str, ' '))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Invalid 'cpu.max' data.")); + return -1; + } + *tmp = '\n'; + + if (virAsprintf(&value, "%s %llu", str, cfs_period) < 0) + return -1; + + return virCgroupSetValueStr(group, VIR_CGROUP_CONTROLLER_CPU, + "cpu.max", value); +} + + +static int +virCgroupV2GetCpuCfsPeriod(virCgroupPtr group, + unsigned long long *cfs_period) +{ + VIR_AUTOFREE(char *) str = NULL; + char *tmp; + + if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPU, + "cpu.max", &str) < 0) { + return -1; + } + + if (!(tmp = strchr(str, ' '))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Invalid 'cpu.max' data.")); + return -1; + } + + if (virStrToLong_ull(tmp, NULL, 10, cfs_period) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse value '%s' from cpu.max."), str); + return -1; + } + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1377,6 +1443,8 @@ virCgroupBackend virCgroupV2Backend = { .setCpuShares = virCgroupV2SetCpuShares, .getCpuShares = virCgroupV2GetCpuShares, + .setCpuCfsPeriod = virCgroupV2SetCpuCfsPeriod, + .getCpuCfsPeriod = virCgroupV2GetCpuCfsPeriod, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 52 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 92fe825795..d52e065685 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1393,6 +1393,56 @@ virCgroupV2GetCpuCfsPeriod(virCgroupPtr group, } +static int +virCgroupV2SetCpuCfsQuota(virCgroupPtr group, + long long cfs_quota) +{ + /* The cfs_quota should be greater or equal than 1ms */ + if (cfs_quota >= 0 && + (cfs_quota < 1000 || + cfs_quota > ULLONG_MAX / 1000)) { + virReportError(VIR_ERR_INVALID_ARG, + _("cfs_quota '%lld' must be in range (1000, %llu)"), + cfs_quota, ULLONG_MAX / 1000); + return -1; + } + + if (cfs_quota == ULLONG_MAX / 1000) { + return virCgroupSetValueStr(group, + VIR_CGROUP_CONTROLLER_CPU, + "cpu.max", "max"); + } + + return virCgroupSetValueI64(group, + VIR_CGROUP_CONTROLLER_CPU, + "cpu.max", cfs_quota); +} + + +static int +virCgroupV2GetCpuCfsQuota(virCgroupPtr group, + long long *cfs_quota) +{ + VIR_AUTOFREE(char *) str = NULL; + + if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPU, + "cpu.max", &str) < 0) { + return -1; + } + + if (STREQLEN(str, "max", 3)) + *cfs_quota = ULLONG_MAX / 1000; + + if (virStrToLong_ll(str, NULL, 10, cfs_quota) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse value '%s' from cpu.max."), str); + return -1; + } + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1445,6 +1495,8 @@ virCgroupBackend virCgroupV2Backend = { .getCpuShares = virCgroupV2GetCpuShares, .setCpuCfsPeriod = virCgroupV2SetCpuCfsPeriod, .getCpuCfsPeriod = virCgroupV2GetCpuCfsPeriod, + .setCpuCfsQuota = virCgroupV2SetCpuCfsQuota, + .getCpuCfsQuota = virCgroupV2GetCpuCfsQuota, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index d52e065685..61c881f7e2 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1443,6 +1443,21 @@ virCgroupV2GetCpuCfsQuota(virCgroupPtr group, } +static bool +virCgroupV2SupportsCpuBW(virCgroupPtr cgroup) +{ + VIR_AUTOFREE(char *) path = NULL; + + if (virCgroupV2PathOfController(cgroup, VIR_CGROUP_CONTROLLER_CPU, + "cpu.max", &path) < 0) { + virResetLastError(); + return false; + } + + return virFileExists(path); +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1497,6 +1512,7 @@ virCgroupBackend virCgroupV2Backend = { .getCpuCfsPeriod = virCgroupV2GetCpuCfsPeriod, .setCpuCfsQuota = virCgroupV2SetCpuCfsQuota, .getCpuCfsQuota = virCgroupV2GetCpuCfsQuota, + .supportsCpuBW = virCgroupV2SupportsCpuBW, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 61c881f7e2..c6e66189bb 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1458,6 +1458,37 @@ virCgroupV2SupportsCpuBW(virCgroupPtr cgroup) } +static int +virCgroupV2GetCpuacctUsage(virCgroupPtr group, + unsigned long long *usage) +{ + VIR_AUTOFREE(char *) str = NULL; + char *tmp; + + if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, + "cpu.stat", &str) < 0) { + return -1; + } + + if (!(tmp = strstr(str, "usage_usec "))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse cpu usage stat '%s'"), str); + return -1; + } + tmp += strlen("usage_usec "); + + if (virStrToLong_ull(tmp, &tmp, 10, usage) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse value '%s' as number."), tmp); + return -1; + } + + *usage *= 1000; + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1513,6 +1544,8 @@ virCgroupBackend virCgroupV2Backend = { .setCpuCfsQuota = virCgroupV2SetCpuCfsQuota, .getCpuCfsQuota = virCgroupV2GetCpuCfsQuota, .supportsCpuBW = virCgroupV2SupportsCpuBW, + + .getCpuacctUsage = virCgroupV2GetCpuacctUsage, }; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroupv2.c | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index c6e66189bb..c7248c1145 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -1489,6 +1489,54 @@ virCgroupV2GetCpuacctUsage(virCgroupPtr group, } +static int +virCgroupV2GetCpuacctStat(virCgroupPtr group, + unsigned long long *user, + unsigned long long *sys) +{ + VIR_AUTOFREE(char *) str = NULL; + char *tmp; + unsigned long long userVal = 0; + unsigned long long sysVal = 0; + + if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPUACCT, + "cpu.stat", &str) < 0) { + return -1; + } + + if (!(tmp = strstr(str, "user_usec "))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse cpu user stat '%s'"), str); + return -1; + } + tmp += strlen("user_usec "); + + if (virStrToLong_ull(tmp, &tmp, 10, &userVal) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse value '%s' as number."), tmp); + return -1; + } + + if (!(tmp = strstr(str, "system_usec "))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot parse cpu sys stat '%s'"), str); + return -1; + } + tmp += strlen("system_usec "); + + if (virStrToLong_ull(tmp, &tmp, 10, &sysVal) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse value '%s' as number."), tmp); + return -1; + } + + *user = userVal * 1000; + *sys = sysVal * 1000; + + return 0; +} + + virCgroupBackend virCgroupV2Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V2, @@ -1546,6 +1594,7 @@ virCgroupBackend virCgroupV2Backend = { .supportsCpuBW = virCgroupV2SupportsCpuBW, .getCpuacctUsage = virCgroupV2GetCpuacctUsage, + .getCpuacctStat = virCgroupV2GetCpuacctStat, }; -- 2.17.1

This enables to use both cgroup v1 and v2 at the same time together with libvirt. It is supported by kernel and there is valid use-case, not all controllers are implemented in cgroup v2 so there might be configurations where administrator would enable these missing controllers in cgroup v1. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/util/vircgroup.c | 351 ++++++++++++++++++++++++++---------- src/util/vircgroupbackend.c | 20 ++ src/util/vircgroupbackend.h | 16 +- src/util/vircgrouppriv.h | 2 +- 4 files changed, 291 insertions(+), 98 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index dc249bfe33..4aec5f1bcf 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -228,6 +228,7 @@ virCgroupDetectMounts(virCgroupPtr group) struct mntent entry; char buf[CGROUP_MAX_VAL]; int ret = -1; + size_t i; mounts = fopen("/proc/mounts", "r"); if (mounts == NULL) { @@ -236,11 +237,14 @@ virCgroupDetectMounts(virCgroupPtr group) } while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { - if (group->backend->detectMounts(group, - entry.mnt_type, - entry.mnt_opts, - entry.mnt_dir) < 0) { - goto cleanup; + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->detectMounts(group, + entry.mnt_type, + entry.mnt_opts, + entry.mnt_dir) < 0) { + goto cleanup; + } } } @@ -303,6 +307,7 @@ virCgroupDetectPlacement(virCgroupPtr group, } while (fgets(line, sizeof(line), mapping) != NULL) { + size_t i; char *controllers = strchr(line, ':'); char *selfpath = controllers ? strchr(controllers + 1, ':') : NULL; char *nl = selfpath ? strchr(selfpath, '\n') : NULL; @@ -317,9 +322,12 @@ virCgroupDetectPlacement(virCgroupPtr group, controllers++; selfpath++; - if (group->backend->detectPlacement(group, path, controllers, - selfpath) < 0) { - goto cleanup; + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->detectPlacement(group, path, controllers, + selfpath) < 0) { + goto cleanup; + } } } @@ -338,8 +346,9 @@ virCgroupDetect(virCgroupPtr group, const char *path, virCgroupPtr parent) { - int rc; size_t i; + bool backendAvailable = false; + int controllersAvailable = 0; virCgroupBackendPtr *backends = virCgroupBackendGetAll(); VIR_DEBUG("group=%p controllers=%d path=%s parent=%p", @@ -350,31 +359,40 @@ virCgroupDetect(virCgroupPtr group, for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { if (backends[i] && backends[i]->available()) { - group->backend = backends[i]; - break; + group->backends[i] = backends[i]; + backendAvailable = true; } } - if (!group->backend) { + if (!backendAvailable) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no cgroup backend available")); return -1; } if (parent) { - if (group->backend->copyMounts(group, parent) < 0) - return -1; + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->copyMounts(group, parent) < 0) { + return -1; + } + } } else { if (virCgroupDetectMounts(group) < 0) return -1; } - rc = group->backend->detectControllers(group, controllers); - if (rc < 0) - return -1; + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i]) { + int rc = group->backends[i]->detectControllers(group, controllers); + if (rc < 0) + return -1; + controllersAvailable |= rc; + } + } /* Check that at least 1 controller is available */ - if (rc == 0) { + if (controllersAvailable == 0) { virReportSystemError(ENXIO, "%s", _("At least one cgroup controller is required")); return -1; @@ -383,17 +401,26 @@ virCgroupDetect(virCgroupPtr group, /* In some cases we can copy part of the placement info * based on the parent cgroup... */ - if ((parent || path[0] == '/') && - group->backend->copyPlacement(group, path, parent) < 0) - return -1; + if (parent || path[0] == '/') { + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->copyPlacement(group, path, parent) < 0) { + return -1; + } + } + } /* ... but use /proc/cgroups to fill in the rest */ if (virCgroupDetectPlacement(group, pid, path) < 0) return -1; /* Check that for every mounted controller, we found our placement */ - if (group->backend->validatePlacement(group, pid) < 0) - return -1; + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->validatePlacement(group, pid) < 0) { + return -1; + } + } return 0; } @@ -599,9 +626,14 @@ virCgroupMakeGroup(virCgroupPtr parent, bool create, unsigned int flags) { - if (group->backend->makeGroup(parent, group, create, flags) < 0) { - virCgroupRemove(group); - return -1; + size_t i; + + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->makeGroup(parent, group, create, flags) < 0) { + virCgroupRemove(group); + return -1; + } } return 0; @@ -662,6 +694,24 @@ virCgroupNew(pid_t pid, } +static int +virCgroupAddTaskInternal(virCgroupPtr group, + pid_t pid, + unsigned int flags) +{ + size_t i; + + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->addTask(group, pid, flags) < 0) { + return -1; + } + } + + return 0; +} + + /** * virCgroupAddProcess: * @@ -676,7 +726,7 @@ virCgroupNew(pid_t pid, int virCgroupAddProcess(virCgroupPtr group, pid_t pid) { - return group->backend->addTask(group, pid, VIR_CGROUP_TASK_PROCESS); + return virCgroupAddTaskInternal(group, pid, VIR_CGROUP_TASK_PROCESS); } /** @@ -693,9 +743,9 @@ virCgroupAddProcess(virCgroupPtr group, pid_t pid) int virCgroupAddMachineProcess(virCgroupPtr group, pid_t pid) { - return group->backend->addTask(group, pid, - VIR_CGROUP_TASK_PROCESS | - VIR_CGROUP_TASK_SYSTEMD); + return virCgroupAddTaskInternal(group, pid, + VIR_CGROUP_TASK_PROCESS | + VIR_CGROUP_TASK_SYSTEMD); } /** @@ -713,7 +763,7 @@ int virCgroupAddThread(virCgroupPtr group, pid_t pid) { - return group->backend->addTask(group, pid, VIR_CGROUP_TASK_THREAD); + return virCgroupAddTaskInternal(group, pid, VIR_CGROUP_TASK_THREAD); } @@ -967,17 +1017,24 @@ virCgroupNewDetectMachine(const char *name, char *machinename, virCgroupPtr *group) { + size_t i; + if (virCgroupNewDetect(pid, controllers, group) < 0) { if (virCgroupNewIgnoreError()) return 0; return -1; } - if (!(*group)->backend->validateMachineGroup(*group, name, drivername, machinename)) { - VIR_DEBUG("Failed to validate machine name for '%s' driver '%s'", - name, drivername); - virCgroupFree(group); - return 0; + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if ((*group)->backends[i] && + !(*group)->backends[i]->validateMachineGroup(*group, name, + drivername, + machinename)) { + VIR_DEBUG("Failed to validate machine name for '%s' driver '%s'", + name, drivername); + virCgroupFree(group); + return 0; + } } return 0; @@ -1055,6 +1112,7 @@ virCgroupNewMachineSystemd(const char *name, int rv; virCgroupPtr init; VIR_AUTOFREE(char *) path = NULL; + size_t i; VIR_DEBUG("Trying to setup machine '%s' via systemd", name); if ((rv = virSystemdCreateMachine(name, @@ -1077,7 +1135,12 @@ virCgroupNewMachineSystemd(const char *name, &init) < 0) return -1; - path = init->backend->stealPlacement(init); + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (init->backends[i] && + (path = init->backends[i]->stealPlacement(init))) { + break; + } + } virCgroupFree(&init); if (!path || STREQ(path, "/") || path[0] != '/') { @@ -1256,12 +1319,21 @@ virCgroupFree(virCgroupPtr *group) bool virCgroupHasController(virCgroupPtr cgroup, int controller) { + size_t i; + if (!cgroup) return false; if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) return false; - return cgroup->backend->hasController(cgroup, controller); + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (cgroup->backends[i] && + cgroup->backends[i]->hasController(cgroup, controller)) { + return true; + } + } + + return false; } @@ -1277,7 +1349,8 @@ virCgroupPathOfController(virCgroupPtr group, return -1; } - return group->backend->pathOfController(group, controller, key, path); + VIR_CGROUP_BACKEND_CALL(group, controller, pathOfController, -1, + controller, key, path); } @@ -1299,7 +1372,8 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group, long long *requests_read, long long *requests_write) { - VIR_CGROUP_BACKEND_CALL(group, getBlkioIoServiced, -1, + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + getBlkioIoServiced, -1, bytes_read, bytes_write, requests_read, requests_write); } @@ -1325,7 +1399,8 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group, long long *requests_read, long long *requests_write) { - VIR_CGROUP_BACKEND_CALL(group, getBlkioIoDeviceServiced, -1, + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + getBlkioIoDeviceServiced, -1, path, bytes_read, bytes_write, requests_read, requests_write); } @@ -1342,7 +1417,8 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group, int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight) { - VIR_CGROUP_BACKEND_CALL(group, setBlkioWeight, -1, weight); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + setBlkioWeight, -1, weight); } @@ -1357,7 +1433,8 @@ virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight) int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight) { - VIR_CGROUP_BACKEND_CALL(group, getBlkioWeight, -1, weight); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + getBlkioWeight, -1, weight); } /** @@ -1373,7 +1450,8 @@ virCgroupSetBlkioDeviceReadIops(virCgroupPtr group, const char *path, unsigned int riops) { - VIR_CGROUP_BACKEND_CALL(group, setBlkioDeviceReadIops, -1, path, riops); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + setBlkioDeviceReadIops, -1, path, riops); } @@ -1390,7 +1468,8 @@ virCgroupSetBlkioDeviceWriteIops(virCgroupPtr group, const char *path, unsigned int wiops) { - VIR_CGROUP_BACKEND_CALL(group, setBlkioDeviceWriteIops, -1, path, wiops); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + setBlkioDeviceWriteIops, -1, path, wiops); } @@ -1407,7 +1486,8 @@ virCgroupSetBlkioDeviceReadBps(virCgroupPtr group, const char *path, unsigned long long rbps) { - VIR_CGROUP_BACKEND_CALL(group, setBlkioDeviceReadBps, -1, path, rbps); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + setBlkioDeviceReadBps, -1, path, rbps); } /** @@ -1423,7 +1503,8 @@ virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group, const char *path, unsigned long long wbps) { - VIR_CGROUP_BACKEND_CALL(group, setBlkioDeviceWriteBps, -1, path, wbps); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + setBlkioDeviceWriteBps, -1, path, wbps); } @@ -1441,7 +1522,8 @@ virCgroupSetBlkioDeviceWeight(virCgroupPtr group, const char *path, unsigned int weight) { - VIR_CGROUP_BACKEND_CALL(group, setBlkioDeviceWeight, -1, path, weight); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + setBlkioDeviceWeight, -1, path, weight); } /** @@ -1457,7 +1539,8 @@ virCgroupGetBlkioDeviceReadIops(virCgroupPtr group, const char *path, unsigned int *riops) { - VIR_CGROUP_BACKEND_CALL(group, getBlkioDeviceReadIops, -1, path, riops); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + getBlkioDeviceReadIops, -1, path, riops); } /** @@ -1473,7 +1556,8 @@ virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group, const char *path, unsigned int *wiops) { - VIR_CGROUP_BACKEND_CALL(group, getBlkioDeviceWriteIops, -1, path, wiops); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + getBlkioDeviceWriteIops, -1, path, wiops); } /** @@ -1489,7 +1573,8 @@ virCgroupGetBlkioDeviceReadBps(virCgroupPtr group, const char *path, unsigned long long *rbps) { - VIR_CGROUP_BACKEND_CALL(group, getBlkioDeviceReadBps, -1, path, rbps); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + getBlkioDeviceReadBps, -1, path, rbps); } /** @@ -1505,7 +1590,8 @@ virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, const char *path, unsigned long long *wbps) { - VIR_CGROUP_BACKEND_CALL(group, getBlkioDeviceWriteBps, -1, path, wbps); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + getBlkioDeviceWriteBps, -1, path, wbps); } /** @@ -1521,7 +1607,8 @@ virCgroupGetBlkioDeviceWeight(virCgroupPtr group, const char *path, unsigned int *weight) { - VIR_CGROUP_BACKEND_CALL(group, getBlkioDeviceWeight, -1, path, weight); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO, + getBlkioDeviceWeight, -1, path, weight); } @@ -1536,7 +1623,8 @@ virCgroupGetBlkioDeviceWeight(virCgroupPtr group, int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb) { - VIR_CGROUP_BACKEND_CALL(group, setMemory, -1, kb); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + setMemory, -1, kb); } @@ -1562,7 +1650,8 @@ virCgroupGetMemoryStat(virCgroupPtr group, unsigned long long *inactiveFile, unsigned long long *unevictable) { - VIR_CGROUP_BACKEND_CALL(group, getMemoryStat, -1, cache, + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + getMemoryStat, -1, cache, activeAnon, inactiveAnon, activeFile, inactiveFile, unevictable); @@ -1580,7 +1669,8 @@ virCgroupGetMemoryStat(virCgroupPtr group, int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb) { - VIR_CGROUP_BACKEND_CALL(group, getMemoryUsage, -1, kb); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + getMemoryUsage, -1, kb); } @@ -1595,7 +1685,8 @@ virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb) int virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb) { - VIR_CGROUP_BACKEND_CALL(group, setMemoryHardLimit, -1, kb); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + setMemoryHardLimit, -1, kb); } @@ -1610,7 +1701,8 @@ virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb) int virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb) { - VIR_CGROUP_BACKEND_CALL(group, getMemoryHardLimit, -1, kb); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + getMemoryHardLimit, -1, kb); } @@ -1625,7 +1717,8 @@ virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb) int virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb) { - VIR_CGROUP_BACKEND_CALL(group, setMemorySoftLimit, -1, kb); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + setMemorySoftLimit, -1, kb); } @@ -1640,7 +1733,8 @@ virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb) int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb) { - VIR_CGROUP_BACKEND_CALL(group, getMemorySoftLimit, -1, kb); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + getMemorySoftLimit, -1, kb); } @@ -1655,7 +1749,8 @@ virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb) int virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb) { - VIR_CGROUP_BACKEND_CALL(group, setMemSwapHardLimit, -1, kb); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + setMemSwapHardLimit, -1, kb); } @@ -1670,7 +1765,8 @@ virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb) int virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb) { - VIR_CGROUP_BACKEND_CALL(group, getMemSwapHardLimit, -1, kb); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + getMemSwapHardLimit, -1, kb); } @@ -1685,7 +1781,8 @@ virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb) int virCgroupGetMemSwapUsage(virCgroupPtr group, unsigned long long *kb) { - VIR_CGROUP_BACKEND_CALL(group, getMemSwapUsage, -1, kb); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY, + getMemSwapUsage, -1, kb); } @@ -1700,7 +1797,8 @@ virCgroupGetMemSwapUsage(virCgroupPtr group, unsigned long long *kb) int virCgroupSetCpusetMems(virCgroupPtr group, const char *mems) { - VIR_CGROUP_BACKEND_CALL(group, setCpusetMems, -1, mems); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET, + setCpusetMems, -1, mems); } @@ -1715,7 +1813,8 @@ virCgroupSetCpusetMems(virCgroupPtr group, const char *mems) int virCgroupGetCpusetMems(virCgroupPtr group, char **mems) { - VIR_CGROUP_BACKEND_CALL(group, getCpusetMems, -1, mems); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET, + getCpusetMems, -1, mems); } @@ -1730,7 +1829,8 @@ virCgroupGetCpusetMems(virCgroupPtr group, char **mems) int virCgroupSetCpusetMemoryMigrate(virCgroupPtr group, bool migrate) { - VIR_CGROUP_BACKEND_CALL(group, setCpusetMemoryMigrate, -1, migrate); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET, + setCpusetMemoryMigrate, -1, migrate); } @@ -1745,7 +1845,8 @@ virCgroupSetCpusetMemoryMigrate(virCgroupPtr group, bool migrate) int virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate) { - VIR_CGROUP_BACKEND_CALL(group, getCpusetMemoryMigrate, -1, migrate); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET, + getCpusetMemoryMigrate, -1, migrate); } @@ -1760,7 +1861,8 @@ virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate) int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus) { - VIR_CGROUP_BACKEND_CALL(group, setCpusetCpus, -1, cpus); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET, + setCpusetCpus, -1, cpus); } @@ -1775,7 +1877,8 @@ virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus) int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus) { - VIR_CGROUP_BACKEND_CALL(group, getCpusetCpus, -1, cpus); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET, + getCpusetCpus, -1, cpus); } @@ -1789,7 +1892,8 @@ virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus) int virCgroupDenyAllDevices(virCgroupPtr group) { - VIR_CGROUP_BACKEND_CALL(group, denyAllDevices, -1); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES, + denyAllDevices, -1); } /** @@ -1809,7 +1913,8 @@ virCgroupDenyAllDevices(virCgroupPtr group) int virCgroupAllowAllDevices(virCgroupPtr group, int perms) { - VIR_CGROUP_BACKEND_CALL(group, allowAllDevices, -1, perms); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES, + allowAllDevices, -1, perms); } @@ -1828,7 +1933,8 @@ int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor, int perms) { - VIR_CGROUP_BACKEND_CALL(group, allowDevice, -1, type, major, minor, perms); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES, + allowDevice, -1, type, major, minor, perms); } @@ -1867,7 +1973,8 @@ virCgroupAllowDevicePath(virCgroupPtr group, if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode)) return 1; - VIR_CGROUP_BACKEND_CALL(group, allowDevice, -1, + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES, + allowDevice, -1, S_ISCHR(sb.st_mode) ? 'c' : 'b', major(sb.st_rdev), minor(sb.st_rdev), @@ -1890,7 +1997,8 @@ int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor, int perms) { - VIR_CGROUP_BACKEND_CALL(group, denyDevice, -1, type, major, minor, perms); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES, + denyDevice, -1, type, major, minor, perms); } @@ -1929,7 +2037,8 @@ virCgroupDenyDevicePath(virCgroupPtr group, if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode)) return 1; - VIR_CGROUP_BACKEND_CALL(group, denyDevice, -1, + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES, + denyDevice, -1, S_ISCHR(sb.st_mode) ? 'c' : 'b', major(sb.st_rdev), minor(sb.st_rdev), @@ -2172,14 +2281,16 @@ virCgroupGetDomainTotalCpuStats(virCgroupPtr group, int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares) { - VIR_CGROUP_BACKEND_CALL(group, setCpuShares, -1, shares); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU, + setCpuShares, -1, shares); } int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares) { - VIR_CGROUP_BACKEND_CALL(group, getCpuShares, -1, shares); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU, + getCpuShares, -1, shares); } @@ -2194,7 +2305,8 @@ virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares) int virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period) { - VIR_CGROUP_BACKEND_CALL(group, setCpuCfsPeriod, -1, cfs_period); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU, + setCpuCfsPeriod, -1, cfs_period); } @@ -2209,7 +2321,8 @@ virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period) int virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period) { - VIR_CGROUP_BACKEND_CALL(group, getCpuCfsPeriod, -1, cfs_period); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU, + getCpuCfsPeriod, -1, cfs_period); } @@ -2225,14 +2338,16 @@ virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period) int virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota) { - VIR_CGROUP_BACKEND_CALL(group, setCpuCfsQuota, -1, cfs_quota); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU, + setCpuCfsQuota, -1, cfs_quota); } int virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage) { - VIR_CGROUP_BACKEND_CALL(group, getCpuacctPercpuUsage, -1, usage); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUACCT, + getCpuacctPercpuUsage, -1, usage); } @@ -2299,7 +2414,16 @@ virCgroupRemoveRecursively(char *grppath) int virCgroupRemove(virCgroupPtr group) { - return group->backend->remove(group); + size_t i; + + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->remove(group) < 0) { + return -1; + } + } + + return 0; } @@ -2308,11 +2432,16 @@ virCgroupPathOfAnyController(virCgroupPtr group, const char *name, char **keypath) { + size_t i; int controller; - controller = group->backend->getAnyController(group); - if (controller >= 0) - return virCgroupPathOfController(group, controller, name, keypath); + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i]) { + controller = group->backends[i]->getAnyController(group); + if (controller >= 0) + return virCgroupPathOfController(group, controller, name, keypath); + } + } virReportSystemError(ENOSYS, "%s", _("No controllers are mounted")); @@ -2548,14 +2677,16 @@ virCgroupKillPainfully(virCgroupPtr group) int virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota) { - VIR_CGROUP_BACKEND_CALL(group, getCpuCfsQuota, -1, cfs_quota); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU, + getCpuCfsQuota, -1, cfs_quota); } int virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage) { - VIR_CGROUP_BACKEND_CALL(group, getCpuacctUsage, -1, usage); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUACCT, + getCpuacctUsage, -1, usage); } @@ -2563,21 +2694,24 @@ int virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user, unsigned long long *sys) { - VIR_CGROUP_BACKEND_CALL(group, getCpuacctStat, -1, user, sys); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUACCT, + getCpuacctStat, -1, user, sys); } int virCgroupSetFreezerState(virCgroupPtr group, const char *state) { - VIR_CGROUP_BACKEND_CALL(group, setFreezerState, -1, state); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_FREEZER, + setFreezerState, -1, state); } int virCgroupGetFreezerState(virCgroupPtr group, char **state) { - VIR_CGROUP_BACKEND_CALL(group, getFreezerState, -1, state); + VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_FREEZER, + getFreezerState, -1, state); } @@ -2585,7 +2719,16 @@ int virCgroupBindMount(virCgroupPtr group, const char *oldroot, const char *mountopts) { - return group->backend->bindMount(group, oldroot, mountopts); + size_t i; + + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->bindMount(group, oldroot, mountopts) < 0) { + return -1; + } + } + + return 0; } @@ -2594,7 +2737,16 @@ int virCgroupSetOwner(virCgroupPtr cgroup, gid_t gid, int controllers) { - return cgroup->backend->setOwner(cgroup, uid, gid, controllers); + size_t i; + + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (cgroup->backends[i] && + cgroup->backends[i]->setOwner(cgroup, uid, gid, controllers) < 0) { + return -1; + } + } + + return 0; } @@ -2608,13 +2760,24 @@ int virCgroupSetOwner(virCgroupPtr cgroup, bool virCgroupSupportsCpuBW(virCgroupPtr cgroup) { - VIR_CGROUP_BACKEND_CALL(cgroup, supportsCpuBW, false); + VIR_CGROUP_BACKEND_CALL(cgroup, VIR_CGROUP_CONTROLLER_CPU, + supportsCpuBW, false); } int virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller) { - return cgroup->backend->hasEmptyTasks(cgroup, controller); + size_t i; + + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (cgroup->backends[i]) { + int rc = cgroup->backends[i]->hasEmptyTasks(cgroup, controller); + if (rc <= 0) + return rc; + } + } + + return 1; } bool diff --git a/src/util/vircgroupbackend.c b/src/util/vircgroupbackend.c index 7ee39ac8ca..2e90781dc3 100644 --- a/src/util/vircgroupbackend.c +++ b/src/util/vircgroupbackend.c @@ -20,6 +20,9 @@ #include <config.h> #include "vircgroupbackend.h" +#define __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__ +#include "vircgrouppriv.h" +#undef __VIR_CGROUP_ALLOW_INCLUDE_PRIV_H__ #include "vircgroupv1.h" #include "vircgroupv2.h" #include "virerror.h" @@ -67,3 +70,20 @@ virCgroupBackendGetAll(void) } return virCgroupBackends; } + + +virCgroupBackendPtr +virCgroupBackendForController(virCgroupPtr group, + unsigned int controller) +{ + size_t i; + + for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (group->backends[i] && + group->backends[i]->hasController(group, controller)) { + return group->backends[i]; + } + } + + return NULL; +} diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 86d1539e07..bc60b44643 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -436,12 +436,22 @@ virCgroupBackendRegister(virCgroupBackendPtr backend); virCgroupBackendPtr * virCgroupBackendGetAll(void); -# define VIR_CGROUP_BACKEND_CALL(group, func, ret, ...) \ - if (!group->backend->func) { \ +virCgroupBackendPtr +virCgroupBackendForController(virCgroupPtr group, + unsigned int controller); + +# define VIR_CGROUP_BACKEND_CALL(group, controller, func, ret, ...) \ + virCgroupBackendPtr backend = virCgroupBackendForController(group, controller); \ + if (!backend) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + _("failed to get cgroup backend for '%s'"), #func); \ + return ret; \ + } \ + if (!backend->func) { \ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, \ _("operation '%s' not supported"), #func); \ return ret; \ } \ - return group->backend->func(group, ##__VA_ARGS__); + return backend->func(group, ##__VA_ARGS__); #endif /* __VIR_CGROUP_BACKEND_H__ */ diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index 4a0d75ddbc..8f24b0891e 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -56,7 +56,7 @@ typedef virCgroupV2Controller *virCgroupV2ControllerPtr; struct _virCgroup { char *path; - virCgroupBackendPtr backend; + virCgroupBackendPtr backends[VIR_CGROUP_BACKEND_TYPE_LAST]; virCgroupV1Controller legacy[VIR_CGROUP_CONTROLLER_LAST]; virCgroupV2Controller unified; -- 2.17.1

On Fri, 2018-10-05 at 12:14 +0200, Michal Privoznik wrote: We already have a macro for something similar that could be expanded and reused in all of those. And, yes, I totally agree that it could be done as a follow-up patch.

Remove the trailing '/' from prefix. This change is required in order to introduce tests for unified cgroups. They are usually mounted in '/sys/fs/cgroup'. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgroupmock.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index cfff1f0b7a..5c48ef012a 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -56,7 +56,7 @@ const char *fakedevicedir0 = FAKEDEVDIR0; const char *fakedevicedir1 = FAKEDEVDIR1; -# define SYSFS_CGROUP_PREFIX "/not/really/sys/fs/cgroup/" +# define SYSFS_CGROUP_PREFIX "/not/really/sys/fs/cgroup" # define SYSFS_CPU_PRESENT "/sys/devices/system/cpu/present" # define SYSFS_CPU_PRESENT_MOCKED "devices_system_cpu_present" @@ -354,7 +354,7 @@ int access(const char *path, int mode) if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; - if (asprintf(&newpath, "%s/%s", + if (asprintf(&newpath, "%s%s", fakesysfscgroupdir, path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; @@ -386,7 +386,7 @@ int __lxstat(int ver, const char *path, struct stat *sb) if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; - if (asprintf(&newpath, "%s/%s", + if (asprintf(&newpath, "%s%s", fakesysfscgroupdir, path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; @@ -417,7 +417,7 @@ int lstat(const char *path, struct stat *sb) if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; - if (asprintf(&newpath, "%s/%s", + if (asprintf(&newpath, "%s%s", fakesysfscgroupdir, path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; @@ -448,7 +448,7 @@ int __xstat(int ver, const char *path, struct stat *sb) if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; - if (asprintf(&newpath, "%s/%s", + if (asprintf(&newpath, "%s%s", fakesysfscgroupdir, path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; @@ -487,7 +487,7 @@ int stat(const char *path, struct stat *sb) } } else if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); - if (asprintf(&newpath, "%s/%s", + if (asprintf(&newpath, "%s%s", fakesysfscgroupdir, path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; @@ -519,7 +519,7 @@ int mkdir(const char *path, mode_t mode) if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); char *newpath; - if (asprintf(&newpath, "%s/%s", + if (asprintf(&newpath, "%s%s", fakesysfscgroupdir, path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; @@ -552,7 +552,7 @@ int open(const char *path, int flags, ...) if (STRPREFIX(path, SYSFS_CGROUP_PREFIX)) { init_sysfs(); - if (asprintf(&newpath, "%s/%s", + if (asprintf(&newpath, "%s%s", fakesysfscgroupdir, path + strlen(SYSFS_CGROUP_PREFIX)) < 0) { errno = ENOMEM; -- 2.17.1

We need to create the cgroup v2 sysfs the same way as we do for cgroup v1. This introduces new VIR_CGROUP_MOCK_MODE env variable which will configure which cgroup mode each test requires. There are three different modes: - legacy: only cgroup v1 is available and it's the default mode - hybrid: both cgroup v1 and cgroup v2 are available and have some controllers - unified: only cgroup v2 is available Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgroupmock.c | 153 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 138 insertions(+), 15 deletions(-) diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c index 5c48ef012a..1ed00ea91c 100644 --- a/tests/vircgroupmock.c +++ b/tests/vircgroupmock.c @@ -105,7 +105,8 @@ static int make_file(const char *path, return ret; } -static int make_controller(const char *path, mode_t mode) + +static int make_controller_v1(const char *path, mode_t mode) { int ret = -1; const char *controller; @@ -231,11 +232,86 @@ static int make_controller(const char *path, mode_t mode) goto cleanup; } +# undef MAKE_FILE + ret = 0; cleanup: return ret; } + +static int make_controller_v2(const char *path, mode_t mode) +{ + if (!STRPREFIX(path, fakesysfscgroupdir)) { + errno = EINVAL; + return -1; + } + + if (real_mkdir(path, mode) < 0 && errno != EEXIST) + return -1; + +# define MAKE_FILE(name, value) \ + do { \ + if (make_file(path, name, value) < 0) \ + return -1; \ + } while (0) + + MAKE_FILE("cgroup.controllers", "cpu io memory\n"); + MAKE_FILE("cgroup.subtree_control", ""); + MAKE_FILE("cgroup.type", "domain\n"); + MAKE_FILE("cpu.max", "max 100000\n"); + MAKE_FILE("cpu.stat", + "usage_usec 0\n" + "user_usec 0\n" + "system_usec 0\n" + "nr_periods 0\n" + "nr_throttled 0\n" + "throttled_usec 0\n"); + MAKE_FILE("cpu.weight", "100\n"); + MAKE_FILE("memory.current", "1455321088\n"); + MAKE_FILE("memory.high", "max\n"); + MAKE_FILE("memory.max", "max\n"); + MAKE_FILE("memory.stat", + "anon 0\n" + "file 0\n" + "kernel_stack 0\n" + "slab 0\n" + "sock 0\n" + "shmem 0\n" + "file_mapped 0\n" + "file_dirty 0\n" + "file_writeback 0\n" + "inactive_anon 0\n" + "active_anon 0\n" + "inactive_file 0\n" + "active_file 0\n" + "unevictable 0\n" + "slab_reclaimable 0\n" + "slab_unreclaimable 0\n" + "pgfault 0\n" + "pgmajfault 0\n" + "pgrefill 0\n" + "pgscan 0\n" + "pgsteal 0\n" + "pgactivate 0\n" + "pgdeactivate 0\n" + "pglazyfree 0\n" + "pglazyfreed 0\n" + "workingset_refault 0\n" + "workingset_activate 0\n" + "workingset_nodereclaim 0\n"); + MAKE_FILE("memory.swap.current", "0\n"); + MAKE_FILE("memory.swap.max", "max\n"); + MAKE_FILE("io.stat", "8:0 rbytes=26828800 wbytes=77062144 rios=2256 wios=7849 dbytes=0 dios=0\n"); + MAKE_FILE("io.max", ""); + MAKE_FILE("io.weight", "default 100\n"); + +# undef MAKE_FILE + + return 0; +} + + static void init_syms(void) { if (real_fopen) @@ -249,16 +325,56 @@ static void init_syms(void) VIR_MOCK_REAL_INIT(open); } + +static int make_controller(const char *path, mode_t mode) +{ + const char *mock; + bool unified = false; + bool hybrid = false; + + mock = getenv("VIR_CGROUP_MOCK_MODE"); + if (mock) { + if (STREQ(mock, "unified")) + unified = true; + else if (STREQ(mock, "hybrid")) + hybrid = true; + } + + if (unified || (hybrid && strstr(path, "unified"))) { + return make_controller_v2(path, mode); + } else { + return make_controller_v1(path, mode); + } +} + + static void init_sysfs(void) { - if (fakerootdir && fakesysfscgroupdir) - return; + const char *mock; + char *newfakerootdir; + bool unified = false; + bool hybrid = false; - if (!(fakerootdir = getenv("LIBVIRT_FAKE_ROOT_DIR"))) { + if (!(newfakerootdir = getenv("LIBVIRT_FAKE_ROOT_DIR"))) { fprintf(stderr, "Missing LIBVIRT_FAKE_ROOT_DIR env variable\n"); abort(); } + if (fakerootdir && STREQ(fakerootdir, newfakerootdir)) + return; + + fakerootdir = newfakerootdir; + + mock = getenv("VIR_CGROUP_MOCK_MODE"); + if (mock) { + if (STREQ(mock, "unified")) + unified = true; + else if (STREQ(mock, "hybrid")) + hybrid = true; + } + + VIR_FREE(fakesysfscgroupdir); + if (virAsprintfQuiet(&fakesysfscgroupdir, "%s%s", fakerootdir, SYSFS_CGROUP_PREFIX) < 0) abort(); @@ -281,18 +397,25 @@ static void init_sysfs(void) free(path); \ } while (0) - MAKE_CONTROLLER("cpu"); - MAKE_CONTROLLER("cpuacct"); - MAKE_CONTROLLER("cpu,cpuacct"); - MAKE_CONTROLLER("cpu,cpuacct/system"); - MAKE_CONTROLLER("cpuset"); - MAKE_CONTROLLER("blkio"); - MAKE_CONTROLLER("memory"); - MAKE_CONTROLLER("freezer"); + if (unified) { + MAKE_CONTROLLER(""); + } else if (hybrid) { + MAKE_CONTROLLER("unified"); + MAKE_CONTROLLER("cpuset"); + MAKE_CONTROLLER("freezer"); + } else { + MAKE_CONTROLLER("cpu"); + MAKE_CONTROLLER("cpuacct"); + MAKE_CONTROLLER("cpu,cpuacct"); + MAKE_CONTROLLER("cpuset"); + MAKE_CONTROLLER("blkio"); + MAKE_CONTROLLER("memory"); + MAKE_CONTROLLER("freezer"); - if (make_file(fakesysfscgroupdir, - SYSFS_CPU_PRESENT_MOCKED, "8-23,48-159\n") < 0) - abort(); + if (make_file(fakesysfscgroupdir, + SYSFS_CPU_PRESENT_MOCKED, "8-23,48-159\n") < 0) + abort(); + } } -- 2.17.1

We need to configure multiple env variables for each set of tests so create helper functions to do that. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgrouptest.c | 48 ++++++++++++++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 4f62014ead..3e8793a6c3 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -830,10 +830,10 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args ATTRIBUTE_UNUSED) # define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX" -static int -mymain(void) +static char * +initFakeFS(const char *mode, + const char *filename) { - int ret = 0; char *fakerootdir; if (VIR_STRDUP_QUIET(fakerootdir, FAKEROOTDIRTEMPLATE) < 0) { @@ -848,6 +848,33 @@ mymain(void) setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1); + if (mode) + setenv("VIR_CGROUP_MOCK_MODE", mode, 1); + + if (filename) + setenv("VIR_CGROUP_MOCK_FILENAME", filename, 1); + + return fakerootdir; +} + +static void +cleanupFakeFS(char *fakerootdir) +{ + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) + virFileDeleteTree(fakerootdir); + + VIR_FREE(fakerootdir); + unsetenv("LIBVIRT_FAKE_ROOT_DIR"); + unsetenv("VIR_CGROUP_MOCK_MODE"); + unsetenv("VIR_CGROUP_MOCK_FILENAME"); +} + +static int +mymain(void) +{ + int ret = 0; + char *fakerootdir; + # define DETECT_MOUNTS_FULL(file, fail) \ do { \ struct _detectMountsData data = { file, fail }; \ @@ -871,7 +898,7 @@ mymain(void) DETECT_MOUNTS_FAIL("no-cgroups"); DETECT_MOUNTS("kubevirt"); - setenv("VIR_CGROUP_MOCK_FILENAME", "systemd", 1); + fakerootdir = initFakeFS(NULL, "systemd"); if (virTestRun("New cgroup for self", testCgroupNewForSelf, NULL) < 0) ret = -1; @@ -907,26 +934,23 @@ mymain(void) if (virTestRun("virCgroupGetPercpuStats works", testCgroupGetPercpuStats, NULL) < 0) ret = -1; - unsetenv("VIR_CGROUP_MOCK_FILENAME"); + cleanupFakeFS(fakerootdir); - setenv("VIR_CGROUP_MOCK_FILENAME", "all-in-one", 1); + fakerootdir = initFakeFS(NULL, "all-in-one"); if (virTestRun("New cgroup for self (allinone)", testCgroupNewForSelfAllInOne, NULL) < 0) ret = -1; if (virTestRun("Cgroup available", testCgroupAvailable, (void*)0x1) < 0) ret = -1; - unsetenv("VIR_CGROUP_MOCK_FILENAME"); + cleanupFakeFS(fakerootdir); - setenv("VIR_CGROUP_MOCK_FILENAME", "logind", 1); + fakerootdir = initFakeFS(NULL, "logind"); if (virTestRun("New cgroup for self (logind)", testCgroupNewForSelfLogind, NULL) < 0) ret = -1; if (virTestRun("Cgroup available", testCgroupAvailable, (void*)0x0) < 0) ret = -1; - unsetenv("VIR_CGROUP_MOCK_FILENAME"); + cleanupFakeFS(fakerootdir); - if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) - virFileDeleteTree(fakerootdir); - VIR_FREE(fakerootdir); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgroupdata/all-in-one.parsed | 1 + tests/vircgroupdata/cgroups1.parsed | 1 + tests/vircgroupdata/cgroups2.parsed | 1 + tests/vircgroupdata/cgroups3.parsed | 1 + tests/vircgroupdata/fedora-18.parsed | 1 + tests/vircgroupdata/fedora-21.parsed | 1 + tests/vircgroupdata/kubevirt.parsed | 1 + tests/vircgroupdata/ovirt-node-6.6.parsed | 1 + tests/vircgroupdata/ovirt-node-7.1.parsed | 1 + tests/vircgroupdata/rhel-7.1.parsed | 1 + tests/vircgrouptest.c | 2 ++ 11 files changed, 12 insertions(+) diff --git a/tests/vircgroupdata/all-in-one.parsed b/tests/vircgroupdata/all-in-one.parsed index d703d08fb9..97c96e8ecd 100644 --- a/tests/vircgroupdata/all-in-one.parsed +++ b/tests/vircgroupdata/all-in-one.parsed @@ -8,3 +8,4 @@ blkio /not/really/sys/fs/cgroup net_cls <null> perf_event <null> name=systemd <null> +unified <null> diff --git a/tests/vircgroupdata/cgroups1.parsed b/tests/vircgroupdata/cgroups1.parsed index b6916f17a1..16431aa006 100644 --- a/tests/vircgroupdata/cgroups1.parsed +++ b/tests/vircgroupdata/cgroups1.parsed @@ -8,3 +8,4 @@ blkio /sys/fs/cgroup/blkio net_cls /sys/fs/cgroup/net_cls perf_event <null> name=systemd <null> +unified <null> diff --git a/tests/vircgroupdata/cgroups2.parsed b/tests/vircgroupdata/cgroups2.parsed index 5eb2bc7bb2..d1bb0dcb7c 100644 --- a/tests/vircgroupdata/cgroups2.parsed +++ b/tests/vircgroupdata/cgroups2.parsed @@ -8,3 +8,4 @@ blkio /sys/fs/cgroup/blkio net_cls <null> perf_event /sys/fs/cgroup/perf_event name=systemd <null> +unified <null> diff --git a/tests/vircgroupdata/cgroups3.parsed b/tests/vircgroupdata/cgroups3.parsed index 2b1f3825c1..44e475c9d2 100644 --- a/tests/vircgroupdata/cgroups3.parsed +++ b/tests/vircgroupdata/cgroups3.parsed @@ -8,3 +8,4 @@ blkio /sys/fs/cgroup/blkio net_cls /sys/fs/cgroup/net_cls perf_event /sys/fs/cgroup/perf_event name=systemd <null> +unified <null> diff --git a/tests/vircgroupdata/fedora-18.parsed b/tests/vircgroupdata/fedora-18.parsed index 8d5ba75c7e..662a38a9e8 100644 --- a/tests/vircgroupdata/fedora-18.parsed +++ b/tests/vircgroupdata/fedora-18.parsed @@ -8,3 +8,4 @@ blkio /sys/fs/cgroup/blkio net_cls /sys/fs/cgroup/net_cls perf_event /sys/fs/cgroup/perf_event name=systemd /sys/fs/cgroup/systemd +unified <null> diff --git a/tests/vircgroupdata/fedora-21.parsed b/tests/vircgroupdata/fedora-21.parsed index 3377af0382..4e447fd7bd 100644 --- a/tests/vircgroupdata/fedora-21.parsed +++ b/tests/vircgroupdata/fedora-21.parsed @@ -8,3 +8,4 @@ blkio /sys/fs/cgroup/blkio net_cls /sys/fs/cgroup/net_cls,net_prio perf_event /sys/fs/cgroup/perf_event name=systemd /sys/fs/cgroup/systemd +unified <null> diff --git a/tests/vircgroupdata/kubevirt.parsed b/tests/vircgroupdata/kubevirt.parsed index 6948707238..bf977f8363 100644 --- a/tests/vircgroupdata/kubevirt.parsed +++ b/tests/vircgroupdata/kubevirt.parsed @@ -8,3 +8,4 @@ blkio /sys/fs/cgroup/blkio net_cls <null> perf_event /sys/fs/cgroup/perf_event name=systemd <null> +unified <null> diff --git a/tests/vircgroupdata/ovirt-node-6.6.parsed b/tests/vircgroupdata/ovirt-node-6.6.parsed index 01bf466be6..9d10813d8c 100644 --- a/tests/vircgroupdata/ovirt-node-6.6.parsed +++ b/tests/vircgroupdata/ovirt-node-6.6.parsed @@ -8,3 +8,4 @@ blkio /cgroup/blkio net_cls /cgroup/net_cls perf_event <null> name=systemd <null> +unified <null> diff --git a/tests/vircgroupdata/ovirt-node-7.1.parsed b/tests/vircgroupdata/ovirt-node-7.1.parsed index 8d5ba75c7e..662a38a9e8 100644 --- a/tests/vircgroupdata/ovirt-node-7.1.parsed +++ b/tests/vircgroupdata/ovirt-node-7.1.parsed @@ -8,3 +8,4 @@ blkio /sys/fs/cgroup/blkio net_cls /sys/fs/cgroup/net_cls perf_event /sys/fs/cgroup/perf_event name=systemd /sys/fs/cgroup/systemd +unified <null> diff --git a/tests/vircgroupdata/rhel-7.1.parsed b/tests/vircgroupdata/rhel-7.1.parsed index 8d5ba75c7e..662a38a9e8 100644 --- a/tests/vircgroupdata/rhel-7.1.parsed +++ b/tests/vircgroupdata/rhel-7.1.parsed @@ -8,3 +8,4 @@ blkio /sys/fs/cgroup/blkio net_cls /sys/fs/cgroup/net_cls perf_event /sys/fs/cgroup/perf_event name=systemd /sys/fs/cgroup/systemd +unified <null> diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 3e8793a6c3..75187216d9 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -175,6 +175,8 @@ testCgroupDetectMounts(const void *args) virCgroupControllerTypeToString(i), NULLSTR(group->legacy[i].mountPoint)); } + virBufferAsprintf(&buf, "%-12s %s\n", + "unified", NULLSTR(group->unified.mountPoint)); if (virBufferCheckError(&buf) < 0) goto cleanup; -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgroupdata/unified.cgroups | 13 +++++++++++++ tests/vircgroupdata/unified.mounts | 20 ++++++++++++++++++++ tests/vircgroupdata/unified.parsed | 11 +++++++++++ tests/vircgroupdata/unified.self.cgroup | 1 + tests/vircgrouptest.c | 3 +++ 5 files changed, 48 insertions(+) create mode 100644 tests/vircgroupdata/unified.cgroups create mode 100644 tests/vircgroupdata/unified.mounts create mode 100644 tests/vircgroupdata/unified.parsed create mode 100644 tests/vircgroupdata/unified.self.cgroup diff --git a/tests/vircgroupdata/unified.cgroups b/tests/vircgroupdata/unified.cgroups new file mode 100644 index 0000000000..e0d8a3561c --- /dev/null +++ b/tests/vircgroupdata/unified.cgroups @@ -0,0 +1,13 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 0 1 1 +cpu 0 1 1 +cpuacct 0 1 1 +blkio 0 1 1 +memory 0 1 1 +devices 0 1 1 +freezer 0 1 1 +net_cls 0 1 1 +perf_event 0 1 1 +net_prio 0 1 1 +hugetlb 0 1 1 +pids 0 1 1 diff --git a/tests/vircgroupdata/unified.mounts b/tests/vircgroupdata/unified.mounts new file mode 100644 index 0000000000..b4ab94a2c3 --- /dev/null +++ b/tests/vircgroupdata/unified.mounts @@ -0,0 +1,20 @@ +sysfs /sys sysfs rw,seclabel,nosuid,nodev,noexec,relatime 0 0 +proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0 +devtmpfs /dev devtmpfs rw,seclabel,nosuid,size=1009844k,nr_inodes=252461,mode=755 0 0 +securityfs /sys/kernel/security securityfs rw,nosuid,nodev,noexec,relatime 0 0 +tmpfs /dev/shm tmpfs rw,seclabel,nosuid,nodev 0 0 +devpts /dev/pts devpts rw,seclabel,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0 +tmpfs /run tmpfs rw,seclabel,nosuid,nodev,mode=755 0 0 +cgroup2 /not/really/sys/fs/cgroup cgroup2 rw,seclabel,nosuid,nodev,noexec,relatime,nsdelegate 0 0 +pstore /sys/fs/pstore pstore rw,seclabel,nosuid,nodev,noexec,relatime 0 0 +bpf /sys/fs/bpf bpf rw,nosuid,nodev,noexec,relatime,mode=700 0 0 +configfs /sys/kernel/config configfs rw,relatime 0 0 +/dev/vda2 / ext4 rw,seclabel,relatime 0 0 +selinuxfs /sys/fs/selinux selinuxfs rw,relatime 0 0 +debugfs /sys/kernel/debug debugfs rw,seclabel,relatime 0 0 +hugetlbfs /dev/hugepages hugetlbfs rw,seclabel,relatime,pagesize=2M 0 0 +systemd-1 /proc/sys/fs/binfmt_misc autofs rw,relatime,fd=40,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=14586 0 0 +mqueue /dev/mqueue mqueue rw,seclabel,relatime 0 0 +tmpfs /tmp tmpfs rw,seclabel,nosuid,nodev 0 0 +sunrpc /var/lib/nfs/rpc_pipefs rpc_pipefs rw,relatime 0 0 +tmpfs /run/user/0 tmpfs rw,seclabel,nosuid,nodev,relatime,size=204000k,mode=700 0 0 diff --git a/tests/vircgroupdata/unified.parsed b/tests/vircgroupdata/unified.parsed new file mode 100644 index 0000000000..3de0fc643d --- /dev/null +++ b/tests/vircgroupdata/unified.parsed @@ -0,0 +1,11 @@ +cpu <null> +cpuacct <null> +cpuset <null> +memory <null> +devices <null> +freezer <null> +blkio <null> +net_cls <null> +perf_event <null> +name=systemd <null> +unified /not/really/sys/fs/cgroup diff --git a/tests/vircgroupdata/unified.self.cgroup b/tests/vircgroupdata/unified.self.cgroup new file mode 100644 index 0000000000..1e027b2a3c --- /dev/null +++ b/tests/vircgroupdata/unified.self.cgroup @@ -0,0 +1 @@ +0::/ diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 75187216d9..800522e311 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -899,6 +899,9 @@ mymain(void) DETECT_MOUNTS("all-in-one"); DETECT_MOUNTS_FAIL("no-cgroups"); DETECT_MOUNTS("kubevirt"); + fakerootdir = initFakeFS("unified", NULL); + DETECT_MOUNTS("unified"); + cleanupFakeFS(fakerootdir); fakerootdir = initFakeFS(NULL, "systemd"); if (virTestRun("New cgroup for self", testCgroupNewForSelf, NULL) < 0) -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgroupdata/hybrid.cgroups | 12 ++++++++++++ tests/vircgroupdata/hybrid.mounts | 23 +++++++++++++++++++++++ tests/vircgroupdata/hybrid.parsed | 11 +++++++++++ tests/vircgroupdata/hybrid.self.cgroup | 9 +++++++++ tests/vircgrouptest.c | 3 +++ 5 files changed, 58 insertions(+) create mode 100644 tests/vircgroupdata/hybrid.cgroups create mode 100644 tests/vircgroupdata/hybrid.mounts create mode 100644 tests/vircgroupdata/hybrid.parsed create mode 100644 tests/vircgroupdata/hybrid.self.cgroup diff --git a/tests/vircgroupdata/hybrid.cgroups b/tests/vircgroupdata/hybrid.cgroups new file mode 100644 index 0000000000..7f3bc7b8cb --- /dev/null +++ b/tests/vircgroupdata/hybrid.cgroups @@ -0,0 +1,12 @@ +#subsys_name hierarchy num_cgroups enabled +cpuset 2 1 1 +cpu 0 1 1 +cpuacct 0 1 1 +blkio 0 1 1 +memory 0 1 1 +devices 3 1 1 +freezer 5 1 1 +net_cls 4 1 1 +perf_event 6 1 1 +hugetlb 7 1 1 +pids 8 1 1 diff --git a/tests/vircgroupdata/hybrid.mounts b/tests/vircgroupdata/hybrid.mounts new file mode 100644 index 0000000000..d6f5f82115 --- /dev/null +++ b/tests/vircgroupdata/hybrid.mounts @@ -0,0 +1,23 @@ +proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0 +sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0 +devtmpfs /dev devtmpfs rw,nosuid,size=10240k,nr_inodes=502705,mode=755 0 0 +devpts /dev/pts devpts rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0 +tmpfs /dev/shm tmpfs rw,nosuid,nodev,noexec 0 0 +tmpfs /run tmpfs rw,nosuid,nodev,noexec,mode=755 0 0 +mqueue /dev/mqueue mqueue rw,nosuid,nodev,noexec,relatime 0 0 +securityfs /sys/kernel/security securityfs rw,nosuid,nodev,noexec,relatime 0 0 +debugfs /sys/kernel/debug debugfs rw,nosuid,nodev,noexec,relatime 0 0 +configfs /sys/kernel/config configfs rw,nosuid,nodev,noexec,relatime 0 0 +fusectl /sys/fs/fuse/connections fusectl rw,nosuid,nodev,noexec,relatime 0 0 +selinuxfs /sys/fs/selinux selinuxfs rw,relatime 0 0 +pstore /sys/fs/pstore pstore rw,nosuid,nodev,noexec,relatime 0 0 +cgroup_root /not/really/sys/fs/cgroup tmpfs rw,nosuid,nodev,noexec,relatime,size=10240k,mode=755 0 0 +openrc /not/really/sys/fs/cgroup/openrc cgroup rw,nosuid,nodev,noexec,relatime,release_agent=/lib64/rc/sh/cgroup-release-agent.sh,name=openrc 0 0 +none /not/really/sys/fs/cgroup/unified cgroup2 rw,nosuid,nodev,noexec,relatime,nsdelegate 0 0 +cpuset /not/really/sys/fs/cgroup/cpuset cgroup rw,nosuid,nodev,noexec,relatime,cpuset 0 0 +devices /not/really/sys/fs/cgroup/devices cgroup rw,nosuid,nodev,noexec,relatime,devices 0 0 +net_cls /not/really/sys/fs/cgroup/net_cls cgroup rw,nosuid,nodev,noexec,relatime,net_cls 0 0 +freezer /not/really/sys/fs/cgroup/freezer cgroup rw,nosuid,nodev,noexec,relatime,freezer 0 0 +perf_event /not/really/sys/fs/cgroup/perf_event cgroup rw,nosuid,nodev,noexec,relatime,perf_event 0 0 +hugetlb /not/really/sys/fs/cgroup/hugetlb cgroup rw,nosuid,nodev,noexec,relatime,hugetlb 0 0 +pids /not/really/sys/fs/cgroup/pids cgroup rw,nosuid,nodev,noexec,relatime,pids 0 0 diff --git a/tests/vircgroupdata/hybrid.parsed b/tests/vircgroupdata/hybrid.parsed new file mode 100644 index 0000000000..7600de5f45 --- /dev/null +++ b/tests/vircgroupdata/hybrid.parsed @@ -0,0 +1,11 @@ +cpu <null> +cpuacct <null> +cpuset /not/really/sys/fs/cgroup/cpuset +memory <null> +devices /not/really/sys/fs/cgroup/devices +freezer /not/really/sys/fs/cgroup/freezer +blkio <null> +net_cls /not/really/sys/fs/cgroup/net_cls +perf_event /not/really/sys/fs/cgroup/perf_event +name=systemd <null> +unified /not/really/sys/fs/cgroup/unified diff --git a/tests/vircgroupdata/hybrid.self.cgroup b/tests/vircgroupdata/hybrid.self.cgroup new file mode 100644 index 0000000000..2a08905c91 --- /dev/null +++ b/tests/vircgroupdata/hybrid.self.cgroup @@ -0,0 +1,9 @@ +8:pids:/ +7:hugetlb:/ +6:perf_event:/ +5:freezer:/ +4:net_cls:/ +3:devices:/ +2:cpuset:/ +1:name=openrc:/ +0::/ diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 800522e311..e766ad7cbe 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -902,6 +902,9 @@ mymain(void) fakerootdir = initFakeFS("unified", NULL); DETECT_MOUNTS("unified"); cleanupFakeFS(fakerootdir); + fakerootdir = initFakeFS("hybrid", NULL); + DETECT_MOUNTS("hybrid"); + cleanupFakeFS(fakerootdir); fakerootdir = initFakeFS(NULL, "systemd"); if (virTestRun("New cgroup for self", testCgroupNewForSelf, NULL) < 0) -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgrouptest.c | 53 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index e766ad7cbe..ead5046600 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -43,7 +43,10 @@ static int validateCgroup(virCgroupPtr cgroup, const char *expectPath, const char **expectMountPoint, const char **expectLinkPoint, - const char **expectPlacement) + const char **expectPlacement, + const char *expectUnifiedMountPoint, + const char *expectUnifiedPlacement, + unsigned int expectUnifiedControllers) { size_t i; @@ -80,6 +83,38 @@ static int validateCgroup(virCgroupPtr cgroup, } } + if (STRNEQ_NULLABLE(expectUnifiedMountPoint, + cgroup->unified.mountPoint)) { + fprintf(stderr, "Wrong mount '%s', expected '%s' for 'unified'\n", + cgroup->unified.mountPoint, + expectUnifiedMountPoint); + return -1; + } + if (STRNEQ_NULLABLE(expectUnifiedPlacement, + cgroup->unified.placement)) { + fprintf(stderr, "Wrong placement '%s', expected '%s' for 'unified'\n", + cgroup->unified.placement, + expectUnifiedPlacement); + return -1; + } + if (expectUnifiedControllers != cgroup->unified.controllers) { + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + int type = 1 << i; + if ((expectUnifiedControllers & type) != (cgroup->unified.controllers & type)) { + const char *typeStr = virCgroupControllerTypeToString(i); + if (expectUnifiedControllers & type) { + fprintf(stderr, "expected controller '%s' for 'unified', " + "but it's missing\n", typeStr); + } else { + fprintf(stderr, "existing controller '%s' for 'unified', " + "but it's not expected\n", typeStr); + } + } + + } + return -1; + } + return 0; } @@ -215,7 +250,7 @@ static int testCgroupNewForSelf(const void *args ATTRIBUTE_UNUSED) goto cleanup; } - ret = validateCgroup(cgroup, "", mountsFull, links, placement); + ret = validateCgroup(cgroup, "", mountsFull, links, placement, NULL, NULL, 0); cleanup: virCgroupFree(&cgroup); @@ -294,14 +329,14 @@ static int testCgroupNewForPartition(const void *args ATTRIBUTE_UNUSED) fprintf(stderr, "Cannot create /virtualmachines cgroup: %d\n", -rv); goto cleanup; } - ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsSmall, links, placementSmall); + ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsSmall, links, placementSmall, NULL, NULL, 0); virCgroupFree(&cgroup); if ((rv = virCgroupNewPartition("/virtualmachines", true, -1, &cgroup)) != 0) { fprintf(stderr, "Cannot create /virtualmachines cgroup: %d\n", -rv); goto cleanup; } - ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsFull, links, placementFull); + ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsFull, links, placementFull, NULL, NULL, 0); cleanup: virCgroupFree(&cgroup); @@ -351,7 +386,7 @@ static int testCgroupNewForPartitionNested(const void *args ATTRIBUTE_UNUSED) } ret = validateCgroup(cgroup, "/deployment.partition/production.partition", - mountsFull, links, placementFull); + mountsFull, links, placementFull, NULL, NULL, 0); cleanup: virCgroupFree(&cgroup); @@ -407,7 +442,7 @@ static int testCgroupNewForPartitionNestedDeep(const void *args ATTRIBUTE_UNUSED } ret = validateCgroup(cgroup, "/user/berrange.user/production.partition", - mountsFull, links, placementFull); + mountsFull, links, placementFull, NULL, NULL, 0); cleanup: virCgroupFree(&cgroup); @@ -443,7 +478,7 @@ static int testCgroupNewForPartitionDomain(const void *args ATTRIBUTE_UNUSED) goto cleanup; } - ret = validateCgroup(domaincgroup, "/production.partition/foo.libvirt-lxc", mountsFull, links, placement); + ret = validateCgroup(domaincgroup, "/production.partition/foo.libvirt-lxc", mountsFull, links, placement, NULL, NULL, 0); cleanup: virCgroupFree(&partitioncgroup); @@ -494,7 +529,7 @@ static int testCgroupNewForPartitionDomainEscaped(const void *args ATTRIBUTE_UNU * since our fake /proc/cgroups pretends this controller * isn't compiled into the kernel */ - ret = validateCgroup(domaincgroup, "/_cgroup.evil/net_cls.evil/__evil.evil/_cpu.foo.libvirt-lxc", mountsFull, links, placement); + ret = validateCgroup(domaincgroup, "/_cgroup.evil/net_cls.evil/__evil.evil/_cpu.foo.libvirt-lxc", mountsFull, links, placement, NULL, NULL, 0); cleanup: virCgroupFree(&partitioncgroup3); @@ -523,7 +558,7 @@ static int testCgroupNewForSelfAllInOne(const void *args ATTRIBUTE_UNUSED) goto cleanup; } - ret = validateCgroup(cgroup, "", mountsAllInOne, linksAllInOne, placement); + ret = validateCgroup(cgroup, "", mountsAllInOne, linksAllInOne, placement, NULL, NULL, 0); cleanup: virCgroupFree(&cgroup); -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgrouptest.c | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index ead5046600..fe0cb9585a 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -567,16 +567,37 @@ static int testCgroupNewForSelfAllInOne(const void *args ATTRIBUTE_UNUSED) static int testCgroupNewForSelfLogind(const void *args ATTRIBUTE_UNUSED) +{ + virCgroupPtr cgroup = NULL; + + if (virCgroupNewSelf(&cgroup) >= 0) { + fprintf(stderr, "Expected to fail, only systemd cgroup available.\n"); + virCgroupFree(&cgroup); + return -1; + } + + return 0; +} + + +static int testCgroupNewForSelfUnified(const void *args ATTRIBUTE_UNUSED) { virCgroupPtr cgroup = NULL; int ret = -1; + const char *empty[VIR_CGROUP_CONTROLLER_LAST] = { 0 }; + unsigned int controllers = + (1 << VIR_CGROUP_CONTROLLER_CPU) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | + (1 << VIR_CGROUP_CONTROLLER_MEMORY) | + (1 << VIR_CGROUP_CONTROLLER_BLKIO); - if (virCgroupNewSelf(&cgroup) == 0) { - fprintf(stderr, "Expected cgroup creation to fail.\n"); + if (virCgroupNewSelf(&cgroup) < 0) { + fprintf(stderr, "Cannot create cgroup for self\n"); goto cleanup; } - ret = 0; + ret = validateCgroup(cgroup, "", empty, empty, empty, + "/not/really/sys/fs/cgroup", "/", controllers); cleanup: virCgroupFree(&cgroup); return ret; @@ -993,7 +1014,14 @@ mymain(void) ret = -1; cleanupFakeFS(fakerootdir); + /* cgroup unified */ + fakerootdir = initFakeFS("unified", "unified"); + if (virTestRun("New cgroup for self (unified)", testCgroupNewForSelfUnified, NULL) < 0) + ret = -1; + if (virTestRun("Cgroup available (unified)", testCgroupAvailable, (void*)0x1) < 0) + ret = -1; + cleanupFakeFS(fakerootdir); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.17.1

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgrouptest.c | 48 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index fe0cb9585a..8fcee21bb2 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -604,6 +604,45 @@ static int testCgroupNewForSelfUnified(const void *args ATTRIBUTE_UNUSED) } +static int testCgroupNewForSelfHybrid(const void *args ATTRIBUTE_UNUSED) +{ + virCgroupPtr cgroup = NULL; + int ret = -1; + const char *empty[VIR_CGROUP_CONTROLLER_LAST] = { 0 }; + const char *mounts[VIR_CGROUP_CONTROLLER_LAST] = { + [VIR_CGROUP_CONTROLLER_CPUSET] = "/not/really/sys/fs/cgroup/cpuset", + [VIR_CGROUP_CONTROLLER_DEVICES] = "/not/really/sys/fs/cgroup/devices", + [VIR_CGROUP_CONTROLLER_FREEZER] = "/not/really/sys/fs/cgroup/freezer", + [VIR_CGROUP_CONTROLLER_NET_CLS] = "/not/really/sys/fs/cgroup/net_cls", + [VIR_CGROUP_CONTROLLER_PERF_EVENT] = "/not/really/sys/fs/cgroup/perf_event", + }; + const char *placement[VIR_CGROUP_CONTROLLER_LAST] = { + [VIR_CGROUP_CONTROLLER_CPUSET] = "/", + [VIR_CGROUP_CONTROLLER_DEVICES] = "/", + [VIR_CGROUP_CONTROLLER_FREEZER] = "/", + [VIR_CGROUP_CONTROLLER_NET_CLS] = "/", + [VIR_CGROUP_CONTROLLER_PERF_EVENT] = "/", + }; + unsigned int controllers = + (1 << VIR_CGROUP_CONTROLLER_CPU) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | + (1 << VIR_CGROUP_CONTROLLER_MEMORY) | + (1 << VIR_CGROUP_CONTROLLER_BLKIO); + + if (virCgroupNewSelf(&cgroup) < 0) { + fprintf(stderr, "Cannot create cgroup for self\n"); + goto cleanup; + } + + ret = validateCgroup(cgroup, "", mounts, empty, placement, + "/not/really/sys/fs/cgroup/unified", "/", controllers); + + cleanup: + virCgroupFree(&cgroup); + return ret; +} + + static int testCgroupAvailable(const void *args) { bool got = virCgroupAvailable(); @@ -1023,6 +1062,15 @@ mymain(void) ret = -1; cleanupFakeFS(fakerootdir); + /* cgroup hybrid */ + + fakerootdir = initFakeFS("hybrid", "hybrid"); + if (virTestRun("New cgroup for self (hybrid)", testCgroupNewForSelfHybrid, NULL) < 0) + ret = -1; + if (virTestRun("Cgroup available (hybrid)", testCgroupAvailable, (void*)0x1) < 0) + ret = -1; + cleanupFakeFS(fakerootdir); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.17.1

This removes code duplication and simplifies cgroup detection. As a drawback we will not have separate messages to enable cgroup controller in kernel or to mount it. On the other side the rewrite adds support for cgroup v2. The kernel config support was wrong because it was parsing '/proc/self/cgroup' instead of '/proc/cgroups/' file. The mount suggestion is removed as well because it will not work with cgroup v2. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tools/virt-host-validate-common.c | 162 ++++++------------------------ tools/virt-host-validate-common.h | 7 +- tools/virt-host-validate-lxc.c | 40 ++------ tools/virt-host-validate-qemu.c | 40 ++------ 4 files changed, 54 insertions(+), 195 deletions(-) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index ccbd764c84..4e70fe9e9c 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -24,12 +24,10 @@ #include <stdarg.h> #include <unistd.h> #include <sys/utsname.h> -#ifdef HAVE_MNTENT_H -# include <mntent.h> -#endif /* HAVE_MNTENT_H */ #include <sys/stat.h> #include "viralloc.h" +#include "vircgroup.h" #include "virfile.h" #include "virt-host-validate-common.h" #include "virstring.h" @@ -288,152 +286,50 @@ int virHostValidateLinuxKernel(const char *hvname, } } - -static int virHostValidateCGroupSupport(const char *hvname, - const char *cg_name, - virHostValidateLevel level, - const char *config_name) -{ - virHostMsgCheck(hvname, "for cgroup '%s' controller support", cg_name); - FILE *fp = fopen("/proc/self/cgroup", "r"); - size_t len = 0; - char *line = NULL; - ssize_t ret; - bool matched = false; - - if (!fp) - goto error; - - while ((ret = getline(&line, &len, fp)) >= 0 && !matched) { - char **cgroups; - char *start; - char *end; - size_t ncgroups; - size_t i; - - /* Each line in this file looks like - * - * 4:cpu,cpuacct:/machine.slice/machine-qemu\x2dtest.scope/emulator - * - * Since multiple cgroups can be part of the same line and some cgroup - * names can appear as part of other cgroup names (eg. 'cpu' is a - * prefix for both 'cpuacct' and 'cpuset'), it's not enough to simply - * check whether the cgroup name is present somewhere inside the file. - * - * Moreover, there's nothing stopping the cgroup name from appearing - * in an unrelated mount point name as well */ - - /* Look for the first colon. - * The part we're interested in starts right after it */ - if (!(start = strchr(line, ':'))) - continue; - start++; - - /* Look for the second colon. - * The part we're interested in ends exactly there */ - if (!(end = strchr(start, ':'))) - continue; - *end = '\0'; - - if (!(cgroups = virStringSplitCount(start, ",", 0, &ncgroups))) - continue; - - /* Look for the matching cgroup */ - for (i = 0; i < ncgroups; i++) { - if (STREQ(cgroups[i], cg_name)) - matched = true; - } - - virStringListFreeCount(cgroups, ncgroups); - } - - VIR_FREE(line); - VIR_FORCE_FCLOSE(fp); - if (!matched) - goto error; - - virHostMsgPass(); - return 0; - - error: - VIR_FREE(line); - virHostMsgFail(level, "Enable CONFIG_%s in kernel Kconfig file", config_name); - return -1; -} - -#ifdef HAVE_MNTENT_H -static int virHostValidateCGroupMount(const char *hvname, - const char *cg_name, - virHostValidateLevel level) +#ifdef __linux__ +int virHostValidateCGroupControllers(const char *hvname, + int controllers, + virHostValidateLevel level) { - virHostMsgCheck(hvname, "for cgroup '%s' controller mount-point", cg_name); - FILE *fp = setmntent("/proc/mounts", "r"); - struct mntent ent; - char mntbuf[1024]; - bool matched = false; + virCgroupPtr group = NULL; + int ret = 0; + size_t i; - if (!fp) - goto error; + if (virCgroupNewSelf(&group) < 0) + return -1; - while (getmntent_r(fp, &ent, mntbuf, sizeof(mntbuf)) && !matched) { - char **opts; - size_t nopts; - size_t i; + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + int flag = 1 << i; + const char *cg_name = virCgroupControllerTypeToString(i); - /* Ignore non-cgroup mounts */ - if (STRNEQ(ent.mnt_type, "cgroup")) + if (!(controllers & flag)) continue; - if (!(opts = virStringSplitCount(ent.mnt_opts, ",", 0, &nopts))) - continue; + virHostMsgCheck(hvname, "for cgroup '%s' controller support", cg_name); - /* Look for a mount option matching the cgroup name */ - for (i = 0; i < nopts; i++) { - if (STREQ(opts[i], cg_name)) - matched = true; + if (!virCgroupHasController(group, i)) { + ret = -1; + virHostMsgFail(level, "Enable '%s' in kernel Kconfig file or " + "mount/enable cgroup controller in your system", + cg_name); + } else { + virHostMsgPass(); } - - virStringListFreeCount(opts, nopts); } - endmntent(fp); - if (!matched) - goto error; - virHostMsgPass(); - return 0; + virCgroupFree(&group); - error: - virHostMsgFail(level, "Mount '%s' cgroup controller (suggested at /sys/fs/cgroup/%s)", - cg_name, cg_name); - return -1; + return ret; } -#else /* ! HAVE_MNTENT_H */ -static int virHostValidateCGroupMount(const char *hvname, - const char *cg_name, - virHostValidateLevel level) +#else /* !__linux__ */ +int virHostValidateCGroupControllers(const char *hvname, + int controllers, + virHostValidateLevel level) { - virHostMsgCheck(hvname, "for cgroup '%s' controller mount-point", cg_name); virHostMsgFail(level, "%s", "This platform does not support cgroups"); return -1; } -#endif /* ! HAVE_MNTENT_H */ - -int virHostValidateCGroupController(const char *hvname, - const char *cg_name, - virHostValidateLevel level, - const char *config_name) -{ - if (virHostValidateCGroupSupport(hvname, - cg_name, - level, - config_name) < 0) - return -1; - if (virHostValidateCGroupMount(hvname, - cg_name, - level) < 0) - return -1; - return 0; -} +#endif /* !__linux__ */ int virHostValidateIOMMU(const char *hvname, virHostValidateLevel level) diff --git a/tools/virt-host-validate-common.h b/tools/virt-host-validate-common.h index b6fe17daa7..b23dd7cdbe 100644 --- a/tools/virt-host-validate-common.h +++ b/tools/virt-host-validate-common.h @@ -77,10 +77,9 @@ int virHostValidateNamespace(const char *hvname, virHostValidateLevel level, const char *hint); -int virHostValidateCGroupController(const char *hvname, - const char *cg_name, - virHostValidateLevel level, - const char *config_name); +int virHostValidateCGroupControllers(const char *hvname, + int controllers, + virHostValidateLevel level); int virHostValidateIOMMU(const char *hvname, virHostValidateLevel level); diff --git a/tools/virt-host-validate-lxc.c b/tools/virt-host-validate-lxc.c index 64d9279c30..3c55b1b26d 100644 --- a/tools/virt-host-validate-lxc.c +++ b/tools/virt-host-validate-lxc.c @@ -23,6 +23,7 @@ #include "virt-host-validate-lxc.h" #include "virt-host-validate-common.h" +#include "vircgroup.h" int virHostValidateLXC(void) { @@ -63,35 +64,16 @@ int virHostValidateLXC(void) _("User namespace support is recommended")) < 0) ret = -1; - if (virHostValidateCGroupController("LXC", "memory", - VIR_HOST_VALIDATE_FAIL, - "MEMCG") < 0) - ret = -1; - - if (virHostValidateCGroupController("LXC", "cpu", - VIR_HOST_VALIDATE_FAIL, - "CGROUP_CPU") < 0) - ret = -1; - - if (virHostValidateCGroupController("LXC", "cpuacct", - VIR_HOST_VALIDATE_FAIL, - "CGROUP_CPUACCT") < 0) - ret = -1; - - if (virHostValidateCGroupController("LXC", "cpuset", - VIR_HOST_VALIDATE_FAIL, - "CPUSETS") < 0) - ret = -1; - - if (virHostValidateCGroupController("LXC", "devices", - VIR_HOST_VALIDATE_FAIL, - "CGROUP_DEVICE") < 0) - ret = -1; - - if (virHostValidateCGroupController("LXC", "blkio", - VIR_HOST_VALIDATE_FAIL, - "BLK_CGROUP") < 0) - ret = -1; + if (virHostValidateCGroupControllers("LXC", + (1 << VIR_CGROUP_CONTROLLER_MEMORY) | + (1 << VIR_CGROUP_CONTROLLER_CPU) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | + (1 << VIR_CGROUP_CONTROLLER_CPUSET) | + (1 << VIR_CGROUP_CONTROLLER_DEVICES) | + (1 << VIR_CGROUP_CONTROLLER_BLKIO), + VIR_HOST_VALIDATE_FAIL) < 0) { + ret = -1; + } #if WITH_FUSE if (virHostValidateDeviceExists("LXC", "/sys/fs/fuse/connections", diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c index d7573ea8b3..ff3c1f0231 100644 --- a/tools/virt-host-validate-qemu.c +++ b/tools/virt-host-validate-qemu.c @@ -26,6 +26,7 @@ #include "virt-host-validate-common.h" #include "virarch.h" #include "virbitmap.h" +#include "vircgroup.h" int virHostValidateQEMU(void) { @@ -96,35 +97,16 @@ int virHostValidateQEMU(void) _("Load the 'tun' module to enable networking for QEMU guests")) < 0) ret = -1; - if (virHostValidateCGroupController("QEMU", "memory", - VIR_HOST_VALIDATE_WARN, - "MEMCG") < 0) - ret = -1; - - if (virHostValidateCGroupController("QEMU", "cpu", - VIR_HOST_VALIDATE_WARN, - "CGROUP_CPU") < 0) - ret = -1; - - if (virHostValidateCGroupController("QEMU", "cpuacct", - VIR_HOST_VALIDATE_WARN, - "CGROUP_CPUACCT") < 0) - ret = -1; - - if (virHostValidateCGroupController("QEMU", "cpuset", - VIR_HOST_VALIDATE_WARN, - "CPUSETS") < 0) - ret = -1; - - if (virHostValidateCGroupController("QEMU", "devices", - VIR_HOST_VALIDATE_WARN, - "CGROUP_DEVICES") < 0) - ret = -1; - - if (virHostValidateCGroupController("QEMU", "blkio", - VIR_HOST_VALIDATE_WARN, - "BLK_CGROUP") < 0) - ret = -1; + if (virHostValidateCGroupControllers("QEMU", + (1 << VIR_CGROUP_CONTROLLER_MEMORY) | + (1 << VIR_CGROUP_CONTROLLER_CPU) | + (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | + (1 << VIR_CGROUP_CONTROLLER_CPUSET) | + (1 << VIR_CGROUP_CONTROLLER_DEVICES) | + (1 << VIR_CGROUP_CONTROLLER_BLKIO), + VIR_HOST_VALIDATE_WARN) < 0) { + ret = -1; + } if (virHostValidateIOMMU("QEMU", VIR_HOST_VALIDATE_WARN) < 0) -- 2.17.1

Cgroup freezer support for LXC was added in libvirt-0.7.2. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tools/virt-host-validate-lxc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virt-host-validate-lxc.c b/tools/virt-host-validate-lxc.c index 3c55b1b26d..8613f37cc7 100644 --- a/tools/virt-host-validate-lxc.c +++ b/tools/virt-host-validate-lxc.c @@ -70,6 +70,7 @@ int virHostValidateLXC(void) (1 << VIR_CGROUP_CONTROLLER_CPUACCT) | (1 << VIR_CGROUP_CONTROLLER_CPUSET) | (1 << VIR_CGROUP_CONTROLLER_DEVICES) | + (1 << VIR_CGROUP_CONTROLLER_FREEZER) | (1 << VIR_CGROUP_CONTROLLER_BLKIO), VIR_HOST_VALIDATE_FAIL) < 0) { ret = -1; -- 2.17.1