Re: [PATCH] qemu_cgroup.c: use VIR_AUTOSTRINGLIST, g_autofree and g_autoptr
On 31. 3. 2020 17:44, Seeteena Thoufeek wrote:
Signed-off-by: Seeteena Thoufeek <s1seetee(a)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