On 10/23/22 14:08, Eric van Blokland wrote:
systemd in hybrid mode uses v1 hierarchies for controllers and v2 for process tracking.
The LXC code uses virCgroupAddMachineProcess() to move processes into appropriate cgroup by manipulating cgroupfs directly. (Note, despite libvirt also supports talking to systemd directly via org.freedesktop.machine1 API.)
If this path is taken, libvirt/lxc must convince systemd that processes really belong to new cgroup, i.e. also the tracking v2 hierarchy must undergo migration too.
The current check would evaluate v2 backend as unavailable with hybrid mode (because there are no available controllers). Simplify the condition and consider the mounted cgroup2 as sufficient to touch v2 hierarchy.
This consequently creates an issue with binding the V2 mount. In hybrid mode the V2 filesystem may be mounted upon the V1 filesystem. By reversing the order in which backends are mounted in virCgroupBindMount this problem is circumvented.
Fixes: #182
We prefer full URL because it enables to just click the link when viewing git log. Also, v3? Somehow I missed v2 and v1 :-D
Signed-off-by: Eric van Blokland <mail@ericvanblokland.nl> --- src/util/vircgroup.c | 8 +++++--- src/util/vircgroupv2.c | 12 ------------ 2 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index a6a409af3d..48fbcf625a 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2924,9 +2924,11 @@ virCgroupBindMount(virCgroup *group, const char *oldroot, size_t i; virCgroup *parent = virCgroupGetNested(group);
- for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { - if (parent->backends[i] && - parent->backends[i]->bindMount(parent, oldroot, mountopts) < 0) { + /* In hybrid environments, V2 may be mounted over V1. + * Mount the backends in reverse order. */ + for (i = 1; i <= VIR_CGROUP_BACKEND_TYPE_LAST; i++) { + if (parent->backends[VIR_CGROUP_BACKEND_TYPE_LAST - i] && + parent->backends[VIR_CGROUP_BACKEND_TYPE_LAST - i]->bindMount(parent, oldroot, mountopts) < 0) {
This works, but I'd rather have the variable count down. For that we'd need to change the type of it from size_t to ssize_t but that's okay.
return -1; } } diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 4c110940cf..0e0c61d466 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -75,22 +75,10 @@ virCgroupV2Available(void) if (STRNEQ(entry.mnt_type, "cgroup2")) continue;
- /* Systemd uses cgroup v2 for process tracking but no controller is - * available. We should consider this configuration as cgroup v2 is - * not available. */ - contFile = g_strdup_printf("%s/cgroup.controllers", entry.mnt_dir); - - if (virFileReadAll(contFile, 1024 * 1024, &contStr) < 0) - goto cleanup; - - if (STREQ(contStr, "")) - continue; - ret = true; break; }
- cleanup: VIR_FORCE_FCLOSE(mounts); return ret; }
I'm fixing the small nit in the first hunk, cleaning up the commit message and merging. Congratulations on your first libvirt contribution! Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal