From 07497fc6dac1e64f14c7eb0c62572bfc360142d0 Mon Sep 17 00:00:00 2001 From: Pavel Hrdina Date: Wed, 14 Apr 2021 12:01:23 +0200 Subject: [PATCH] vircgroupv2devices: refactor virCgroupV2DevicesRemoveProg When running on systemd host the cgroup itself is removed by machined so when we reach this code the directory no longer exist. If libvirtd was running the whole time between starting and destroying VM the detection is skipped because we still have both FD in memory. But if libvirtd was restarted and no operation requiring cgroup devices executed the FDs would be 0 and libvirt would try to detect them using the cgroup directory. This results in reporting following errors: libvirtd[955]: unable to open '/sys/fs/cgroup/machine.slice/machine-qemu\x2d1\x2dguest.scope/': No such file or directory libvirtd[955]: Failed to remove cgroup for guest When running on non-systemd host where we handle cgroups manually this would not happen. When destroying VM it is not necessary to detect the BPF prog and map because the following code only closes the FDs without doing anything else. We could run code that would try to detach the BPF prog from the cgroup but that is not necessary as well. If the cgroup is removed and there is no other FD open to the prog kernel will cleanup the prog and map eventually. Reported-by: Eric Farman Signed-off-by: Pavel Hrdina Tested-by: Eric Farman Reviewed-by: Eric Farman Reviewed-by: Michal Privoznik --- src/libvirt_private.syms | 2 +- src/util/vircgroupv2.c | 2 +- src/util/vircgroupv2devices.c | 14 ++++---------- src/util/vircgroupv2devices.h | 2 +- 4 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b3f9c9681a..a727537c76 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1963,12 +1963,12 @@ virCgroupV2Register; # util/vircgroupv2devices.h virCgroupV2DevicesAvailable; +virCgroupV2DevicesCloseProg; virCgroupV2DevicesCreateProg; virCgroupV2DevicesDetectProg; virCgroupV2DevicesGetKey; virCgroupV2DevicesGetPerms; virCgroupV2DevicesPrepareProg; -virCgroupV2DevicesRemoveProg; # util/vircommand.h virCommandAbort; diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 1c3a78427c..e555217355 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -535,7 +535,7 @@ virCgroupV2Remove(virCgroup *group) if (virCgroupV2PathOfController(group, controller, "", &grppath) < 0) return 0; - if (virCgroupV2DevicesRemoveProg(parent) < 0) + if (virCgroupV2DevicesCloseProg(parent) < 0) return -1; return virCgroupRemoveRecursively(grppath); diff --git a/src/util/vircgroupv2devices.c b/src/util/vircgroupv2devices.c index 2c6e083132..ffa65bdd00 100644 --- a/src/util/vircgroupv2devices.c +++ b/src/util/vircgroupv2devices.c @@ -548,18 +548,12 @@ virCgroupV2DevicesPrepareProg(virCgroup *group) int -virCgroupV2DevicesRemoveProg(virCgroup *group) +virCgroupV2DevicesCloseProg(virCgroup *group) { - if (virCgroupV2DevicesDetectProg(group) < 0) - return -1; - - if (group->unified.devices.progfd <= 0 && group->unified.devices.mapfd <= 0) - return 0; - - if (group->unified.devices.mapfd >= 0) + if (group->unified.devices.mapfd > 0) VIR_FORCE_CLOSE(group->unified.devices.mapfd); - if (group->unified.devices.progfd >= 0) + if (group->unified.devices.progfd > 0) VIR_FORCE_CLOSE(group->unified.devices.progfd); return 0; @@ -629,7 +623,7 @@ virCgroupV2DevicesPrepareProg(virCgroup *group G_GNUC_UNUSED) int -virCgroupV2DevicesRemoveProg(virCgroup *group G_GNUC_UNUSED) +virCgroupV2DevicesCloseProg(virCgroup *group G_GNUC_UNUSED) { return 0; } diff --git a/src/util/vircgroupv2devices.h b/src/util/vircgroupv2devices.h index 21f1d882f7..1ff46987e8 100644 --- a/src/util/vircgroupv2devices.h +++ b/src/util/vircgroupv2devices.h @@ -38,7 +38,7 @@ int virCgroupV2DevicesPrepareProg(virCgroup *group); int -virCgroupV2DevicesRemoveProg(virCgroup *group); +virCgroupV2DevicesCloseProg(virCgroup *group); uint32_t virCgroupV2DevicesGetPerms(int perms,