On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrd...@redhat.com> wrote:

> Signed-off-by: Pavel Hrdina <phrd...@redhat.com>
>

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

Reply via email to