[libvirt] [PATCH] vircgroup: no need to ifdef virCgroupFree

Pavel Hrdina posted 1 patch 5 years ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/ff5c7395adcd45b611a776c9511749a09619d6e5.1556112791.git.phrdina@redhat.com
src/util/vircgroup.c | 62 +++++++++++++++++++-------------------------
1 file changed, 27 insertions(+), 35 deletions(-)
[libvirt] [PATCH] vircgroup: no need to ifdef virCgroupFree
Posted by Pavel Hrdina 5 years ago
virCgroup struct is always defined and the free function is not calling
anything that would require OS supporting cgroups.

This fixes an issue if we try to start a VM with QEMU binary that
doesn't support QXL.  The start operation will fail in
qemuProcessStartValidateVideo() which will set correct error message,
but later in one of the cleanup paths we will call
qemuDomainObjPrivateDataClear() which always calls virCgroupFree()
and that will fail on OS that doesn't support cgroups and it will
set a new error which will be eventually reported to user.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/util/vircgroup.c | 62 +++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 35 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 4238d7014b..f58e336404 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1279,33 +1279,6 @@ virCgroupNewIgnoreError(void)
 }
 
 
-/**
- * virCgroupFree:
- *
- * @group: The group structure to free
- */
-void
-virCgroupFree(virCgroupPtr *group)
-{
-    size_t i;
-
-    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)->unified.mountPoint);
-    VIR_FREE((*group)->unified.placement);
-
-    VIR_FREE((*group)->path);
-    VIR_FREE(*group);
-}
-
-
 /**
  * virCgroupHasController: query whether a cgroup controller is present
  *
@@ -2917,14 +2890,6 @@ virCgroupNewIgnoreError(void)
 }
 
 
-void
-virCgroupFree(virCgroupPtr *group ATTRIBUTE_UNUSED)
-{
-    virReportSystemError(ENXIO, "%s",
-                         _("Control groups not supported on this platform"));
-}
-
-
 bool
 virCgroupHasController(virCgroupPtr cgroup ATTRIBUTE_UNUSED,
                        int controller ATTRIBUTE_UNUSED)
@@ -3565,6 +3530,33 @@ virCgroupControllerAvailable(int controller ATTRIBUTE_UNUSED)
 #endif /* !__linux__ */
 
 
+/**
+ * virCgroupFree:
+ *
+ * @group: The group structure to free
+ */
+void
+virCgroupFree(virCgroupPtr *group)
+{
+    size_t i;
+
+    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)->unified.mountPoint);
+    VIR_FREE((*group)->unified.placement);
+
+    VIR_FREE((*group)->path);
+    VIR_FREE(*group);
+}
+
+
 int
 virCgroupDelThread(virCgroupPtr cgroup,
                    virCgroupThreadName nameval,
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vircgroup: no need to ifdef virCgroupFree
Posted by Martin Kletzander 5 years ago
ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list