Re: [libvirt] [PATCH 07/47] vircgroup: extract v1 detect functions

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt;

--- src/util/vircgroup.c | 138 ++----------------------------- src/util/vircgroupbackend.h | 14 ++++ src/util/vircgroupv1.c | 158 ++++++++++++++++++++++++++++++++++++ 3 files changed, 180 insertions(+), 130 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index f5b2a14df9..7557fc5576 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -237,82 +237,6 @@ virCgroupPartitionEscape(char **path) } -static int -virCgroupResolveMountLink(const char *mntDir, - const char *typeStr, - virCgroupControllerPtr controller) -{ - VIR_AUTOFREE(char *) linkSrc = NULL; - VIR_AUTOFREE(char *) tmp = NULL; - char *dirName; - struct stat sb; - - if (VIR_STRDUP(tmp, mntDir) < 0) - return -1; - - dirName = strrchr(tmp, '/'); - if (!dirName) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing '/' separator in cgroup mount '%s'"), tmp); - return -1; - } - - if (!strchr(dirName + 1, ',')) - return 0; - - *dirName = '\0'; - if (virAsprintf(&linkSrc, "%s/%s", tmp, 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, tmp, 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; -} - - -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 @@ -320,7 +244,6 @@ virCgroupMountOptsMatchController(const char *mntOpts, static int virCgroupDetectMounts(virCgroupPtr group) { - size_t i; FILE *mounts = NULL; struct mntent entry; char buf[CGROUP_MAX_VAL]; @@ -333,34 +256,11 @@ virCgroupDetectMounts(virCgroupPtr group) } while (getmntent_r(mounts, &entry, buf, sizeof(buf)) != NULL) { - if (STRNEQ(entry.mnt_type, "cgroup")) - continue; - - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - const char *typestr = virCgroupControllerTypeToString(i); - - 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 (virCgroupResolveMountLink(entry.mnt_dir, typestr, - controller) < 0) { - goto cleanup; - } - } + if (group->backend->detectMounts(group, + entry.mnt_type, + entry.mnt_opts, + entry.mnt_dir) < 0) { + goto cleanup; } } @@ -434,7 +334,6 @@ virCgroupDetectPlacement(virCgroupPtr group, pid_t pid, const char *path) { - size_t i; FILE *mapping = NULL; char line[1024]; int ret = -1; @@ -474,30 +373,9 @@ virCgroupDetectPlacement(virCgroupPtr group, controllers++; selfpath++; - for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - const char *typestr = virCgroupControllerTypeToString(i); - - 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 (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 (group->backend->detectPlacement(group, path, controllers, + selfpath) < 0) { + goto cleanup; } } diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h index 81ee597fc8..fadc7efdcf 100644 --- a/src/util/vircgroupbackend.h +++ b/src/util/vircgroupbackend.h @@ -45,6 +45,18 @@ typedef int (*virCgroupCopyMountsCB)(virCgroupPtr group, virCgroupPtr parent); +typedef int +(*virCgroupDetectMountsCB)(virCgroupPtr group, + const char *mntType, + const char *mntOpts, + const char *mntDir); + +typedef int +(*virCgroupDetectPlacementCB)(virCgroupPtr group, + const char *path, + const char *controllers, + const char *selfpath); + struct _virCgroupBackend { virCgroupBackendType type; @@ -52,6 +64,8 @@ struct _virCgroupBackend { virCgroupAvailableCB available; virCgroupValidateMachineGroupCB validateMachineGroup; virCgroupCopyMountsCB copyMounts; + virCgroupDetectMountsCB detectMounts; + virCgroupDetectPlacementCB detectPlacement; }; typedef struct _virCgroupBackend virCgroupBackend; typedef virCgroupBackend *virCgroupBackendPtr; diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 00c3349aee..2882a19be2 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -23,6 +23,7 @@ #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R # include <mntent.h> #endif +#include <sys/stat.h> #include "internal.h" @@ -37,6 +38,7 @@ #include "virlog.h" #include "virstring.h" #include "virsystemd.h" +#include "virerror.h" VIR_LOG_INIT("util.cgroup"); @@ -180,12 +182,168 @@ virCgroupV1CopyMounts(virCgroupPtr group, } +static int +virCgroupV1ResolveMountLink(const char *mntDir, + const char *typeStr, + virCgroupControllerPtr controller) +{ + VIR_AUTOFREE(char *) linkSrc = NULL; + VIR_AUTOFREE(char *) tmp = NULL; + char *dirName; + struct stat sb; + + if (VIR_STRDUP(tmp, mntDir) < 0) + return -1; + + dirName = strrchr(tmp, '/'); + if (!dirName) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing '/' separator in cgroup mount '%s'"), tmp); + return -1; + } + + if (!strchr(dirName + 1, ',')) + return 0; + + *dirName = '\0'; + if (virAsprintf(&linkSrc, "%s/%s", tmp, 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, tmp, 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; +} + + +static bool +virCgroupV1MountOptsMatchController(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; +} + + +static int +virCgroupV1DetectMounts(virCgroupPtr group, + const char *mntType, + const char *mntOpts, + const char *mntDir) +{ + size_t i; + + if (STRNEQ(mntType, "cgroup")) + return 0; + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + const char *typestr = virCgroupV1ControllerTypeToString(i); + + if (virCgroupV1MountOptsMatchController(mntOpts, 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, mntDir) < 0) + return -1; + + /* If it is a co-mount it has a filename like "cpu,cpuacct" + * and we must identify the symlink path */ + if (virCgroupV1ResolveMountLink(mntDir, typestr, controller) < 0) + return -1; + } + } + + return 0; +} + + +static int +virCgroupV1DetectPlacement(virCgroupPtr group, + const char *path, + const char *controllers, + const char *selfpath) +{ + size_t i; + + for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { + const char *typestr = virCgroupV1ControllerTypeToString(i); + + if (virCgroupV1MountOptsMatchController(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 (i == VIR_CGROUP_CONTROLLER_SYSTEMD) { + if (VIR_STRDUP(group->controllers[i].placement, + selfpath) < 0) + return -1; + } else { + if (virAsprintf(&group->controllers[i].placement, + "%s%s%s", selfpath, + (STREQ(selfpath, "/") || + STREQ(path, "") ? "" : "/"), + path) < 0) + return -1; + } + } + } + + return 0; +} + + virCgroupBackend virCgroupV1Backend = { .type = VIR_CGROUP_BACKEND_TYPE_V1, .available = virCgroupV1Available, .validateMachineGroup = virCgroupV1ValidateMachineGroup, .copyMounts = virCgroupV1CopyMounts, + .detectMounts = virCgroupV1DetectMounts, + .detectPlacement = virCgroupV1DetectPlacement, }; -- 2.17.1 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list