Re: [PATCH 05/53] vircgroup: introduce virCgroupV2CopyPlacement

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 04, 2018 at 01:18:27PM +0200, Michal Privoznik wrote:
> On 10/02/2018 10:43 AM, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> > ---
> >  src/util/vircgroupv2.c | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> > index d3f72b9006..11d9876d36 100644
> > --- a/src/util/vircgroupv2.c
> > +++ b/src/util/vircgroupv2.c
> > @@ -141,12 +141,39 @@ virCgroupV2CopyMounts(virCgroupPtr group,
> >  }
> >  
> >  
> > +static int
> > +virCgroupV2CopyPlacement(virCgroupPtr group,
> > +                         const char *path,
> > +                         virCgroupPtr parent)
> > +{
> > +    if (path[0] == '/') {
> > +        if (VIR_STRDUP(group->unified.placement, path) < 0)
> > +            return -1;
> > +    } else {
> 
> Maybe it's the lack of morning coffee, but I had some difficulties
> parsing this.
> 
> > +        /*
> > +         * parent == "/" + path="" => "/"
> > +         * parent == "/libvirt.service" + path == "" => "/libvirt.service"
> > +         * parent == "/libvirt.service" + path == "foo" => "/libvirt.service/foo"
> 
> s/\+/&&/ so that it looks like a C condition.
> 
> > +         */
> > +        if (virAsprintf(&group->unified.placement, "%s%s%s",
> > +                        parent->unified.placement,
> > +                        (STREQ(parent->unified.placement, "/") ||
> > +                         STREQ(path, "") ? "" : "/"),
> > +                        path) < 0)
> 
> Maybe if you got rid of the ternary operator it would be more readable.
> But now that I finally understood this, I don't care that much :-)

Yes, this bit is kind of weird and should be improved, it's basically
copy&paste from cgroupv1 backend code.  As a followup we can export it
into a function and reuse it in both backends.

> 
> > +            return -1;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +
> >  virCgroupBackend virCgroupV2Backend = {
> >      .type = VIR_CGROUP_BACKEND_TYPE_V2,
> >  
> >      .available = virCgroupV2Available,
> >      .validateMachineGroup = virCgroupV2ValidateMachineGroup,
> >      .copyMounts = virCgroupV2CopyMounts,
> > +    .copyPlacement = virCgroupV2CopyPlacement,
> >  };
> >  
> >  
> > 
> 
> ACK
> 
> Michal

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux