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@xxxxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
Michal
