From 0ede215415509022d94092b75ef075495c91a5d5 Mon Sep 17 00:00:00 2001

Message-Id: <0ede215415509022d94092b75ef075495c91a5d5@dist-git>

From: Pavel Hrdina <phrdina@redhat.com>

Date: Mon, 1 Jul 2019 17:08:03 +0200

Subject: [PATCH] virt-host-validate: rewrite cgroup detection to use

 util/vircgroup

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

This removes code duplication and simplifies cgroup detection.

As a drawback we will not have separate messages to enable cgroup

controller in kernel or to mount it.  On the other side the rewrite

adds support for cgroup v2.

The kernel config support was wrong because it was parsing

'/proc/self/cgroup' instead of '/proc/cgroups/' file.

The mount suggestion is removed as well because it will not work

with cgroup v2.

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

(cherry picked from commit 0f4d7daa8cd43b62911413c2cc1614f87380e459)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1689297

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

Message-Id: <0a79b9dcaee25d04b993c4e6576d75ce7c45ac0f.1561993100.git.phrdina@redhat.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

---

 tools/virt-host-validate-common.c | 164 ++++++------------------------

 tools/virt-host-validate-common.h |   7 +-

 tools/virt-host-validate-lxc.c    |  38 ++-----

 tools/virt-host-validate-qemu.c   |  38 ++-----

 4 files changed, 53 insertions(+), 194 deletions(-)

diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c

index c45dfc5d8c..73165d673a 100644

--- a/tools/virt-host-validate-common.c

+++ b/tools/virt-host-validate-common.c

@@ -26,12 +26,10 @@

 #include <stdio.h>

 #include <unistd.h>

 #include <sys/utsname.h>

-#ifdef HAVE_MNTENT_H

-# include <mntent.h>

-#endif /* HAVE_MNTENT_H */

 #include <sys/stat.h>

 #include "viralloc.h"

+#include "vircgroup.h"

 #include "virfile.h"

 #include "virt-host-validate-common.h"

 #include "virstring.h"

@@ -290,152 +288,50 @@ int virHostValidateLinuxKernel(const char *hvname,

     }

 }

-

-static int virHostValidateCGroupSupport(const char *hvname,

-                                        const char *cg_name,

-                                        virHostValidateLevel level,

-                                        const char *config_name)

+#ifdef __linux__

+int virHostValidateCGroupControllers(const char *hvname,

+                                     int controllers,

+                                     virHostValidateLevel level)

 {

-    virHostMsgCheck(hvname, "for cgroup '%s' controller support", cg_name);

-    FILE *fp = fopen("/proc/self/cgroup", "r");

-    size_t len = 0;

-    char *line = NULL;

-    ssize_t ret;

-    bool matched = false;

+    virCgroupPtr group = NULL;

+    int ret = 0;

+    size_t i;

-    if (!fp)

-        goto error;

+    if (virCgroupNewSelf(&group) < 0)

+        return -1;

-    while ((ret = getline(&line, &len, fp)) >= 0 && !matched) {

-        char **cgroups;

-        char *start;

-        char *end;

-        size_t ncgroups;

-        size_t i;

+    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {

+        int flag = 1 << i;

+        const char *cg_name = virCgroupControllerTypeToString(i);

-        /* Each line in this file looks like

-         *

-         *   4:cpu,cpuacct:/machine.slice/machine-qemu\x2dtest.scope/emulator

-         *

-         * Since multiple cgroups can be part of the same line and some cgroup

-         * names can appear as part of other cgroup names (eg. 'cpu' is a

-         * prefix for both 'cpuacct' and 'cpuset'), it's not enough to simply

-         * check whether the cgroup name is present somewhere inside the file.

-         *

-         * Moreover, there's nothing stopping the cgroup name from appearing

-         * in an unrelated mount point name as well */

-

-        /* Look for the first colon.

-         * The part we're interested in starts right after it */

-        if (!(start = strchr(line, ':')))

-            continue;

-        start++;

-

-        /* Look for the second colon.

-         * The part we're interested in ends exactly there */

-        if (!(end = strchr(start, ':')))

-            continue;

-        *end = '\0';

-

-        if (!(cgroups = virStringSplitCount(start, ",", 0, &ncgroups)))

+        if (!(controllers & flag))

             continue;

-        /* Look for the matching cgroup */

-        for (i = 0; i < ncgroups; i++) {

-            if (STREQ(cgroups[i], cg_name))

-                matched = true;

+        virHostMsgCheck(hvname, "for cgroup '%s' controller support", cg_name);

+

+        if (!virCgroupHasController(group, i)) {

+            ret = -1;

+            virHostMsgFail(level, "Enable '%s' in kernel Kconfig file or "

+                           "mount/enable cgroup controller in your system",

+                           cg_name);

+        } else {

+            virHostMsgPass();

         }

-

-        virStringListFreeCount(cgroups, ncgroups);

     }

-    VIR_FREE(line);

-    VIR_FORCE_FCLOSE(fp);

-    if (!matched)

-        goto error;

+    virCgroupFree(&group);

-    virHostMsgPass();

-    return 0;

-

- error:

-    VIR_FREE(line);

-    virHostMsgFail(level, "Enable CONFIG_%s in kernel Kconfig file", config_name);

-    return -1;

+    return ret;

 }

-

-#ifdef HAVE_MNTENT_H

-static int virHostValidateCGroupMount(const char *hvname,

-                                      const char *cg_name,

-                                      virHostValidateLevel level)

+#else /*  !__linux__ */

+int virHostValidateCGroupControllers(const char *hvname,

+                                     int controllers,

+                                     virHostValidateLevel level)

 {

-    virHostMsgCheck(hvname, "for cgroup '%s' controller mount-point", cg_name);

-    FILE *fp = setmntent("/proc/mounts", "r");

-    struct mntent ent;

-    char mntbuf[1024];

-    bool matched = false;

-

-    if (!fp)

-        goto error;

-

-    while (getmntent_r(fp, &ent, mntbuf, sizeof(mntbuf)) && !matched) {

-        char **opts;

-        size_t nopts;

-        size_t i;

-

-        /* Ignore non-cgroup mounts */

-        if (STRNEQ(ent.mnt_type, "cgroup"))

-            continue;

-

-        if (!(opts = virStringSplitCount(ent.mnt_opts, ",", 0, &nopts)))

-            continue;

-

-        /* Look for a mount option matching the cgroup name */

-        for (i = 0; i < nopts; i++) {

-            if (STREQ(opts[i], cg_name))

-                matched = true;

-        }

-

-        virStringListFreeCount(opts, nopts);

-    }

-    endmntent(fp);

-    if (!matched)

-        goto error;

-

-    virHostMsgPass();

-    return 0;

-

- error:

-    virHostMsgFail(level, "Mount '%s' cgroup controller (suggested at /sys/fs/cgroup/%s)",

-                   cg_name, cg_name);

-    return -1;

-}

-#else /* ! HAVE_MNTENT_H */

-static int virHostValidateCGroupMount(const char *hvname,

-                                      const char *cg_name,

-                                      virHostValidateLevel level)

-{

-    virHostMsgCheck(hvname, "for cgroup '%s' controller mount-point", cg_name);

     virHostMsgFail(level, "%s", "This platform does not support cgroups");

     return -1;

 }

-#endif /* ! HAVE_MNTENT_H */

-

-int virHostValidateCGroupController(const char *hvname,

-                                    const char *cg_name,

-                                    virHostValidateLevel level,

-                                    const char *config_name)

-{

-    if (virHostValidateCGroupSupport(hvname,

-                                     cg_name,

-                                     level,

-                                     config_name) < 0)

-        return -1;

-    if (virHostValidateCGroupMount(hvname,

-                                   cg_name,

-                                   level) < 0)

-        return -1;

-    return 0;

-}

+#endif /* !__linux__ */

 int virHostValidateIOMMU(const char *hvname,

                          virHostValidateLevel level)

diff --git a/tools/virt-host-validate-common.h b/tools/virt-host-validate-common.h

index b6fe17daa7..b23dd7cdbe 100644

--- a/tools/virt-host-validate-common.h

+++ b/tools/virt-host-validate-common.h

@@ -77,10 +77,9 @@ int virHostValidateNamespace(const char *hvname,

                              virHostValidateLevel level,

                              const char *hint);

-int virHostValidateCGroupController(const char *hvname,

-                                    const char *cg_name,

-                                    virHostValidateLevel level,

-                                    const char *config_name);

+int virHostValidateCGroupControllers(const char *hvname,

+                                     int controllers,

+                                     virHostValidateLevel level);

 int virHostValidateIOMMU(const char *hvname,

                          virHostValidateLevel level);

diff --git a/tools/virt-host-validate-lxc.c b/tools/virt-host-validate-lxc.c

index 64d9279c30..3c55b1b26d 100644

--- a/tools/virt-host-validate-lxc.c

+++ b/tools/virt-host-validate-lxc.c

@@ -23,6 +23,7 @@

 #include "virt-host-validate-lxc.h"

 #include "virt-host-validate-common.h"

+#include "vircgroup.h"

 int virHostValidateLXC(void)

 {

@@ -63,35 +64,16 @@ int virHostValidateLXC(void)

                                  _("User namespace support is recommended")) < 0)

         ret = -1;

-    if (virHostValidateCGroupController("LXC", "memory",

-                                        VIR_HOST_VALIDATE_FAIL,

-                                        "MEMCG") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("LXC", "cpu",

-                                        VIR_HOST_VALIDATE_FAIL,

-                                        "CGROUP_CPU") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("LXC", "cpuacct",

-                                        VIR_HOST_VALIDATE_FAIL,

-                                        "CGROUP_CPUACCT") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("LXC", "cpuset",

-                                        VIR_HOST_VALIDATE_FAIL,

-                                        "CPUSETS") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("LXC", "devices",

-                                        VIR_HOST_VALIDATE_FAIL,

-                                        "CGROUP_DEVICE") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("LXC", "blkio",

-                                        VIR_HOST_VALIDATE_FAIL,

-                                        "BLK_CGROUP") < 0)

+    if (virHostValidateCGroupControllers("LXC",

+                                         (1 << VIR_CGROUP_CONTROLLER_MEMORY) |

+                                         (1 << VIR_CGROUP_CONTROLLER_CPU) |

+                                         (1 << VIR_CGROUP_CONTROLLER_CPUACCT) |

+                                         (1 << VIR_CGROUP_CONTROLLER_CPUSET) |

+                                         (1 << VIR_CGROUP_CONTROLLER_DEVICES) |

+                                         (1 << VIR_CGROUP_CONTROLLER_BLKIO),

+                                         VIR_HOST_VALIDATE_FAIL) < 0) {

         ret = -1;

+    }

 #if WITH_FUSE

     if (virHostValidateDeviceExists("LXC", "/sys/fs/fuse/connections",

diff --git a/tools/virt-host-validate-qemu.c b/tools/virt-host-validate-qemu.c

index d7573ea8b3..ff3c1f0231 100644

--- a/tools/virt-host-validate-qemu.c

+++ b/tools/virt-host-validate-qemu.c

@@ -26,6 +26,7 @@

 #include "virt-host-validate-common.h"

 #include "virarch.h"

 #include "virbitmap.h"

+#include "vircgroup.h"

 int virHostValidateQEMU(void)

 {

@@ -96,35 +97,16 @@ int virHostValidateQEMU(void)

                                     _("Load the 'tun' module to enable networking for QEMU guests")) < 0)

         ret = -1;

-    if (virHostValidateCGroupController("QEMU", "memory",

-                                        VIR_HOST_VALIDATE_WARN,

-                                        "MEMCG") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("QEMU", "cpu",

-                                        VIR_HOST_VALIDATE_WARN,

-                                        "CGROUP_CPU") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("QEMU", "cpuacct",

-                                        VIR_HOST_VALIDATE_WARN,

-                                        "CGROUP_CPUACCT") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("QEMU", "cpuset",

-                                        VIR_HOST_VALIDATE_WARN,

-                                        "CPUSETS") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("QEMU", "devices",

-                                        VIR_HOST_VALIDATE_WARN,

-                                        "CGROUP_DEVICES") < 0)

-        ret = -1;

-

-    if (virHostValidateCGroupController("QEMU", "blkio",

-                                        VIR_HOST_VALIDATE_WARN,

-                                        "BLK_CGROUP") < 0)

+    if (virHostValidateCGroupControllers("QEMU",

+                                         (1 << VIR_CGROUP_CONTROLLER_MEMORY) |

+                                         (1 << VIR_CGROUP_CONTROLLER_CPU) |

+                                         (1 << VIR_CGROUP_CONTROLLER_CPUACCT) |

+                                         (1 << VIR_CGROUP_CONTROLLER_CPUSET) |

+                                         (1 << VIR_CGROUP_CONTROLLER_DEVICES) |

+                                         (1 << VIR_CGROUP_CONTROLLER_BLKIO),

+                                         VIR_HOST_VALIDATE_WARN) < 0) {

         ret = -1;

+    }

     if (virHostValidateIOMMU("QEMU",

                              VIR_HOST_VALIDATE_WARN) < 0)

-- 

2.22.0