[libvirt] [PATCH 15/47] vircgroup: extract virCgroupV1MakeGroup

Fabiano Fidêncio fidencio at redhat.com
Thu Sep 20 06:29:38 UTC 2018


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

> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>

Reviewed-by: Fabiano Fidêncio <fidencio at redhat.com>


> ---
>  src/util/vircgroup.c        | 141 ++----------------------------------
>  src/util/vircgroupbackend.h |  15 ++++
>  src/util/vircgrouppriv.h    |  20 +++++
>  src/util/vircgroupv1.c      | 132 +++++++++++++++++++++++++++++++++
>  4 files changed, 174 insertions(+), 134 deletions(-)
>
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 55122a5ab9..8083e4596d 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -80,14 +80,6 @@ VIR_ENUM_IMPL(virCgroupController,
> VIR_CGROUP_CONTROLLER_LAST,
>                "freezer", "blkio", "net_cls", "perf_event",
>                "name=systemd");
>
> -typedef enum {
> -    VIR_CGROUP_NONE = 0, /* create subdir under each cgroup if possible.
> */
> -    VIR_CGROUP_MEM_HIERACHY = 1 << 0, /* call
> virCgroupSetMemoryUseHierarchy
> -                                       * before creating subcgroups and
> -                                       * attaching tasks
> -                                       */
> -} virCgroupFlags;
> -
>
>  /**
>   * virCgroupGetDevicePermsString:
> @@ -448,7 +440,7 @@ virCgroupGetBlockDevString(const char *path)
>  }
>
>
> -static int
> +int
>  virCgroupSetValueStr(virCgroupPtr group,
>                       int controller,
>                       const char *key,
> @@ -478,7 +470,7 @@ virCgroupSetValueStr(virCgroupPtr group,
>  }
>
>
> -static int
> +int
>  virCgroupGetValueStr(virCgroupPtr group,
>                       int controller,
>                       const char *key,
> @@ -539,7 +531,7 @@ virCgroupGetValueForBlkDev(virCgroupPtr group,
>  }
>
>
> -static int
> +int
>  virCgroupSetValueU64(virCgroupPtr group,
>                       int controller,
>                       const char *key,
> @@ -591,7 +583,7 @@ virCgroupGetValueI64(virCgroupPtr group,
>  }
>
>
> -static int
> +int
>  virCgroupGetValueU64(virCgroupPtr group,
>                       int controller,
>                       const char *key,
> @@ -613,137 +605,18 @@ virCgroupGetValueU64(virCgroupPtr group,
>  }
>
>
> -static int
> -virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group)
> -{
> -    size_t i;
> -    const char *inherit_values[] = {
> -        "cpuset.cpus",
> -        "cpuset.mems",
> -        "cpuset.memory_migrate",
> -    };
> -
> -    VIR_DEBUG("Setting up inheritance %s -> %s", parent->path,
> group->path);
> -    for (i = 0; i < ARRAY_CARDINALITY(inherit_values); i++) {
> -        VIR_AUTOFREE(char *) value = NULL;
> -
> -        if (virCgroupGetValueStr(parent,
> -                                 VIR_CGROUP_CONTROLLER_CPUSET,
> -                                 inherit_values[i],
> -                                 &value) < 0)
> -            return -1;
> -
> -        VIR_DEBUG("Inherit %s = %s", inherit_values[i], value);
> -
> -        if (virCgroupSetValueStr(group,
> -                                 VIR_CGROUP_CONTROLLER_CPUSET,
> -                                 inherit_values[i],
> -                                 value) < 0)
> -            return -1;
> -    }
> -
> -    return 0;
> -}
> -
> -
> -static int
> -virCgroupSetMemoryUseHierarchy(virCgroupPtr group)
> -{
> -    unsigned long long value;
> -    const char *filename = "memory.use_hierarchy";
> -
> -    if (virCgroupGetValueU64(group,
> -                             VIR_CGROUP_CONTROLLER_MEMORY,
> -                             filename, &value) < 0)
> -        return -1;
> -
> -    /* Setting twice causes error, so if already enabled, skip setting */
> -    if (value == 1)
> -        return 0;
> -
> -    VIR_DEBUG("Setting up %s/%s", group->path, filename);
> -    if (virCgroupSetValueU64(group,
> -                             VIR_CGROUP_CONTROLLER_MEMORY,
> -                             filename, 1) < 0)
> -        return -1;
> -
> -    return 0;
> -}
> -
> -
>  static int
>  virCgroupMakeGroup(virCgroupPtr parent,
>                     virCgroupPtr group,
>                     bool create,
>                     unsigned int flags)
>  {
> -    size_t i;
> -
> -    VIR_DEBUG("Make group %s", group->path);
> -    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
> -        VIR_AUTOFREE(char *) path = NULL;
> -
> -        /* We must never mkdir() in systemd's hierarchy */
> -        if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
> -            VIR_DEBUG("Not creating systemd controller group");
> -            continue;
> -        }
> -
> -        /* Skip over controllers that aren't mounted */
> -        if (!group->controllers[i].mountPoint) {
> -            VIR_DEBUG("Skipping unmounted controller %s",
> -                      virCgroupControllerTypeToString(i));
> -            continue;
> -        }
> -
> -        if (virCgroupPathOfController(group, i, "", &path) < 0)
> -            goto error;
> -
> -        VIR_DEBUG("Make controller %s", path);
> -        if (!virFileExists(path)) {
> -            if (!create ||
> -                mkdir(path, 0755) < 0) {
> -                if (errno == EEXIST)
> -                    continue;
> -                /* With a kernel that doesn't support multi-level
> directory
> -                 * for blkio controller, libvirt will fail and disable all
> -                 * other controllers even though they are available. So
> -                 * treat blkio as unmounted if mkdir fails. */
> -                if (i == VIR_CGROUP_CONTROLLER_BLKIO) {
> -                    VIR_DEBUG("Ignoring mkdir failure with blkio
> controller. Kernel probably too old");
> -                    VIR_FREE(group->controllers[i].mountPoint);
> -                    continue;
> -                } else {
> -                    virReportSystemError(errno,
> -                                         _("Failed to create controller
> %s for group"),
> -                                         virCgroupControllerTypeToStrin
> g(i));
> -                    goto error;
> -                }
> -            }
> -            if (i == VIR_CGROUP_CONTROLLER_CPUSET &&
> -                group->controllers[i].mountPoint != NULL &&
> -                virCgroupCpuSetInherit(parent, group) < 0) {
> -                goto error;
> -            }
> -            /*
> -             * Note that virCgroupSetMemoryUseHierarchy should always be
> -             * called prior to creating subcgroups and attaching tasks.
> -             */
> -            if ((flags & VIR_CGROUP_MEM_HIERACHY) &&
> -                i == VIR_CGROUP_CONTROLLER_MEMORY &&
> -                group->controllers[i].mountPoint != NULL &&
> -                virCgroupSetMemoryUseHierarchy(group) < 0) {
> -                goto error;
> -            }
> -        }
> +    if (group->backend->makeGroup(parent, group, create, flags) < 0) {
> +        virCgroupRemove(group);
> +        return -1;
>      }
>
> -    VIR_DEBUG("Done making controllers for group");
>      return 0;
> -
> - error:
> -    virCgroupRemove(group);
> -    return -1;
>  }
>
>
> diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h
> index cf000529c4..214c9f0726 100644
> --- a/src/util/vircgroupbackend.h
> +++ b/src/util/vircgroupbackend.h
> @@ -27,6 +27,14 @@
>
>  # define CGROUP_MAX_VAL 512
>
> +typedef enum {
> +    VIR_CGROUP_NONE = 0, /* create subdir under each cgroup if possible.
> */
> +    VIR_CGROUP_MEM_HIERACHY = 1 << 0, /* call
> virCgroupSetMemoryUseHierarchy
> +                                       * before creating subcgroups and
> +                                       * attaching tasks
> +                                       */
> +} virCgroupBackendFlags;
> +
>  typedef enum {
>      VIR_CGROUP_BACKEND_TYPE_V1 = 0,
>      VIR_CGROUP_BACKEND_TYPE_LAST,
> @@ -86,6 +94,12 @@ typedef int
>                                 const char *key,
>                                 char **path);
>
> +typedef int
> +(*virCgroupMakeGroupCB)(virCgroupPtr parent,
> +                        virCgroupPtr group,
> +                        bool create,
> +                        unsigned int flags);
> +
>  struct _virCgroupBackend {
>      virCgroupBackendType type;
>
> @@ -102,6 +116,7 @@ struct _virCgroupBackend {
>      virCgroupHasControllerCB hasController;
>      virCgroupGetAnyControllerCB getAnyController;
>      virCgroupPathOfControllerCB pathOfController;
> +    virCgroupMakeGroupCB makeGroup;
>  };
>  typedef struct _virCgroupBackend virCgroupBackend;
>  typedef virCgroupBackend *virCgroupBackendPtr;
> diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h
> index e7f4a1f0fc..2e731458d5 100644
> --- a/src/util/vircgrouppriv.h
> +++ b/src/util/vircgrouppriv.h
> @@ -53,6 +53,26 @@ struct _virCgroup {
>      virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST];
>  };
>
> +int virCgroupSetValueStr(virCgroupPtr group,
> +                         int controller,
> +                         const char *key,
> +                         const char *value);
> +
> +int virCgroupGetValueStr(virCgroupPtr group,
> +                         int controller,
> +                         const char *key,
> +                         char **value);
> +
> +int virCgroupSetValueU64(virCgroupPtr group,
> +                         int controller,
> +                         const char *key,
> +                         unsigned long long int value);
> +
> +int virCgroupGetValueU64(virCgroupPtr group,
> +                         int controller,
> +                         const char *key,
> +                         unsigned long long int *value);
> +
>  int virCgroupPartitionEscape(char **path);
>
>  int virCgroupNewPartition(const char *path,
> diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
> index 7d92150dc3..cf484216cc 100644
> --- a/src/util/vircgroupv1.c
> +++ b/src/util/vircgroupv1.c
> @@ -536,6 +536,137 @@ virCgroupV1PathOfController(virCgroupPtr group,
>  }
>
>
> +static int
> +virCgroupV1CpuSetInherit(virCgroupPtr parent,
> +                         virCgroupPtr group)
> +{
> +    size_t i;
> +    const char *inherit_values[] = {
> +        "cpuset.cpus",
> +        "cpuset.mems",
> +        "cpuset.memory_migrate",
> +    };
> +
> +    VIR_DEBUG("Setting up inheritance %s -> %s", parent->path,
> group->path);
> +    for (i = 0; i < ARRAY_CARDINALITY(inherit_values); i++) {
> +        VIR_AUTOFREE(char *) value = NULL;
> +
> +        if (virCgroupGetValueStr(parent,
> +                                 VIR_CGROUP_CONTROLLER_CPUSET,
> +                                 inherit_values[i],
> +                                 &value) < 0)
> +            return -1;
> +
> +        VIR_DEBUG("Inherit %s = %s", inherit_values[i], value);
> +
> +        if (virCgroupSetValueStr(group,
> +                                 VIR_CGROUP_CONTROLLER_CPUSET,
> +                                 inherit_values[i],
> +                                 value) < 0)
> +            return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static int
> +virCgroupV1SetMemoryUseHierarchy(virCgroupPtr group)
> +{
> +    unsigned long long value;
> +    const char *filename = "memory.use_hierarchy";
> +
> +    if (virCgroupGetValueU64(group,
> +                             VIR_CGROUP_CONTROLLER_MEMORY,
> +                             filename, &value) < 0)
> +        return -1;
> +
> +    /* Setting twice causes error, so if already enabled, skip setting */
> +    if (value == 1)
> +        return 0;
> +
> +    VIR_DEBUG("Setting up %s/%s", group->path, filename);
> +    if (virCgroupSetValueU64(group,
> +                             VIR_CGROUP_CONTROLLER_MEMORY,
> +                             filename, 1) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +
> +static int
> +virCgroupV1MakeGroup(virCgroupPtr parent,
> +                     virCgroupPtr group,
> +                     bool create,
> +                     unsigned int flags)
> +{
> +    size_t i;
> +
> +    VIR_DEBUG("Make group %s", group->path);
> +    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
> +        VIR_AUTOFREE(char *) path = NULL;
> +
> +        /* We must never mkdir() in systemd's hierarchy */
> +        if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
> +            VIR_DEBUG("Not creating systemd controller group");
> +            continue;
> +        }
> +
> +        /* Skip over controllers that aren't mounted */
> +        if (!group->controllers[i].mountPoint) {
> +            VIR_DEBUG("Skipping unmounted controller %s",
> +                      virCgroupV1ControllerTypeToString(i));
> +            continue;
> +        }
> +
> +        if (virCgroupV1PathOfController(group, i, "", &path) < 0)
> +            return -1;
> +
> +        VIR_DEBUG("Make controller %s", path);
> +        if (!virFileExists(path)) {
> +            if (!create ||
> +                mkdir(path, 0755) < 0) {
> +                if (errno == EEXIST)
> +                    continue;
> +                /* With a kernel that doesn't support multi-level
> directory
> +                 * for blkio controller, libvirt will fail and disable all
> +                 * other controllers even though they are available. So
> +                 * treat blkio as unmounted if mkdir fails. */
> +                if (i == VIR_CGROUP_CONTROLLER_BLKIO) {
> +                    VIR_DEBUG("Ignoring mkdir failure with blkio
> controller. Kernel probably too old");
> +                    VIR_FREE(group->controllers[i].mountPoint);
> +                    continue;
> +                } else {
> +                    virReportSystemError(errno,
> +                                         _("Failed to create v1
> controller %s for group"),
> +                                         virCgroupV1ControllerTypeToStr
> ing(i));
> +                    return -1;
> +                }
> +            }
> +            if (i == VIR_CGROUP_CONTROLLER_CPUSET &&
> +                group->controllers[i].mountPoint != NULL &&
> +                virCgroupV1CpuSetInherit(parent, group) < 0) {
> +                return -1;
> +            }
> +            /*
> +             * Note that virCgroupV1SetMemoryUseHierarchy should always
> be
> +             * called prior to creating subcgroups and attaching tasks.
> +             */
> +            if ((flags & VIR_CGROUP_MEM_HIERACHY) &&
> +                i == VIR_CGROUP_CONTROLLER_MEMORY &&
> +                group->controllers[i].mountPoint != NULL &&
> +                virCgroupV1SetMemoryUseHierarchy(group) < 0) {
> +                return -1;
> +            }
> +        }
> +    }
> +
> +    VIR_DEBUG("Done making controllers for group");
> +    return 0;
> +}
> +
> +
>  virCgroupBackend virCgroupV1Backend = {
>      .type = VIR_CGROUP_BACKEND_TYPE_V1,
>
> @@ -551,6 +682,7 @@ virCgroupBackend virCgroupV1Backend = {
>      .hasController = virCgroupV1HasController,
>      .getAnyController = virCgroupV1GetAnyController,
>      .pathOfController = virCgroupV1PathOfController,
> +    .makeGroup = virCgroupV1MakeGroup,
>  };
>
>
> --
> 2.17.1
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180920/648ef340/attachment-0001.htm>


More information about the libvir-list mailing list