[libvirt] [PATCH 00/17] some cgroup cleanup patches
Preparation for cgroupv2 support. Pavel Hrdina (17): docs: List cpuacct in controllers used by QEMU driver docs: Update how we create cgroup directory names vircgroup: Rename structs to start with underscore vircgroup: Introduce standard set of typedefs and use them vircgroup: Extract file link resolving into separate function vircgroup: Remove unused function virCgroupKill() vircgroup: Unexport unused function virCgroupAddTaskController() vircgroup: Unexport unused function virCgroupRemoveRecursively vircgroup: Move function used in tests into vircgrouppriv.h vircgroup: Remove pointless bool parameter vircgroup: Extract mount options matching into function vircgroup: Use virCgroupMountOptsMatchController in virCgroupDetectPlacement vircgroup: Introduce virCgroupEnableMissingControllers vircgroup: machinename will never be NULL vircgroup: Remove virCgroupAddTaskController vircgroup: Introduce virCgroupGetMemoryStat lxc: Use virCgroupGetMemoryStat docs/cgroups.html.in | 47 ++-- src/libvirt_private.syms | 4 +- src/lxc/lxc_cgroup.c | 65 +---- src/util/vircgroup.c | 521 ++++++++++++++++++++------------------- src/util/vircgroup.h | 31 +-- src/util/vircgrouppriv.h | 21 +- 6 files changed, 330 insertions(+), 359 deletions(-) -- 2.17.1
The cpuacct controller is used to get cpu stats. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/cgroups.html.in | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/cgroups.html.in b/docs/cgroups.html.in index e33d07a82b..a498590c45 100644 --- a/docs/cgroups.html.in +++ b/docs/cgroups.html.in @@ -23,12 +23,13 @@ <p> The QEMU driver is capable of using the <code>cpuset</code>, - <code>cpu</code>, <code>memory</code>, <code>blkio</code> and - <code>devices</code> controllers. None of them are compulsory. - If any controller is not mounted, the resource management APIs - which use it will cease to operate. It is possible to explicitly - turn off use of a controller, even when mounted, via the - <code>/etc/libvirt/qemu.conf</code> configuration file. + <code>cpu</code>, <code>cpuacct</code>, <code>memory</code>, + <code>blkio</code> and <code>devices</code> controllers. + None of them are compulsory. If any controller is not mounted, + the resource management APIs which use it will cease to operate. + It is possible to explicitly turn off use of a controller, + even when mounted, via the <code>/etc/libvirt/qemu.conf</code> + configuration file. </p> <p> -- 2.17.1
Commit <c3bd0019c0> changed the way how cgroup directory names are constructed but the documentation was not updated. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/cgroups.html.in | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/docs/cgroups.html.in b/docs/cgroups.html.in index a498590c45..081ba2eae1 100644 --- a/docs/cgroups.html.in +++ b/docs/cgroups.html.in @@ -76,11 +76,13 @@ <p> The systemd convention is for the scope name of virtual machines / containers to be of the general format <code>machine-$NAME.scope</code>. Libvirt forms the - <code>$NAME</code> part of this by concatenating the driver type with the name - of the guest, and then escaping any systemd reserved characters. + <code>$NAME</code> part of this by concatenating the driver type with the id + and truncated name of the guest, and then escaping any systemd reserved + characters. So for a guest <code>demo</code> running under the <code>lxc</code> driver, - we get a <code>$NAME</code> of <code>lxc-demo</code> which when escaped is - <code>lxc\x2ddemo</code>. So the complete scope name is <code>machine-lxc\x2ddemo.scope</code>. + we get a <code>$NAME</code> of <code>lxc-12345-demo</code> which when escaped + is <code>lxc\x2d12345\x2ddemo</code>. So the complete scope name is + <code>machine-lxc\x2d12345\x2ddemo.scope</code>. The scope names map directly to the cgroup directory names. </p> @@ -113,19 +115,19 @@ $ROOT | +- machine.slice | - +- machine-qemu\x2dvm1.scope + +- machine-qemu\x2d1\x2dvm1.scope | | | +- emulator | +- vcpu0 | +- vcpu1 | - +- machine-qemu\x2dvm2.scope + +- machine-qemu\x2d2\x2dvm2.scope | | | +- emulator | +- vcpu0 | +- vcpu1 | - +- machine-qemu\x2dvm3.scope + +- machine-qemu\x2d3\x2dvm3.scope | | | +- emulator | +- vcpu0 @@ -135,15 +137,15 @@ $ROOT | | | +- machine-engineering-testing.slice | | | - | | +- machine-lxc\x2dcontainer1.scope + | | +- machine-lxc\x2d11111\x2dcontainer1.scope | | | +- machine-engineering-production.slice | | - | +- machine-lxc\x2dcontainer2.scope + | +- machine-lxc\x2d22222\x2dcontainer2.scope | +- machine-marketing.slice | - +- machine-lxc\x2dcontainer3.scope + +- machine-lxc\x2d33333\x2dcontainer3.scope </pre> <h3><a id="currentLayoutGeneric">Non-systemd cgroups layout</a></h3> @@ -174,19 +176,19 @@ $ROOT | +- machine | - +- vm1.libvirt-qemu + +- qemu-1-vm1.libvirt-qemu | | | +- emulator | +- vcpu0 | +- vcpu1 | - +- vm2.libvirt-qemu + +- qeme-2-vm2.libvirt-qemu | | | +- emulator | +- vcpu0 | +- vcpu1 | - +- vm3.libvirt-qemu + +- qemu-3-vm3.libvirt-qemu | | | +- emulator | +- vcpu0 @@ -196,15 +198,15 @@ $ROOT | | | +- testing.partition | | | - | | +- container1.libvirt-lxc + | | +- lxc-11111-container1.libvirt-lxc | | | +- production.partition | | - | +- container2.libvirt-lxc + | +- lxc-22222-container2.libvirt-lxc | +- marketing.partition | - +- container3.libvirt-lxc + +- lxc-33333-container3.libvirt-lxc </pre> <h2><a id="customPartiton">Using custom partitions</a></h2> -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 2 +- src/util/vircgroup.h | 4 ++-- src/util/vircgrouppriv.h | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 6bf4e88da1..7602641713 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -385,7 +385,7 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, const char *typestr = virCgroupControllerTypeToString(i); int typelen = strlen(typestr); char *tmp = entry.mnt_opts; - struct virCgroupController *controller = &group->controllers[i]; + struct _virCgroupController *controller = &group->controllers[i]; while (tmp) { char *next = strchr(tmp, ','); int len; diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index d833927678..cfa69b67cb 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -28,8 +28,8 @@ # include "virutil.h" # include "virbitmap.h" -struct virCgroup; -typedef struct virCgroup *virCgroupPtr; +struct _virCgroup; +typedef struct _virCgroup *virCgroupPtr; enum { VIR_CGROUP_CONTROLLER_CPU, diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index 722863e5b6..71788639d6 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -31,7 +31,7 @@ # include "vircgroup.h" -struct virCgroupController { +struct _virCgroupController { int type; char *mountPoint; /* If mountPoint holds several controllers co-mounted, @@ -42,10 +42,10 @@ struct virCgroupController { char *placement; }; -struct virCgroup { +struct _virCgroup { char *path; - struct virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; + struct _virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; }; int virCgroupDetectMountsFromFile(virCgroupPtr group, -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 2 +- src/util/vircgroup.h | 3 ++- src/util/vircgrouppriv.h | 4 +++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 7602641713..5144b33d43 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -385,7 +385,7 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, const char *typestr = virCgroupControllerTypeToString(i); int typelen = strlen(typestr); char *tmp = entry.mnt_opts; - struct _virCgroupController *controller = &group->controllers[i]; + virCgroupControllerPtr controller = &group->controllers[i]; while (tmp) { char *next = strchr(tmp, ','); int len; diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index cfa69b67cb..af93316197 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -29,7 +29,8 @@ # include "virbitmap.h" struct _virCgroup; -typedef struct _virCgroup *virCgroupPtr; +typedef struct _virCgroup virCgroup; +typedef virCgroup *virCgroupPtr; enum { VIR_CGROUP_CONTROLLER_CPU, diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index 71788639d6..1b47d3b858 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -41,11 +41,13 @@ struct _virCgroupController { char *linkPoint; char *placement; }; +typedef struct _virCgroupController virCgroupController; +typedef virCgroupController *virCgroupControllerPtr; struct _virCgroup { char *path; - struct _virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; + virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST]; }; int virCgroupDetectMountsFromFile(virCgroupPtr group, -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 85 +++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 37 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 5144b33d43..fd58ba154f 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -356,6 +356,51 @@ virCgroupCopyMounts(virCgroupPtr group, } +static int +virCgroupResolveMountLink(const char *mntDir, + const char *typeStr, + virCgroupControllerPtr controller) +{ + VIR_AUTOFREE(char *) linkSrc = NULL; + char *dirName; + struct stat sb; + + dirName = strrchr(mntDir, '/'); + if (!dirName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing '/' separator in cgroup mount '%s'"), mntDir); + return -1; + } + + if (!strchr(dirName + 1, ',')) + return 0; + + *dirName = '\0'; + if (virAsprintf(&linkSrc, "%s/%s", mntDir, typeStr) < 0) + return -1; + *dirName = '/'; + + if (lstat(linkSrc, &sb) < 0) { + if (errno == ENOENT) { + VIR_WARN("Controller %s co-mounted at %s is missing symlink at %s", + typeStr, mntDir, linkSrc); + } else { + virReportSystemError(errno, _("Cannot stat %s"), linkSrc); + return -1; + } + } else { + if (!S_ISLNK(sb.st_mode)) { + VIR_WARN("Expecting a symlink at %s for controller %s", + linkSrc, typeStr); + } else { + VIR_STEAL_PTR(controller->linkPoint, linkSrc); + } + } + + return 0; +} + + /* * Process /proc/mounts figuring out what controllers are * mounted and where @@ -397,8 +442,6 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, } if (typelen == len && STREQLEN(typestr, tmp, len)) { - struct stat sb; - char *tmp2; /* Note that the lines in /proc/mounts have the same * order than the mount operations, and that there may @@ -412,44 +455,12 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, if (VIR_STRDUP(controller->mountPoint, entry.mnt_dir) < 0) goto cleanup; - tmp2 = strrchr(entry.mnt_dir, '/'); - if (!tmp2) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing '/' separator in cgroup mount '%s'"), - entry.mnt_dir); - goto cleanup; - } - /* If it is a co-mount it has a filename like "cpu,cpuacct" * and we must identify the symlink path */ - if (checkLinks && strchr(tmp2 + 1, ',')) { - VIR_AUTOFREE(char *) linksrc = NULL; - - *tmp2 = '\0'; - if (virAsprintf(&linksrc, "%s/%s", - entry.mnt_dir, typestr) < 0) + if (checkLinks && + virCgroupResolveMountLink(entry.mnt_dir, typestr, + controller) < 0) { goto cleanup; - *tmp2 = '/'; - - if (lstat(linksrc, &sb) < 0) { - if (errno == ENOENT) { - VIR_WARN("Controller %s co-mounted at %s is missing symlink at %s", - typestr, entry.mnt_dir, linksrc); - } else { - virReportSystemError(errno, - _("Cannot stat %s"), - linksrc); - goto cleanup; - } - } else { - if (!S_ISLNK(sb.st_mode)) { - VIR_WARN("Expecting a symlink at %s for controller %s", - linksrc, typestr); - } else { - controller->linkPoint = linksrc; - linksrc = NULL; - } - } } } tmp = next; -- 2.17.1
On 08/09/2018 03:44 PM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 85 +++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 37 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 5144b33d43..fd58ba154f 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -356,6 +356,51 @@ virCgroupCopyMounts(virCgroupPtr group, }
+static int +virCgroupResolveMountLink(const char *mntDir,
@mntDir shouldn't be const char. You're changing it in the function, even though not directly rather than via @dirName.
+ const char *typeStr, + virCgroupControllerPtr controller) +{ + VIR_AUTOFREE(char *) linkSrc = NULL; + char *dirName; + struct stat sb; + + dirName = strrchr(mntDir, '/'); + if (!dirName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing '/' separator in cgroup mount '%s'"), mntDir); + return -1; + } + + if (!strchr(dirName + 1, ',')) + return 0; + + *dirName = '\0'; + if (virAsprintf(&linkSrc, "%s/%s", mntDir, typeStr) < 0) + return -1; + *dirName = '/';
Pre-existing and probably doesn't matter, but if above virAsprintf() fails, @dirName is not written back and thus @mntDir is left mangled.
+ + if (lstat(linkSrc, &sb) < 0) { + if (errno == ENOENT) { + VIR_WARN("Controller %s co-mounted at %s is missing symlink at %s", + typeStr, mntDir, linkSrc); + } else { + virReportSystemError(errno, _("Cannot stat %s"), linkSrc); + return -1; + } + } else { + if (!S_ISLNK(sb.st_mode)) { + VIR_WARN("Expecting a symlink at %s for controller %s", + linkSrc, typeStr); + } else { + VIR_STEAL_PTR(controller->linkPoint, linkSrc); + } + } + + return 0; +}
Michal
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 - src/util/vircgroup.c | 37 ------------------------------------- src/util/vircgroup.h | 1 - 3 files changed, 39 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 32ed5a09f9..8f80ee2250 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1539,7 +1539,6 @@ virCgroupGetMemSwapUsage; virCgroupGetPercpuStats; virCgroupHasController; virCgroupHasEmptyTasks; -virCgroupKill; virCgroupKillPainfully; virCgroupKillRecursive; virCgroupNewDetect; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index fd58ba154f..d9f6c5b06f 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3510,33 +3510,6 @@ virCgroupPidCopy(const void *name) } -/* - * Returns 1 if some PIDs are killed, 0 if none are killed, or -1 on error - */ -int -virCgroupKill(virCgroupPtr group, int signum) -{ - VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum); - int ret; - /* The 'tasks' file in cgroups can contain duplicated - * pids, so we use a hash to track which we've already - * killed. - */ - virHashTablePtr pids = virHashCreateFull(100, - NULL, - virCgroupPidCode, - virCgroupPidEqual, - virCgroupPidCopy, - NULL); - - ret = virCgroupKillInternal(group, signum, pids); - - virHashFree(pids); - - return ret; -} - - static int virCgroupKillRecursiveInternal(virCgroupPtr group, int signum, @@ -4585,16 +4558,6 @@ virCgroupRemove(virCgroupPtr group ATTRIBUTE_UNUSED) } -int -virCgroupKill(virCgroupPtr group ATTRIBUTE_UNUSED, - int signum ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENOSYS, "%s", - _("Control groups not supported on this platform")); - return -1; -} - - int virCgroupKillRecursive(virCgroupPtr group ATTRIBUTE_UNUSED, int signum ATTRIBUTE_UNUSED) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index af93316197..a23a491d95 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -280,7 +280,6 @@ int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus); int virCgroupRemoveRecursively(char *grppath); int virCgroupRemove(virCgroupPtr group); -int virCgroupKill(virCgroupPtr group, int signum); int virCgroupKillRecursive(virCgroupPtr group, int signum); int virCgroupKillPainfully(virCgroupPtr group); -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 - src/util/vircgroup.c | 69 +++++++++++++++++----------------------- src/util/vircgroup.h | 4 --- 3 files changed, 29 insertions(+), 45 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8f80ee2250..95d7c9f834 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1496,7 +1496,6 @@ virBufferVasprintf; # util/vircgroup.h virCgroupAddMachineTask; virCgroupAddTask; -virCgroupAddTaskController; virCgroupAllowAllDevices; virCgroupAllowDevice; virCgroupAllowDevicePath; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index d9f6c5b06f..6d937626e6 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1136,6 +1136,35 @@ virCgroupNew(pid_t pid, } +/** + * virCgroupAddTaskController: + * + * @group: The cgroup to add a task to + * @pid: The pid of the task to add + * @controller: The cgroup controller to be operated on + * + * Returns: 0 on success or -1 on error + */ +static int +virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller) +{ + if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Controller %d out of range"), controller); + return -1; + } + + if (!group->controllers[controller].mountPoint) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Controller '%s' not mounted"), + virCgroupControllerTypeToString(controller)); + return -1; + } + + return virCgroupSetValueI64(group, controller, "tasks", pid); +} + + static int virCgroupAddTaskInternal(virCgroupPtr group, pid_t pid, bool withSystemd) { @@ -1197,35 +1226,6 @@ virCgroupAddMachineTask(virCgroupPtr group, pid_t pid) } -/** - * virCgroupAddTaskController: - * - * @group: The cgroup to add a task to - * @pid: The pid of the task to add - * @controller: The cgroup controller to be operated on - * - * Returns: 0 on success or -1 on error - */ -int -virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller) -{ - if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Controller %d out of range"), controller); - return -1; - } - - if (!group->controllers[controller].mountPoint) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Controller '%s' not mounted"), - virCgroupControllerTypeToString(controller)); - return -1; - } - - return virCgroupSetValueI64(group, controller, "tasks", pid); -} - - static int virCgroupSetPartitionSuffix(const char *path, char **res) { @@ -4115,17 +4115,6 @@ virCgroupAddMachineTask(virCgroupPtr group ATTRIBUTE_UNUSED, } -int -virCgroupAddTaskController(virCgroupPtr group ATTRIBUTE_UNUSED, - pid_t pid ATTRIBUTE_UNUSED, - int controller ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENXIO, "%s", - _("Control groups not supported on this platform")); - return -1; -} - - int virCgroupGetBlkioIoServiced(virCgroupPtr group ATTRIBUTE_UNUSED, long long *bytes_read ATTRIBUTE_UNUSED, diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index a23a491d95..74c7dbcccc 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -134,10 +134,6 @@ int virCgroupPathOfController(virCgroupPtr group, int virCgroupAddTask(virCgroupPtr group, pid_t pid); int virCgroupAddMachineTask(virCgroupPtr group, pid_t pid); -int virCgroupAddTaskController(virCgroupPtr group, - pid_t pid, - int controller); - int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight); int virCgroupGetBlkioWeight(virCgroupPtr group, unsigned int *weight); -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 - src/util/vircgroup.c | 11 +---------- src/util/vircgroup.h | 1 - 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 95d7c9f834..59d9bc380e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1550,7 +1550,6 @@ virCgroupNewSelf; virCgroupNewThread; virCgroupPathOfController; virCgroupRemove; -virCgroupRemoveRecursively; virCgroupSetBlkioDeviceReadBps; virCgroupSetBlkioDeviceReadIops; virCgroupSetBlkioDeviceWeight; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 6d937626e6..280d781f41 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3312,7 +3312,7 @@ virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage) } -int +static int virCgroupRemoveRecursively(char *grppath) { DIR *grpdir; @@ -4529,15 +4529,6 @@ virCgroupSetCpuCfsQuota(virCgroupPtr group ATTRIBUTE_UNUSED, } -int -virCgroupRemoveRecursively(char *grppath ATTRIBUTE_UNUSED) -{ - virReportSystemError(ENXIO, "%s", - _("Control groups not supported on this platform")); - return -1; -} - - int virCgroupRemove(virCgroupPtr group ATTRIBUTE_UNUSED) { diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 74c7dbcccc..138bb3c076 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -273,7 +273,6 @@ int virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate); int virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus); int virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus); -int virCgroupRemoveRecursively(char *grppath); int virCgroupRemove(virCgroupPtr group); int virCgroupKillRecursive(virCgroupPtr group, int signum); -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.h | 13 ------------- src/util/vircgrouppriv.h | 13 +++++++++++++ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 138bb3c076..48be077aba 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -64,22 +64,9 @@ typedef enum { bool virCgroupAvailable(void); -int virCgroupNewPartition(const char *path, - bool create, - int controllers, - virCgroupPtr *group) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); - int virCgroupNewSelf(virCgroupPtr *group) ATTRIBUTE_NONNULL(1); -int virCgroupNewDomainPartition(virCgroupPtr partition, - const char *driver, - const char *name, - bool create, - virCgroupPtr *group) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5); - int virCgroupNewThread(virCgroupPtr domain, virCgroupThreadName nameval, int id, diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h index 1b47d3b858..a0034f3889 100644 --- a/src/util/vircgrouppriv.h +++ b/src/util/vircgrouppriv.h @@ -54,4 +54,17 @@ int virCgroupDetectMountsFromFile(virCgroupPtr group, const char *path, bool checkLinks); +int virCgroupNewPartition(const char *path, + bool create, + int controllers, + virCgroupPtr *group) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); + +int virCgroupNewDomainPartition(virCgroupPtr partition, + const char *driver, + const char *name, + bool create, + virCgroupPtr *group) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5); + #endif /* __VIR_CGROUP_PRIV_H__ */ -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 280d781f41..6ffa1e9c58 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -251,7 +251,6 @@ static bool virCgroupValidateMachineGroup(virCgroupPtr group, const char *name, const char *drivername, - bool stripEmulatorSuffix, const char *machinename) { size_t i; @@ -303,10 +302,9 @@ virCgroupValidateMachineGroup(virCgroupPtr group, if (!tmp) return false; - if (stripEmulatorSuffix && - (i == VIR_CGROUP_CONTROLLER_CPU || - i == VIR_CGROUP_CONTROLLER_CPUACCT || - i == VIR_CGROUP_CONTROLLER_CPUSET)) { + if (i == VIR_CGROUP_CONTROLLER_CPU || + i == VIR_CGROUP_CONTROLLER_CPUACCT || + i == VIR_CGROUP_CONTROLLER_CPUSET) { if (STREQ(tmp, "/emulator")) *tmp = '\0'; tmp = strrchr(group->controllers[i].placement, '/'); @@ -1486,8 +1484,7 @@ virCgroupNewDetectMachine(const char *name, return -1; } - if (!virCgroupValidateMachineGroup(*group, name, drivername, - true, machinename)) { + if (!virCgroupValidateMachineGroup(*group, name, drivername, machinename)) { VIR_DEBUG("Failed to validate machine name for '%s' driver '%s'", name, drivername); virCgroupFree(group); -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 80 ++++++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 33 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 6ffa1e9c58..52be5e5fb8 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -399,6 +399,33 @@ virCgroupResolveMountLink(const char *mntDir, } +static bool +virCgroupMountOptsMatchController(const char *mntOpts, + const char *typeStr) +{ + const char *tmp = mntOpts; + int typeLen = strlen(typeStr); + + while (tmp) { + const char *next = strchr(tmp, ','); + int len; + if (next) { + len = next - tmp; + next++; + } else { + len = strlen(tmp); + } + + if (typeLen == len && STREQLEN(typeStr, tmp, len)) + return true; + + tmp = next; + } + + return false; +} + + /* * Process /proc/mounts figuring out what controllers are * mounted and where @@ -426,42 +453,29 @@ virCgroupDetectMountsFromFile(virCgroupPtr group, for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { const char *typestr = virCgroupControllerTypeToString(i); - int typelen = strlen(typestr); - char *tmp = entry.mnt_opts; - virCgroupControllerPtr controller = &group->controllers[i]; - while (tmp) { - char *next = strchr(tmp, ','); - int len; - if (next) { - len = next-tmp; - next++; - } else { - len = strlen(tmp); - } - if (typelen == len && STREQLEN(typestr, tmp, len)) { - - /* Note that the lines in /proc/mounts have the same - * order than the mount operations, and that there may - * be duplicates due to bind mounts. This means - * that the same mount point may be processed more than - * once. We need to save the results of the last one, - * and we need to be careful to release the memory used - * by previous processing. */ - VIR_FREE(controller->mountPoint); - VIR_FREE(controller->linkPoint); - if (VIR_STRDUP(controller->mountPoint, entry.mnt_dir) < 0) - goto cleanup; + if (virCgroupMountOptsMatchController(entry.mnt_opts, typestr)) { + /* Note that the lines in /proc/mounts have the same + * order than the mount operations, and that there may + * be duplicates due to bind mounts. This means + * that the same mount point may be processed more than + * once. We need to save the results of the last one, + * and we need to be careful to release the memory used + * by previous processing. */ + virCgroupControllerPtr controller = &group->controllers[i]; + + VIR_FREE(controller->mountPoint); + VIR_FREE(controller->linkPoint); + if (VIR_STRDUP(controller->mountPoint, entry.mnt_dir) < 0) + goto cleanup; - /* If it is a co-mount it has a filename like "cpu,cpuacct" - * and we must identify the symlink path */ - if (checkLinks && - virCgroupResolveMountLink(entry.mnt_dir, typestr, - controller) < 0) { - goto cleanup; - } + /* If it is a co-mount it has a filename like "cpu,cpuacct" + * and we must identify the symlink path */ + if (checkLinks && + virCgroupResolveMountLink(entry.mnt_dir, typestr, + controller) < 0) { + goto cleanup; } - tmp = next; } } } -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 43 ++++++++++++++----------------------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 52be5e5fb8..45fe2595d1 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -598,42 +598,27 @@ virCgroupDetectPlacement(virCgroupPtr group, for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { const char *typestr = virCgroupControllerTypeToString(i); - int typelen = strlen(typestr); - char *tmp = controllers; - - while (tmp) { - char *next = strchr(tmp, ','); - int len; - if (next) { - len = next - tmp; - next++; - } else { - len = strlen(tmp); - } + if (virCgroupMountOptsMatchController(controllers, typestr) && + group->controllers[i].mountPoint != NULL && + group->controllers[i].placement == NULL) { /* * selfpath == "/" + path="" -> "/" * selfpath == "/libvirt.service" + path == "" -> "/libvirt.service" * selfpath == "/libvirt.service" + path == "foo" -> "/libvirt.service/foo" */ - if (typelen == len && STREQLEN(typestr, tmp, len) && - group->controllers[i].mountPoint != NULL && - group->controllers[i].placement == NULL) { - if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) { - if (VIR_STRDUP(group->controllers[i].placement, - selfpath) < 0) - goto cleanup; - } else { - if (virAsprintf(&group->controllers[i].placement, - "%s%s%s", selfpath, - (STREQ(selfpath, "/") || - STREQ(path, "") ? "" : "/"), - path) < 0) - goto cleanup; - } + if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) { + if (VIR_STRDUP(group->controllers[i].placement, + selfpath) < 0) + goto cleanup; + } else { + if (virAsprintf(&group->controllers[i].placement, + "%s%s%s", selfpath, + (STREQ(selfpath, "/") || + STREQ(path, "") ? "" : "/"), + path) < 0) + goto cleanup; } - - tmp = next; } } } -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 103 ++++++++++++++++++++++++------------------- 1 file changed, 58 insertions(+), 45 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 45fe2595d1..99cbdaa59b 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1494,6 +1494,58 @@ virCgroupNewDetectMachine(const char *name, } +static int +virCgroupEnableMissingControllers(char *path, + pid_t pidleader, + int controllers, + virCgroupPtr *group) +{ + virCgroupPtr parent = NULL; + char *offset = path; + int ret = -1; + + if (virCgroupNew(pidleader, + "", + NULL, + controllers, + &parent) < 0) + return ret; + + for (;;) { + virCgroupPtr tmp; + char *t = strchr(offset + 1, '/'); + if (t) + *t = '\0'; + + if (virCgroupNew(pidleader, + path, + parent, + controllers, + &tmp) < 0) + goto cleanup; + + if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_NONE) < 0) { + virCgroupFree(&tmp); + goto cleanup; + } + if (t) { + *t = '/'; + offset = t; + virCgroupFree(&parent); + parent = tmp; + } else { + *group = tmp; + break; + } + } + + ret = 0; + cleanup: + virCgroupFree(&parent); + return ret; +} + + /* * Returns 0 on success, -1 on fatal error, -2 on systemd not available */ @@ -1510,11 +1562,9 @@ virCgroupNewMachineSystemd(const char *name, int controllers, virCgroupPtr *group) { - int ret = -1; int rv; - virCgroupPtr init, parent = NULL; + virCgroupPtr init; VIR_AUTOFREE(char *) path = NULL; - char *offset; VIR_DEBUG("Trying to setup machine '%s' via systemd", name); if ((rv = virSystemdCreateMachine(name, @@ -1543,46 +1593,12 @@ virCgroupNewMachineSystemd(const char *name, if (!path || STREQ(path, "/") || path[0] != '/') { VIR_DEBUG("Systemd didn't setup its controller"); - ret = -2; - goto cleanup; + return -2; } - offset = path; - - if (virCgroupNew(pidleader, - "", - NULL, - controllers, - &parent) < 0) - goto cleanup; - - - for (;;) { - virCgroupPtr tmp; - char *t = strchr(offset + 1, '/'); - if (t) - *t = '\0'; - - if (virCgroupNew(pidleader, - path, - parent, - controllers, - &tmp) < 0) - goto cleanup; - - if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_NONE) < 0) { - virCgroupFree(&tmp); - goto cleanup; - } - if (t) { - *t = '/'; - offset = t; - virCgroupFree(&parent); - parent = tmp; - } else { - *group = tmp; - break; - } + if (virCgroupEnableMissingControllers(path, pidleader, + controllers, group) < 0) { + return -1; } if (virCgroupAddTask(*group, pidleader) < 0) { @@ -1595,10 +1611,7 @@ virCgroupNewMachineSystemd(const char *name, } } - ret = 0; - cleanup: - virCgroupFree(&parent); - return ret; + return 0; } -- 2.17.1
Commit <eaf2c9f89107b9f60cf8db2c919f78b987ff7177> moved machineName generation before virCgroupNewDetectMachine() is called. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 99cbdaa59b..84f5c4197f 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -266,27 +266,22 @@ virCgroupValidateMachineGroup(virCgroupPtr group, if (virCgroupPartitionEscape(&partname) < 0) return false; - if (machinename && - (virAsprintf(&partmachinename, "%s.libvirt-%s", - machinename, drivername) < 0 || - virCgroupPartitionEscape(&partmachinename) < 0)) + if (virAsprintf(&partmachinename, "%s.libvirt-%s", + machinename, drivername) < 0 || + virCgroupPartitionEscape(&partmachinename) < 0) return false; if (!(scopename_old = virSystemdMakeScopeName(name, drivername, true))) return false; - /* We should keep trying even if this failed */ - if (!machinename) - virResetLastError(); - else if (!(scopename_new = virSystemdMakeScopeName(machinename, - drivername, false))) + if (!(scopename_new = virSystemdMakeScopeName(machinename, + drivername, false))) return false; if (virCgroupPartitionEscape(&scopename_old) < 0) return false; - if (scopename_new && - virCgroupPartitionEscape(&scopename_new) < 0) + if (virCgroupPartitionEscape(&scopename_new) < 0) return false; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { @@ -315,16 +310,16 @@ virCgroupValidateMachineGroup(virCgroupPtr group, tmp++; if (STRNEQ(tmp, name) && - STRNEQ_NULLABLE(tmp, machinename) && + STRNEQ(tmp, machinename) && STRNEQ(tmp, partname) && - STRNEQ_NULLABLE(tmp, partmachinename) && + STRNEQ(tmp, partmachinename) && STRNEQ(tmp, scopename_old) && - STRNEQ_NULLABLE(tmp, scopename_new)) { + STRNEQ(tmp, scopename_new)) { VIR_DEBUG("Name '%s' for controller '%s' does not match " "'%s', '%s', '%s', '%s' or '%s'", tmp, virCgroupControllerTypeToString(i), - name, NULLSTR(machinename), partname, - scopename_old, NULLSTR(scopename_new)); + name, machinename, partname, + scopename_old, scopename_new); return false; } } -- 2.17.1
There is no need for this function, both of the checks are done later by virCgroupGetControllerPath. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/vircgroup.c | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 84f5c4197f..37982a9607 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1128,35 +1128,6 @@ virCgroupNew(pid_t pid, } -/** - * virCgroupAddTaskController: - * - * @group: The cgroup to add a task to - * @pid: The pid of the task to add - * @controller: The cgroup controller to be operated on - * - * Returns: 0 on success or -1 on error - */ -static int -virCgroupAddTaskController(virCgroupPtr group, pid_t pid, int controller) -{ - if (controller < 0 || controller >= VIR_CGROUP_CONTROLLER_LAST) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Controller %d out of range"), controller); - return -1; - } - - if (!group->controllers[controller].mountPoint) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Controller '%s' not mounted"), - virCgroupControllerTypeToString(controller)); - return -1; - } - - return virCgroupSetValueI64(group, controller, "tasks", pid); -} - - static int virCgroupAddTaskInternal(virCgroupPtr group, pid_t pid, bool withSystemd) { @@ -1174,7 +1145,7 @@ virCgroupAddTaskInternal(virCgroupPtr group, pid_t pid, bool withSystemd) if (i == VIR_CGROUP_CONTROLLER_SYSTEMD && !withSystemd) continue; - if (virCgroupAddTaskController(group, pid, i) < 0) + if (virCgroupSetValueI64(group, i, "tasks", pid) < 0) goto cleanup; } -- 2.17.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 + src/util/vircgroup.c | 88 ++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 7 ++++ 3 files changed, 96 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 59d9bc380e..ee0dca6129 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1532,6 +1532,7 @@ virCgroupGetDomainTotalCpuStats; virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; +virCgroupGetMemoryStat; virCgroupGetMemoryUsage; virCgroupGetMemSwapHardLimit; virCgroupGetMemSwapUsage; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 37982a9607..b91acd13c7 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2427,6 +2427,94 @@ virCgroupSetMemory(virCgroupPtr group, unsigned long long kb) } +/** + * virCgroupGetMemoryStat: + * + * @group: The cgroup to change memory for + * @cache: page cache memory in KiB + * @activeAnon: anonymous and swap cache memory in KiB + * @inactiveAnon: anonymous and swap cache memory in KiB + * @activeFile: file-backed memory in KiB + * @inactiveFile: file-backed memory in KiB + * @unevictable: memory that cannot be reclaimed KiB + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupGetMemoryStat(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) +{ + int ret = -1; + 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.")); + goto cleanup; + } + *valueStr = '\0'; + + if (virStrToLong_ull(valueStr + 1, NULL, 10, &value) < 0) + goto cleanup; + + if (STREQ(line, "cache")) + 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; + + ret = 0; + + cleanup: + VIR_FREE(stat); + return ret; +} + + /** * virCgroupGetMemoryUsage: * diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 48be077aba..c7fdaaede4 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -177,6 +177,13 @@ int virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group, unsigned long long *wbps); int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb); +int virCgroupGetMemoryStat(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); int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb); int virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb); -- 2.17.1
On 08/09/2018 03:44 PM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 + src/util/vircgroup.c | 88 ++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 7 ++++ 3 files changed, 96 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 59d9bc380e..ee0dca6129 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1532,6 +1532,7 @@ virCgroupGetDomainTotalCpuStats; virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; +virCgroupGetMemoryStat; virCgroupGetMemoryUsage; virCgroupGetMemSwapHardLimit; virCgroupGetMemSwapUsage; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 37982a9607..b91acd13c7 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2427,6 +2427,94 @@ virCgroupSetMemory(virCgroupPtr group, unsigned long long kb) }
+/** + * virCgroupGetMemoryStat: + * + * @group: The cgroup to change memory for + * @cache: page cache memory in KiB + * @activeAnon: anonymous and swap cache memory in KiB + * @inactiveAnon: anonymous and swap cache memory in KiB + * @activeFile: file-backed memory in KiB + * @inactiveFile: file-backed memory in KiB + * @unevictable: memory that cannot be reclaimed KiB + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupGetMemoryStat(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) +{ + int ret = -1; + 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.")); + goto cleanup; + } + *valueStr = '\0'; + + if (virStrToLong_ull(valueStr + 1, NULL, 10, &value) < 0) + goto cleanup; + + if (STREQ(line, "cache")) + cacheVal = value >> 10;
Can't we assign directly to *cache? Sure, you'd need to initialize it before, just like you're initializing cacheVal. Michal
On Fri, Aug 10, 2018 at 10:44:39AM +0200, Michal Privoznik wrote:
On 08/09/2018 03:44 PM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/libvirt_private.syms | 1 + src/util/vircgroup.c | 88 ++++++++++++++++++++++++++++++++++++++++ src/util/vircgroup.h | 7 ++++ 3 files changed, 96 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 59d9bc380e..ee0dca6129 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1532,6 +1532,7 @@ virCgroupGetDomainTotalCpuStats; virCgroupGetFreezerState; virCgroupGetMemoryHardLimit; virCgroupGetMemorySoftLimit; +virCgroupGetMemoryStat; virCgroupGetMemoryUsage; virCgroupGetMemSwapHardLimit; virCgroupGetMemSwapUsage; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 37982a9607..b91acd13c7 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2427,6 +2427,94 @@ virCgroupSetMemory(virCgroupPtr group, unsigned long long kb) }
+/** + * virCgroupGetMemoryStat: + * + * @group: The cgroup to change memory for + * @cache: page cache memory in KiB + * @activeAnon: anonymous and swap cache memory in KiB + * @inactiveAnon: anonymous and swap cache memory in KiB + * @activeFile: file-backed memory in KiB + * @inactiveFile: file-backed memory in KiB + * @unevictable: memory that cannot be reclaimed KiB + * + * Returns: 0 on success, -1 on error + */ +int +virCgroupGetMemoryStat(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) +{ + int ret = -1; + 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.")); + goto cleanup; + } + *valueStr = '\0'; + + if (virStrToLong_ull(valueStr + 1, NULL, 10, &value) < 0) + goto cleanup; + + if (STREQ(line, "cache")) + cacheVal = value >> 10;
Can't we assign directly to *cache? Sure, you'd need to initialize it before, just like you're initializing cacheVal.
We can do that, some other functions in vircgroup.c are assigning values directly into the passed parameters, I wanted to play it safe and update the passed parameters only if everything succeeds. Pavel
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/lxc/lxc_cgroup.c | 65 +++++--------------------------------------- 1 file changed, 7 insertions(+), 58 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 8e937ec389..d93a19d684 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -220,64 +220,13 @@ static int virLXCCgroupGetMemTotal(virCgroupPtr cgroup, static int virLXCCgroupGetMemStat(virCgroupPtr cgroup, virLXCMeminfoPtr meminfo) { - int ret = 0; - FILE *statfd = NULL; - char *statFile = NULL; - char *line = NULL; - size_t n; - - ret = virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_MEMORY, - "memory.stat", &statFile); - if (ret != 0) { - virReportSystemError(-ret, "%s", - _("cannot get the path of MEMORY cgroup controller")); - return ret; - } - - statfd = fopen(statFile, "r"); - if (statfd == NULL) { - ret = -errno; - goto cleanup; - } - - while (getline(&line, &n, statfd) > 0) { - - char *value = strchr(line, ' '); - char *nl = value ? strchr(line, '\n') : NULL; - unsigned long long stat_value; - - if (!value) - continue; - - if (nl) - *nl = '\0'; - - *value = '\0'; - - if (virStrToLong_ull(value + 1, NULL, 10, &stat_value) < 0) { - ret = -EINVAL; - goto cleanup; - } - if (STREQ(line, "cache")) - meminfo->cached = stat_value >> 10; - else if (STREQ(line, "inactive_anon")) - meminfo->inactive_anon = stat_value >> 10; - else if (STREQ(line, "active_anon")) - meminfo->active_anon = stat_value >> 10; - else if (STREQ(line, "inactive_file")) - meminfo->inactive_file = stat_value >> 10; - else if (STREQ(line, "active_file")) - meminfo->active_file = stat_value >> 10; - else if (STREQ(line, "unevictable")) - meminfo->unevictable = stat_value >> 10; - } - ret = 0; - - cleanup: - VIR_FREE(line); - VIR_FREE(statFile); - VIR_FORCE_FCLOSE(statfd); - return ret; + return virCgroupGetMemoryStat(cgroup, + &meminfo->cached, + &meminfo->inactive_anon, + &meminfo->active_anon, + &meminfo->inactive_file, + &meminfo->active_file, + &meminfo->unevictable); } -- 2.17.1
On 08/09/2018 03:43 PM, Pavel Hrdina wrote:
Preparation for cgroupv2 support.
Pavel Hrdina (17): docs: List cpuacct in controllers used by QEMU driver docs: Update how we create cgroup directory names vircgroup: Rename structs to start with underscore vircgroup: Introduce standard set of typedefs and use them vircgroup: Extract file link resolving into separate function vircgroup: Remove unused function virCgroupKill() vircgroup: Unexport unused function virCgroupAddTaskController() vircgroup: Unexport unused function virCgroupRemoveRecursively vircgroup: Move function used in tests into vircgrouppriv.h vircgroup: Remove pointless bool parameter vircgroup: Extract mount options matching into function vircgroup: Use virCgroupMountOptsMatchController in virCgroupDetectPlacement vircgroup: Introduce virCgroupEnableMissingControllers vircgroup: machinename will never be NULL vircgroup: Remove virCgroupAddTaskController vircgroup: Introduce virCgroupGetMemoryStat lxc: Use virCgroupGetMemoryStat
docs/cgroups.html.in | 47 ++-- src/libvirt_private.syms | 4 +- src/lxc/lxc_cgroup.c | 65 +---- src/util/vircgroup.c | 521 ++++++++++++++++++++------------------- src/util/vircgroup.h | 31 +-- src/util/vircgrouppriv.h | 21 +- 6 files changed, 330 insertions(+), 359 deletions(-)
ACK series. Michal
participants (2)
-
Michal Privoznik -
Pavel Hrdina