[libvirt] [PATCH 04/47] vircgroup: detect available backend for cgroup

Pavel Hrdina phrdina at redhat.com
Thu Sep 20 08:03:29 UTC 2018


On Thu, Sep 20, 2018 at 08:28:57AM +0200, Fabiano Fidêncio wrote:
> On Tue, Sep 18, 2018 at 5:45 PM, Pavel Hrdina <phrdina at redhat.com> wrote:
> 
> > We need to update one test-case because now new cgroup object will be
> > created only if there is any cgroup backend available.
> >
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> >  src/util/vircgroup.c     | 18 ++++++++++++++++++
> >  src/util/vircgrouppriv.h |  3 +++
> >  tests/vircgrouptest.c    | 38 +++-----------------------------------
> >  3 files changed, 24 insertions(+), 35 deletions(-)
> >
> > diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> > index a5478a3fa4..0ffb5db93c 100644
> > --- a/src/util/vircgroup.c
> > +++ b/src/util/vircgroup.c
> > @@ -713,10 +713,28 @@ virCgroupDetect(virCgroupPtr group,
> >                  virCgroupPtr parent)
> >  {
> >      int rc;
> > +    size_t i;
> > +    virCgroupBackendPtr *backends = virCgroupBackendGetAll();
> >
> >      VIR_DEBUG("group=%p controllers=%d path=%s parent=%p",
> >                group, controllers, path, parent);
> >
> > +    if (!backends)
> > +        return -1;
> > +
> > +    for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) {
> > +        if (backends[i] && backends[i]->available()) {
> > +            group->backend = backends[i];
> > +            break;
> > +        }
> > +    }
> > +
> > +    if (!group->backend) {
> > +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +                       _("no cgroup backend available"));
> > +        return -1;
> > +    }
> > +
> >      if (parent) {
> >          if (virCgroupCopyMounts(group, parent) < 0)
> >              return -1;
> > diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h
> > index 046c96c52c..2caa966fee 100644
> > --- a/src/util/vircgrouppriv.h
> > +++ b/src/util/vircgrouppriv.h
> > @@ -30,6 +30,7 @@
> >  # define __VIR_CGROUP_PRIV_H__
> >
> >  # include "vircgroup.h"
> > +# include "vircgroupbackend.h"
> >
> >  struct _virCgroupController {
> >      int type;
> > @@ -47,6 +48,8 @@ typedef virCgroupController *virCgroupControllerPtr;
> >  struct _virCgroup {
> >      char *path;
> >
> > +    virCgroupBackendPtr backend;
> > +
> >      virCgroupController controllers[VIR_CGROUP_CONTROLLER_LAST];
> >  };
> >
> > diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
> > index bbbe6ffbe5..be3143ea52 100644
> > --- a/tests/vircgrouptest.c
> > +++ b/tests/vircgrouptest.c
> > @@ -114,16 +114,6 @@ const char *mountsAllInOne[VIR_CGROUP_CONTROLLER_LAST]
> > = {
> >      [VIR_CGROUP_CONTROLLER_BLKIO] = "/not/really/sys/fs/cgroup",
> >      [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL,
> >  };
> > -const char *mountsLogind[VIR_CGROUP_CONTROLLER_LAST] = {
> > -    [VIR_CGROUP_CONTROLLER_CPU] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_CPUSET] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_MEMORY] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_FREEZER] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_BLKIO] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/not/really/sys/fs/cgroup/sys
> > temd",
> > -};
> >
> >  const char *links[VIR_CGROUP_CONTROLLER_LAST] = {
> >      [VIR_CGROUP_CONTROLLER_CPU] = "/not/really/sys/fs/cgroup/cpu",
> > @@ -147,17 +137,6 @@ const char *linksAllInOne[VIR_CGROUP_CONTROLLER_LAST]
> > = {
> >      [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL,
> >  };
> >
> > -const char *linksLogind[VIR_CGROUP_CONTROLLER_LAST] = {
> > -    [VIR_CGROUP_CONTROLLER_CPU] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_CPUSET] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_MEMORY] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_FREEZER] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_BLKIO] = NULL,
> > -    [VIR_CGROUP_CONTROLLER_SYSTEMD] = NULL,
> > -};
> > -
> >
> >  static int
> >  testCgroupDetectMounts(const void *args)
> > @@ -541,24 +520,13 @@ static int testCgroupNewForSelfLogind(const void
> > *args ATTRIBUTE_UNUSED)
> >  {
> >      virCgroupPtr cgroup = NULL;
> >      int ret = -1;
> > -    const char *placement[VIR_CGROUP_CONTROLLER_LAST] = {
> > -        [VIR_CGROUP_CONTROLLER_CPU] = NULL,
> > -        [VIR_CGROUP_CONTROLLER_CPUACCT] = NULL,
> > -        [VIR_CGROUP_CONTROLLER_CPUSET] = NULL,
> > -        [VIR_CGROUP_CONTROLLER_MEMORY] = NULL,
> > -        [VIR_CGROUP_CONTROLLER_DEVICES] = NULL,
> > -        [VIR_CGROUP_CONTROLLER_FREEZER] = NULL,
> > -        [VIR_CGROUP_CONTROLLER_BLKIO] = NULL,
> > -        [VIR_CGROUP_CONTROLLER_SYSTEMD] = "/",
> > -    };
> >
> > -    if (virCgroupNewSelf(&cgroup) < 0) {
> > -        fprintf(stderr, "Cannot create cgroup for self\n");
> > +    if (virCgroupNewSelf(&cgroup) == 0) {
> > +        fprintf(stderr, "Expected cgroup creation to fail.\n");
> >
> 
> Seems that code here would fit quite well in the last patch of the previous
> series in order to avoid the test failure introduced there.

I'm not sure if I follow.  This patch introduces a new restriction in
the code that if no backend is available the creation of new cgroup will
fail.  In the previous series there is no such thing as backend.

I cannot move only the test code into different patch because the test
suit would start failing.

The only thing I would be able to do is add the virCgroupAvailable()
check into virCgroupDetect() without any backend code which would
require the same test code change but still it would be separate patch,
it would not be simple enough to fit into the last patch of the previous
series.

Pavel

> 
> 
> >          goto cleanup;
> >      }
> >
> > -    ret = validateCgroup(cgroup, "", mountsLogind, linksLogind,
> > placement);
> > -
> > +    ret = 0;
> >   cleanup:
> >      virCgroupFree(&cgroup);
> >      return ret;
> > --
> > 2.17.1
> >
> > --
> > libvir-list mailing list
> > libvir-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
> >

> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180920/aefa37ad/attachment-0001.sig>


More information about the libvir-list mailing list