mirror of
https://gitlab.com/libvirt/libvirt.git
synced 2025-01-03 11:35:19 +00:00
vircgroup: Fix virCgroupKillRecursive() wrt nested controllers
I've encountered the following bug, but only on Gentoo with systemd and CGroupsV2. I've started an LXC container successfully but destroying it reported the following error: error: Failed to destroy domain 'amd64' error: internal error: failed to get cgroup backend for 'pathOfController' Debugging showed, that CGroup hierarchy is full of surprises: /sys/fs/cgroup/machine.slice/machine-lxc\x2d861\x2damd64.scope/ └── libvirt ├── dev-hugepages.mount ├── dev-mqueue.mount ├── init.scope ├── sys-fs-fuse-connections.mount ├── sys-kernel-config.mount ├── sys-kernel-debug.mount ├── sys-kernel-tracing.mount ├── system.slice │ ├── console-getty.service │ ├── dbus.service │ ├── system-getty.slice │ ├── system-modprobe.slice │ ├── systemd-journald.service │ ├── systemd-logind.service │ └── tmp.mount └── user.slice For comparison, here's the same container on recent Rawhide: /sys/fs/cgroup/machine.slice/machine-lxc\x2d13550\x2damd64.scope/ └── libvirt Anyway, those nested directories should not be a problem, because virCgroupKillRecursiveInternal() removes them recursively, right? Sort of. The function really does remove nested directories, but it assumes that every directory has the same controller as the rest. Just take a look at virCgroupV2KillRecursive() - it gets 'Any' controller (the first one it found in ".scope") and then passes it to virCgroupKillRecursiveInternal(). This assumption is not true though. The controllers found in ".scope" are the following: cpuset cpu io memory pids while "libvirt" has fewer: cpuset cpu io memory Up until now it's not problem, because of how we order controllers internally - "cpu" is the first and thus picking "Any" controller returns just that. But the rest of directories has no controllers, their "cgroup.controllers" is just empty. What fixes the bug is dropping @controller argument from virCgroupKillRecursiveInternal() and letting each iteration work pick its own controller. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
This commit is contained in:
parent
a0815484b1
commit
ea7d0ca37c
@ -1477,6 +1477,24 @@ virCgroupHasController(virCgroup *cgroup, int controller)
|
||||
}
|
||||
|
||||
|
||||
static int
|
||||
virCgroupGetAnyController(virCgroup *cgroup)
|
||||
{
|
||||
size_t i;
|
||||
|
||||
for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
|
||||
if (!cgroup->backends[i])
|
||||
continue;
|
||||
|
||||
return cgroup->backends[i]->getAnyController(cgroup);
|
||||
}
|
||||
|
||||
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||||
_("Unable to get any controller"));
|
||||
return -1;
|
||||
}
|
||||
|
||||
|
||||
int
|
||||
virCgroupPathOfController(virCgroup *group,
|
||||
unsigned int controller,
|
||||
@ -2715,11 +2733,11 @@ int
|
||||
virCgroupKillRecursiveInternal(virCgroup *group,
|
||||
int signum,
|
||||
GHashTable *pids,
|
||||
int controller,
|
||||
const char *taskFile,
|
||||
bool dormdir)
|
||||
{
|
||||
int rc;
|
||||
int controller;
|
||||
bool killedAny = false;
|
||||
g_autofree char *keypath = NULL;
|
||||
g_autoptr(DIR) dp = NULL;
|
||||
@ -2728,6 +2746,9 @@ virCgroupKillRecursiveInternal(virCgroup *group,
|
||||
VIR_DEBUG("group=%p signum=%d pids=%p taskFile=%s dormdir=%d",
|
||||
group, signum, pids, taskFile, dormdir);
|
||||
|
||||
if ((controller = virCgroupGetAnyController(group)) < 0)
|
||||
return -1;
|
||||
|
||||
if (virCgroupPathOfController(group, controller, "", &keypath) < 0)
|
||||
return -1;
|
||||
|
||||
@ -2760,7 +2781,7 @@ virCgroupKillRecursiveInternal(virCgroup *group,
|
||||
return -1;
|
||||
|
||||
if ((rc = virCgroupKillRecursiveInternal(subgroup, signum, pids,
|
||||
controller, taskFile, true)) < 0)
|
||||
taskFile, true)) < 0)
|
||||
return -1;
|
||||
if (rc == 1)
|
||||
killedAny = true;
|
||||
|
@ -135,6 +135,5 @@ int virCgroupRemoveRecursively(char *grppath);
|
||||
int virCgroupKillRecursiveInternal(virCgroup *group,
|
||||
int signum,
|
||||
GHashTable *pids,
|
||||
int controller,
|
||||
const char *taskFile,
|
||||
bool dormdir);
|
||||
|
@ -812,12 +812,7 @@ virCgroupV1KillRecursive(virCgroup *group,
|
||||
int signum,
|
||||
GHashTable *pids)
|
||||
{
|
||||
int controller = virCgroupV1GetAnyController(group);
|
||||
|
||||
if (controller < 0)
|
||||
return -1;
|
||||
|
||||
return virCgroupKillRecursiveInternal(group, signum, pids, controller,
|
||||
return virCgroupKillRecursiveInternal(group, signum, pids,
|
||||
"tasks", false);
|
||||
}
|
||||
|
||||
|
@ -577,12 +577,7 @@ virCgroupV2KillRecursive(virCgroup *group,
|
||||
int signum,
|
||||
GHashTable *pids)
|
||||
{
|
||||
int controller = virCgroupV2GetAnyController(group);
|
||||
|
||||
if (controller < 0)
|
||||
return -1;
|
||||
|
||||
return virCgroupKillRecursiveInternal(group, signum, pids, controller,
|
||||
return virCgroupKillRecursiveInternal(group, signum, pids,
|
||||
"cgroup.threads", false);
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user