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

On 10/02/2018 10:43 AM, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > 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 :-)

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