On 31. 3. 2020 17:44, Seeteena Thoufeek wrote:
Signed-off-by: Seeteena Thoufeek <s1seetee@linux.vnet.ibm.com> --- src/qemu/qemu_cgroup.c | 91 +++++++++++++++++++------------------------------- 1 file changed, 35 insertions(+), 56 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index c0e30f6..d34c515 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -62,7 +62,7 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm, { qemuDomainObjPrivatePtr priv = vm->privateData; int perms = VIR_CGROUP_DEVICE_READ; - char **targetPaths = NULL; + VIR_AUTOSTRINGLIST targetPaths = NULL; size_t i; int rv; int ret = -1;> @@ -82,12 +82,11 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm, virCgroupGetDevicePermsString(perms), rv); if (rv < 0) - goto cleanup; + return ret;
I don't quite understand. Why not return -1 immediatelly? This also applies to the test of the patch.
if (rv > 0) { /* @path is neither character device nor block device. */ - ret = 0; - goto cleanup; + return 0; }
if (virDevMapperGetTargets(path, &targetPaths) < 0 &&
@@ -964,16 +956,9 @@ qemuInitCgroup(virDomainObjPtr vm, cfg->maxThreadsPerProc, &priv->cgroup) < 0) { if (virCgroupNewIgnoreError()) - goto done; - - goto cleanup; + return 0;
So previouslly, it this has failed, then if the error was to be ingored then 0 was returned and -1 if it wasn't. But with this change 0 is returned no matter what.
} - - done: - ret = 0; - cleanup: - virObjectUnref(cfg); - return ret; + return ret;
I don't think this is right. Previously, in success path ret was set to 0. But you removed that, so this returns -1, allways.
}
static void @@ -1058,14 +1043,14 @@ int qemuConnectCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(priv->driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver); int ret = -1;
if (!virQEMUDriverIsPrivileged(priv->driver)) - goto done; + return 0;
if (!virCgroupAvailable()) - goto done; + return 0;
virCgroupFree(&priv->cgroup);
@@ -1075,14 +1060,9 @@ qemuConnectCgroup(virDomainObjPtr vm) cfg->cgroupControllers, priv->machineName, &priv->cgroup) < 0) - goto cleanup; + return ret;
qemuRestoreCgroupState(vm); - - done: - ret = 0; - cleanup: - virObjectUnref(cfg); return ret; }
@@ -1269,7 +1249,7 @@ qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, qemuCgroupEmulatorAllNodesDataPtr *retData) { qemuCgroupEmulatorAllNodesDataPtr data = NULL; - char *all_nodes_str = NULL; + g_autofree char *all_nodes_str = NULL; virBitmapPtr all_nodes = NULL;
virBitmap can be made g_autoptr() too.
int ret = -1;
@@ -1298,7 +1278,6 @@ qemuCgroupEmulatorAllNodesAllow(virCgroupPtr cgroup, ret = 0;
cleanup: - VIR_FREE(all_nodes_str); virBitmapFree(all_nodes); qemuCgroupEmulatorAllNodesDataFree(data);
Nevertheless, I'm fixing all the issues, ACKing and pushing. Michal