Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/qemu/qemu_cgroup.c | 71 +++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index b671991ad6..9017753053 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -960,16 +960,41 @@ qemuInitCgroup(virDomainObjPtr vm, return 0; } -static void -qemuRestoreCgroupState(virDomainObjPtr vm) +static int +qemuRestoreCgroupThread(virCgroupPtr cgroup, + virCgroupThreadName thread, + int id) { - g_autofree char *mem_mask = NULL; + virCgroupPtr cgroup_temp = NULL; g_autofree char *nodeset = NULL; + int ret = -1; + + if (virCgroupNewThread(cgroup, thread, id, false, &cgroup_temp) < 0) + goto cleanup; + + if (virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0) + goto cleanup; + + if (virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0) + goto cleanup; + + if (virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0) + goto cleanup; + + ret = 0; + cleanup: + virCgroupFree(&cgroup_temp); + return ret; +} + +static void +qemuRestoreCgroupState(virDomainObjPtr vm) +{ + g_autofree char *mem_mask = NULL; int empty = -1; qemuDomainObjPrivatePtr priv = vm->privateData; size_t i = 0; g_autoptr(virBitmap) all_nodes = NULL; - virCgroupPtr cgroup_temp = NULL; if (!virNumaIsAvailable() || !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) @@ -994,45 +1019,27 @@ qemuRestoreCgroupState(virDomainObjPtr vm) if (!vcpu->online) continue; - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i, - false, &cgroup_temp) < 0 || - virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 || - virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 || - virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0) - goto cleanup; - - VIR_FREE(nodeset); - virCgroupFree(&cgroup_temp); + if (qemuRestoreCgroupThread(priv->cgroup, + VIR_CGROUP_THREAD_VCPU, i) < 0) + return; } for (i = 0; i < vm->def->niothreadids; i++) { - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, - vm->def->iothreadids[i]->iothread_id, - false, &cgroup_temp) < 0 || - virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 || - virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 || - virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0) - goto cleanup; - - VIR_FREE(nodeset); - virCgroupFree(&cgroup_temp); + if (qemuRestoreCgroupThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, + vm->def->iothreadids[i]->iothread_id) < 0) + return; } - if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, - false, &cgroup_temp) < 0 || - virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0 || - virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0 || - virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0) - goto cleanup; + if (qemuRestoreCgroupThread(priv->cgroup, + VIR_CGROUP_THREAD_EMULATOR, 0) < 0) + return; - cleanup: - virCgroupFree(&cgroup_temp); return; error: virResetLastError(); VIR_DEBUG("Couldn't restore cgroups to meaningful state"); - goto cleanup; + return; } int -- 2.26.2

As preparation for g_autoptr() we need to change the function to take only virCgroupPtr. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/libvirt-lxc.c | 4 +- src/lxc/lxc_cgroup.c | 4 +- src/lxc/lxc_container.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_domain.c | 2 +- src/lxc/lxc_process.c | 11 +++--- src/qemu/qemu_cgroup.c | 12 +++--- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_driver.c | 32 +++++++-------- src/qemu/qemu_process.c | 2 +- src/util/vircgroup.c | 65 +++++++++++++++++-------------- src/util/vircgroup.h | 2 +- src/util/vircgroupv1.c | 2 +- tests/vircgrouptest.c | 48 +++++++++++------------ tools/virt-host-validate-common.c | 2 +- 15 files changed, 102 insertions(+), 91 deletions(-) diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c index 25f1cfc5f7..73daf123f0 100644 --- a/src/libvirt-lxc.c +++ b/src/libvirt-lxc.c @@ -307,12 +307,12 @@ int virDomainLxcEnterCGroup(virDomainPtr domain, if (virCgroupAddProcess(cgroup, getpid()) < 0) goto error; - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return 0; error: virDispatchError(NULL); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return -1; } diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index d13f2adde5..b80a8911f9 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -168,7 +168,7 @@ int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -417,7 +417,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, def->idmap.uidmap[0].target, def->idmap.gidmap[0].target, (1 << VIR_CGROUP_CONTROLLER_SYSTEMD)) < 0) { - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return NULL; } } diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d1aa622be4..913f4de26a 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1668,7 +1668,7 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef, ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index f70731bc64..e6dee85ec7 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -310,7 +310,7 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) g_free(ctrl->nbdpids); g_free(ctrl->nsFDs); - virCgroupFree(&ctrl->cgroup); + virCgroupFree(ctrl->cgroup); /* This must always be the last thing to be closed */ VIR_FORCE_CLOSE(ctrl->handshakeFd); diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index d8aebe06d9..df60519fca 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -168,7 +168,7 @@ virLXCDomainObjPrivateFree(void *data) { virLXCDomainObjPrivatePtr priv = data; - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); virLXCDomainObjFreeJob(priv); g_free(priv); } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 16969dbf33..a98a090893 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -236,7 +236,8 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, if (priv->cgroup) { virCgroupRemove(priv->cgroup); - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); + priv->cgroup = NULL; } /* Get machined to terminate the machine as it may not have cleaned it @@ -1202,26 +1203,26 @@ int virLXCProcessStart(virConnectPtr conn, if (!virCgroupHasController(selfcgroup, VIR_CGROUP_CONTROLLER_CPUACCT)) { - virCgroupFree(&selfcgroup); + virCgroupFree(selfcgroup); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'cpuacct' cgroups controller mount")); return -1; } if (!virCgroupHasController(selfcgroup, VIR_CGROUP_CONTROLLER_DEVICES)) { - virCgroupFree(&selfcgroup); + virCgroupFree(selfcgroup); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'devices' cgroups controller mount")); return -1; } if (!virCgroupHasController(selfcgroup, VIR_CGROUP_CONTROLLER_MEMORY)) { - virCgroupFree(&selfcgroup); + virCgroupFree(selfcgroup); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to find 'memory' cgroups controller mount")); return -1; } - virCgroupFree(&selfcgroup); + virCgroupFree(selfcgroup); if (vm->def->nconsoles == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 9017753053..473ca8a414 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -921,7 +921,8 @@ qemuInitCgroup(virDomainObjPtr vm, if (!virCgroupAvailable()) return 0; - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); + priv->cgroup = NULL; if (!vm->def->resource) { virDomainResourceDefPtr res; @@ -983,7 +984,7 @@ qemuRestoreCgroupThread(virCgroupPtr cgroup, ret = 0; cleanup: - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); return ret; } @@ -1054,7 +1055,8 @@ qemuConnectCgroup(virDomainObjPtr vm) if (!virCgroupAvailable()) return 0; - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); + priv->cgroup = NULL; if (virCgroupNewDetectMachine(vm->def->name, "qemu", @@ -1150,7 +1152,7 @@ qemuSetupCgroupForExtDevices(virDomainObjPtr vm, ret = qemuExtDevicesSetupCgroup(driver, vm, cgroup_temp); cleanup: - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); return ret; } @@ -1221,7 +1223,7 @@ qemuCgroupEmulatorAllNodesDataFree(qemuCgroupEmulatorAllNodesDataPtr data) if (!data) return; - virCgroupFree(&data->emulatorCgroup); + virCgroupFree(data->emulatorCgroup); VIR_FREE(data->emulatorMemMask); VIR_FREE(data); } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9623123d3c..2d7f61f5e9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1725,7 +1725,8 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) g_strfreev(priv->qemuDevices); priv->qemuDevices = NULL; - virCgroupFree(&priv->cgroup); + virCgroupFree(priv->cgroup); + priv->cgroup = NULL; virPerfFree(priv->perf); priv->perf = NULL; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8ef812cd94..60e043115f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4583,7 +4583,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, cleanup: virBitmapFree(tmpmap); - virCgroupFree(&cgroup_vcpu); + virCgroupFree(cgroup_vcpu); virObjectEventStateQueue(driver->domainEventState, event); return ret; } @@ -4809,7 +4809,7 @@ qemuDomainPinEmulator(virDomainPtr dom, cleanup: if (cgroup_emulator) - virCgroupFree(&cgroup_emulator); + virCgroupFree(cgroup_emulator); virObjectEventStateQueue(driver->domainEventState, event); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); @@ -5287,7 +5287,7 @@ qemuDomainPinIOThread(virDomainPtr dom, cleanup: if (cgroup_iothread) - virCgroupFree(&cgroup_iothread); + virCgroupFree(cgroup_iothread); virObjectEventStateQueue(driver->domainEventState, event); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); @@ -8717,7 +8717,7 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) { virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i); @@ -8729,7 +8729,7 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); } for (i = 0; i < vm->def->niothreadids; i++) { @@ -8738,7 +8738,7 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, false, &cgroup_temp) < 0 || virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) goto cleanup; - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); } /* set nodeset for root cgroup */ @@ -8747,7 +8747,7 @@ qemuDomainSetNumaParamsLive(virDomainObjPtr vm, ret = 0; cleanup: - virCgroupFree(&cgroup_temp); + virCgroupFree(cgroup_temp); return ret; } @@ -9164,13 +9164,13 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) goto cleanup; - virCgroupFree(&cgroup_vcpu); + virCgroupFree(cgroup_vcpu); } return 0; cleanup: - virCgroupFree(&cgroup_vcpu); + virCgroupFree(cgroup_vcpu); return -1; } @@ -9191,11 +9191,11 @@ qemuSetEmulatorBandwidthLive(virCgroupPtr cgroup, if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) goto cleanup; - virCgroupFree(&cgroup_emulator); + virCgroupFree(cgroup_emulator); return 0; cleanup: - virCgroupFree(&cgroup_emulator); + virCgroupFree(cgroup_emulator); return -1; } @@ -9222,13 +9222,13 @@ qemuSetIOThreadsBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, if (qemuSetupCgroupVcpuBW(cgroup_iothread, period, quota) < 0) goto cleanup; - virCgroupFree(&cgroup_iothread); + virCgroupFree(cgroup_iothread); } return 0; cleanup: - virCgroupFree(&cgroup_iothread); + virCgroupFree(cgroup_iothread); return -1; } @@ -9601,7 +9601,7 @@ qemuGetVcpusBWLive(virDomainObjPtr vm, ret = 0; cleanup: - virCgroupFree(&cgroup_vcpu); + virCgroupFree(cgroup_vcpu); return ret; } @@ -9624,7 +9624,7 @@ qemuGetEmulatorBandwidthLive(virCgroupPtr cgroup, ret = 0; cleanup: - virCgroupFree(&cgroup_emulator); + virCgroupFree(cgroup_emulator); return ret; } @@ -9660,7 +9660,7 @@ qemuGetIOThreadsBWLive(virDomainObjPtr vm, ret = 0; cleanup: - virCgroupFree(&cgroup_iothread); + virCgroupFree(cgroup_iothread); return ret; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6b5de29fdb..5bc76a75e3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2750,7 +2750,7 @@ qemuProcessSetupPid(virDomainObjPtr vm, if (cgroup) { if (ret < 0) virCgroupRemove(cgroup); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); } return ret; diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index d52c0305f4..04876c6596 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -680,7 +680,7 @@ virCgroupNew(pid_t pid, return 0; error: - virCgroupFree(group); + virCgroupFree(*group); *group = NULL; return -1; @@ -860,9 +860,11 @@ virCgroupNewPartition(const char *path, ret = 0; cleanup: - if (ret != 0) - virCgroupFree(group); - virCgroupFree(&parent); + if (ret != 0) { + virCgroupFree(*group); + *group = NULL; + } + virCgroupFree(parent); return ret; } @@ -923,7 +925,8 @@ virCgroupNewDomainPartition(virCgroupPtr partition, */ if (virCgroupMakeGroup(partition, *group, create, VIR_CGROUP_MEM_HIERACHY) < 0) { - virCgroupFree(group); + virCgroupFree(*group); + *group = NULL; return -1; } @@ -976,7 +979,8 @@ virCgroupNewThread(virCgroupPtr domain, return -1; if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_THREAD) < 0) { - virCgroupFree(group); + virCgroupFree(*group); + *group = NULL; return -1; } @@ -1019,7 +1023,8 @@ virCgroupNewDetectMachine(const char *name, machinename)) { VIR_DEBUG("Failed to validate machine name for '%s' driver '%s'", name, drivername); - virCgroupFree(group); + virCgroupFree(*group); + *group = NULL; return 0; } } @@ -1059,13 +1064,13 @@ virCgroupEnableMissingControllers(char *path, goto cleanup; if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_SYSTEMD) < 0) { - virCgroupFree(&tmp); + virCgroupFree(tmp); goto cleanup; } if (t) { *t = '/'; offset = t; - virCgroupFree(&parent); + virCgroupFree(parent); parent = tmp; } else { *group = tmp; @@ -1075,7 +1080,7 @@ virCgroupEnableMissingControllers(char *path, ret = 0; cleanup: - virCgroupFree(&parent); + virCgroupFree(parent); return ret; } @@ -1130,7 +1135,7 @@ virCgroupNewMachineSystemd(const char *name, break; } } - virCgroupFree(&init); + virCgroupFree(init); if (!path || STREQ(path, "/") || path[0] != '/') { VIR_DEBUG("Systemd didn't setup its controller, path=%s", @@ -1148,7 +1153,8 @@ virCgroupNewMachineSystemd(const char *name, virErrorPreserveLast(&saved); virCgroupRemove(*group); - virCgroupFree(group); + virCgroupFree(*group); + *group = NULL; virErrorRestore(&saved); } @@ -1199,7 +1205,8 @@ virCgroupNewMachineManual(const char *name, virErrorPreserveLast(&saved); virCgroupRemove(*group); - virCgroupFree(group); + virCgroupFree(*group); + *group = NULL; virErrorRestore(&saved); } @@ -1207,7 +1214,7 @@ virCgroupNewMachineManual(const char *name, ret = 0; cleanup: - virCgroupFree(&parent); + virCgroupFree(parent); return ret; } @@ -2059,12 +2066,12 @@ virCgroupGetPercpuVcpuSum(virCgroupPtr group, sum_cpu_time[j] += tmp; } - virCgroupFree(&group_vcpu); + virCgroupFree(group_vcpu); } ret = 0; cleanup: - virCgroupFree(&group_vcpu); + virCgroupFree(group_vcpu); return ret; } @@ -2556,7 +2563,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, if (dormdir) virCgroupRemove(subgroup); - virCgroupFree(&subgroup); + virCgroupFree(subgroup); } if (direrr < 0) goto cleanup; @@ -2565,7 +2572,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, ret = killedAny ? 1 : 0; cleanup: - virCgroupFree(&subgroup); + virCgroupFree(subgroup); VIR_DIR_CLOSE(dp); return ret; } @@ -2767,7 +2774,7 @@ virCgroupControllerAvailable(int controller) return ret; ret = virCgroupHasController(cgroup, controller); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -3534,24 +3541,24 @@ virCgroupControllerAvailable(int controller G_GNUC_UNUSED) * @group: The group structure to free */ void -virCgroupFree(virCgroupPtr *group) +virCgroupFree(virCgroupPtr group) { size_t i; - if (*group == NULL) + if (group == NULL) return; for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) { - VIR_FREE((*group)->legacy[i].mountPoint); - VIR_FREE((*group)->legacy[i].linkPoint); - VIR_FREE((*group)->legacy[i].placement); + VIR_FREE(group->legacy[i].mountPoint); + VIR_FREE(group->legacy[i].linkPoint); + VIR_FREE(group->legacy[i].placement); } - VIR_FREE((*group)->unified.mountPoint); - VIR_FREE((*group)->unified.placement); + VIR_FREE(group->unified.mountPoint); + VIR_FREE(group->unified.placement); - VIR_FREE((*group)->path); - VIR_FREE(*group); + VIR_FREE(group->path); + VIR_FREE(group); } @@ -3568,7 +3575,7 @@ virCgroupDelThread(virCgroupPtr cgroup, /* Remove the offlined cgroup */ virCgroupRemove(new_cgroup); - virCgroupFree(&new_cgroup); + virCgroupFree(new_cgroup); } return 0; diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h index 1dcd0688f1..691bec610f 100644 --- a/src/util/vircgroup.h +++ b/src/util/vircgroup.h @@ -107,7 +107,7 @@ int virCgroupTerminateMachine(const char *name) bool virCgroupNewIgnoreError(void); -void virCgroupFree(virCgroupPtr *group); +void virCgroupFree(virCgroupPtr group); bool virCgroupHasController(virCgroupPtr cgroup, int controller); int virCgroupPathOfController(virCgroupPtr group, diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 5504441fa6..52d2a17d39 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1484,7 +1484,7 @@ virCgroupV1MemoryOnceInit(void) "memory.limit_in_bytes", &mem_unlimited)); cleanup: - virCgroupFree(&group); + virCgroupFree(group); virCgroupV1MemoryUnlimitedKB = mem_unlimited >> 10; } diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index e5be556104..632d33efcd 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -217,7 +217,7 @@ testCgroupDetectMounts(const void *args) cleanup: g_unsetenv("VIR_CGROUP_MOCK_FILENAME"); VIR_FREE(parsed); - virCgroupFree(&group); + virCgroupFree(group); return result; } @@ -245,7 +245,7 @@ static int testCgroupNewForSelf(const void *args G_GNUC_UNUSED) ret = validateCgroup(cgroup, "", mountsFull, links, placement, NULL, NULL, 0); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -322,7 +322,7 @@ static int testCgroupNewForPartition(const void *args G_GNUC_UNUSED) goto cleanup; } ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsSmall, links, placementSmall, NULL, NULL, 0); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/virtualmachines", true, -1, &cgroup)) != 0) { fprintf(stderr, "Cannot create /virtualmachines cgroup: %d\n", -rv); @@ -331,7 +331,7 @@ static int testCgroupNewForPartition(const void *args G_GNUC_UNUSED) ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsFull, links, placementFull, NULL, NULL, 0); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -371,7 +371,7 @@ static int testCgroupNewForPartitionNested(const void *args G_GNUC_UNUSED) } /* Should now work */ - virCgroupFree(&cgroup); + virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/deployment/production", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /deployment/production cgroup: %d\n", -rv); goto cleanup; @@ -381,7 +381,7 @@ static int testCgroupNewForPartitionNested(const void *args G_GNUC_UNUSED) mountsFull, links, placementFull, NULL, NULL, 0); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -420,14 +420,14 @@ static int testCgroupNewForPartitionNestedDeep(const void *args G_GNUC_UNUSED) goto cleanup; } - virCgroupFree(&cgroup); + virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/user/berrange.user", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user cgroup: %d\n", -rv); goto cleanup; } /* Should now work */ - virCgroupFree(&cgroup); + virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/user/berrange.user/production", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user/production cgroup: %d\n", -rv); goto cleanup; @@ -437,7 +437,7 @@ static int testCgroupNewForPartitionNestedDeep(const void *args G_GNUC_UNUSED) mountsFull, links, placementFull, NULL, NULL, 0); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -473,8 +473,8 @@ static int testCgroupNewForPartitionDomain(const void *args G_GNUC_UNUSED) ret = validateCgroup(domaincgroup, "/production.partition/foo.libvirt-lxc", mountsFull, links, placement, NULL, NULL, 0); cleanup: - virCgroupFree(&partitioncgroup); - virCgroupFree(&domaincgroup); + virCgroupFree(partitioncgroup); + virCgroupFree(domaincgroup); return ret; } @@ -524,10 +524,10 @@ static int testCgroupNewForPartitionDomainEscaped(const void *args G_GNUC_UNUSED ret = validateCgroup(domaincgroup, "/_cgroup.evil/net_cls.evil/__evil.evil/_cpu.foo.libvirt-lxc", mountsFull, links, placement, NULL, NULL, 0); cleanup: - virCgroupFree(&partitioncgroup3); - virCgroupFree(&partitioncgroup2); - virCgroupFree(&partitioncgroup1); - virCgroupFree(&domaincgroup); + virCgroupFree(partitioncgroup3); + virCgroupFree(partitioncgroup2); + virCgroupFree(partitioncgroup1); + virCgroupFree(domaincgroup); return ret; } @@ -553,7 +553,7 @@ static int testCgroupNewForSelfAllInOne(const void *args G_GNUC_UNUSED) ret = validateCgroup(cgroup, "", mountsAllInOne, linksAllInOne, placement, NULL, NULL, 0); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -564,7 +564,7 @@ static int testCgroupNewForSelfLogind(const void *args G_GNUC_UNUSED) if (virCgroupNewSelf(&cgroup) >= 0) { fprintf(stderr, "Expected to fail, only systemd cgroup available.\n"); - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return -1; } @@ -592,7 +592,7 @@ static int testCgroupNewForSelfUnified(const void *args G_GNUC_UNUSED) ret = validateCgroup(cgroup, "", empty, empty, empty, "/not/really/sys/fs/cgroup", "/", controllers); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -630,7 +630,7 @@ static int testCgroupNewForSelfHybrid(const void *args G_GNUC_UNUSED) "/not/really/sys/fs/cgroup/unified", "/", controllers); cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -756,7 +756,7 @@ static int testCgroupGetPercpuStats(const void *args G_GNUC_UNUSED) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); VIR_FREE(params); return ret; } @@ -789,7 +789,7 @@ static int testCgroupGetMemoryUsage(const void *args G_GNUC_UNUSED) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -849,7 +849,7 @@ testCgroupGetMemoryStat(const void *args G_GNUC_UNUSED) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -900,7 +900,7 @@ static int testCgroupGetBlkioIoServiced(const void *args G_GNUC_UNUSED) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } @@ -973,7 +973,7 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args G_GNUC_UNUSED) ret = 0; cleanup: - virCgroupFree(&cgroup); + virCgroupFree(cgroup); return ret; } diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index c3784bb91d..4d8ec38f88 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -315,7 +315,7 @@ int virHostValidateCGroupControllers(const char *hvname, } } - virCgroupFree(&group); + virCgroupFree(group); return ret; } -- 2.26.2

On Thu, Oct 08, 2020 at 11:36:56AM -0500, Jonathon Jongsma wrote: Nice catch! I'll fix it before pushing. Pavel

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/libvirt-lxc.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libvirt-lxc.c b/src/libvirt-lxc.c index 73daf123f0..f6391214be 100644 --- a/src/libvirt-lxc.c +++ b/src/libvirt-lxc.c @@ -289,7 +289,7 @@ int virDomainLxcEnterCGroup(virDomainPtr domain, unsigned int flags) { virConnectPtr conn; - virCgroupPtr cgroup = NULL; + g_autoptr(virCgroup) cgroup = NULL; VIR_DOMAIN_DEBUG(domain, "flags=0x%x", flags); @@ -307,12 +307,9 @@ int virDomainLxcEnterCGroup(virDomainPtr domain, if (virCgroupAddProcess(cgroup, getpid()) < 0) goto error; - virCgroupFree(cgroup); - return 0; error: virDispatchError(NULL); - virCgroupFree(cgroup); return -1; } -- 2.26.2

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- src/qemu/qemu_cgroup.c | 28 +++------ src/qemu/qemu_driver.c | 139 ++++++++++++++--------------------------- 2 files changed, 57 insertions(+), 110 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 473ca8a414..210cac9866 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -966,26 +966,22 @@ qemuRestoreCgroupThread(virCgroupPtr cgroup, virCgroupThreadName thread, int id) { - virCgroupPtr cgroup_temp = NULL; + g_autoptr(virCgroup) cgroup_temp = NULL; g_autofree char *nodeset = NULL; - int ret = -1; if (virCgroupNewThread(cgroup, thread, id, false, &cgroup_temp) < 0) - goto cleanup; + return -1; if (virCgroupSetCpusetMemoryMigrate(cgroup_temp, true) < 0) - goto cleanup; + return -1; if (virCgroupGetCpusetMems(cgroup_temp, &nodeset) < 0) - goto cleanup; + return -1; if (virCgroupSetCpusetMems(cgroup_temp, nodeset) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - virCgroupFree(cgroup_temp); - return ret; + return 0; } static void @@ -1129,8 +1125,7 @@ qemuSetupCgroupForExtDevices(virDomainObjPtr vm, virQEMUDriverPtr driver) { qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup_temp = NULL; - int ret = -1; + g_autoptr(virCgroup) cgroup_temp = NULL; if (!qemuExtDevicesHasDevice(vm->def) || priv->cgroup == NULL) @@ -1147,14 +1142,9 @@ qemuSetupCgroupForExtDevices(virDomainObjPtr vm, if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, false, &cgroup_temp) < 0) - goto cleanup; + return -1; - ret = qemuExtDevicesSetupCgroup(driver, vm, cgroup_temp); - - cleanup: - virCgroupFree(cgroup_temp); - - return ret; + return qemuExtDevicesSetupCgroup(driver, vm, cgroup_temp); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 60e043115f..825bdd9119 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4519,7 +4519,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, virBitmapPtr tmpmap = NULL; virDomainVcpuDefPtr vcpuinfo; qemuDomainObjPrivatePtr priv = vm->privateData; - virCgroupPtr cgroup_vcpu = NULL; + g_autoptr(virCgroup) cgroup_vcpu = NULL; g_autofree char *str = NULL; virObjectEventPtr event = NULL; char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ""; @@ -4583,7 +4583,6 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, cleanup: virBitmapFree(tmpmap); - virCgroupFree(cgroup_vcpu); virObjectEventStateQueue(driver->domainEventState, event); return ret; } @@ -4720,7 +4719,7 @@ qemuDomainPinEmulator(virDomainPtr dom, { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; - virCgroupPtr cgroup_emulator = NULL; + g_autoptr(virCgroup) cgroup_emulator = NULL; virDomainDefPtr def; virDomainDefPtr persistentDef; int ret = -1; @@ -4808,8 +4807,6 @@ qemuDomainPinEmulator(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - if (cgroup_emulator) - virCgroupFree(cgroup_emulator); virObjectEventStateQueue(driver->domainEventState, event); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); @@ -5174,7 +5171,7 @@ qemuDomainPinIOThread(virDomainPtr dom, virDomainDefPtr persistentDef; virBitmapPtr pcpumap = NULL; qemuDomainObjPrivatePtr priv; - virCgroupPtr cgroup_iothread = NULL; + g_autoptr(virCgroup) cgroup_iothread = NULL; virObjectEventPtr event = NULL; char paramField[VIR_TYPED_PARAM_FIELD_LENGTH] = ""; g_autofree char *str = NULL; @@ -5286,8 +5283,6 @@ qemuDomainPinIOThread(virDomainPtr dom, qemuDomainObjEndJob(driver, vm); cleanup: - if (cgroup_iothread) - virCgroupFree(cgroup_iothread); virObjectEventStateQueue(driver->domainEventState, event); virBitmapFree(pcpumap); virDomainObjEndAPI(&vm); @@ -8691,65 +8686,60 @@ static int qemuDomainSetNumaParamsLive(virDomainObjPtr vm, virBitmapPtr nodeset) { - virCgroupPtr cgroup_temp = NULL; + g_autoptr(virCgroup) cgroup_thread = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; g_autofree char *nodeset_str = NULL; virDomainNumatuneMemMode mode; size_t i = 0; - int ret = -1; if (virDomainNumatuneGetMode(vm->def->numa, -1, &mode) == 0 && mode != VIR_DOMAIN_NUMATUNE_MEM_STRICT) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("change of nodeset for running domain " "requires strict numa mode")); - goto cleanup; + return -1; } if (!virNumaNodesetIsAvailable(nodeset)) - goto cleanup; + return -1; /* Ensure the cpuset string is formatted before passing to cgroup */ if (!(nodeset_str = virBitmapFormat(nodeset))) - goto cleanup; + return -1; if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, - false, &cgroup_temp) < 0 || - virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) - goto cleanup; - virCgroupFree(cgroup_temp); + false, &cgroup_thread) < 0 || + virCgroupSetCpusetMems(cgroup_thread, nodeset_str) < 0) + return -1; for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) { + g_autoptr(virCgroup) cgroup_vcpu = NULL; virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i); if (!vcpu->online) continue; if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, i, - false, &cgroup_temp) < 0 || - virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) - goto cleanup; - virCgroupFree(cgroup_temp); + false, &cgroup_vcpu) < 0 || + virCgroupSetCpusetMems(cgroup_vcpu, nodeset_str) < 0) + return -1; } for (i = 0; i < vm->def->niothreadids; i++) { + g_autoptr(virCgroup) cgroup_iothread = NULL; + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, vm->def->iothreadids[i]->iothread_id, - false, &cgroup_temp) < 0 || - virCgroupSetCpusetMems(cgroup_temp, nodeset_str) < 0) - goto cleanup; - virCgroupFree(cgroup_temp); + false, &cgroup_iothread) < 0 || + virCgroupSetCpusetMems(cgroup_iothread, nodeset_str) < 0) + return -1; } /* set nodeset for root cgroup */ if (virCgroupSetCpusetMems(priv->cgroup, nodeset_str) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - virCgroupFree(cgroup_temp); - - return ret; + return 0; } static int @@ -9143,7 +9133,6 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, unsigned long long period, long long quota) { size_t i; - virCgroupPtr cgroup_vcpu = NULL; if (period == 0 && quota == 0) return 0; @@ -9152,26 +9141,21 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, return 0; for (i = 0; i < virDomainDefGetVcpusMax(vm->def); i++) { + g_autoptr(virCgroup) cgroup_vcpu = NULL; virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i); if (!vcpu->online) - continue; + return -1; if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_VCPU, i, false, &cgroup_vcpu) < 0) - goto cleanup; + return -1; if (qemuSetupCgroupVcpuBW(cgroup_vcpu, period, quota) < 0) - goto cleanup; - - virCgroupFree(cgroup_vcpu); + return -1; } return 0; - - cleanup: - virCgroupFree(cgroup_vcpu); - return -1; } static int @@ -9179,24 +9163,19 @@ qemuSetEmulatorBandwidthLive(virCgroupPtr cgroup, unsigned long long period, long long quota) { - virCgroupPtr cgroup_emulator = NULL; + g_autoptr(virCgroup) cgroup_emulator = NULL; if (period == 0 && quota == 0) return 0; if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, false, &cgroup_emulator) < 0) - goto cleanup; + return -1; if (qemuSetupCgroupVcpuBW(cgroup_emulator, period, quota) < 0) - goto cleanup; + return -1; - virCgroupFree(cgroup_emulator); return 0; - - cleanup: - virCgroupFree(cgroup_emulator); - return -1; } @@ -9205,7 +9184,6 @@ qemuSetIOThreadsBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, unsigned long long period, long long quota) { size_t i; - virCgroupPtr cgroup_iothread = NULL; if (period == 0 && quota == 0) return 0; @@ -9214,22 +9192,18 @@ qemuSetIOThreadsBWLive(virDomainObjPtr vm, virCgroupPtr cgroup, return 0; for (i = 0; i < vm->def->niothreadids; i++) { + g_autoptr(virCgroup) cgroup_iothread = NULL; + if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_IOTHREAD, vm->def->iothreadids[i]->iothread_id, false, &cgroup_iothread) < 0) - goto cleanup; + return -1; if (qemuSetupCgroupVcpuBW(cgroup_iothread, period, quota) < 0) - goto cleanup; - - virCgroupFree(cgroup_iothread); + return -1; } return 0; - - cleanup: - virCgroupFree(cgroup_iothread); - return -1; } @@ -9571,38 +9545,32 @@ static int qemuGetVcpusBWLive(virDomainObjPtr vm, unsigned long long *period, long long *quota) { - virCgroupPtr cgroup_vcpu = NULL; + g_autoptr(virCgroup) cgroup_vcpu = NULL; qemuDomainObjPrivatePtr priv = NULL; int rc; - int ret = -1; priv = vm->privateData; if (!qemuDomainHasVcpuPids(vm)) { /* We do not create sub dir for each vcpu */ rc = qemuGetVcpuBWLive(priv->cgroup, period, quota); if (rc < 0) - goto cleanup; + return -1; if (*quota > 0) *quota /= virDomainDefGetVcpus(vm->def); - goto out; + return 0; } /* get period and quota for vcpu0 */ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, 0, false, &cgroup_vcpu) < 0) - goto cleanup; + return -1; rc = qemuGetVcpuBWLive(cgroup_vcpu, period, quota); if (rc < 0) - goto cleanup; + return -1; - out: - ret = 0; - - cleanup: - virCgroupFree(cgroup_vcpu); - return ret; + return 0; } static int @@ -9610,58 +9578,47 @@ qemuGetEmulatorBandwidthLive(virCgroupPtr cgroup, unsigned long long *period, long long *quota) { - virCgroupPtr cgroup_emulator = NULL; - int ret = -1; + g_autoptr(virCgroup) cgroup_emulator = NULL; /* get period and quota for emulator */ if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, false, &cgroup_emulator) < 0) - goto cleanup; + return -1; if (qemuGetVcpuBWLive(cgroup_emulator, period, quota) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - virCgroupFree(cgroup_emulator); - return ret; + return 0; } static int qemuGetIOThreadsBWLive(virDomainObjPtr vm, unsigned long long *period, long long *quota) { - virCgroupPtr cgroup_iothread = NULL; + g_autoptr(virCgroup) cgroup_iothread = NULL; qemuDomainObjPrivatePtr priv = NULL; int rc; - int ret = -1; priv = vm->privateData; if (!vm->def->niothreadids) { /* We do not create sub dir for each iothread */ if ((rc = qemuGetVcpuBWLive(priv->cgroup, period, quota)) < 0) - goto cleanup; + return -1; - goto out; + return 0; } /* get period and quota for the "first" IOThread */ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_IOTHREAD, vm->def->iothreadids[0]->iothread_id, false, &cgroup_iothread) < 0) - goto cleanup; + return -1; rc = qemuGetVcpuBWLive(cgroup_iothread, period, quota); if (rc < 0) - goto cleanup; + return -1; - out: - ret = 0; - - cleanup: - virCgroupFree(cgroup_iothread); - return ret; + return 0; } -- 2.26.2

Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; --- tests/vircgrouptest.c | 228 +++++++++++++++--------------------------- 1 file changed, 80 insertions(+), 148 deletions(-) diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c index 632d33efcd..6c1a766fa2 100644 --- a/tests/vircgrouptest.c +++ b/tests/vircgrouptest.c @@ -183,7 +183,7 @@ testCgroupDetectMounts(const void *args) const struct _detectMountsData *data = args; char *parsed = NULL; const char *actual; - virCgroupPtr group = NULL; + g_autoptr(virCgroup) group = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; size_t i; @@ -217,15 +217,13 @@ testCgroupDetectMounts(const void *args) cleanup: g_unsetenv("VIR_CGROUP_MOCK_FILENAME"); VIR_FREE(parsed); - virCgroupFree(group); return result; } static int testCgroupNewForSelf(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; - int ret = -1; + g_autoptr(virCgroup) cgroup = NULL; const char *placement[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/system", [VIR_CGROUP_CONTROLLER_CPUACCT] = "/system", @@ -239,14 +237,10 @@ static int testCgroupNewForSelf(const void *args G_GNUC_UNUSED) if (virCgroupNewSelf(&cgroup) < 0) { fprintf(stderr, "Cannot create cgroup for self\n"); - goto cleanup; + return -1; } - ret = validateCgroup(cgroup, "", mountsFull, links, placement, NULL, NULL, 0); - - cleanup: - virCgroupFree(cgroup); - return ret; + return validateCgroup(cgroup, "", mountsFull, links, placement, NULL, NULL, 0); } @@ -256,7 +250,7 @@ static int testCgroupNewForSelf(const void *args G_GNUC_UNUSED) virErrorPtr err = virGetLastError(); \ fprintf(stderr, "Did not get " #en " error code: %d:%d\n", \ err ? err->code : 0, err ? err->int1 : 0); \ - goto cleanup; \ + return -1; \ } } while (0) /* Asking for impossible combination since CPU is co-mounted */ @@ -264,8 +258,7 @@ static int testCgroupNewForSelf(const void *args G_GNUC_UNUSED) static int testCgroupNewForPartition(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; - int ret = -1; + g_autoptr(virCgroup) cgroup = NULL; int rv; const char *placementSmall[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/virtualmachines.partition", @@ -290,7 +283,7 @@ static int testCgroupNewForPartition(const void *args G_GNUC_UNUSED) if ((rv = virCgroupNewPartition("/virtualmachines", false, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected found /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } ENSURE_ERRNO(ENOENT); @@ -299,7 +292,7 @@ static int testCgroupNewForPartition(const void *args G_GNUC_UNUSED) (1 << VIR_CGROUP_CONTROLLER_CPU), &cgroup)) != -1) { fprintf(stderr, "Should not have created /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } ENSURE_ERRNO(EINVAL); @@ -308,7 +301,7 @@ static int testCgroupNewForPartition(const void *args G_GNUC_UNUSED) (1 << VIR_CGROUP_CONTROLLER_DEVICES), &cgroup)) != -1) { fprintf(stderr, "Should not have created /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } ENSURE_ERRNO(ENXIO); @@ -319,27 +312,22 @@ static int testCgroupNewForPartition(const void *args G_GNUC_UNUSED) (1 << VIR_CGROUP_CONTROLLER_MEMORY), &cgroup)) != 0) { fprintf(stderr, "Cannot create /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } - ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsSmall, links, placementSmall, NULL, NULL, 0); + rv = validateCgroup(cgroup, "/virtualmachines.partition", mountsSmall, links, placementSmall, NULL, NULL, 0); virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/virtualmachines", true, -1, &cgroup)) != 0) { fprintf(stderr, "Cannot create /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } - ret = validateCgroup(cgroup, "/virtualmachines.partition", mountsFull, links, placementFull, NULL, NULL, 0); - - cleanup: - virCgroupFree(cgroup); - return ret; + return validateCgroup(cgroup, "/virtualmachines.partition", mountsFull, links, placementFull, NULL, NULL, 0); } static int testCgroupNewForPartitionNested(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; - int ret = -1; + g_autoptr(virCgroup) cgroup = NULL; int rv; const char *placementFull[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/deployment.partition/production.partition", @@ -354,42 +342,37 @@ static int testCgroupNewForPartitionNested(const void *args G_GNUC_UNUSED) if ((rv = virCgroupNewPartition("/deployment/production", false, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected found /deployment/production cgroup: %d\n", -rv); - goto cleanup; + return -1; } ENSURE_ERRNO(ENOENT); /* Should not work, since we require /deployment to be pre-created */ if ((rv = virCgroupNewPartition("/deployment/production", true, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected created /deployment/production cgroup: %d\n", -rv); - goto cleanup; + return -1; } ENSURE_ERRNO(ENOENT); if ((rv = virCgroupNewPartition("/deployment", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /deployment cgroup: %d\n", -rv); - goto cleanup; + return -1; } /* Should now work */ virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/deployment/production", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /deployment/production cgroup: %d\n", -rv); - goto cleanup; + return -1; } - ret = validateCgroup(cgroup, "/deployment.partition/production.partition", - mountsFull, links, placementFull, NULL, NULL, 0); - - cleanup: - virCgroupFree(cgroup); - return ret; + return validateCgroup(cgroup, "/deployment.partition/production.partition", + mountsFull, links, placementFull, NULL, NULL, 0); } static int testCgroupNewForPartitionNestedDeep(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; - int ret = -1; + g_autoptr(virCgroup) cgroup = NULL; int rv; const char *placementFull[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/user/berrange.user/production.partition", @@ -404,50 +387,45 @@ static int testCgroupNewForPartitionNestedDeep(const void *args G_GNUC_UNUSED) if ((rv = virCgroupNewPartition("/user/berrange.user/production", false, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected found /user/berrange.user/production cgroup: %d\n", -rv); - goto cleanup; + return -1; } ENSURE_ERRNO(ENOENT); /* Should not work, since we require /user/berrange.user to be pre-created */ if ((rv = virCgroupNewPartition("/user/berrange.user/production", true, -1, &cgroup)) != -1) { fprintf(stderr, "Unexpected created /user/berrange.user/production cgroup: %d\n", -rv); - goto cleanup; + return -1; } ENSURE_ERRNO(ENOENT); if ((rv = virCgroupNewPartition("/user", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user cgroup: %d\n", -rv); - goto cleanup; + return -1; } virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/user/berrange.user", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user cgroup: %d\n", -rv); - goto cleanup; + return -1; } /* Should now work */ virCgroupFree(cgroup); if ((rv = virCgroupNewPartition("/user/berrange.user/production", true, -1, &cgroup)) != 0) { fprintf(stderr, "Failed to create /user/berrange.user/production cgroup: %d\n", -rv); - goto cleanup; + return -1; } - ret = validateCgroup(cgroup, "/user/berrange.user/production.partition", - mountsFull, links, placementFull, NULL, NULL, 0); - - cleanup: - virCgroupFree(cgroup); - return ret; + return validateCgroup(cgroup, "/user/berrange.user/production.partition", + mountsFull, links, placementFull, NULL, NULL, 0); } static int testCgroupNewForPartitionDomain(const void *args G_GNUC_UNUSED) { - virCgroupPtr partitioncgroup = NULL; - virCgroupPtr domaincgroup = NULL; - int ret = -1; + g_autoptr(virCgroup) partitioncgroup = NULL; + g_autoptr(virCgroup) domaincgroup = NULL; int rv; const char *placement[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/production.partition/foo.libvirt-lxc", @@ -462,29 +440,23 @@ static int testCgroupNewForPartitionDomain(const void *args G_GNUC_UNUSED) if ((rv = virCgroupNewPartition("/production", true, -1, &partitioncgroup)) != 0) { fprintf(stderr, "Failed to create /production cgroup: %d\n", -rv); - goto cleanup; + return -1; } if ((rv = virCgroupNewDomainPartition(partitioncgroup, "lxc", "foo", true, &domaincgroup)) != 0) { fprintf(stderr, "Cannot create LXC cgroup: %d\n", -rv); - goto cleanup; + return -1; } - ret = validateCgroup(domaincgroup, "/production.partition/foo.libvirt-lxc", mountsFull, links, placement, NULL, NULL, 0); - - cleanup: - virCgroupFree(partitioncgroup); - virCgroupFree(domaincgroup); - return ret; + return validateCgroup(domaincgroup, "/production.partition/foo.libvirt-lxc", mountsFull, links, placement, NULL, NULL, 0); } static int testCgroupNewForPartitionDomainEscaped(const void *args G_GNUC_UNUSED) { - virCgroupPtr partitioncgroup1 = NULL; - virCgroupPtr partitioncgroup2 = NULL; - virCgroupPtr partitioncgroup3 = NULL; - virCgroupPtr domaincgroup = NULL; - int ret = -1; + g_autoptr(virCgroup) partitioncgroup1 = NULL; + g_autoptr(virCgroup) partitioncgroup2 = NULL; + g_autoptr(virCgroup) partitioncgroup3 = NULL; + g_autoptr(virCgroup) domaincgroup = NULL; int rv; const char *placement[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/_cgroup.evil/net_cls.evil/__evil.evil/_cpu.foo.libvirt-lxc", @@ -499,42 +471,34 @@ static int testCgroupNewForPartitionDomainEscaped(const void *args G_GNUC_UNUSED if ((rv = virCgroupNewPartition("/cgroup.evil", true, -1, &partitioncgroup1)) != 0) { fprintf(stderr, "Failed to create /cgroup.evil cgroup: %d\n", -rv); - goto cleanup; + return -1; } if ((rv = virCgroupNewPartition("/cgroup.evil/net_cls.evil", true, -1, &partitioncgroup2)) != 0) { fprintf(stderr, "Failed to create /cgroup.evil/cpu.evil cgroup: %d\n", -rv); - goto cleanup; + return -1; } if ((rv = virCgroupNewPartition("/cgroup.evil/net_cls.evil/_evil.evil", true, -1, &partitioncgroup3)) != 0) { fprintf(stderr, "Failed to create /cgroup.evil cgroup: %d\n", -rv); - goto cleanup; + return -1; } if ((rv = virCgroupNewDomainPartition(partitioncgroup3, "lxc", "cpu.foo", true, &domaincgroup)) != 0) { fprintf(stderr, "Cannot create LXC cgroup: %d\n", -rv); - goto cleanup; + return -1; } /* NB we're not expecting 'net_cls.evil' to be escaped, * since our fake /proc/cgroups pretends this controller * isn't compiled into the kernel */ - ret = validateCgroup(domaincgroup, "/_cgroup.evil/net_cls.evil/__evil.evil/_cpu.foo.libvirt-lxc", mountsFull, links, placement, NULL, NULL, 0); - - cleanup: - virCgroupFree(partitioncgroup3); - virCgroupFree(partitioncgroup2); - virCgroupFree(partitioncgroup1); - virCgroupFree(domaincgroup); - return ret; + return validateCgroup(domaincgroup, "/_cgroup.evil/net_cls.evil/__evil.evil/_cpu.foo.libvirt-lxc", mountsFull, links, placement, NULL, NULL, 0); } static int testCgroupNewForSelfAllInOne(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; - int ret = -1; + g_autoptr(virCgroup) cgroup = NULL; const char *placement[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPU] = "/", [VIR_CGROUP_CONTROLLER_CPUACCT] = "/", @@ -547,24 +511,19 @@ static int testCgroupNewForSelfAllInOne(const void *args G_GNUC_UNUSED) if (virCgroupNewSelf(&cgroup) < 0) { fprintf(stderr, "Cannot create cgroup for self\n"); - goto cleanup; + return -1; } - ret = validateCgroup(cgroup, "", mountsAllInOne, linksAllInOne, placement, NULL, NULL, 0); - - cleanup: - virCgroupFree(cgroup); - return ret; + return validateCgroup(cgroup, "", mountsAllInOne, linksAllInOne, placement, NULL, NULL, 0); } static int testCgroupNewForSelfLogind(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; + g_autoptr(virCgroup) cgroup = NULL; if (virCgroupNewSelf(&cgroup) >= 0) { fprintf(stderr, "Expected to fail, only systemd cgroup available.\n"); - virCgroupFree(cgroup); return -1; } @@ -574,8 +533,7 @@ static int testCgroupNewForSelfLogind(const void *args G_GNUC_UNUSED) static int testCgroupNewForSelfUnified(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; - int ret = -1; + g_autoptr(virCgroup) cgroup = NULL; const char *empty[VIR_CGROUP_CONTROLLER_LAST] = { 0 }; unsigned int controllers = (1 << VIR_CGROUP_CONTROLLER_CPU) | @@ -586,21 +544,17 @@ static int testCgroupNewForSelfUnified(const void *args G_GNUC_UNUSED) if (virCgroupNewSelf(&cgroup) < 0) { fprintf(stderr, "Cannot create cgroup for self\n"); - goto cleanup; + return -1; } - ret = validateCgroup(cgroup, "", empty, empty, empty, - "/not/really/sys/fs/cgroup", "/", controllers); - cleanup: - virCgroupFree(cgroup); - return ret; + return validateCgroup(cgroup, "", empty, empty, empty, + "/not/really/sys/fs/cgroup", "/", controllers); } static int testCgroupNewForSelfHybrid(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; - int ret = -1; + g_autoptr(virCgroup) cgroup = NULL; const char *empty[VIR_CGROUP_CONTROLLER_LAST] = { 0 }; const char *mounts[VIR_CGROUP_CONTROLLER_LAST] = { [VIR_CGROUP_CONTROLLER_CPUSET] = "/not/really/sys/fs/cgroup/cpuset", @@ -623,15 +577,11 @@ static int testCgroupNewForSelfHybrid(const void *args G_GNUC_UNUSED) if (virCgroupNewSelf(&cgroup) < 0) { fprintf(stderr, "Cannot create cgroup for self\n"); - goto cleanup; + return -1; } - ret = validateCgroup(cgroup, "", mounts, empty, placement, - "/not/really/sys/fs/cgroup/unified", "/", controllers); - - cleanup: - virCgroupFree(cgroup); - return ret; + return validateCgroup(cgroup, "", mounts, empty, placement, + "/not/really/sys/fs/cgroup/unified", "/", controllers); } @@ -679,7 +629,7 @@ static int testCgroupControllerAvailable(const void *args G_GNUC_UNUSED) static int testCgroupGetPercpuStats(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; + g_autoptr(virCgroup) cgroup = NULL; size_t i; int rv, ret = -1; virTypedParameterPtr params = NULL; @@ -756,50 +706,44 @@ static int testCgroupGetPercpuStats(const void *args G_GNUC_UNUSED) ret = 0; cleanup: - virCgroupFree(cgroup); VIR_FREE(params); return ret; } static int testCgroupGetMemoryUsage(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; - int rv, ret = -1; + g_autoptr(virCgroup) cgroup = NULL; + int rv; unsigned long kb; if ((rv = virCgroupNewPartition("/virtualmachines", true, (1 << VIR_CGROUP_CONTROLLER_MEMORY), &cgroup)) < 0) { fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } if ((rv = virCgroupGetMemoryUsage(cgroup, &kb)) < 0) { fprintf(stderr, "Could not retrieve GetMemoryUsage for /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } if (kb != 1421212UL) { fprintf(stderr, "Wrong value from virCgroupGetMemoryUsage (expected %ld)\n", 1421212UL); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - virCgroupFree(cgroup); - return ret; + return 0; } static int testCgroupGetMemoryStat(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; + g_autoptr(virCgroup) cgroup = NULL; int rv; - int ret = -1; size_t i; const unsigned long long expected_values[] = { @@ -824,7 +768,7 @@ testCgroupGetMemoryStat(const void *args G_GNUC_UNUSED) (1 << VIR_CGROUP_CONTROLLER_MEMORY), &cgroup)) < 0) { fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } if ((rv = virCgroupGetMemoryStat(cgroup, &values[0], @@ -832,7 +776,7 @@ testCgroupGetMemoryStat(const void *args G_GNUC_UNUSED) &values[3], &values[4], &values[5])) < 0) { fprintf(stderr, "Could not retrieve GetMemoryStat for /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } for (i = 0; i < G_N_ELEMENTS(expected_values); i++) { @@ -842,23 +786,19 @@ testCgroupGetMemoryStat(const void *args G_GNUC_UNUSED) "Wrong value (%llu) for %s from virCgroupGetMemoryStat " "(expected %llu)\n", values[i], names[i], (expected_values[i] >> 10)); - goto cleanup; + return -1; } } - ret = 0; - - cleanup: - virCgroupFree(cgroup); - return ret; + return 0; } static int testCgroupGetBlkioIoServiced(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; + g_autoptr(virCgroup) cgroup = NULL; size_t i; - int rv, ret = -1; + int rv; const long long expected_values[] = { 119084214273ULL, @@ -878,14 +818,14 @@ static int testCgroupGetBlkioIoServiced(const void *args G_GNUC_UNUSED) (1 << VIR_CGROUP_CONTROLLER_BLKIO), &cgroup)) < 0) { fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } if ((rv = virCgroupGetBlkioIoServiced(cgroup, values, &values[1], &values[2], &values[3])) < 0) { fprintf(stderr, "Could not retrieve BlkioIoServiced for /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } for (i = 0; i < G_N_ELEMENTS(expected_values); i++) { @@ -893,22 +833,18 @@ static int testCgroupGetBlkioIoServiced(const void *args G_GNUC_UNUSED) fprintf(stderr, "Wrong value for %s from virCgroupBlkioIoServiced (expected %lld)\n", names[i], expected_values[i]); - goto cleanup; + return -1; } } - ret = 0; - - cleanup: - virCgroupFree(cgroup); - return ret; + return 0; } static int testCgroupGetBlkioIoDeviceServiced(const void *args G_GNUC_UNUSED) { - virCgroupPtr cgroup = NULL; + g_autoptr(virCgroup) cgroup = NULL; size_t i; - int rv, ret = -1; + int rv; const long long expected_values0[] = { 59542107136ULL, 411440480256ULL, @@ -933,7 +869,7 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args G_GNUC_UNUSED) (1 << VIR_CGROUP_CONTROLLER_BLKIO), &cgroup)) < 0) { fprintf(stderr, "Could not create /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } if ((rv = virCgroupGetBlkioIoDeviceServiced(cgroup, @@ -941,7 +877,7 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args G_GNUC_UNUSED) values, &values[1], &values[2], &values[3])) < 0) { fprintf(stderr, "Could not retrieve BlkioIoDeviceServiced for /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } for (i = 0; i < G_N_ELEMENTS(expected_values0); i++) { @@ -949,7 +885,7 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args G_GNUC_UNUSED) fprintf(stderr, "Wrong value for %s from virCgroupGetBlkioIoDeviceServiced (expected %lld)\n", names[i], expected_values0[i]); - goto cleanup; + return -1; } } @@ -958,7 +894,7 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args G_GNUC_UNUSED) values, &values[1], &values[2], &values[3])) < 0) { fprintf(stderr, "Could not retrieve BlkioIoDeviceServiced for /virtualmachines cgroup: %d\n", -rv); - goto cleanup; + return -1; } for (i = 0; i < G_N_ELEMENTS(expected_values1); i++) { @@ -966,15 +902,11 @@ static int testCgroupGetBlkioIoDeviceServiced(const void *args G_GNUC_UNUSED) fprintf(stderr, "Wrong value for %s from virCgroupGetBlkioIoDeviceServiced (expected %lld)\n", names[i], expected_values1[i]); - goto cleanup; + return -1; } } - ret = 0; - - cleanup: - virCgroupFree(cgroup); - return ret; + return 0; } # define FAKEROOTDIRTEMPLATE abs_builddir "/fakerootdir-XXXXXX" -- 2.26.2