[PATCH v2 01/40] util: virresctrl: convert classes to GObject

Rafael Fonseca posted 40 patches 5 years, 8 months ago
[PATCH v2 01/40] util: virresctrl: convert classes to GObject
Posted by Rafael Fonseca 5 years, 8 months ago
This patch changes virResctrlInfo, virResctrlAlloc and virResctrlMonitor
from virObject to GObject.

Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
---
 src/conf/capabilities.c  |   2 +-
 src/conf/domain_conf.c   |  19 ++---
 src/libvirt_private.syms |   3 +
 src/util/virresctrl.c    | 164 ++++++++++++++++++++-------------------
 src/util/virresctrl.h    |  15 ++--
 tests/virresctrltest.c   |   3 +-
 6 files changed, 107 insertions(+), 99 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 99b69aebb5..0741d76731 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -266,7 +266,7 @@ virCapsDispose(void *object)
     VIR_FREE(caps->host.netprefix);
     VIR_FREE(caps->host.pagesSize);
     virCPUDefFree(caps->host.cpu);
-    virObjectUnref(caps->host.resctrl);
+    g_clear_object(&caps->host.resctrl);
 }
 
 /**
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8700d56761..674cfa408e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3304,7 +3304,7 @@ virDomainResctrlMonDefFree(virDomainResctrlMonDefPtr domresmon)
         return;
 
     virBitmapFree(domresmon->vcpus);
-    virObjectUnref(domresmon->instance);
+    g_clear_object(&domresmon->instance);
     VIR_FREE(domresmon);
 }
 
@@ -3320,7 +3320,7 @@ virDomainResctrlDefFree(virDomainResctrlDefPtr resctrl)
     for (i = 0; i < resctrl->nmonitors; i++)
         virDomainResctrlMonDefFree(resctrl->monitors[i]);
 
-    virObjectUnref(resctrl->alloc);
+    g_clear_object(&resctrl->alloc);
     virBitmapFree(resctrl->vcpus);
     VIR_FREE(resctrl->monitors);
     VIR_FREE(resctrl);
@@ -19950,11 +19950,6 @@ virDomainResctrlMonDefParse(virDomainDefPtr def,
         domresmon->tag = tag;
 
         domresmon->instance = virResctrlMonitorNew();
-        if (!domresmon->instance) {
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("Could not create monitor"));
-            goto cleanup;
-        }
 
         if (tag == VIR_RESCTRL_MONITOR_TYPE_CACHE) {
             tmp = virXMLPropString(nodes[i], "level");
@@ -20064,7 +20059,7 @@ virDomainResctrlNew(xmlNodePtr node,
         goto cleanup;
     }
 
-    resctrl->alloc = virObjectRef(alloc);
+    resctrl->alloc = g_object_ref(alloc);
 
     ret = g_steal_pointer(&resctrl);
  cleanup:
@@ -20111,8 +20106,7 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
         return -1;
     }
 
-    if (!(alloc = virResctrlAllocNew()))
-        return -1;
+    alloc = virResctrlAllocNew();
 
     for (i = 0; i < n; i++) {
         if (virDomainCachetuneDefParseCache(ctxt, nodes[i], alloc) < 0)
@@ -20297,10 +20291,9 @@ virDomainMemorytuneDefParse(virDomainDefPtr def,
         return -1;
 
     if (resctrl) {
-        alloc = virObjectRef(resctrl->alloc);
+        alloc = g_object_ref(resctrl->alloc);
     } else {
-        if (!(alloc = virResctrlAllocNew()))
-            return -1;
+        alloc = virResctrlAllocNew();
     }
 
     /* First, parse <memorytune/node> element if any <node> element exists */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 182a570382..7bba191b03 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2916,6 +2916,9 @@ virRandomInt;
 
 
 # util/virresctrl.h
+vir_resctrl_alloc_get_type;
+vir_resctrl_info_get_type;
+vir_resctrl_monitor_get_type;
 virCacheKernelTypeFromString;
 virCacheKernelTypeToString;
 virCacheTypeFromString;
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index c537d606cc..ce11a24a54 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -112,12 +112,6 @@ typedef struct _virResctrlAllocMemBW virResctrlAllocMemBW;
 typedef virResctrlAllocMemBW *virResctrlAllocMemBWPtr;
 
 
-/* Class definitions and initializations */
-static virClassPtr virResctrlInfoClass;
-static virClassPtr virResctrlAllocClass;
-static virClassPtr virResctrlMonitorClass;
-
-
 /* virResctrlInfo */
 struct _virResctrlInfoPerType {
     /* Kernel-provided information */
@@ -182,7 +176,7 @@ struct _virResctrlInfoMongrp {
 };
 
 struct _virResctrlInfo {
-    virObject parent;
+    GObject parent;
 
     virResctrlInfoPerLevelPtr *levels;
     size_t nlevels;
@@ -192,14 +186,17 @@ struct _virResctrlInfo {
     virResctrlInfoMongrpPtr monitor_info;
 };
 
+/* Class definitions and initializations */
+G_DEFINE_TYPE(virResctrlInfo, vir_resctrl_info, G_TYPE_OBJECT);
+
 
 static void
-virResctrlInfoDispose(void *obj)
+virResctrlInfoFinalize(GObject *obj)
 {
     size_t i = 0;
     size_t j = 0;
 
-    virResctrlInfoPtr resctrl = obj;
+    virResctrlInfoPtr resctrl = VIR_RESCTRL_INFO(obj);
 
     for (i = 0; i < resctrl->nlevels; i++) {
         virResctrlInfoPerLevelPtr level = resctrl->levels[i];
@@ -221,6 +218,8 @@ virResctrlInfoDispose(void *obj)
     VIR_FREE(resctrl->membw_info);
     VIR_FREE(resctrl->levels);
     VIR_FREE(resctrl->monitor_info);
+
+    G_OBJECT_CLASS(vir_resctrl_info_parent_class)->finalize(obj);
 }
 
 
@@ -334,7 +333,7 @@ struct _virResctrlAllocMemBW {
 };
 
 struct _virResctrlAlloc {
-    virObject parent;
+    GObject parent;
 
     virResctrlAllocPerLevelPtr *levels;
     size_t nlevels;
@@ -355,7 +354,7 @@ struct _virResctrlAlloc {
  * memory bandwidth.
  */
 struct _virResctrlMonitor {
-    virObject parent;
+    GObject parent;
 
     /* Each virResctrlMonitor is associated with one specific allocation,
      * either the root directory allocation under /sys/fs/resctrl or a
@@ -372,15 +371,20 @@ struct _virResctrlMonitor {
     char *path;
 };
 
+/* Class definitions and initializations */
+G_DEFINE_TYPE(virResctrlAlloc, vir_resctrl_alloc, G_TYPE_OBJECT);
+G_DEFINE_TYPE(virResctrlMonitor, vir_resctrl_monitor, G_TYPE_OBJECT);
+
+
 
 static void
-virResctrlAllocDispose(void *obj)
+virResctrlAllocFinalize(GObject *obj)
 {
     size_t i = 0;
     size_t j = 0;
     size_t k = 0;
 
-    virResctrlAllocPtr alloc = obj;
+    virResctrlAllocPtr alloc = VIR_RESCTRL_ALLOC(obj);
 
     for (i = 0; i < alloc->nlevels; i++) {
         virResctrlAllocPerLevelPtr level = alloc->levels[i];
@@ -419,38 +423,71 @@ virResctrlAllocDispose(void *obj)
     VIR_FREE(alloc->id);
     VIR_FREE(alloc->path);
     VIR_FREE(alloc->levels);
+
+    G_OBJECT_CLASS(vir_resctrl_alloc_parent_class)->finalize(obj);
 }
 
 
 static void
-virResctrlMonitorDispose(void *obj)
+virResctrlMonitorDispose(GObject *obj)
 {
-    virResctrlMonitorPtr monitor = obj;
+    virResctrlMonitorPtr monitor = VIR_RESCTRL_MONITOR(obj);
+
+    g_clear_object(&monitor->alloc);
+
+    G_OBJECT_CLASS(vir_resctrl_alloc_parent_class)->dispose(obj);
+}
+
+static void
+virResctrlMonitorFinalize(GObject *obj)
+{
+    virResctrlMonitorPtr monitor = VIR_RESCTRL_MONITOR(obj);
 
-    virObjectUnref(monitor->alloc);
     VIR_FREE(monitor->id);
     VIR_FREE(monitor->path);
+
+    G_OBJECT_CLASS(vir_resctrl_monitor_parent_class)->finalize(obj);
 }
 
+static void
+vir_resctrl_info_init(virResctrlInfo *resctrlinfo G_GNUC_UNUSED)
+{
+}
 
-/* Global initialization for classes */
-static int
-virResctrlOnceInit(void)
+static void
+vir_resctrl_info_class_init(virResctrlInfoClass *klass)
 {
-    if (!VIR_CLASS_NEW(virResctrlInfo, virClassForObject()))
-        return -1;
+    GObjectClass *obj = G_OBJECT_CLASS(klass);
 
-    if (!VIR_CLASS_NEW(virResctrlAlloc, virClassForObject()))
-        return -1;
+    obj->finalize = virResctrlInfoFinalize;
+}
 
-    if (!VIR_CLASS_NEW(virResctrlMonitor, virClassForObject()))
-        return -1;
+static void
+vir_resctrl_alloc_init(virResctrlAlloc *resctrlalloc G_GNUC_UNUSED)
+{
+}
 
-    return 0;
+static void
+vir_resctrl_alloc_class_init(virResctrlAllocClass *klass)
+{
+    GObjectClass *obj = G_OBJECT_CLASS(klass);
+
+    obj->finalize = virResctrlAllocFinalize;
+}
+
+static void
+vir_resctrl_monitor_init(virResctrlMonitor *resctrlmon G_GNUC_UNUSED)
+{
 }
 
-VIR_ONCE_GLOBAL_INIT(virResctrl);
+static void
+vir_resctrl_monitor_class_init(virResctrlMonitorClass *klass)
+{
+    GObjectClass *obj = G_OBJECT_CLASS(klass);
 
+    obj->dispose = virResctrlMonitorDispose;
+    obj->finalize = virResctrlMonitorFinalize;
+}
 
 /* Common functions */
 static int
@@ -793,21 +830,13 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl)
 virResctrlInfoPtr
 virResctrlInfoNew(void)
 {
-    virResctrlInfoPtr ret = NULL;
-
-    if (virResctrlInitialize() < 0)
-        return NULL;
-
-    ret = virObjectNew(virResctrlInfoClass);
-    if (!ret)
-        return NULL;
+    g_autoptr(virResctrlInfo) ret = VIR_RESCTRL_INFO(g_object_new(VIR_TYPE_RESCTRL_INFO, NULL));
 
     if (virResctrlGetInfo(ret) < 0) {
-        virObjectUnref(ret);
         return NULL;
     }
 
-    return ret;
+    return g_steal_pointer(&ret);
 }
 
 
@@ -1034,10 +1063,7 @@ virResctrlInfoGetMonitorPrefix(virResctrlInfoPtr resctrl,
 virResctrlAllocPtr
 virResctrlAllocNew(void)
 {
-    if (virResctrlInitialize() < 0)
-        return NULL;
-
-    return virObjectNew(virResctrlAllocClass);
+    return VIR_RESCTRL_ALLOC(g_object_new(VIR_TYPE_RESCTRL_ALLOC, NULL));
 }
 
 
@@ -1769,8 +1795,7 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl,
 
  error:
     VIR_FREE(schemata);
-    virObjectUnref(*alloc);
-    *alloc = NULL;
+    g_clear_object(alloc);
     return -1;
 }
 
@@ -1779,9 +1804,7 @@ static virResctrlAllocPtr
 virResctrlAllocGetDefault(virResctrlInfoPtr resctrl)
 {
     virResctrlAllocPtr ret = NULL;
-    int rv = virResctrlAllocGetGroup(resctrl, ".", &ret);
-
-    if (rv == -2) {
+    if (virResctrlAllocGetGroup(resctrl, ".", &ret) == -2) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("Could not read schemata file for the default group"));
     }
@@ -1836,9 +1859,6 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
     virResctrlAllocPtr ret = virResctrlAllocNew();
     virBitmapPtr mask = NULL;
 
-    if (!ret)
-        return NULL;
-
     for (i = 0; i < info->nlevels; i++) {
         virResctrlInfoPerLevelPtr i_level = info->levels[i];
 
@@ -1884,8 +1904,7 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info)
     virBitmapFree(mask);
     return ret;
  error:
-    virObjectUnref(ret);
-    ret = NULL;
+    g_clear_object(&ret);
     goto cleanup;
 }
 
@@ -1907,7 +1926,7 @@ virResctrlAllocPtr
 virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
 {
     virResctrlAllocPtr ret = NULL;
-    virResctrlAllocPtr alloc = NULL;
+    g_autoptr(virResctrlAlloc) alloc = NULL;
     struct dirent *ent = NULL;
     DIR *dirp = NULL;
     int rv = -1;
@@ -1927,7 +1946,7 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
         goto error;
 
     virResctrlAllocSubtract(ret, alloc);
-    virObjectUnref(alloc);
+    g_clear_object(&alloc);
 
     if (virDirOpen(&dirp, SYSFS_RESCTRL_PATH) < 0)
         goto error;
@@ -1948,20 +1967,17 @@ virResctrlAllocGetUnused(virResctrlInfoPtr resctrl)
         }
 
         virResctrlAllocSubtract(ret, alloc);
-        virObjectUnref(alloc);
-        alloc = NULL;
+        g_clear_object(&alloc);
     }
     if (rv < 0)
         goto error;
 
  cleanup:
-    virObjectUnref(alloc);
     VIR_DIR_CLOSE(dirp);
     return ret;
 
  error:
-    virObjectUnref(ret);
-    ret = NULL;
+    g_clear_object(&ret);
     goto cleanup;
 }
 
@@ -2229,10 +2245,9 @@ static int
 virResctrlAllocAssign(virResctrlInfoPtr resctrl,
                       virResctrlAllocPtr alloc)
 {
-    int ret = -1;
     unsigned int level = 0;
-    virResctrlAllocPtr alloc_free = NULL;
-    virResctrlAllocPtr alloc_default = NULL;
+    g_autoptr(virResctrlAlloc) alloc_free = NULL;
+    g_autoptr(virResctrlAlloc) alloc_default = NULL;
 
     alloc_free = virResctrlAllocGetUnused(resctrl);
     if (!alloc_free)
@@ -2240,16 +2255,16 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
 
     alloc_default = virResctrlAllocGetDefault(resctrl);
     if (!alloc_default)
-        goto cleanup;
+        return -1;
 
     if (virResctrlAllocMemoryBandwidth(resctrl, alloc) < 0)
-        goto cleanup;
+        return -1;
 
     if (virResctrlAllocCopyMasks(alloc, alloc_default) < 0)
-        goto cleanup;
+        return -1;
 
     if (virResctrlAllocCopyMemBW(alloc, alloc_default) < 0)
-        goto cleanup;
+        return -1;
 
     for (level = 0; level < alloc->nlevels; level++) {
         virResctrlAllocPerLevelPtr a_level = alloc->levels[level];
@@ -2266,7 +2281,7 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("Cache level %d does not support tuning"),
                            level);
-            goto cleanup;
+            return -1;
         }
 
         for (type = 0; type < VIR_CACHE_TYPE_LAST; type++) {
@@ -2282,7 +2297,7 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
                                _("Cache level %d does not support tuning for "
                                  "scope type '%s'"),
                                level, virCacheTypeToString(type));
-                goto cleanup;
+                return -1;
             }
 
             for (cache = 0; cache < a_type->nsizes; cache++) {
@@ -2290,16 +2305,12 @@ virResctrlAllocAssign(virResctrlInfoPtr resctrl,
                 virResctrlInfoPerTypePtr i_type = i_level->types[type];
 
                 if (virResctrlAllocFindUnused(alloc, i_type, f_type, level, type, cache) < 0)
-                    goto cleanup;
+                    return -1;
             }
         }
     }
 
-    ret = 0;
- cleanup:
-    virObjectUnref(alloc_free);
-    virObjectUnref(alloc_default);
-    return ret;
+    return 0;
 }
 
 
@@ -2506,10 +2517,7 @@ virResctrlAllocRemove(virResctrlAllocPtr alloc)
 virResctrlMonitorPtr
 virResctrlMonitorNew(void)
 {
-    if (virResctrlInitialize() < 0)
-        return NULL;
-
-    return virObjectNew(virResctrlMonitorClass);
+    return VIR_RESCTRL_MONITOR(g_object_new(VIR_TYPE_RESCTRL_MONITOR, NULL));
 }
 
 
@@ -2623,7 +2631,7 @@ void
 virResctrlMonitorSetAlloc(virResctrlMonitorPtr monitor,
                           virResctrlAllocPtr alloc)
 {
-    monitor->alloc = virObjectRef(alloc);
+    monitor->alloc = g_object_ref(alloc);
 }
 
 
diff --git a/src/util/virresctrl.h b/src/util/virresctrl.h
index dcd9ca2bb2..6724d904d1 100644
--- a/src/util/virresctrl.h
+++ b/src/util/virresctrl.h
@@ -19,8 +19,8 @@
 #pragma once
 
 #include "internal.h"
+#include <glib-object.h>
 
-#include "virobject.h"
 #include "virbitmap.h"
 #include "virenum.h"
 
@@ -93,7 +93,9 @@ struct _virResctrlInfoMon {
     unsigned int cache_level;
 };
 
-typedef struct _virResctrlInfo virResctrlInfo;
+#define VIR_TYPE_RESCTRL_INFO vir_resctrl_info_get_type()
+G_DECLARE_FINAL_TYPE(virResctrlInfo, vir_resctrl_info, VIR, RESCTRL_INFO, GObject);
+
 typedef virResctrlInfo *virResctrlInfoPtr;
 
 virResctrlInfoPtr
@@ -111,10 +113,11 @@ virResctrlInfoGetMemoryBandwidth(virResctrlInfoPtr resctrl,
                                  unsigned int level,
                                  virResctrlInfoMemBWPerNodePtr control);
 /* Alloc-related things */
-typedef struct _virResctrlAlloc virResctrlAlloc;
+#define VIR_TYPE_RESCTRL_ALLOC vir_resctrl_alloc_get_type()
+G_DECLARE_FINAL_TYPE(virResctrlAlloc, vir_resctrl_alloc, VIR, RESCTRL_ALLOC, GObject);
+
 typedef virResctrlAlloc *virResctrlAllocPtr;
 
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(virResctrlAlloc, virObjectUnref);
 
 
 typedef int virResctrlAllocForeachCacheCallback(unsigned int level,
@@ -190,7 +193,9 @@ virResctrlInfoGetMonitorPrefix(virResctrlInfoPtr resctrl,
 
 /* Monitor-related things */
 
-typedef struct _virResctrlMonitor virResctrlMonitor;
+#define VIR_TYPE_RESCTRL_MONITOR vir_resctrl_monitor_get_type()
+G_DECLARE_FINAL_TYPE(virResctrlMonitor, vir_resctrl_monitor, VIR, RESCTRL_MONITOR, GObject);
+
 typedef virResctrlMonitor *virResctrlMonitorPtr;
 
 typedef struct _virResctrlMonitorStats virResctrlMonitorStats;
diff --git a/tests/virresctrltest.c b/tests/virresctrltest.c
index bb6d0fe48e..91baef506d 100644
--- a/tests/virresctrltest.c
+++ b/tests/virresctrltest.c
@@ -21,7 +21,7 @@ test_virResctrlGetUnused(const void *opaque)
     char *system_dir = NULL;
     char *resctrl_dir = NULL;
     int ret = -1;
-    virResctrlAllocPtr alloc = NULL;
+    g_autoptr(virResctrlAlloc) alloc = NULL;
     char *schemata_str = NULL;
     char *schemata_file;
     virCapsPtr caps = NULL;
@@ -66,7 +66,6 @@ test_virResctrlGetUnused(const void *opaque)
     ret = 0;
  cleanup:
     virObjectUnref(caps);
-    virObjectUnref(alloc);
     VIR_FREE(system_dir);
     VIR_FREE(resctrl_dir);
     VIR_FREE(schemata_str);
-- 
2.25.3


Re: [PATCH v2 01/40] util: virresctrl: convert classes to GObject
Posted by Jonathon Jongsma 5 years, 8 months ago
On Tue, 2020-04-21 at 15:48 +0200, Rafael Fonseca wrote:
> @@ -793,21 +830,13 @@ virResctrlGetInfo(virResctrlInfoPtr resctrl)
>  virResctrlInfoPtr
>  virResctrlInfoNew(void)
>  {
> -    virResctrlInfoPtr ret = NULL;
> -
> -    if (virResctrlInitialize() < 0)
> -        return NULL;
> -
> -    ret = virObjectNew(virResctrlInfoClass);
> -    if (!ret)
> -        return NULL;
> +    g_autoptr(virResctrlInfo) ret =
> VIR_RESCTRL_INFO(g_object_new(VIR_TYPE_RESCTRL_INFO, NULL));
>  
>      if (virResctrlGetInfo(ret) < 0) {
> -        virObjectUnref(ret);
>          return NULL;
>      }
>  
> -    return ret;
> +    return g_steal_pointer(&ret);
>  }


(FYI: Glib provides a base class GInitiable (and an async variant) for
an object that requires initialization immediately after allocation and
returns NULL if said initialization fails. Not sure that it would gain
us anything here, but just thought I'd mention it.)

Jonathon