From 205289d2792aacf68ed2cb8563d1860bd36137a0 Mon Sep 17 00:00:00 2001

Message-Id: <205289d2792aacf68ed2cb8563d1860bd36137a0@dist-git>

From: Pavel Hrdina <phrdina@redhat.com>

Date: Fri, 19 Feb 2021 13:33:55 +0100

Subject: [PATCH] vircgroup: use DBus call to systemd for some APIs

MIME-Version: 1.0

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

Content-Transfer-Encoding: 8bit

When running on host with systemd we register VMs with machined.

In this case systemd creates the root VM cgroup for us. This has some

implications where one of them is that systemd owns all files inside

the root VM cgroup and we should not touch them.

If we change any value in file that systemd knows about it will be

changed to what systemd thinks it should be when executing

`systemctl daemon-reload`.

These are the APIs that we need to call using systemd because they set

limits that are proportional to sibling cgroups.

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

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

(cherry picked from commit 9c1693eff427661616ce1bd2795688f87288a412)

Conflicts:

    src/util/vircgroup.c

        - missing upstream g_autofree rewrite

        - missing upstream glib dbus rewrite, hence the ugly macro

          instead of a function

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

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

Message-Id: <5d22d307112333f1da565cb642ea9001a7b8b55b.1613737828.git.phrdina@redhat.com>

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

---

 src/util/vircgroup.c     | 11 ++++++++++

 src/util/vircgrouppriv.h | 25 +++++++++++++++++++++++

 src/util/vircgroupv1.c   | 44 +++++++++++++++++++++++++++-------------

 src/util/vircgroupv2.c   | 44 +++++++++++++++++++++++++++-------------

 tests/Makefile.am        |  1 +

 5 files changed, 97 insertions(+), 28 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c

index a45c2e7f2f..10b934291c 100644

--- a/src/util/vircgroup.c

+++ b/src/util/vircgroup.c

@@ -1027,6 +1027,10 @@ virCgroupNewDetectMachine(const char *name,

         }

     }

+    (*group)->unitName = virSystemdGetMachineUnitByPID(pid);

+    if (virSystemdHasMachined() == 0 && !(*group)->unitName)

+        return -1;

+

     return 0;

 }

@@ -1146,6 +1150,12 @@ virCgroupNewMachineSystemd(const char *name,

         return -1;

     }

+    (*group)->unitName = virSystemdGetMachineUnitByPID(pidleader);

+    if (!(*group)->unitName) {

+        virCgroupFree(group);

+        return -1;

+    }

+

     if (virCgroupAddProcess(*group, pidleader) < 0) {

         virErrorPtr saved;

@@ -3553,6 +3563,7 @@ virCgroupFree(virCgroupPtr *group)

     VIR_FREE((*group)->unified.mountPoint);

     VIR_FREE((*group)->unified.placement);

+    VIR_FREE((*group)->unitName);

     VIR_FREE((*group)->path);

     VIR_FREE(*group);

diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h

index f2a80aeb82..b4a9e0b379 100644

--- a/src/util/vircgrouppriv.h

+++ b/src/util/vircgrouppriv.h

@@ -27,6 +27,7 @@

 #include "vircgroup.h"

 #include "vircgroupbackend.h"

+#include "virdbus.h"

 struct _virCgroupV1Controller {

     int type;

@@ -66,8 +67,32 @@ struct _virCgroup {

     virCgroupV1Controller legacy[VIR_CGROUP_CONTROLLER_LAST];

     virCgroupV2Controller unified;

+

+    char *unitName;

 };

+#define virCgroupSetValueDBus(unitName, key, ...) \

+    ({ \

+        int __ret = -1; \

+        do { \

+            DBusConnection *__conn; \

+            if (!(__conn = virDBusGetSystemBus())) \

+                break; \

+            __ret = virDBusCallMethod(__conn, NULL, NULL, \

+                                      "org.freedesktop.systemd1", \

+                                      "/org/freedesktop/systemd1", \

+                                      "org.freedesktop.systemd1.Manager", \

+                                      "SetUnitProperties", \

+                                      "sba(sv)", \

+                                      unitName, \

+                                      true, \

+                                      1, \

+                                      key, \

+                                      __VA_ARGS__); \

+        } while (0); \

+        __ret; \

+    })

+

 int virCgroupSetValueRaw(const char *path,

                          const char *value);

diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c

index c35088a3c4..7ec8f3a316 100644

--- a/src/util/vircgroupv1.c

+++ b/src/util/vircgroupv1.c

@@ -931,7 +931,6 @@ virCgroupV1SetBlkioWeight(virCgroupPtr group,

                           unsigned int weight)

 {

     g_autofree char *path = NULL;

-    g_autofree char *value = NULL;

     if (virCgroupV1PathOfController(group, VIR_CGROUP_CONTROLLER_BLKIO,

                                     "blkio.bfq.weight", &path) < 0) {

@@ -953,9 +952,14 @@ virCgroupV1SetBlkioWeight(virCgroupPtr group,

         return -1;

     }

-    value = g_strdup_printf("%u", weight);

+    if (group->unitName) {

+        return virCgroupSetValueDBus(group->unitName, "BlockIOWeight",

+                                     "t", (unsigned long long) weight);

+    } else {

+        g_autofree char *value = g_strdup_printf("%u", weight);

-    return virCgroupSetValueRaw(path, value);

+        return virCgroupSetValueRaw(path, value);

+    }

 }

@@ -1188,15 +1192,8 @@ virCgroupV1SetBlkioDeviceWeight(virCgroupPtr group,

                                 const char *devPath,

                                 unsigned int weight)

 {

-    g_autofree char *str = NULL;

-    g_autofree char *blkstr = NULL;

     g_autofree char *path = NULL;

-    if (!(blkstr = virCgroupGetBlockDevString(devPath)))

-        return -1;

-

-    str = g_strdup_printf("%s%d", blkstr, weight);

-

     if (virCgroupV1PathOfController(group, VIR_CGROUP_CONTROLLER_BLKIO,

                                     "blkio.weight_device", &path) < 0) {

         return -1;

@@ -1208,7 +1205,21 @@ virCgroupV1SetBlkioDeviceWeight(virCgroupPtr group,

         return -1;

     }

-    return virCgroupSetValueRaw(path, str);

+    if (group->unitName) {

+        return virCgroupSetValueDBus(group->unitName, "BlockIODeviceWeight",

+                                     "a(st)",

+                                     1, path, (unsigned long long) weight);

+    } else {

+        g_autofree char *str = NULL;

+        g_autofree char *blkstr = NULL;

+

+        if (!(blkstr = virCgroupGetBlockDevString(devPath)))

+            return -1;

+

+        str = g_strdup_printf("%s%d", blkstr, weight);

+

+        return virCgroupSetValueRaw(path, str);

+    }

 }

@@ -1849,9 +1860,14 @@ static int

 virCgroupV1SetCpuShares(virCgroupPtr group,

                         unsigned long long shares)

 {

-    return virCgroupSetValueU64(group,

-                                VIR_CGROUP_CONTROLLER_CPU,

-                                "cpu.shares", shares);

+    if (group->unitName) {

+        return virCgroupSetValueDBus(group->unitName, "CPUShares",

+                                     "t", shares);

+    } else {

+        return virCgroupSetValueU64(group,

+                                    VIR_CGROUP_CONTROLLER_CPU,

+                                    "cpu.shares", shares);

+    }

 }

diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c

index 4682a6a920..8fe4894a9e 100644

--- a/src/util/vircgroupv2.c

+++ b/src/util/vircgroupv2.c

@@ -606,7 +606,6 @@ virCgroupV2SetBlkioWeight(virCgroupPtr group,

                           unsigned int weight)

 {

     g_autofree char *path = NULL;

-    g_autofree char *value = NULL;

     const char *format = "%u";

     if (virCgroupV2PathOfController(group, VIR_CGROUP_CONTROLLER_BLKIO,

@@ -630,9 +629,14 @@ virCgroupV2SetBlkioWeight(virCgroupPtr group,

         return -1;

     }

-    value = g_strdup_printf(format, weight);

+    if (group->unitName) {

+        return virCgroupSetValueDBus(group->unitName, "IOWeight",

+                                     "t", (unsigned long long) weight);

+    } else {

+        g_autofree char *value = g_strdup_printf(format, weight);

-    return virCgroupSetValueRaw(path, value);

+        return virCgroupSetValueRaw(path, value);

+    }

 }

@@ -817,13 +821,6 @@ virCgroupV2SetBlkioDeviceWeight(virCgroupPtr group,

                                 unsigned int weight)

 {

     g_autofree char *path = NULL;

-    g_autofree char *str = NULL;

-    g_autofree char *blkstr = NULL;

-

-    if (!(blkstr = virCgroupGetBlockDevString(devPath)))

-        return -1;

-

-    str = g_strdup_printf("%s%d", blkstr, weight);

     if (virCgroupV2PathOfController(group, VIR_CGROUP_CONTROLLER_BLKIO,

                                     "io.weight", &path) < 0) {

@@ -836,7 +833,21 @@ virCgroupV2SetBlkioDeviceWeight(virCgroupPtr group,

         return -1;

     }

-    return virCgroupSetValueRaw(path, str);

+    if (group->unitName) {

+        return virCgroupSetValueDBus(group->unitName, "IODeviceWeight",

+                                     "a(st)",

+                                     1, path, (unsigned long long) weight);

+    } else {

+        g_autofree char *str = NULL;

+        g_autofree char *blkstr = NULL;

+

+        if (!(blkstr = virCgroupGetBlockDevString(devPath)))

+            return -1;

+

+        str = g_strdup_printf("%s%d", blkstr, weight);

+

+        return virCgroupSetValueRaw(path, str);

+    }

 }

@@ -1455,9 +1466,14 @@ static int

 virCgroupV2SetCpuShares(virCgroupPtr group,

                         unsigned long long shares)

 {

-    return virCgroupSetValueU64(group,

-                                VIR_CGROUP_CONTROLLER_CPU,

-                                "cpu.weight", shares);

+    if (group->unitName) {

+        return virCgroupSetValueDBus(group->unitName, "CPUWeight",

+                                     "t", shares);

+    } else {

+        return virCgroupSetValueU64(group,

+                                    VIR_CGROUP_CONTROLLER_CPU,

+                                    "cpu.weight", shares);

+    }

 }

diff --git a/tests/Makefile.am b/tests/Makefile.am

index f957c7d1ba..b030d0e8f6 100644

--- a/tests/Makefile.am

+++ b/tests/Makefile.am

@@ -1188,6 +1188,7 @@ libvirportallocatormock_la_LIBADD = $(MOCKLIBS_LIBS)

 vircgrouptest_SOURCES = \

 	vircgrouptest.c testutils.h testutils.c

+vircgrouptest_CFLAGS = $(DBUS_CFLAGS) $(AM_CFLAGS)

 vircgrouptest_LDADD = $(LDADDS)

 libvircgroupmock_la_SOURCES = \

-- 

2.30.0