On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina@redhat.com> wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Reviewed-by: Fabiano Fidêncio <fidencio@redhat.com>
 
---
 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list