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

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(a)redhat.com&gt; 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(a)redhat.com&gt; > > --- > > 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(a)redhat.com > > https://www.redhat.com/mailman/listinfo/libvir-list > > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list