On Thu, Sep 20, 2018 at 10:03 AM, Pavel Hrdina <phrdina@redhat.com> wrote:
> 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@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@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.
>
Aha, I see!
I'll leave it for you to decide what's the best way to handle the failure
in the previous series and just wait for v2.
>
> 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@redhat.com
> > > https://www.redhat.com/mailman/listinfo/libvir-list
> > >
>
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com
> > https://www.redhat.com/mailman/listinfo/libvir-list
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list