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

[Pavel Hrdina]

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 at redhat.com>
> > ---
> >  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
-------------- 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/20181004/e0949ea7/attachment-0001.sig>