7:26 a.m.
When running on host with systemd there is an ownership issue of
the root VM cgroup. When it is created for us by systemd using machined
the owner of the root VM cgroup is systemd and we should not touch any
of the files as systemd can and will modify any values configured there.
Basically we had the issue since introduction of machined support
in libvirt 1.1.1 back in 2013. With systemd implementing more cgroup
APIs the `systemctl daemon-reload` would change more values configured
by libvirt.
The solution to the issue is to use systemd DBus APIs to configure
cgroups but unfortunately they don't cover everything that libvirt
needs.
For that reason we will use systemd DBus APIs only for values that
affect sibling cgroups where the resources are distributed
proportionally, such as blkio.weight or cpu.shares. For the remaining
resources we will keep the current code where we work with the files
directly but we move everything into a child cgroup of the VM root
cgroup where we are free to do whatever we like including thread
configuration.
Pavel Hrdina (9):
virsystemd: export virSystemdHasMachined
virsystemd: introduce virSystemdGetMachineByPID
virsystemd: introduce virSystemdGetMachineUnitByPID
vircgroup: use DBus call to systemd for some APIs
vircgroupv1: refactor virCgroupV1DetectPlacement
vircgroupv2: move task into cgroup before enabling controllers
vircgroup: introduce virCgroupV1Exists and virCgroupV2Exists
vircgroup: introduce nested cgroup to properly work with systemd
tests: add cgroup nested tests
docs/cgroups.html.in | 29 +-
src/libvirt_private.syms | 2 +
src/util/vircgroup.c | 300 ++++++++++++++----
src/util/vircgroupbackend.h | 5 +
src/util/vircgrouppriv.h | 10 +
src/util/vircgroupv1.c | 122 +++++--
src/util/vircgroupv2.c | 82 ++++-
src/util/virsystemd.c | 105 +++++-
src/util/virsystemd.h | 4 +
tests/vircgroupdata/systemd-legacy.cgroups | 12 +
tests/vircgroupdata/systemd-legacy.mounts | 11 +
.../vircgroupdata/systemd-legacy.self.cgroup | 11 +
tests/vircgroupdata/systemd-unified.cgroups | 13 +
tests/vircgroupdata/systemd-unified.mounts | 1 +
.../vircgroupdata/systemd-unified.self.cgroup | 1 +
tests/vircgrouptest.c | 72 +++++
tests/virsystemdtest.c | 39 ++-
17 files changed, 687 insertions(+), 132 deletions(-)
create mode 100644 tests/vircgroupdata/systemd-legacy.cgroups
create mode 100644 tests/vircgroupdata/systemd-legacy.mounts
create mode 100644 tests/vircgroupdata/systemd-legacy.self.cgroup
create mode 100644 tests/vircgroupdata/systemd-unified.cgroups
create mode 100644 tests/vircgroupdata/systemd-unified.mounts
create mode 100644 tests/vircgroupdata/systemd-unified.self.cgroup
--
2.29.2
7:26 a.m.
New subject: [libvirt PATCH 1/9] virsystemd: export virSystemdHasMachined
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virsystemd.c | 2 +-
src/util/virsystemd.h | 2 ++
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0636b0d8c9..b2d2282ce0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3304,6 +3304,7 @@ virSystemdGetActivation;
virSystemdGetMachineNameByPID;
virSystemdHasLogind;
virSystemdHasLogindResetCachedValue;
+virSystemdHasMachined;
virSystemdHasMachinedResetCachedValue;
virSystemdMakeScopeName;
virSystemdMakeSliceName;
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 3f689365e4..58b5c650b5 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -147,7 +147,7 @@ void virSystemdHasLogindResetCachedValue(void)
* -1 = error
* 0 = machine1 is available
*/
-static int
+int
virSystemdHasMachined(void)
{
int ret;
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
index dfea75948b..9ce16b7de1 100644
--- a/src/util/virsystemd.h
+++ b/src/util/virsystemd.h
@@ -57,6 +57,8 @@ int virSystemdTerminateMachine(const char *name);
void virSystemdNotifyStartup(void);
+int virSystemdHasMachined(void);
+
int virSystemdHasLogind(void);
int virSystemdCanSuspend(bool *result);
--
2.29.2
7:26 a.m.
New subject: [libvirt PATCH 2/9] virsystemd: introduce virSystemdGetMachineByPID
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/util/virsystemd.c | 52 ++++++++++++++++++++++++++++++-------------
1 file changed, 36 insertions(+), 16 deletions(-)
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index 58b5c650b5..d2fcf0bf0c 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -194,21 +194,21 @@ virSystemdHasLogind(void)
}
-char *
-virSystemdGetMachineNameByPID(pid_t pid)
+/**
+ * virSystemdGetMachineByPID:
+ * @conn: dbus connection
+ * @pid: pid of running VM
+ *
+ * Returns dbus object path to VM registered with machined.
+ * On error returns NULL.
+ */
+static char *
+virSystemdGetMachineByPID(GDBusConnection *conn,
+ pid_t pid)
{
- GDBusConnection *conn;
g_autoptr(GVariant) message = NULL;
g_autoptr(GVariant) reply = NULL;
- g_autoptr(GVariant) gvar = NULL;
- g_autofree char *object = NULL;
- char *name = NULL;
-
- if (virSystemdHasMachined() < 0)
- return NULL;
-
- if (!(conn = virGDBusGetSystemBus()))
- return NULL;
+ char *object = NULL;
message = g_variant_new("(u)", pid);
@@ -225,13 +225,33 @@ virSystemdGetMachineNameByPID(pid_t pid)
g_variant_get(reply, "(o)", &object);
- g_variant_unref(reply);
- reply = NULL;
-
VIR_DEBUG("Domain with pid %lld has object path '%s'",
(long long) pid, object);
- g_variant_unref(message);
+ return object;
+}
+
+
+char *
+virSystemdGetMachineNameByPID(pid_t pid)
+{
+ GDBusConnection *conn;
+ g_autoptr(GVariant) message = NULL;
+ g_autoptr(GVariant) reply = NULL;
+ g_autoptr(GVariant) gvar = NULL;
+ g_autofree char *object = NULL;
+ char *name = NULL;
+
+ if (virSystemdHasMachined() < 0)
+ return NULL;
+
+ if (!(conn = virGDBusGetSystemBus()))
+ return NULL;
+
+ object = virSystemdGetMachineByPID(conn, pid);
+ if (!object)
+ return NULL;
+
message = g_variant_new("(ss)",
"org.freedesktop.machine1.Machine",
"Name");
--
2.29.2
7:26 a.m.
New subject: [libvirt PATCH 3/9] virsystemd: introduce virSystemdGetMachineUnitByPID
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virsystemd.c | 51 ++++++++++++++++++++++++++++++++++++++++
src/util/virsystemd.h | 2 ++
tests/virsystemdtest.c | 39 ++++++++++++++++++++++++++----
4 files changed, 88 insertions(+), 5 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b2d2282ce0..837986845f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3302,6 +3302,7 @@ virSystemdCanSuspend;
virSystemdCreateMachine;
virSystemdGetActivation;
virSystemdGetMachineNameByPID;
+virSystemdGetMachineUnitByPID;
virSystemdHasLogind;
virSystemdHasLogindResetCachedValue;
virSystemdHasMachined;
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
index d2fcf0bf0c..c109324e96 100644
--- a/src/util/virsystemd.c
+++ b/src/util/virsystemd.c
@@ -276,6 +276,57 @@ virSystemdGetMachineNameByPID(pid_t pid)
}
+/**
+ * virSystemdGetMachineUnitByPID:
+ * @pid: pid of running VM
+ *
+ * Returns systemd Unit name of a running VM registered with machined.
+ * On error returns NULL.
+ */
+char *
+virSystemdGetMachineUnitByPID(pid_t pid)
+{
+ GDBusConnection *conn;
+ g_autoptr(GVariant) message = NULL;
+ g_autoptr(GVariant) reply = NULL;
+ g_autoptr(GVariant) gvar = NULL;
+ g_autofree char *object = NULL;
+ char *unit = NULL;
+
+ if (virSystemdHasMachined() < 0)
+ return NULL;
+
+ if (!(conn = virGDBusGetSystemBus()))
+ return NULL;
+
+ object = virSystemdGetMachineByPID(conn, pid);
+ if (!object)
+ return NULL;
+
+ message = g_variant_new("(ss)",
+ "org.freedesktop.machine1.Machine",
"Unit");
+
+ if (virGDBusCallMethod(conn,
+ &reply,
+ G_VARIANT_TYPE("(v)"),
+ NULL,
+ "org.freedesktop.machine1",
+ object,
+ "org.freedesktop.DBus.Properties",
+ "Get",
+ message) < 0)
+ return NULL;
+
+ g_variant_get(reply, "(v)", &gvar);
+ g_variant_get(gvar, "s", &unit);
+
+ VIR_DEBUG("Domain with pid %lld has unit name '%s'",
+ (long long) pid, unit);
+
+ return unit;
+}
+
+
/**
* virSystemdCreateMachine:
* @name: driver unique name of the machine
diff --git a/src/util/virsystemd.h b/src/util/virsystemd.h
index 9ce16b7de1..cd329c49f9 100644
--- a/src/util/virsystemd.h
+++ b/src/util/virsystemd.h
@@ -69,6 +69,8 @@ int virSystemdCanHybridSleep(bool *result);
char *virSystemdGetMachineNameByPID(pid_t pid);
+char *virSystemdGetMachineUnitByPID(pid_t pid);
+
int virSystemdGetActivation(virSystemdActivationMap *map,
size_t nmap,
virSystemdActivationPtr *act);
diff --git a/tests/virsystemdtest.c b/tests/virsystemdtest.c
index bd0ca51140..b48cdb950c 100644
--- a/tests/virsystemdtest.c
+++ b/tests/virsystemdtest.c
@@ -53,11 +53,10 @@ VIR_MOCK_WRAP_RET_ARGS(g_dbus_connection_call_sync,
GError **, error)
{
GVariant *reply = NULL;
+ g_autoptr(GVariant) params = parameters;
- if (parameters) {
- g_variant_ref_sink(parameters);
- g_variant_unref(parameters);
- }
+ if (params)
+ g_variant_ref_sink(params);
VIR_MOCK_REAL_INIT(g_dbus_connection_call_sync);
@@ -71,7 +70,19 @@ VIR_MOCK_WRAP_RET_ARGS(g_dbus_connection_call_sync,
reply = g_variant_new("(o)",
"/org/freedesktop/machine1/machine/qemu_2ddemo");
} else if (STREQ(method_name, "Get")) {
- reply = g_variant_new("(v)",
g_variant_new_string("qemu-demo"));
+ const char *prop;
+ g_variant_get(params, "(@s&s)", NULL, &prop);
+
+ if (STREQ(prop, "Name")) {
+ reply = g_variant_new("(v)",
g_variant_new_string("qemu-demo"));
+ } else if (STREQ(prop, "Unit")) {
+ reply = g_variant_new("(v)",
+
g_variant_new_string("machine-qemu-demo.scope"));
+ } else {
+ *error = g_dbus_error_new_for_dbus_error(
+ "org.freedesktop.systemd.badthing",
+ "Unknown machine property");
+ }
} else {
reply = g_variant_new("()");
}
@@ -330,6 +341,23 @@ testGetMachineName(const void *opaque G_GNUC_UNUSED)
}
+static int
+testGetMachineUnit(const void *opaque G_GNUC_UNUSED)
+{
+ g_autofree char *tmp = virSystemdGetMachineUnitByPID(1234);
+
+ if (!tmp) {
+ fprintf(stderr, "%s", "Failed to create get machine
unit\n");
+ return -1;
+ }
+
+ if (STREQ(tmp, "machine-qemu-demo.scope"))
+ return 0;
+
+ return -1;
+}
+
+
struct testNameData {
const char *name;
const char *expected;
@@ -656,6 +684,7 @@ mymain(void)
DO_TEST("Test create bad systemd ", testCreateBadSystemd);
DO_TEST("Test create with network ", testCreateNetwork);
DO_TEST("Test getting machine name ", testGetMachineName);
+ DO_TEST("Test getting machine unit ", testGetMachineUnit);
# define TEST_SCOPE(_name, unitname, _legacy) \
do { \
--
2.29.2
7:26 a.m.
New subject: [libvirt PATCH 4/9] vircgroup: use DBus call to systemd for some APIs
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(a)redhat.com>
---
src/util/vircgroup.c | 41 ++++++++++++++++++++++++++++++++++
src/util/vircgrouppriv.h | 6 +++++
src/util/vircgroupv1.c | 48 ++++++++++++++++++++++++++++------------
src/util/vircgroupv2.c | 48 ++++++++++++++++++++++++++++------------
4 files changed, 115 insertions(+), 28 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index b5c74e94de..7a0e14eb73 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -41,6 +41,7 @@
#include "virerror.h"
#include "virlog.h"
#include "virfile.h"
+#include "virgdbus.h"
#include "virhash.h"
#include "virstring.h"
#include "virsystemd.h"
@@ -484,6 +485,37 @@ virCgroupGetBlockDevString(const char *path)
}
+int
+virCgroupSetValueDBus(const char *unitName,
+ const char *key,
+ GVariant *value)
+{
+ GDBusConnection *conn;
+ g_autoptr(GVariant) message = NULL;
+ GVariantBuilder builder;
+ GVariant *props = NULL;
+
+ g_variant_builder_init(&builder, G_VARIANT_TYPE("a(sv)"));
+ g_variant_builder_add(&builder, "(sv)", key, value);
+ props = g_variant_builder_end(&builder);
+
+ message = g_variant_new("(sb@a(sv))", unitName, true, props);
+
+ if (!(conn = virGDBusGetSystemBus()))
+ return -1;
+
+ return virGDBusCallMethod(conn,
+ NULL,
+ NULL,
+ NULL,
+ "org.freedesktop.systemd1",
+ "/org/freedesktop/systemd1",
+ "org.freedesktop.systemd1.Manager",
+ "SetUnitProperties",
+ message);
+}
+
+
int
virCgroupSetValueRaw(const char *path,
const char *value)
@@ -1129,6 +1161,10 @@ virCgroupNewDetectMachine(const char *name,
}
}
+ newGroup->unitName = virSystemdGetMachineUnitByPID(pid);
+ if (virSystemdHasMachined() == 0 && !newGroup->unitName)
+ return -1;
+
*group = g_steal_pointer(&newGroup);
return 0;
}
@@ -1229,6 +1265,10 @@ virCgroupNewMachineSystemd(const char *name,
if (virCgroupEnableMissingControllers(path, controllers, &newGroup) < 0)
return -1;
+ newGroup->unitName = virSystemdGetMachineUnitByPID(pidleader);
+ if (!newGroup->unitName)
+ return -1;
+
if (virCgroupAddProcess(newGroup, pidleader) < 0) {
virErrorPtr saved;
@@ -3588,6 +3628,7 @@ virCgroupFree(virCgroupPtr group)
g_free(group->unified.mountPoint);
g_free(group->unified.placement);
+ g_free(group->unitName);
g_free(group);
}
diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h
index 85ba5393e0..f88d00814d 100644
--- a/src/util/vircgrouppriv.h
+++ b/src/util/vircgrouppriv.h
@@ -64,8 +64,14 @@ struct _virCgroup {
virCgroupV1Controller legacy[VIR_CGROUP_CONTROLLER_LAST];
virCgroupV2Controller unified;
+
+ char *unitName;
};
+int virCgroupSetValueDBus(const char *unitName,
+ const char *key,
+ GVariant *value);
+
int virCgroupSetValueRaw(const char *path,
const char *value);
diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index 2b4d625c35..edb2c23bbc 100644
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -946,7 +946,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) {
@@ -968,9 +967,15 @@ virCgroupV1SetBlkioWeight(virCgroupPtr group,
return -1;
}
- value = g_strdup_printf("%u", weight);
+ if (group->unitName) {
+ GVariant *value = g_variant_new("t", weight);
- return virCgroupSetValueRaw(path, value);
+ return virCgroupSetValueDBus(group->unitName, "BlockIOWeight",
value);
+ } else {
+ g_autofree char *value = g_strdup_printf("%u", weight);
+
+ return virCgroupSetValueRaw(path, value);
+ }
}
@@ -1203,15 +1208,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;
@@ -1223,7 +1221,23 @@ virCgroupV1SetBlkioDeviceWeight(virCgroupPtr group,
return -1;
}
- return virCgroupSetValueRaw(path, str);
+ if (group->unitName) {
+ GVariant *value = NULL;
+
+ value = g_variant_new_parsed("[(%s, uint64 %u)]", path, weight);
+
+ return virCgroupSetValueDBus(group->unitName, "BlockIODeviceWeight",
value);
+ } 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);
+ }
}
@@ -1862,9 +1876,15 @@ static int
virCgroupV1SetCpuShares(virCgroupPtr group,
unsigned long long shares)
{
- return virCgroupSetValueU64(group,
- VIR_CGROUP_CONTROLLER_CPU,
- "cpu.shares", shares);
+ if (group->unitName) {
+ GVariant *value = g_variant_new("t", shares);
+
+ return virCgroupSetValueDBus(group->unitName, "CPUShares", value);
+ } else {
+ return virCgroupSetValueU64(group,
+ VIR_CGROUP_CONTROLLER_CPU,
+ "cpu.shares", shares);
+ }
}
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index 4a239f067a..3f3e5ac83d 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -609,7 +609,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,
@@ -633,9 +632,15 @@ virCgroupV2SetBlkioWeight(virCgroupPtr group,
return -1;
}
- value = g_strdup_printf(format, weight);
+ if (group->unitName) {
+ GVariant *value = g_variant_new("t", weight);
- return virCgroupSetValueRaw(path, value);
+ return virCgroupSetValueDBus(group->unitName, "IOWeight", value);
+ } else {
+ g_autofree char *value = g_strdup_printf(format, weight);
+
+ return virCgroupSetValueRaw(path, value);
+ }
}
@@ -820,13 +825,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) {
@@ -839,7 +837,23 @@ virCgroupV2SetBlkioDeviceWeight(virCgroupPtr group,
return -1;
}
- return virCgroupSetValueRaw(path, str);
+ if (group->unitName) {
+ GVariant *value = NULL;
+
+ value = g_variant_new_parsed("[(%s, uint64 %u)]", path, weight);
+
+ return virCgroupSetValueDBus(group->unitName, "IODeviceWeight",
value);
+ } 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);
+ }
}
@@ -1458,9 +1472,15 @@ static int
virCgroupV2SetCpuShares(virCgroupPtr group,
unsigned long long shares)
{
- return virCgroupSetValueU64(group,
- VIR_CGROUP_CONTROLLER_CPU,
- "cpu.weight", shares);
+ if (group->unitName) {
+ GVariant *value = g_variant_new("t", shares);
+
+ return virCgroupSetValueDBus(group->unitName, "CPUWeight", value);
+ } else {
+ return virCgroupSetValueU64(group,
+ VIR_CGROUP_CONTROLLER_CPU,
+ "cpu.weight", shares);
+ }
}
--
2.29.2
7:26 a.m.
New subject: [libvirt PATCH 5/9] vircgroupv1: refactor virCgroupV1DetectPlacement
Remove one level of indentation by splitting the condition.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/util/vircgroupv1.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index edb2c23bbc..f4dd20fd73 100644
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -339,23 +339,28 @@ virCgroupV1DetectPlacement(virCgroupPtr group,
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
const char *typestr = virCgroupV1ControllerTypeToString(i);
- if (virCgroupV1MountOptsMatchController(controllers, typestr) &&
- group->legacy[i].mountPoint != NULL &&
- group->legacy[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) {
- group->legacy[i].placement = g_strdup(selfpath);
- } else {
- bool delim = STREQ(selfpath, "/") || STREQ(path,
"");
+ if (!virCgroupV1MountOptsMatchController(controllers, typestr))
+ continue;
- group->legacy[i].placement = g_strdup_printf("%s%s%s",
selfpath,
- delim ? "" :
"/",
- path);
- }
+ if (!group->legacy[i].mountPoint)
+ continue;
+
+ if (group->legacy[i].placement)
+ continue;
+
+ /*
+ * selfpath == "/" + path="" -> "/"
+ * selfpath == "/libvirt.service" + path == "" ->
"/libvirt.service"
+ * selfpath == "/libvirt.service" + path == "foo" ->
"/libvirt.service/foo"
+ */
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
+ group->legacy[i].placement = g_strdup(selfpath);
+ } else {
+ bool delim = STREQ(selfpath, "/") || STREQ(path, "");
+
+ group->legacy[i].placement = g_strdup_printf("%s%s%s",
selfpath,
+ delim ? "" :
"/",
+ path);
}
}
--
2.29.2
7:26 a.m.
New subject: [libvirt PATCH 6/9] vircgroupv2: move task into cgroup before enabling controllers
When we create a new child cgroup and the parent cgroup has any process
attached to it enabling controllers for the child cgroup fails with
error. We need to move the process into the child cgroup first before
enabling any controllers.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/util/vircgroup.c | 11 ++++++-----
src/util/vircgroupbackend.h | 1 +
src/util/vircgroupv1.c | 1 +
src/util/vircgroupv2.c | 13 +++++++++++++
4 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 7a0e14eb73..44a3064876 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -694,13 +694,14 @@ static int
virCgroupMakeGroup(virCgroupPtr parent,
virCgroupPtr group,
bool create,
+ pid_t pid,
unsigned int flags)
{
size_t i;
for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
if (group->backends[i] &&
- group->backends[i]->makeGroup(parent, group, create, flags) < 0) {
+ group->backends[i]->makeGroup(parent, group, create, pid, flags) <
0) {
virCgroupRemove(group);
return -1;
}
@@ -970,7 +971,7 @@ virCgroupNewPartition(const char *path,
return -1;
if (parent) {
- if (virCgroupMakeGroup(parent, newGroup, create, VIR_CGROUP_NONE) < 0)
+ if (virCgroupMakeGroup(parent, newGroup, create, -1, VIR_CGROUP_NONE) < 0)
return -1;
}
@@ -1033,7 +1034,7 @@ virCgroupNewDomainPartition(virCgroupPtr partition,
* a group for driver, is to avoid overhead to track
* cumulative usage that we don't need.
*/
- if (virCgroupMakeGroup(partition, newGroup, true,
+ if (virCgroupMakeGroup(partition, newGroup, true, -1,
VIR_CGROUP_MEM_HIERACHY) < 0) {
return -1;
}
@@ -1090,7 +1091,7 @@ virCgroupNewThread(virCgroupPtr domain,
if (virCgroupNewFromParent(domain, name, controllers, &newGroup) < 0)
return -1;
- if (virCgroupMakeGroup(domain, newGroup, create, VIR_CGROUP_THREAD) < 0)
+ if (virCgroupMakeGroup(domain, newGroup, create, -1, VIR_CGROUP_THREAD) < 0)
return -1;
*group = g_steal_pointer(&newGroup);
@@ -1192,7 +1193,7 @@ virCgroupEnableMissingControllers(char *path,
&tmp) < 0)
return -1;
- if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_SYSTEMD) < 0)
+ if (virCgroupMakeGroup(parent, tmp, true, -1, VIR_CGROUP_SYSTEMD) < 0)
return -1;
virCgroupFree(parent);
diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h
index 4ca2e38af2..497dde5625 100644
--- a/src/util/vircgroupbackend.h
+++ b/src/util/vircgroupbackend.h
@@ -122,6 +122,7 @@ typedef int
(*virCgroupMakeGroupCB)(virCgroupPtr parent,
virCgroupPtr group,
bool create,
+ pid_t pid,
unsigned int flags);
typedef int
diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index f4dd20fd73..51ada12f6e 100644
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -622,6 +622,7 @@ static int
virCgroupV1MakeGroup(virCgroupPtr parent,
virCgroupPtr group,
bool create,
+ pid_t pid G_GNUC_UNUSED,
unsigned int flags)
{
size_t i;
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index 3f3e5ac83d..d590f83187 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -406,10 +406,17 @@ virCgroupV2EnableController(virCgroupPtr group,
}
+static int
+virCgroupV2AddTask(virCgroupPtr group,
+ pid_t pid,
+ unsigned int flags);
+
+
static int
virCgroupV2MakeGroup(virCgroupPtr parent,
virCgroupPtr group,
bool create,
+ pid_t pid,
unsigned int flags)
{
g_autofree char *path = NULL;
@@ -455,6 +462,12 @@ virCgroupV2MakeGroup(virCgroupPtr parent,
}
} else {
size_t i;
+
+ if (pid > 0) {
+ if (virCgroupV2AddTask(group, pid, VIR_CGROUP_TASK_PROCESS) < 0)
+ return -1;
+ }
+
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
int rc;
--
2.29.2
7:26 a.m.
New subject: [libvirt PATCH 7/9] vircgroup: introduce virCgroupV1Exists and virCgroupV2Exists
This will check if the cgroup actually exists on the system.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/util/vircgroupbackend.h | 4 ++++
src/util/vircgroupv1.c | 27 +++++++++++++++++++++++++++
src/util/vircgroupv2.c | 15 +++++++++++++++
3 files changed, 46 insertions(+)
diff --git a/src/util/vircgroupbackend.h b/src/util/vircgroupbackend.h
index 497dde5625..a338ffb618 100644
--- a/src/util/vircgroupbackend.h
+++ b/src/util/vircgroupbackend.h
@@ -118,6 +118,9 @@ typedef int
const char *key,
char **path);
+typedef bool
+(*virCgroupExistsCB)(virCgroupPtr group);
+
typedef int
(*virCgroupMakeGroupCB)(virCgroupPtr parent,
virCgroupPtr group,
@@ -382,6 +385,7 @@ struct _virCgroupBackend {
virCgroupGetAnyControllerCB getAnyController;
virCgroupPathOfControllerCB pathOfController;
virCgroupMakeGroupCB makeGroup;
+ virCgroupExistsCB exists;
virCgroupRemoveCB remove;
virCgroupAddTaskCB addTask;
virCgroupHasEmptyTasksCB hasEmptyTasks;
diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index 51ada12f6e..8a9a63e0b7 100644
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -690,6 +690,32 @@ virCgroupV1MakeGroup(virCgroupPtr parent,
}
+static bool
+virCgroupV1Exists(virCgroupPtr group)
+{
+ size_t i;
+
+ for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
+ g_autofree char *path = NULL;
+
+ if (i == VIR_CGROUP_CONTROLLER_SYSTEMD)
+ continue;
+
+ if (!group->legacy[i].mountPoint)
+ continue;
+
+ if (virCgroupV1PathOfController(group, i, "", &path) < 0)
+ return false;
+
+ if (!virFileExists(path)) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
+
static int
virCgroupV1Remove(virCgroupPtr group)
{
@@ -2154,6 +2180,7 @@ virCgroupBackend virCgroupV1Backend = {
.getAnyController = virCgroupV1GetAnyController,
.pathOfController = virCgroupV1PathOfController,
.makeGroup = virCgroupV1MakeGroup,
+ .exists = virCgroupV1Exists,
.remove = virCgroupV1Remove,
.addTask = virCgroupV1AddTask,
.hasEmptyTasks = virCgroupV1HasEmptyTasks,
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index d590f83187..c96f405dfa 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -499,6 +499,20 @@ virCgroupV2MakeGroup(virCgroupPtr parent,
}
+static bool
+virCgroupV2Exists(virCgroupPtr group)
+{
+ g_autofree char *path = NULL;
+ int controller;
+
+ controller = virCgroupV2GetAnyController(group);
+ if (virCgroupV2PathOfController(group, controller, "", &path) < 0)
+ return false;
+
+ return virFileExists(path);
+}
+
+
static int
virCgroupV2Remove(virCgroupPtr group)
{
@@ -1895,6 +1909,7 @@ virCgroupBackend virCgroupV2Backend = {
.getAnyController = virCgroupV2GetAnyController,
.pathOfController = virCgroupV2PathOfController,
.makeGroup = virCgroupV2MakeGroup,
+ .exists = virCgroupV2Exists,
.remove = virCgroupV2Remove,
.addTask = virCgroupV2AddTask,
.hasEmptyTasks = virCgroupV2HasEmptyTasks,
--
2.29.2
7:26 a.m.
New subject: [libvirt PATCH 8/9] vircgroup: introduce nested cgroup to properly work with systemd
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.
We already use DBus calls for some of the APIs but for the remaining
ones we will continue accessing the files directly. Systemd doesn't
support threaded cgroups so we need to do this.
The reason why we don't use DBus for most of the APIs is that we already
have a code that works with files and we would have to check if systemd
supports each API.
This change introduces new topology on systemd hosts:
$ROOT
|
+- machine.slice
|
+- machine-qemu\x2d1\x2dvm1.scope
|
+- libvirt
|
+- emulator
+- vcpu0
+- vcpu0
compared to the previous topology:
$ROOT
|
+- machine.slice
|
+- machine-qemu\x2d1\x2dvm1.scope
|
+- emulator
+- vcpu0
+- vcpu0
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
docs/cgroups.html.in | 29 +++--
src/util/vircgroup.c | 250 ++++++++++++++++++++++++++++++---------
src/util/vircgrouppriv.h | 4 +
src/util/vircgroupv1.c | 15 ++-
src/util/vircgroupv2.c | 6 +
5 files changed, 239 insertions(+), 65 deletions(-)
diff --git a/docs/cgroups.html.in b/docs/cgroups.html.in
index 78dede1bba..412a9360ff 100644
--- a/docs/cgroups.html.in
+++ b/docs/cgroups.html.in
@@ -117,21 +117,27 @@ $ROOT
|
+- machine-qemu\x2d1\x2dvm1.scope
| |
- | +- emulator
- | +- vcpu0
- | +- vcpu1
+ | +- libvirt
+ | |
+ | +- emulator
+ | +- vcpu0
+ | +- vcpu1
|
+- machine-qemu\x2d2\x2dvm2.scope
| |
- | +- emulator
- | +- vcpu0
- | +- vcpu1
+ | +- libvirt
+ | |
+ | +- emulator
+ | +- vcpu0
+ | +- vcpu1
|
+- machine-qemu\x2d3\x2dvm3.scope
| |
- | +- emulator
- | +- vcpu0
- | +- vcpu1
+ | +- libvirt
+ | |
+ | +- emulator
+ | +- vcpu0
+ | +- vcpu1
|
+- machine-engineering.slice
| |
@@ -148,6 +154,11 @@ $ROOT
+- machine-lxc\x2d33333\x2dcontainer3.scope
</pre>
+ <p>
+ Prior libvirt 7.1.0 the topology doesn't have extra
+ <code>libvirt</code> directory.
+ </p>
+
<h3><a id="currentLayoutGeneric">Non-systemd cgroups
layout</a></h3>
<p>
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 44a3064876..402224813b 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -711,6 +711,22 @@ virCgroupMakeGroup(virCgroupPtr parent,
}
+static bool
+virCgroupExists(virCgroupPtr group)
+{
+ size_t i;
+
+ for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
+ if (group->backends[i] &&
+ !group->backends[i]->exists(group)) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
+
/**
* virCgroupNew:
* @path: path for the new group
@@ -764,10 +780,11 @@ virCgroupAddTaskInternal(virCgroupPtr group,
unsigned int flags)
{
size_t i;
+ virCgroupPtr parent = virCgroupGetNested(group);
for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
- if (group->backends[i] &&
- group->backends[i]->addTask(group, pid, flags) < 0) {
+ if (parent->backends[i] &&
+ parent->backends[i]->addTask(parent, pid, flags) < 0) {
return -1;
}
}
@@ -980,6 +997,28 @@ virCgroupNewPartition(const char *path,
}
+static int
+virCgroupNewNested(virCgroupPtr parent,
+ int controllers,
+ bool create,
+ pid_t pid,
+ virCgroupPtr *nested)
+{
+ g_autoptr(virCgroup) new = NULL;
+
+ if (virCgroupNewFromParent(parent, "libvirt", controllers, &new) <
0)
+ return -1;
+
+ if (create) {
+ if (virCgroupMakeGroup(parent, new, create, pid, VIR_CGROUP_NONE) < 0)
+ return -1;
+ }
+
+ *nested = g_steal_pointer(&new);
+ return 0;
+}
+
+
/**
* virCgroupNewSelf:
*
@@ -1064,6 +1103,7 @@ virCgroupNewThread(virCgroupPtr domain,
{
g_autofree char *name = NULL;
g_autoptr(virCgroup) newGroup = NULL;
+ virCgroupPtr parent = NULL;
int controllers;
*group = NULL;
@@ -1088,10 +1128,12 @@ virCgroupNewThread(virCgroupPtr domain,
(1 << VIR_CGROUP_CONTROLLER_CPUACCT) |
(1 << VIR_CGROUP_CONTROLLER_CPUSET));
- if (virCgroupNewFromParent(domain, name, controllers, &newGroup) < 0)
+ parent = virCgroupGetNested(domain);
+
+ if (virCgroupNewFromParent(parent, name, controllers, &newGroup) < 0)
return -1;
- if (virCgroupMakeGroup(domain, newGroup, create, -1, VIR_CGROUP_THREAD) < 0)
+ if (virCgroupMakeGroup(parent, newGroup, create, -1, VIR_CGROUP_THREAD) < 0)
return -1;
*group = g_steal_pointer(&newGroup);
@@ -1142,6 +1184,7 @@ virCgroupNewDetectMachine(const char *name,
{
size_t i;
g_autoptr(virCgroup) newGroup = NULL;
+ g_autoptr(virCgroup) nested = NULL;
*group = NULL;
@@ -1166,6 +1209,12 @@ virCgroupNewDetectMachine(const char *name,
if (virSystemdHasMachined() == 0 && !newGroup->unitName)
return -1;
+ if (virCgroupNewNested(newGroup, controllers, false, -1, &nested) < 0)
+ return -1;
+
+ if (virCgroupExists(nested))
+ newGroup->nested = g_steal_pointer(&nested);
+
*group = g_steal_pointer(&newGroup);
return 0;
}
@@ -1225,6 +1274,7 @@ virCgroupNewMachineSystemd(const char *name,
int rv;
g_autoptr(virCgroup) init = NULL;
g_autoptr(virCgroup) newGroup = NULL;
+ g_autoptr(virCgroup) nested = NULL;
g_autofree char *path = NULL;
size_t i;
@@ -1270,6 +1320,11 @@ virCgroupNewMachineSystemd(const char *name,
if (!newGroup->unitName)
return -1;
+ if (virCgroupNewNested(newGroup, controllers, true, pidleader, &nested) < 0)
+ return -1;
+
+ newGroup->nested = g_steal_pointer(&nested);
+
if (virCgroupAddProcess(newGroup, pidleader) < 0) {
virErrorPtr saved;
@@ -1457,7 +1512,9 @@ virCgroupGetBlkioIoServiced(virCgroupPtr group,
long long *requests_read,
long long *requests_write)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
getBlkioIoServiced, -1,
bytes_read, bytes_write,
requests_read, requests_write);
@@ -1484,7 +1541,9 @@ virCgroupGetBlkioIoDeviceServiced(virCgroupPtr group,
long long *requests_read,
long long *requests_write)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
getBlkioIoDeviceServiced, -1,
path, bytes_read, bytes_write,
requests_read, requests_write);
@@ -1535,7 +1594,9 @@ virCgroupSetBlkioDeviceReadIops(virCgroupPtr group,
const char *path,
unsigned int riops)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
setBlkioDeviceReadIops, -1, path, riops);
}
@@ -1553,7 +1614,9 @@ virCgroupSetBlkioDeviceWriteIops(virCgroupPtr group,
const char *path,
unsigned int wiops)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
setBlkioDeviceWriteIops, -1, path, wiops);
}
@@ -1571,7 +1634,9 @@ virCgroupSetBlkioDeviceReadBps(virCgroupPtr group,
const char *path,
unsigned long long rbps)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
setBlkioDeviceReadBps, -1, path, rbps);
}
@@ -1588,7 +1653,9 @@ virCgroupSetBlkioDeviceWriteBps(virCgroupPtr group,
const char *path,
unsigned long long wbps)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
setBlkioDeviceWriteBps, -1, path, wbps);
}
@@ -1624,7 +1691,9 @@ virCgroupGetBlkioDeviceReadIops(virCgroupPtr group,
const char *path,
unsigned int *riops)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
getBlkioDeviceReadIops, -1, path, riops);
}
@@ -1641,7 +1710,9 @@ virCgroupGetBlkioDeviceWriteIops(virCgroupPtr group,
const char *path,
unsigned int *wiops)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
getBlkioDeviceWriteIops, -1, path, wiops);
}
@@ -1658,7 +1729,9 @@ virCgroupGetBlkioDeviceReadBps(virCgroupPtr group,
const char *path,
unsigned long long *rbps)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
getBlkioDeviceReadBps, -1, path, rbps);
}
@@ -1675,7 +1748,9 @@ virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group,
const char *path,
unsigned long long *wbps)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_BLKIO,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_BLKIO,
getBlkioDeviceWriteBps, -1, path, wbps);
}
@@ -1708,7 +1783,9 @@ virCgroupGetBlkioDeviceWeight(virCgroupPtr group,
int
virCgroupSetMemory(virCgroupPtr group, unsigned long long kb)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
setMemory, -1, kb);
}
@@ -1735,7 +1812,9 @@ virCgroupGetMemoryStat(virCgroupPtr group,
unsigned long long *inactiveFile,
unsigned long long *unevictable)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
getMemoryStat, -1, cache,
activeAnon, inactiveAnon,
activeFile, inactiveFile,
@@ -1754,7 +1833,9 @@ virCgroupGetMemoryStat(virCgroupPtr group,
int
virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
getMemoryUsage, -1, kb);
}
@@ -1770,7 +1851,9 @@ virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb)
int
virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
setMemoryHardLimit, -1, kb);
}
@@ -1786,7 +1869,9 @@ virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long
kb)
int
virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long *kb)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
getMemoryHardLimit, -1, kb);
}
@@ -1802,7 +1887,9 @@ virCgroupGetMemoryHardLimit(virCgroupPtr group, unsigned long long
*kb)
int
virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long kb)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
setMemorySoftLimit, -1, kb);
}
@@ -1818,7 +1905,9 @@ virCgroupSetMemorySoftLimit(virCgroupPtr group, unsigned long long
kb)
int
virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
getMemorySoftLimit, -1, kb);
}
@@ -1834,7 +1923,9 @@ virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long
*kb)
int
virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long kb)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
setMemSwapHardLimit, -1, kb);
}
@@ -1850,7 +1941,9 @@ virCgroupSetMemSwapHardLimit(virCgroupPtr group, unsigned long long
kb)
int
virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long *kb)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
getMemSwapHardLimit, -1, kb);
}
@@ -1866,7 +1959,9 @@ virCgroupGetMemSwapHardLimit(virCgroupPtr group, unsigned long long
*kb)
int
virCgroupGetMemSwapUsage(virCgroupPtr group, unsigned long long *kb)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_MEMORY,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_MEMORY,
getMemSwapUsage, -1, kb);
}
@@ -1882,7 +1977,9 @@ virCgroupGetMemSwapUsage(virCgroupPtr group, unsigned long long
*kb)
int
virCgroupSetCpusetMems(virCgroupPtr group, const char *mems)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPUSET,
setCpusetMems, -1, mems);
}
@@ -1898,7 +1995,9 @@ virCgroupSetCpusetMems(virCgroupPtr group, const char *mems)
int
virCgroupGetCpusetMems(virCgroupPtr group, char **mems)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPUSET,
getCpusetMems, -1, mems);
}
@@ -1914,7 +2013,9 @@ virCgroupGetCpusetMems(virCgroupPtr group, char **mems)
int
virCgroupSetCpusetMemoryMigrate(virCgroupPtr group, bool migrate)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPUSET,
setCpusetMemoryMigrate, -1, migrate);
}
@@ -1930,7 +2031,9 @@ virCgroupSetCpusetMemoryMigrate(virCgroupPtr group, bool migrate)
int
virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPUSET,
getCpusetMemoryMigrate, -1, migrate);
}
@@ -1946,7 +2049,9 @@ virCgroupGetCpusetMemoryMigrate(virCgroupPtr group, bool *migrate)
int
virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPUSET,
setCpusetCpus, -1, cpus);
}
@@ -1962,7 +2067,9 @@ virCgroupSetCpusetCpus(virCgroupPtr group, const char *cpus)
int
virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUSET,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPUSET,
getCpusetCpus, -1, cpus);
}
@@ -1977,7 +2084,9 @@ virCgroupGetCpusetCpus(virCgroupPtr group, char **cpus)
int
virCgroupDenyAllDevices(virCgroupPtr group)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_DEVICES,
denyAllDevices, -1);
}
@@ -1998,7 +2107,9 @@ virCgroupDenyAllDevices(virCgroupPtr group)
int
virCgroupAllowAllDevices(virCgroupPtr group, int perms)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_DEVICES,
allowAllDevices, -1, perms);
}
@@ -2018,7 +2129,9 @@ int
virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor,
int perms)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_DEVICES,
allowDevice, -1, type, major, minor, perms);
}
@@ -2044,6 +2157,7 @@ virCgroupAllowDevicePath(virCgroupPtr group,
bool ignoreEacces)
{
struct stat sb;
+ virCgroupPtr parent = virCgroupGetNested(group);
if (stat(path, &sb) < 0) {
if (errno == EACCES && ignoreEacces)
@@ -2058,7 +2172,7 @@ virCgroupAllowDevicePath(virCgroupPtr group,
if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode))
return 1;
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES,
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_DEVICES,
allowDevice, -1,
S_ISCHR(sb.st_mode) ? 'c' : 'b',
major(sb.st_rdev),
@@ -2082,7 +2196,9 @@ int
virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor,
int perms)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_DEVICES,
denyDevice, -1, type, major, minor, perms);
}
@@ -2108,6 +2224,7 @@ virCgroupDenyDevicePath(virCgroupPtr group,
bool ignoreEacces)
{
struct stat sb;
+ virCgroupPtr parent = virCgroupGetNested(group);
if (stat(path, &sb) < 0) {
if (errno == EACCES && ignoreEacces)
@@ -2122,7 +2239,7 @@ virCgroupDenyDevicePath(virCgroupPtr group,
if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode))
return 1;
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_DEVICES,
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_DEVICES,
denyDevice, -1,
S_ISCHR(sb.st_mode) ? 'c' : 'b',
major(sb.st_rdev),
@@ -2383,7 +2500,9 @@ virCgroupGetCpuShares(virCgroupPtr group, unsigned long long
*shares)
int
virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long cfs_period)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPU,
setCpuCfsPeriod, -1, cfs_period);
}
@@ -2399,7 +2518,9 @@ virCgroupSetCpuCfsPeriod(virCgroupPtr group, unsigned long long
cfs_period)
int
virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long *cfs_period)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPU,
getCpuCfsPeriod, -1, cfs_period);
}
@@ -2416,7 +2537,9 @@ virCgroupGetCpuCfsPeriod(virCgroupPtr group, unsigned long long
*cfs_period)
int
virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPU,
setCpuCfsQuota, -1, cfs_quota);
}
@@ -2424,7 +2547,9 @@ virCgroupSetCpuCfsQuota(virCgroupPtr group, long long cfs_quota)
int
virCgroupGetCpuacctPercpuUsage(virCgroupPtr group, char **usage)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUACCT,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPUACCT,
getCpuacctPercpuUsage, -1, usage);
}
@@ -2727,7 +2852,9 @@ virCgroupKillPainfully(virCgroupPtr group)
int
virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPU,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPU,
getCpuCfsQuota, -1, cfs_quota);
}
@@ -2735,7 +2862,9 @@ virCgroupGetCpuCfsQuota(virCgroupPtr group, long long *cfs_quota)
int
virCgroupGetCpuacctUsage(virCgroupPtr group, unsigned long long *usage)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUACCT,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPUACCT,
getCpuacctUsage, -1, usage);
}
@@ -2744,7 +2873,9 @@ int
virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long *user,
unsigned long long *sys)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_CPUACCT,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPUACCT,
getCpuacctStat, -1, user, sys);
}
@@ -2752,7 +2883,9 @@ virCgroupGetCpuacctStat(virCgroupPtr group, unsigned long long
*user,
int
virCgroupSetFreezerState(virCgroupPtr group, const char *state)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_FREEZER,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_FREEZER,
setFreezerState, -1, state);
}
@@ -2760,7 +2893,9 @@ virCgroupSetFreezerState(virCgroupPtr group, const char *state)
int
virCgroupGetFreezerState(virCgroupPtr group, char **state)
{
- VIR_CGROUP_BACKEND_CALL(group, VIR_CGROUP_CONTROLLER_FREEZER,
+ virCgroupPtr parent = virCgroupGetNested(group);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_FREEZER,
getFreezerState, -1, state);
}
@@ -2770,10 +2905,11 @@ virCgroupBindMount(virCgroupPtr group, const char *oldroot,
const char *mountopts)
{
size_t i;
+ virCgroupPtr parent = virCgroupGetNested(group);
for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
- if (group->backends[i] &&
- group->backends[i]->bindMount(group, oldroot, mountopts) < 0) {
+ if (parent->backends[i] &&
+ parent->backends[i]->bindMount(parent, oldroot, mountopts) < 0) {
return -1;
}
}
@@ -2788,10 +2924,11 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
int controllers)
{
size_t i;
+ virCgroupPtr parent = virCgroupGetNested(cgroup);
for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
- if (cgroup->backends[i] &&
- cgroup->backends[i]->setOwner(cgroup, uid, gid, controllers) < 0) {
+ if (parent->backends[i] &&
+ parent->backends[i]->setOwner(parent, uid, gid, controllers) < 0) {
return -1;
}
}
@@ -2810,7 +2947,9 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
bool
virCgroupSupportsCpuBW(virCgroupPtr cgroup)
{
- VIR_CGROUP_BACKEND_CALL(cgroup, VIR_CGROUP_CONTROLLER_CPU,
+ virCgroupPtr parent = virCgroupGetNested(cgroup);
+
+ VIR_CGROUP_BACKEND_CALL(parent, VIR_CGROUP_CONTROLLER_CPU,
supportsCpuBW, false);
}
@@ -2818,10 +2957,11 @@ int
virCgroupHasEmptyTasks(virCgroupPtr cgroup, int controller)
{
size_t i;
+ virCgroupPtr parent = virCgroupGetNested(cgroup);
for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
- if (cgroup->backends[i]) {
- int rc = cgroup->backends[i]->hasEmptyTasks(cgroup, controller);
+ if (parent->backends[i]) {
+ int rc = parent->backends[i]->hasEmptyTasks(parent, controller);
if (rc <= 0)
return rc;
}
@@ -3630,6 +3770,7 @@ virCgroupFree(virCgroupPtr group)
g_free(group->unified.mountPoint);
g_free(group->unified.placement);
g_free(group->unitName);
+ g_free(group->nested);
g_free(group);
}
@@ -3641,9 +3782,12 @@ virCgroupDelThread(virCgroupPtr cgroup,
int idx)
{
g_autoptr(virCgroup) new_cgroup = NULL;
+ virCgroupPtr parent = NULL;
if (cgroup) {
- if (virCgroupNewThread(cgroup, nameval, idx, false, &new_cgroup) < 0)
+ parent = virCgroupGetNested(cgroup);
+
+ if (virCgroupNewThread(parent, nameval, idx, false, &new_cgroup) < 0)
return -1;
/* Remove the offlined cgroup */
diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h
index f88d00814d..7c63780800 100644
--- a/src/util/vircgrouppriv.h
+++ b/src/util/vircgrouppriv.h
@@ -66,8 +66,12 @@ struct _virCgroup {
virCgroupV2Controller unified;
char *unitName;
+ virCgroupPtr nested;
};
+#define virCgroupGetNested(cgroup) \
+ (cgroup->nested ? cgroup->nested : cgroup)
+
int virCgroupSetValueDBus(const char *unitName,
const char *key,
GVariant *value);
diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index 8a9a63e0b7..79015ddaa4 100644
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -338,6 +338,8 @@ virCgroupV1DetectPlacement(virCgroupPtr group,
for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
const char *typestr = virCgroupV1ControllerTypeToString(i);
+ g_autofree char* placement = NULL;
+ char *tmp = NULL;
if (!virCgroupV1MountOptsMatchController(controllers, typestr))
continue;
@@ -348,17 +350,24 @@ virCgroupV1DetectPlacement(virCgroupPtr group,
if (group->legacy[i].placement)
continue;
+ /* On systemd we create a nested cgroup for some cgroup tasks
+ * but the placement should point to the root cgroup. */
+ placement = g_strdup(selfpath);
+ tmp = g_strrstr(placement, "/libvirt");
+ if (tmp)
+ *tmp = '\0';
+
/*
* selfpath == "/" + path="" -> "/"
* selfpath == "/libvirt.service" + path == "" ->
"/libvirt.service"
* selfpath == "/libvirt.service" + path == "foo" ->
"/libvirt.service/foo"
*/
if (i == VIR_CGROUP_CONTROLLER_SYSTEMD) {
- group->legacy[i].placement = g_strdup(selfpath);
+ group->legacy[i].placement = g_strdup(placement);
} else {
- bool delim = STREQ(selfpath, "/") || STREQ(path, "");
+ bool delim = STREQ(placement, "/") || STREQ(path, "");
- group->legacy[i].placement = g_strdup_printf("%s%s%s",
selfpath,
+ group->legacy[i].placement = g_strdup_printf("%s%s%s",
placement,
delim ? "" :
"/",
path);
}
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index c96f405dfa..0fb5188f45 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -208,6 +208,12 @@ virCgroupV2DetectPlacement(virCgroupPtr group,
if (tmp)
*tmp = '\0';
+ /* On systemd we create a nested cgroup for some cgroup tasks
+ * but the placement should point to the root cgroup. */
+ tmp = g_strrstr(placement, "/libvirt");
+ if (tmp)
+ *tmp = '\0';
+
/*
* selfpath == "/" + path="" -> "/"
* selfpath == "/libvirt.service" + path == "" ->
"/libvirt.service"
--
2.29.2
7:26 a.m.
New subject: [libvirt PATCH 9/9] tests: add cgroup nested tests
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
tests/vircgroupdata/systemd-legacy.cgroups | 12 ++++
tests/vircgroupdata/systemd-legacy.mounts | 11 +++
.../vircgroupdata/systemd-legacy.self.cgroup | 11 +++
tests/vircgroupdata/systemd-unified.cgroups | 13 ++++
tests/vircgroupdata/systemd-unified.mounts | 1 +
.../vircgroupdata/systemd-unified.self.cgroup | 1 +
tests/vircgrouptest.c | 72 +++++++++++++++++++
7 files changed, 121 insertions(+)
create mode 100644 tests/vircgroupdata/systemd-legacy.cgroups
create mode 100644 tests/vircgroupdata/systemd-legacy.mounts
create mode 100644 tests/vircgroupdata/systemd-legacy.self.cgroup
create mode 100644 tests/vircgroupdata/systemd-unified.cgroups
create mode 100644 tests/vircgroupdata/systemd-unified.mounts
create mode 100644 tests/vircgroupdata/systemd-unified.self.cgroup
diff --git a/tests/vircgroupdata/systemd-legacy.cgroups
b/tests/vircgroupdata/systemd-legacy.cgroups
new file mode 100644
index 0000000000..444354e3c8
--- /dev/null
+++ b/tests/vircgroupdata/systemd-legacy.cgroups
@@ -0,0 +1,12 @@
+#subsys_name hierarchy num_cgroups enabled
+blkio 1 1 1
+cpu 2 1 1
+cpuacct 3 1 1
+cpuset 4 1 1
+devices 5 1 1
+freezer 6 1 1
+hugetlb 7 1 1
+memory 8 1 1
+net_cls 9 1 1
+perf_event 10 1 1
+pids 11 1 1
diff --git a/tests/vircgroupdata/systemd-legacy.mounts
b/tests/vircgroupdata/systemd-legacy.mounts
new file mode 100644
index 0000000000..23462e9e68
--- /dev/null
+++ b/tests/vircgroupdata/systemd-legacy.mounts
@@ -0,0 +1,11 @@
+cgroup /not/really/sys/fs/cgroup/blkio cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,blkio 0 0
+cgroup /not/really/sys/fs/cgroup/cpu cgroup rw,seclabel,nosuid,nodev,noexec,relatime,cpu
0 0
+cgroup /not/really/sys/fs/cgroup/cpuacct cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,cpuacct 0 0
+cgroup /not/really/sys/fs/cgroup/cpuset cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,cpuset 0 0
+cgroup /not/really/sys/fs/cgroup/devices cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,devices 0 0
+cgroup /not/really/sys/fs/cgroup/freezer cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,freezer 0 0
+cgroup /not/really/sys/fs/cgroup/hugetlb cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,hugetlb 0 0
+cgroup /not/really/sys/fs/cgroup/memory cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,memory 0 0
+cgroup /not/really/sys/fs/cgroup/net_cls cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,net_cls 0 0
+cgroup /not/really/sys/fs/cgroup/perf_event cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,perf_event 0 0
+cgroup /not/really/sys/fs/cgroup/pids cgroup
rw,seclabel,nosuid,nodev,noexec,relatime,pids 0 0
diff --git a/tests/vircgroupdata/systemd-legacy.self.cgroup
b/tests/vircgroupdata/systemd-legacy.self.cgroup
new file mode 100644
index 0000000000..5c133a3c08
--- /dev/null
+++ b/tests/vircgroupdata/systemd-legacy.self.cgroup
@@ -0,0 +1,11 @@
+1:blkio:/libvirt
+2:cpu:/libvirt/emulator
+3:cpuacct:/libvirt/emulator
+4:cpuset:/libvirt/emulator
+5:devices:/libvirt
+6:freezer:/libvirt
+7:hugetlb:/
+8:memory:/libvirt
+9:net_cls:/libvirt
+10:perf_event:/libvirt
+11:pids:/
diff --git a/tests/vircgroupdata/systemd-unified.cgroups
b/tests/vircgroupdata/systemd-unified.cgroups
new file mode 100644
index 0000000000..e0d8a3561c
--- /dev/null
+++ b/tests/vircgroupdata/systemd-unified.cgroups
@@ -0,0 +1,13 @@
+#subsys_name hierarchy num_cgroups enabled
+cpuset 0 1 1
+cpu 0 1 1
+cpuacct 0 1 1
+blkio 0 1 1
+memory 0 1 1
+devices 0 1 1
+freezer 0 1 1
+net_cls 0 1 1
+perf_event 0 1 1
+net_prio 0 1 1
+hugetlb 0 1 1
+pids 0 1 1
diff --git a/tests/vircgroupdata/systemd-unified.mounts
b/tests/vircgroupdata/systemd-unified.mounts
new file mode 100644
index 0000000000..8225f37f45
--- /dev/null
+++ b/tests/vircgroupdata/systemd-unified.mounts
@@ -0,0 +1 @@
+cgroup2 /not/really/sys/fs/cgroup cgroup2
rw,seclabel,nosuid,nodev,noexec,relatime,nsdelegate 0 0
diff --git a/tests/vircgroupdata/systemd-unified.self.cgroup
b/tests/vircgroupdata/systemd-unified.self.cgroup
new file mode 100644
index 0000000000..6007ce7e18
--- /dev/null
+++ b/tests/vircgroupdata/systemd-unified.self.cgroup
@@ -0,0 +1 @@
+0::/libvirt/emulator
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index 1baa71e61c..85c5a6ebd4 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -576,6 +576,64 @@ static int testCgroupNewForSelfHybrid(const void *args
G_GNUC_UNUSED)
}
+static int testCgroupNewForSelfSystemdLegacy(const void *args G_GNUC_UNUSED)
+{
+ g_autoptr(virCgroup) cgroup = NULL;
+ const char *empty[VIR_CGROUP_CONTROLLER_LAST] = { 0 };
+ const char *mounts[VIR_CGROUP_CONTROLLER_LAST] = {
+ [VIR_CGROUP_CONTROLLER_BLKIO] = "/not/really/sys/fs/cgroup/blkio",
+ [VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup/cpu",
+ [VIR_CGROUP_CONTROLLER_CPUACCT] = "/not/really/sys/fs/cgroup/cpuacct",
+ [VIR_CGROUP_CONTROLLER_CPUSET] = "/not/really/sys/fs/cgroup/cpuset",
+ [VIR_CGROUP_CONTROLLER_DEVICES] = "/not/really/sys/fs/cgroup/devices",
+ [VIR_CGROUP_CONTROLLER_FREEZER] = "/not/really/sys/fs/cgroup/freezer",
+ [VIR_CGROUP_CONTROLLER_MEMORY] = "/not/really/sys/fs/cgroup/memory",
+ [VIR_CGROUP_CONTROLLER_NET_CLS] = "/not/really/sys/fs/cgroup/net_cls",
+ [VIR_CGROUP_CONTROLLER_PERF_EVENT] =
"/not/really/sys/fs/cgroup/perf_event",
+ };
+ const char *placement[VIR_CGROUP_CONTROLLER_LAST] = {
+ [VIR_CGROUP_CONTROLLER_BLKIO] = "",
+ [VIR_CGROUP_CONTROLLER_CPU] = "",
+ [VIR_CGROUP_CONTROLLER_CPUACCT] = "",
+ [VIR_CGROUP_CONTROLLER_CPUSET] = "",
+ [VIR_CGROUP_CONTROLLER_DEVICES] = "",
+ [VIR_CGROUP_CONTROLLER_FREEZER] = "",
+ [VIR_CGROUP_CONTROLLER_MEMORY] = "",
+ [VIR_CGROUP_CONTROLLER_NET_CLS] = "",
+ [VIR_CGROUP_CONTROLLER_PERF_EVENT] = "",
+ };
+
+ if (virCgroupNewSelf(&cgroup) < 0) {
+ fprintf(stderr, "Cannot create cgroup for self\n");
+ return -1;
+ }
+
+ return validateCgroup(cgroup, mounts, empty, placement, NULL, NULL, 0);
+}
+
+
+static int testCgroupNewForSelfSystemdUnified(const void *args G_GNUC_UNUSED)
+{
+ g_autoptr(virCgroup) cgroup = NULL;
+ const char *empty[VIR_CGROUP_CONTROLLER_LAST] = { 0 };
+ unsigned int controllers =
+ (1 << VIR_CGROUP_CONTROLLER_CPU) |
+ (1 << VIR_CGROUP_CONTROLLER_CPUACCT) |
+ (1 << VIR_CGROUP_CONTROLLER_MEMORY) |
+ (1 << VIR_CGROUP_CONTROLLER_DEVICES) |
+ (1 << VIR_CGROUP_CONTROLLER_BLKIO);
+
+ if (virCgroupNewSelf(&cgroup) < 0) {
+ fprintf(stderr, "Cannot create cgroup for self\n");
+ return -1;
+ }
+
+ return validateCgroup(cgroup, empty, empty, empty,
+ "/not/really/sys/fs/cgroup", "",
+ controllers);
+}
+
+
static int testCgroupAvailable(const void *args)
{
bool got = virCgroupAvailable();
@@ -1046,6 +1104,20 @@ mymain(void)
ret = -1;
cleanupFakeFS(fakerootdir);
+ fakerootdir = initFakeFS("legacy", "systemd-legacy");
+ if (virTestRun("New cgroup for self (systemd-legacy)",
+ testCgroupNewForSelfSystemdLegacy, NULL) < 0) {
+ ret = -1;
+ }
+ cleanupFakeFS(fakerootdir);
+
+ fakerootdir = initFakeFS("unified", "systemd-unified");
+ if (virTestRun("New cgroup for self (systemd-unified)",
+ testCgroupNewForSelfSystemdUnified, NULL) < 0) {
+ ret = -1;
+ }
+ cleanupFakeFS(fakerootdir);
+
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.29.2
10:08 a.m.
On 2/9/21 2:26 PM, Pavel Hrdina wrote:
When running on host with systemd there is an ownership issue of
the root VM cgroup. When it is created for us by systemd using machined
the owner of the root VM cgroup is systemd and we should not touch any
of the files as systemd can and will modify any values configured there.
Basically we had the issue since introduction of machined support
in libvirt 1.1.1 back in 2013. With systemd implementing more cgroup
APIs the `systemctl daemon-reload` would change more values configured
by libvirt.
The solution to the issue is to use systemd DBus APIs to configure
cgroups but unfortunately they don't cover everything that libvirt
needs.
For that reason we will use systemd DBus APIs only for values that
affect sibling cgroups where the resources are distributed
proportionally, such as blkio.weight or cpu.shares. For the remaining
resources we will keep the current code where we work with the files
directly but we move everything into a child cgroup of the VM root
cgroup where we are free to do whatever we like including thread
configuration.
Yeah, if only there was a way to tell machined to not touch CGroups
we've created until they offer full set of features.
Pavel Hrdina (9):
virsystemd: export virSystemdHasMachined
virsystemd: introduce virSystemdGetMachineByPID
virsystemd: introduce virSystemdGetMachineUnitByPID
vircgroup: use DBus call to systemd for some APIs
vircgroupv1: refactor virCgroupV1DetectPlacement
vircgroupv2: move task into cgroup before enabling controllers
vircgroup: introduce virCgroupV1Exists and virCgroupV2Exists
vircgroup: introduce nested cgroup to properly work with systemd
tests: add cgroup nested tests
docs/cgroups.html.in | 29 +-
src/libvirt_private.syms | 2 +
src/util/vircgroup.c | 300 ++++++++++++++----
src/util/vircgroupbackend.h | 5 +
src/util/vircgrouppriv.h | 10 +
src/util/vircgroupv1.c | 122 +++++--
src/util/vircgroupv2.c | 82 ++++-
src/util/virsystemd.c | 105 +++++-
src/util/virsystemd.h | 4 +
tests/vircgroupdata/systemd-legacy.cgroups | 12 +
tests/vircgroupdata/systemd-legacy.mounts | 11 +
.../vircgroupdata/systemd-legacy.self.cgroup | 11 +
tests/vircgroupdata/systemd-unified.cgroups | 13 +
tests/vircgroupdata/systemd-unified.mounts | 1 +
.../vircgroupdata/systemd-unified.self.cgroup | 1 +
tests/vircgrouptest.c | 72 +++++
tests/virsystemdtest.c | 39 ++-
17 files changed, 687 insertions(+), 132 deletions(-)
create mode 100644 tests/vircgroupdata/systemd-legacy.cgroups
create mode 100644 tests/vircgroupdata/systemd-legacy.mounts
create mode 100644 tests/vircgroupdata/systemd-legacy.self.cgroup
create mode 100644 tests/vircgroupdata/systemd-unified.cgroups
create mode 100644 tests/vircgroupdata/systemd-unified.mounts
create mode 100644 tests/vircgroupdata/systemd-unified.self.cgroup
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1373
days inactive
1373
days old
10 comments
2 participants
participants (2)
-
Michal Privoznik -
Pavel Hrdina