Re: [libvirt] [PATCH v3 07/16] Refactor cgroups internal data structures

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Currently the virCgroupPtr struct contains 3 pieces of information - path - path of the cgroup, relative to current process' cgroup placement - placement - current process' placement in each controller - mounts - mount point of each controller When reading/writing cgroup settings, the path & placement strings are combined to form the file path. This approach only works if we assume all cgroups will be relative to the current process' cgroup placement. To allow support for managing cgroups at any place in the heirarchy a change is needed. The 'placement' data should reflect the absolute path to the cgroup, and the 'path' value should no longer be used to form the paths to the cgroup attribute files. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/util/vircgroup.c | 222 +++++++++++++++++++++++++++++++++++--------------- tests/vircgrouptest.c | 53 +++++++----- 2 files changed, 188 insertions(+), 87 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 2f52c92..c336806 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -101,6 +101,23 @@ bool virCgroupHasController(virCgroupPtr cgroup, int controller) } #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R +static int virCgroupCopyMounts(virCgroupPtr group, + virCgroupPtr parent) +{ + int i; + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { + if (!parent->controllers[i].mountPoint) + continue; + + group->controllers[i].mountPoint = + strdup(parent->controllers[i].mountPoint); + + if (!group->controllers[i].mountPoint) + return -ENOMEM; + } + return 0; +} + /* * Process /proc/mounts figuring out what controllers are * mounted and where @@ -158,12 +175,61 @@ no_memory: } +static int virCgroupCopyPlacement(virCgroupPtr group, + const char *path, + virCgroupPtr parent) +{ + int i; + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { + if (!group->controllers[i].mountPoint) + continue; + + if (path[0] == '/') { + if (!(group->controllers[i].placement = strdup(path))) + return -ENOMEM; + } else { + /* + * parent=="/" + path="" => "/" + * parent=="/libvirt.service" + path="" => "/libvirt.service" + * parent=="/libvirt.service" + path="foo" => "/libvirt.service/foo" + */

+ if (virAsprintf(&group->controllers[i].placement, + "%s%s%s", + parent->controllers[i].placement, + (STREQ(parent->controllers[i].placement, "/") || + STREQ(path, "") ? "" : "/"), + path) < 0)

+ return -ENOMEM; + } + } + + return 0; +} + + /*

+ * @group: the group to process + * @path: the relative path to append, not starting with '/' + * * Process /proc/self/cgroup figuring out what cgroup * sub-path the current process is assigned to. ie not - * necessarily in the root + * necessarily in the root. The contents of this file + * looks like + * + * 9:perf_event:/ + * 8:blkio:/ + * 7:net_cls:/ + * 6:freezer:/ + * 5:devices:/ + * 4:memory:/ + * 3:cpuacct,cpu:/ + * 2:cpuset:/ + * 1:name=systemd:/user/berrange/2 + * + * It then appends @path to each detected path. */ -static int virCgroupDetectPlacement(virCgroupPtr group) +static int virCgroupDetectPlacement(virCgroupPtr group, + const char *path) { int i; FILE *mapping = NULL; @@ -177,18 +243,18 @@ static int virCgroupDetectPlacement(virCgroupPtr group) while (fgets(line, sizeof(line), mapping) != NULL) { char *controllers = strchr(line, ':'); - char *path = controllers ? strchr(controllers+1, ':') : NULL; - char *nl = path ? strchr(path, '\n') : NULL; + char *selfpath = controllers ? strchr(controllers + 1, ':') : NULL; + char *nl = selfpath ? strchr(selfpath, '\n') : NULL; - if (!controllers || !path) + if (!controllers || !selfpath) continue; if (nl) *nl = '\0'; - *path = '\0'; + *selfpath = '\0'; controllers++; - path++; + selfpath++; for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) { const char *typestr = virCgroupControllerTypeToString(i); @@ -198,14 +264,25 @@ static int virCgroupDetectPlacement(virCgroupPtr group) char *next = strchr(tmp, ','); int len; if (next) { - len = next-tmp; + len = next - tmp; next++; } else { len = strlen(tmp); } - if (typelen == len && STREQLEN(typestr, tmp, len) && - !(group->controllers[i].placement = strdup(STREQ(path, "/") ? "" : path))) - goto no_memory; + + /* + * selfpath=="/" + path="" -> "/" + * selfpath=="/libvirt.service" + path="" -> "/libvirt.service" + * selfpath=="/libvirt.service" + path="foo" -> "/libvirt.service/foo" + */ + if (typelen == len && STREQLEN(typestr, tmp, len)) { + if (virAsprintf(&group->controllers[i].placement, + "%s%s%s", selfpath, + (STREQ(selfpath, "/") || + STREQ(path, "") ? "" : "/"),

+ path) < 0) + goto no_memory; + } tmp = next; }

diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index a68aa88..3f35f2e 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c

@@ -246,4 +255,4 @@ mymain(void) return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/libvircgroupmock.so") +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/vircgroupmock.so")