[libvirt] [PATCH 10/53] vircgroup: introduce virCgroupV2DetectControllers

Pavel Hrdina posted 53 patches 6 years, 7 months ago
There is a newer version of this series
[libvirt] [PATCH 10/53] vircgroup: introduce virCgroupV2DetectControllers
Posted by Pavel Hrdina 6 years, 7 months ago
Cgroup v2 has only single mount point for all controllers.  The list
of controllers is stored in cgroup.controllers file, name of controllers
are separated by space.

In cgroup v2 there is no cpuacct controller, the cpu.stat file always
exists with usage stats.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/util/vircgroupv2.c | 67 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index d4e0366780..3200338fe3 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -32,6 +32,7 @@
 #include "vircgroup.h"
 #include "vircgroupbackend.h"
 #include "vircgroupv2.h"
+#include "virerror.h"
 #include "virfile.h"
 #include "virlog.h"
 #include "virstring.h"
@@ -232,6 +233,71 @@ virCgroupV2StealPlacement(virCgroupPtr group)
 }
 
 
+static int
+virCgroupV2ParseControllersFile(virCgroupPtr group)
+{
+    int rc;
+    VIR_AUTOFREE(char *) contStr = NULL;
+    VIR_AUTOFREE(char *) contFile = NULL;
+    char **contList = NULL;
+    char **tmp;
+
+    if (virAsprintf(&contFile, "%s/cgroup.controllers",
+                    group->unified.mountPoint) < 0)
+        return -1;
+
+    rc = virFileReadAll(contFile, 1024 * 1024, &contStr);
+    if (rc < 0) {
+        virReportSystemError(errno, _("Unable to read from '%s'"), contFile);
+        return -1;
+    }
+
+    virTrimSpaces(contStr, NULL);
+
+    contList = virStringSplit(contStr, " ", 20);
+    if (!contList)
+        return -1;
+
+    tmp = contList;
+
+    while (*tmp) {
+        int type = virCgroupV2ControllerTypeFromString(*tmp);
+
+        if (type >= 0)
+            group->unified.controllers |= 1 << type;
+
+        tmp++;
+    }
+
+    virStringListFree(contList);
+
+    return 0;
+}
+
+
+static int
+virCgroupV2DetectControllers(virCgroupPtr group,
+                             int controllers)
+{
+    size_t i;
+
+    if (virCgroupV2ParseControllersFile(group) < 0)
+        return -1;
+
+    group->unified.controllers |= 1 << VIR_CGROUP_CONTROLLER_CPUACCT;
+
+    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++)
+        VIR_DEBUG("Controller '%s' present=%s",
+                  virCgroupV2ControllerTypeToString(i),
+                  (group->unified.controllers & 1 << i) ? "yes" : "no");
+
+    if (controllers >= 0)
+        return controllers & group->unified.controllers;
+    else
+        return group->unified.controllers;
+}
+
+
 virCgroupBackend virCgroupV2Backend = {
     .type = VIR_CGROUP_BACKEND_TYPE_V2,
 
@@ -243,6 +309,7 @@ virCgroupBackend virCgroupV2Backend = {
     .detectPlacement = virCgroupV2DetectPlacement,
     .validatePlacement = virCgroupV2ValidatePlacement,
     .stealPlacement = virCgroupV2StealPlacement,
+    .detectControllers = virCgroupV2DetectControllers,
 };
 
 
-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/53] vircgroup: introduce virCgroupV2DetectControllers
Posted by Michal Privoznik 6 years, 7 months ago
On 10/02/2018 10:43 AM, Pavel Hrdina wrote:
> Cgroup v2 has only single mount point for all controllers.  The list
> of controllers is stored in cgroup.controllers file, name of controllers
> are separated by space.
> 
> In cgroup v2 there is no cpuacct controller, the cpu.stat file always
> exists with usage stats.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/util/vircgroupv2.c | 67 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
> 
> diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> index d4e0366780..3200338fe3 100644
> --- a/src/util/vircgroupv2.c
> +++ b/src/util/vircgroupv2.c
> @@ -32,6 +32,7 @@
>  #include "vircgroup.h"
>  #include "vircgroupbackend.h"
>  #include "vircgroupv2.h"
> +#include "virerror.h"
>  #include "virfile.h"
>  #include "virlog.h"
>  #include "virstring.h"
> @@ -232,6 +233,71 @@ virCgroupV2StealPlacement(virCgroupPtr group)
>  }
>  
>  
> +static int
> +virCgroupV2ParseControllersFile(virCgroupPtr group)
> +{
> +    int rc;
> +    VIR_AUTOFREE(char *) contStr = NULL;
> +    VIR_AUTOFREE(char *) contFile = NULL;
> +    char **contList = NULL;
> +    char **tmp;
> +
> +    if (virAsprintf(&contFile, "%s/cgroup.controllers",
> +                    group->unified.mountPoint) < 0)
> +        return -1;
> +
> +    rc = virFileReadAll(contFile, 1024 * 1024, &contStr);
> +    if (rc < 0) {
> +        virReportSystemError(errno, _("Unable to read from '%s'"), contFile);
> +        return -1;
> +    }
> +
> +    virTrimSpaces(contStr, NULL);
> +
> +    contList = virStringSplit(contStr, " ", 20);
> +    if (!contList)
> +        return -1;
> +
> +    tmp = contList;
> +
> +    while (*tmp) {
> +        int type = virCgroupV2ControllerTypeFromString(*tmp);
> +
> +        if (type >= 0)
> +            group->unified.controllers |= 1 << type;
> +
> +        tmp++;
> +    }
> +
> +    virStringListFree(contList);
> +
> +    return 0;
> +}
> +
> +
> +static int
> +virCgroupV2DetectControllers(virCgroupPtr group,
> +                             int controllers)
> +{
> +    size_t i;
> +
> +    if (virCgroupV2ParseControllersFile(group) < 0)
> +        return -1;
> +
> +    group->unified.controllers |= 1 << VIR_CGROUP_CONTROLLER_CPUACCT;

I'd put a comment here describing what you do in the commit message.
Sure I can go to commit message if I want to, but a comment is more handy.

> +
> +    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++)
> +        VIR_DEBUG("Controller '%s' present=%s",
> +                  virCgroupV2ControllerTypeToString(i),
> +                  (group->unified.controllers & 1 << i) ? "yes" : "no");
> +
> +    if (controllers >= 0)
> +        return controllers & group->unified.controllers;
> +    else
> +        return group->unified.controllers;
> +}
> +
> +
>  virCgroupBackend virCgroupV2Backend = {
>      .type = VIR_CGROUP_BACKEND_TYPE_V2,
>  
> @@ -243,6 +309,7 @@ virCgroupBackend virCgroupV2Backend = {
>      .detectPlacement = virCgroupV2DetectPlacement,
>      .validatePlacement = virCgroupV2ValidatePlacement,
>      .stealPlacement = virCgroupV2StealPlacement,
> +    .detectControllers = virCgroupV2DetectControllers,
>  };
>  
>  
> 

ACK

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/53] vircgroup: introduce virCgroupV2DetectControllers
Posted by Pavel Hrdina 6 years, 7 months ago
On Thu, Oct 04, 2018 at 01:18:16PM +0200, Michal Privoznik wrote:
> On 10/02/2018 10:43 AM, Pavel Hrdina wrote:
> > Cgroup v2 has only single mount point for all controllers.  The list
> > of controllers is stored in cgroup.controllers file, name of controllers
> > are separated by space.
> > 
> > In cgroup v2 there is no cpuacct controller, the cpu.stat file always
> > exists with usage stats.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  src/util/vircgroupv2.c | 67 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> > 
> > diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> > index d4e0366780..3200338fe3 100644
> > --- a/src/util/vircgroupv2.c
> > +++ b/src/util/vircgroupv2.c
> > @@ -32,6 +32,7 @@
> >  #include "vircgroup.h"
> >  #include "vircgroupbackend.h"
> >  #include "vircgroupv2.h"
> > +#include "virerror.h"
> >  #include "virfile.h"
> >  #include "virlog.h"
> >  #include "virstring.h"
> > @@ -232,6 +233,71 @@ virCgroupV2StealPlacement(virCgroupPtr group)
> >  }
> >  
> >  
> > +static int
> > +virCgroupV2ParseControllersFile(virCgroupPtr group)
> > +{
> > +    int rc;
> > +    VIR_AUTOFREE(char *) contStr = NULL;
> > +    VIR_AUTOFREE(char *) contFile = NULL;
> > +    char **contList = NULL;
> > +    char **tmp;
> > +
> > +    if (virAsprintf(&contFile, "%s/cgroup.controllers",
> > +                    group->unified.mountPoint) < 0)
> > +        return -1;
> > +
> > +    rc = virFileReadAll(contFile, 1024 * 1024, &contStr);
> > +    if (rc < 0) {
> > +        virReportSystemError(errno, _("Unable to read from '%s'"), contFile);
> > +        return -1;
> > +    }
> > +
> > +    virTrimSpaces(contStr, NULL);
> > +
> > +    contList = virStringSplit(contStr, " ", 20);
> > +    if (!contList)
> > +        return -1;
> > +
> > +    tmp = contList;
> > +
> > +    while (*tmp) {
> > +        int type = virCgroupV2ControllerTypeFromString(*tmp);
> > +
> > +        if (type >= 0)
> > +            group->unified.controllers |= 1 << type;
> > +
> > +        tmp++;
> > +    }
> > +
> > +    virStringListFree(contList);
> > +
> > +    return 0;
> > +}
> > +
> > +
> > +static int
> > +virCgroupV2DetectControllers(virCgroupPtr group,
> > +                             int controllers)
> > +{
> > +    size_t i;
> > +
> > +    if (virCgroupV2ParseControllersFile(group) < 0)
> > +        return -1;
> > +
> > +    group->unified.controllers |= 1 << VIR_CGROUP_CONTROLLER_CPUACCT;
> 
> I'd put a comment here describing what you do in the commit message.
> Sure I can go to commit message if I want to, but a comment is more handy.

Since I was considering it I'll fix it before pushing, thanks.

> 
> > +
> > +    for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++)
> > +        VIR_DEBUG("Controller '%s' present=%s",
> > +                  virCgroupV2ControllerTypeToString(i),
> > +                  (group->unified.controllers & 1 << i) ? "yes" : "no");
> > +
> > +    if (controllers >= 0)
> > +        return controllers & group->unified.controllers;
> > +    else
> > +        return group->unified.controllers;
> > +}
> > +
> > +
> >  virCgroupBackend virCgroupV2Backend = {
> >      .type = VIR_CGROUP_BACKEND_TYPE_V2,
> >  
> > @@ -243,6 +309,7 @@ virCgroupBackend virCgroupV2Backend = {
> >      .detectPlacement = virCgroupV2DetectPlacement,
> >      .validatePlacement = virCgroupV2ValidatePlacement,
> >      .stealPlacement = virCgroupV2StealPlacement,
> > +    .detectControllers = virCgroupV2DetectControllers,
> >  };
> >  
> >  
> > 
> 
> ACK
> 
> Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list