[PATCH 13/36] admin: convert virAdmServer to GObject

Rafael Fonseca posted 36 patches 5 years, 8 months ago
There is a newer version of this series
[PATCH 13/36] admin: convert virAdmServer to GObject
Posted by Rafael Fonseca 5 years, 8 months ago
Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
---
 src/admin/libvirt-admin.c            |  2 +-
 src/admin/libvirt_admin_private.syms |  1 -
 src/datatypes.c                      | 42 ++++++++++++++++++----------
 src/datatypes.h                      | 20 +++++++------
 4 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 835b5560d2..ed7a57cbbb 100644
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -586,7 +586,7 @@ int virAdmServerFree(virAdmServerPtr srv)
 
     virCheckAdmServerReturn(srv, -1);
 
-    virObjectUnref(srv);
+    g_object_unref(srv);
     return 0;
 }
 
diff --git a/src/admin/libvirt_admin_private.syms b/src/admin/libvirt_admin_private.syms
index 157a45341e..270fa43995 100644
--- a/src/admin/libvirt_admin_private.syms
+++ b/src/admin/libvirt_admin_private.syms
@@ -40,7 +40,6 @@ virAdmConnectCloseCallbackDataRegister;
 virAdmConnectCloseCallbackDataReset;
 virAdmConnectCloseCallbackDataUnregister;
 virAdmGetServer;
-virAdmServerClass;
 
 # Let emacs know we want case-insensitive sorting
 # Local Variables:
diff --git a/src/datatypes.c b/src/datatypes.c
index 552115c7a3..d6afaf89ad 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -71,11 +71,25 @@ virClassPtr virAdmConnectCloseCallbackDataClass;
 static void virAdmConnectDispose(void *obj);
 static void virAdmConnectCloseCallbackDataDispose(void *obj);
 
-virClassPtr virAdmServerClass;
 virClassPtr virAdmClientClass;
-static void virAdmServerDispose(void *obj);
 static void virAdmClientDispose(void *obj);
 
+G_DEFINE_TYPE(virAdmServer, vir_adm_server, G_TYPE_OBJECT);
+static void virAdmServerFinalize(GObject *obj);
+
+static void
+vir_adm_server_init(virAdmServer *srv G_GNUC_UNUSED)
+{
+}
+
+static void
+vir_adm_server_class_init(virAdmServerClass *klass)
+{
+    GObjectClass *obj = G_OBJECT_CLASS(klass);
+
+    obj->finalize = virAdmServerFinalize;
+}
+
 static int
 virDataTypesOnceInit(void)
 {
@@ -105,7 +119,6 @@ virDataTypesOnceInit(void)
 
     DECLARE_CLASS_LOCKABLE(virAdmConnect);
     DECLARE_CLASS_LOCKABLE(virAdmConnectCloseCallbackData);
-    DECLARE_CLASS(virAdmServer);
     DECLARE_CLASS(virAdmClient);
 
 #undef DECLARE_CLASS_COMMON
@@ -1173,31 +1186,30 @@ virAdmConnectCloseCallbackDataRegister(virAdmConnectCloseCallbackDataPtr cbdata,
 virAdmServerPtr
 virAdmGetServer(virAdmConnectPtr conn, const char *name)
 {
-    virAdmServerPtr ret = NULL;
+    g_autoptr(virAdmServer) ret = NULL;
 
     if (virDataTypesInitialize() < 0)
-        goto error;
+        return NULL;
+
+    ret = VIR_ADM_SERVER(g_object_new(VIR_TYPE_ADM_SERVER, NULL));
 
-    if (!(ret = virObjectNew(virAdmServerClass)))
-        goto error;
     ret->name = g_strdup(name);
 
     ret->conn = virObjectRef(conn);
 
-    return ret;
- error:
-    virObjectUnref(ret);
-    return NULL;
+    return g_steal_pointer(&ret);
 }
 
 static void
-virAdmServerDispose(void *obj)
+virAdmServerFinalize(GObject *obj)
 {
-    virAdmServerPtr srv = obj;
+    virAdmServerPtr srv = VIR_ADM_SERVER(obj);
     VIR_DEBUG("release server srv=%p name=%s", srv, srv->name);
 
     VIR_FREE(srv->name);
     virObjectUnref(srv->conn);
+
+    G_OBJECT_CLASS(vir_adm_server_parent_class)->finalize(obj);
 }
 
 virAdmClientPtr
@@ -1215,7 +1227,7 @@ virAdmGetClient(virAdmServerPtr srv, const unsigned long long id,
     ret->id = id;
     ret->timestamp = timestamp;
     ret->transport = transport;
-    ret->srv = virObjectRef(srv);
+    ret->srv = g_object_ref(srv);
 
     return ret;
  error:
@@ -1229,5 +1241,5 @@ virAdmClientDispose(void *obj)
     virAdmClientPtr clt = obj;
     VIR_DEBUG("release client clt=%p, id=%llu", clt, clt->id);
 
-    virObjectUnref(clt->srv);
+    g_object_unref(clt->srv);
 }
diff --git a/src/datatypes.h b/src/datatypes.h
index 2d0407f7ec..20b64ca788 100644
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -28,6 +28,8 @@
 #include "virobject.h"
 #include "viruuid.h"
 
+#include <glib-object.h>
+
 extern virClassPtr virConnectClass;
 extern virClassPtr virDomainClass;
 extern virClassPtr virDomainCheckpointClass;
@@ -44,9 +46,11 @@ extern virClassPtr virStorageVolClass;
 extern virClassPtr virStoragePoolClass;
 
 extern virClassPtr virAdmConnectClass;
-extern virClassPtr virAdmServerClass;
 extern virClassPtr virAdmClientClass;
 
+#define VIR_TYPE_ADM_SERVER vir_adm_server_get_type()
+G_DECLARE_FINAL_TYPE(virAdmServer, vir_adm_server, VIR, ADM_SERVER, GObject);
+
 #define virCheckConnectReturn(obj, retval) \
     do { \
         if (!virObjectIsClass(obj, virConnectClass)) { \
@@ -419,8 +423,8 @@ extern virClassPtr virAdmClientClass;
 
 #define virCheckAdmServerReturn(obj, retval) \
     do { \
-        virAdmServerPtr _srv = (obj); \
-        if (!virObjectIsClass(_srv, virAdmServerClass) || \
+        virAdmServerPtr _srv = VIR_ADM_SERVER(obj); \
+        if (!G_IS_OBJECT(_srv) || !(G_OBJECT_TYPE(_srv) == VIR_TYPE_ADM_SERVER) || \
             !virObjectIsClass(_srv->conn, virAdmConnectClass)) { \
             virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
                                  __FILE__, __FUNCTION__, __LINE__, \
@@ -431,8 +435,8 @@ extern virClassPtr virAdmClientClass;
     } while (0)
 #define virCheckAdmServerGoto(obj, label) \
     do { \
-        virAdmServerPtr _srv = (obj); \
-        if (!virObjectIsClass(_srv, virAdmServerClass) || \
+        virAdmServerPtr _srv = VIR_ADM_SERVER(obj); \
+        if (!G_IS_OBJECT(_srv) || !(G_OBJECT_TYPE(_srv) == VIR_TYPE_ADM_SERVER) || \
             !virObjectIsClass(_srv->conn, virAdmConnectClass)) { \
             virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
                                  __FILE__, __FUNCTION__, __LINE__, \
@@ -445,7 +449,7 @@ extern virClassPtr virAdmClientClass;
     do { \
         virAdmClientPtr _clt = (obj); \
         if (!virObjectIsClass(_clt, virAdmClientClass) || \
-            !virObjectIsClass(_clt->srv, virAdmServerClass) || \
+            !G_IS_OBJECT(_clt->srv) || !(G_OBJECT_TYPE(_clt->srv) == VIR_TYPE_ADM_SERVER) || \
             !virObjectIsClass(_clt->srv->conn, virAdmConnectClass)) { \
             virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
                                  __FILE__, __FUNCTION__, __LINE__, \
@@ -458,7 +462,7 @@ extern virClassPtr virAdmClientClass;
     do { \
         virAdmClientPtr _clt = (obj); \
         if (!virObjectIsClass(_clt, virAdmClientClass) || \
-            !virObjectIsClass(_clt->srv, virAdmServerClass) || \
+            !G_IS_OBJECT(_clt->srv) || !(G_OBJECT_TYPE(_clt->srv) == VIR_TYPE_ADM_SERVER) || \
             !virObjectIsClass(_clt->srv->conn, virAdmConnectClass)) { \
             virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \
                                  __FILE__, __FUNCTION__, __LINE__, \
@@ -573,7 +577,7 @@ struct _virAdmConnect {
  * Internal structure associated to a daemon server
  */
 struct _virAdmServer {
-    virObject parent;
+    GObject parent;
     virAdmConnectPtr conn;          /* pointer back to the admin connection */
     char *name;                     /* the server external name */
 };
-- 
2.25.1


Re: [PATCH 13/36] admin: convert virAdmServer to GObject
Posted by Jonathon Jongsma 5 years, 8 months ago
On Fri, 2020-04-03 at 17:15 +0200, Rafael Fonseca wrote:
> Signed-off-by: Rafael Fonseca <r4f4rfs@gmail.com>
> ---
>  src/admin/libvirt-admin.c            |  2 +-
>  src/admin/libvirt_admin_private.syms |  1 -
>  src/datatypes.c                      | 42 ++++++++++++++++++------
> ----
>  src/datatypes.h                      | 20 +++++++------
>  4 files changed, 40 insertions(+), 25 deletions(-)
> 
> 

[snip]

> @@ -44,9 +46,11 @@ extern virClassPtr virStorageVolClass;
>  extern virClassPtr virStoragePoolClass;
>  
>  extern virClassPtr virAdmConnectClass;
> -extern virClassPtr virAdmServerClass;
>  extern virClassPtr virAdmClientClass;
>  
> +#define VIR_TYPE_ADM_SERVER vir_adm_server_get_type()
> +G_DECLARE_FINAL_TYPE(virAdmServer, vir_adm_server, VIR, ADM_SERVER,
> GObject);
> +
>  #define virCheckConnectReturn(obj, retval) \
>      do { \
>          if (!virObjectIsClass(obj, virConnectClass)) { \
> @@ -419,8 +423,8 @@ extern virClassPtr virAdmClientClass;
>  
>  #define virCheckAdmServerReturn(obj, retval) \
>      do { \
> -        virAdmServerPtr _srv = (obj); \
> -        if (!virObjectIsClass(_srv, virAdmServerClass) || \
> +        virAdmServerPtr _srv = VIR_ADM_SERVER(obj); \
> +        if (!G_IS_OBJECT(_srv) || !(G_OBJECT_TYPE(_srv) ==
> VIR_TYPE_ADM_SERVER) || \

This additional check is unnecessary with GObject. The VIR_ADM_SERVER()
casting macro that is provided by G_DECLARE_FINAL_TYPE() does this
internally. This function already checks that the type of obj is
correct and castable. If the pointer is not castable to the type, it
emits a warning and returns NULL. So you can just check for (_srv ==
NULL).

>              !virObjectIsClass(_srv->conn, virAdmConnectClass)) { \
>              virReportErrorHelper(VIR_FROM_THIS,
> VIR_ERR_INVALID_CONN, \
>                                   __FILE__, __FUNCTION__, __LINE__, \
> @@ -431,8 +435,8 @@ extern virClassPtr virAdmClientClass;
>      } while (0)
>  #define virCheckAdmServerGoto(obj, label) \
>      do { \
> -        virAdmServerPtr _srv = (obj); \
> -        if (!virObjectIsClass(_srv, virAdmServerClass) || \
> +        virAdmServerPtr _srv = VIR_ADM_SERVER(obj); \
> +        if (!G_IS_OBJECT(_srv) || !(G_OBJECT_TYPE(_srv) ==
> VIR_TYPE_ADM_SERVER) || \

same here

>              !virObjectIsClass(_srv->conn, virAdmConnectClass)) { \
>              virReportErrorHelper(VIR_FROM_THIS,
> VIR_ERR_INVALID_CONN, \
>                                   __FILE__, __FUNCTION__, __LINE__, \
> @@ -445,7 +449,7 @@ extern virClassPtr virAdmClientClass;
>      do { \
>          virAdmClientPtr _clt = (obj); \
>          if (!virObjectIsClass(_clt, virAdmClientClass) || \
> -            !virObjectIsClass(_clt->srv, virAdmServerClass) || \
> +            !G_IS_OBJECT(_clt->srv) || !(G_OBJECT_TYPE(_clt->srv) ==
> VIR_TYPE_ADM_SERVER) || \

Here you should be able to use the macro VIR_IS_ADM_SERVER() that is
automatically provided by G_DECLARE_FINAL_TYPE().

>              !virObjectIsClass(_clt->srv->conn, virAdmConnectClass))
> { \
>              virReportErrorHelper(VIR_FROM_THIS,
> VIR_ERR_INVALID_CONN, \
>                                   __FILE__, __FUNCTION__, __LINE__, \
> @@ -458,7 +462,7 @@ extern virClassPtr virAdmClientClass;
>      do { \
>          virAdmClientPtr _clt = (obj); \
>          if (!virObjectIsClass(_clt, virAdmClientClass) || \
> -            !virObjectIsClass(_clt->srv, virAdmServerClass) || \
> +            !G_IS_OBJECT(_clt->srv) || !(G_OBJECT_TYPE(_clt->srv) ==
> VIR_TYPE_ADM_SERVER) || \

same here

>              !virObjectIsClass(_clt->srv->conn, virAdmConnectClass))
> { \
>              virReportErrorHelper(VIR_FROM_THIS,
> VIR_ERR_INVALID_CONN, \
>                                   __FILE__, __FUNCTION__, __LINE__, \
> @@ -573,7 +577,7 @@ struct _virAdmConnect {
>   * Internal structure associated to a daemon server
>   */
>  struct _virAdmServer {
> -    virObject parent;
> +    GObject parent;
>      virAdmConnectPtr conn;          /* pointer back to the admin
> connection */
>      char *name;                     /* the server external name */
>  };

Re: [PATCH 13/36] admin: convert virAdmServer to GObject
Posted by Rafael Fonseca 5 years, 8 months ago
On Wed, Apr 8, 2020 at 10:47 PM Jonathon Jongsma <jjongsma@redhat.com> wrote:
>
> On Fri, 2020-04-03 at 17:15 +0200, Rafael Fonseca wrote:
> >
> >  #define virCheckAdmServerReturn(obj, retval) \
> >      do { \
> > -        virAdmServerPtr _srv = (obj); \
> > -        if (!virObjectIsClass(_srv, virAdmServerClass) || \
> > +        virAdmServerPtr _srv = VIR_ADM_SERVER(obj); \
> > +        if (!G_IS_OBJECT(_srv) || !(G_OBJECT_TYPE(_srv) ==
> > VIR_TYPE_ADM_SERVER) || \
>
> This additional check is unnecessary with GObject. The VIR_ADM_SERVER()
> casting macro that is provided by G_DECLARE_FINAL_TYPE() does this
> internally. This function already checks that the type of obj is
> correct and castable. If the pointer is not castable to the type, it
> emits a warning and returns NULL. So you can just check for (_srv ==
> NULL).

Thank you for the reviews and comments!

I'm afraid all the patches related to `datatypes.c` will have this
same problem. They'll be fixed in v2.


Att.
-- 
Rafael Fonseca