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@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list