Using virtCgroupNewSelf() is not correct with cgroups v2 because the
the virt-host-validate process is executed from from the same cgroup
context as the terminal and usually not all controllers are enabled
by default.

To do a proper check we need to use the root cgroup to see what
controllers are actually available. Libvirt or systemd ensures that
all controllers are available for VMs as well.

This still doesn't solve the devices controller with cgroups v2 where
there is no controller as it was replaced by eBPF. Currently libvirt
tries to query eBPF programs which usually works only for root as
regular users will get permission denied for that operation.

Fixes: https://gitlab.com/libvirt/libvirt/-/issues/94

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/libvirt_private.syms          | 1 +
 src/util/vircgroup.h              | 4 ++++
 src/util/vircgrouppriv.h          | 4 ----
 tools/virt-host-validate-common.c | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1d98f01334..79a23f34cb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1796,6 +1796,7 @@ virCgroupHasController;
 virCgroupHasEmptyTasks;
 virCgroupKillPainfully;
 virCgroupKillRecursive;
+virCgroupNew;
 virCgroupNewDetect;
 virCgroupNewDetectMachine;
 virCgroupNewDomainPartition;
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 78770f5d3b..f7eed983cc 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -60,6 +60,10 @@ typedef enum {
 
 bool virCgroupAvailable(void);
 
+int virCgroupNew(const char *path,
+                 int controllers,
+                 virCgroupPtr *group);
+
 int virCgroupNewSelf(virCgroupPtr *group)
     ATTRIBUTE_NONNULL(1);
 
diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h
index baa84550f4..85ba5393e0 100644
--- a/src/util/vircgrouppriv.h
+++ b/src/util/vircgrouppriv.h
@@ -110,10 +110,6 @@ int virCgroupGetValueForBlkDev(const char *str,
                                const char *devPath,
                                char **value);
 
-int virCgroupNew(const char *path,
-                 int controllers,
-                 virCgroupPtr *group);
-
 int virCgroupNewPartition(const char *path,
                           bool create,
                           int controllers,
diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c
index a10ac03293..fc43b2ddc8 100644
--- a/tools/virt-host-validate-common.c
+++ b/tools/virt-host-validate-common.c
@@ -293,7 +293,7 @@ int virHostValidateCGroupControllers(const char *hvname,
     int ret = 0;
     size_t i;
 
-    if (virCgroupNewSelf(&group) < 0)
+    if (virCgroupNew("/", -1, &group) < 0)
         return -1;
 
     for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
-- 
2.26.2

On 11/18/20 1:48 PM, Pavel Hrdina wrote:
> Using virtCgroupNewSelf() is not correct with cgroups v2 because the
> the virt-host-validate process is executed from from the same cgroup
> context as the terminal and usually not all controllers are enabled
> by default.
> 
> To do a proper check we need to use the root cgroup to see what
> controllers are actually available. Libvirt or systemd ensures that
> all controllers are available for VMs as well.
> 
> This still doesn't solve the devices controller with cgroups v2 where
> there is no controller as it was replaced by eBPF. Currently libvirt
> tries to query eBPF programs which usually works only for root as
> regular users will get permission denied for that operation.
> 
> Fixes: https://gitlab.com/libvirt/libvirt/-/issues/94
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>   src/libvirt_private.syms          | 1 +
>   src/util/vircgroup.h              | 4 ++++
>   src/util/vircgrouppriv.h          | 4 ----
>   tools/virt-host-validate-common.c | 2 +-
>   4 files changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

On Wed, Nov 18, 2020 at 01:48:56PM +0100, Pavel Hrdina wrote:
> Using virtCgroupNewSelf() is not correct with cgroups v2 because the
> the virt-host-validate process is executed from from the same cgroup
> context as the terminal and usually not all controllers are enabled
> by default.
> 
> To do a proper check we need to use the root cgroup to see what
> controllers are actually available. Libvirt or systemd ensures that
> all controllers are available for VMs as well.
> 
> This still doesn't solve the devices controller with cgroups v2 where
> there is no controller as it was replaced by eBPF. Currently libvirt
> tries to query eBPF programs which usually works only for root as
> regular users will get permission denied for that operation.
> 
> Fixes: https://gitlab.com/libvirt/libvirt/-/issues/94
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/libvirt_private.syms          | 1 +
>  src/util/vircgroup.h              | 4 ++++
>  src/util/vircgrouppriv.h          | 4 ----
>  tools/virt-host-validate-common.c | 2 +-
>  4 files changed, 6 insertions(+), 5 deletions(-)

Unfortunately this broken mingw builds as virCgroupNew is missing a
stub.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|