On 1/14/19 4:47 PM, Pavel Hrdina wrote:
If some program is attached to cgroup and that cgroup is removed
before
detaching the program it might not be freed and will remain in the
system until it's rebooted.
This would not be that big deal to workaround if we wouldn't use
machined to track our guests. Systemd tries to be the nice guy and if
there is no process attached to TransientUnit it will destroy the unit
and remove the cgroup from system which will hit the kernel bug.
In order to prevent this behavior of systemd we can move another process
into the guest cgroup and the cgroup will remain active even if we kill
qemu process while destroying guest. We can then detach our BPF program
from that cgroup and call machined terminate API to cleanup the cgroup.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/lxc/lxc_cgroup.c | 1 +
src/qemu/qemu_cgroup.c | 6 ++-
src/util/vircgroup.c | 16 ++++++++
src/util/vircgroup.h | 1 +
src/util/vircgrouppriv.h | 2 +
src/util/vircgroupv2.c | 2 +
src/util/vircgroupv2devices.c | 70 ++++++++++++++++++++++++++++++++++-
src/util/virsystemd.c | 2 +-
src/util/virsystemd.h | 2 +
10 files changed, 99 insertions(+), 4 deletions(-)
+static pid_t
+virCgroupV2DevicesStartDummyProc(virCgroupPtr group)
+{
+ pid_t ret = -1;
+ pid_t pid = -1;
+ virCommandPtr cmd = NULL;
+ VIR_AUTOFREE(char *) pidfile = NULL;
+ VIR_AUTOFREE(char *) fileName = NULL;
+
+ if (virAsprintf(&fileName, "cgroup-%s", group->vmName) < 0)
+ return -1;
+
+ pidfile = virPidFileBuildPath(group->stateDir, fileName);
+ if (!pidfile)
+ return -1;
+
+ cmd = virCommandNewArgList("/usr/bin/tail", "-f",
"/dev/null",
+ "-s", "3600", NULL);
+ if (!cmd)
+ return -1;
+
+ virCommandDaemonize(cmd);
+ virCommandSetPidFile(cmd, pidfile);
+
+ if (virCommandRun(cmd, NULL) < 0)
+ goto cleanup;
+
+ if (virPidFileReadPath(pidfile, &pid) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("failed to start dummy cgroup helper"));
+ goto cleanup;
+ }
+
+ if (virCgroupAddMachineProcess(group, pid) < 0)
+ goto cleanup;
+
+ ret = pid;
+ cleanup:
+ if (ret < 0) {
+ virCommandAbort(cmd);
+ if (pid > 0)
+ virProcessKillPainfully(pid, true);
+ }
+ if (pidfile)
+ unlink(pidfile);
+ virCommandFree(cmd);
+ return ret;
+}
I don't think we should bother with this. This is clearly a kernel bug
and once fixed (which we can't detect) we would still be placing this
'tail' into the cgroup for nothing. I know we have lots of workarounds
for bugs in other libraries (and even probably kernels too) in our code,
but we should start getting rid of those instead of introducing new ones.
Michal