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