src: make virObjectUnref return void

To prepare for a conversion to GObject, we need virObjectUnref
to have the same API design as g_object_unref, which means it
needs to be void.

A few places do actually care about the return value though,
and in these cases a thread local flag is used to determine
if the dispose method was invoked.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 835b5560d2fe644dd4ed183be331bb2cde26b4a7..1d4ac51296ecb91a30c7a82b68ea3f101527ba84 100644 (file)
--- a/src/admin/libvirt-admin.c
+++ b/src/admin/libvirt-admin.c
@@ -295,7 +295,9 @@ virAdmConnectClose(virAdmConnectPtr conn)
 
     virCheckAdmConnectReturn(conn, -1);
 
-    if (!virObjectUnref(conn))
+    virAdmConnectWatchDispose();
+    virObjectUnref(conn);
+    if (virAdmConnectWasDisposed())
         return 0;
     return 1;
 }

diff --git a/src/datatypes.c b/src/datatypes.c
index 552115c7a33f137fa163b5b248aecb0627a88b34..1db38c5aa6e2f823105781071a13c59991186935 100644 (file)
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -76,6 +76,9 @@ virClassPtr virAdmClientClass;
 static void virAdmServerDispose(void *obj);
 static void virAdmClientDispose(void *obj);
 
+static __thread bool connectDisposed;
+static __thread bool admConnectDisposed;
+
 static int
 virDataTypesOnceInit(void)
 {
@@ -133,6 +136,27 @@ virGetConnect(void)
     return virObjectLockableNew(virConnectClass);
 }
 
+
+void virConnectWatchDispose(void)
+{
+    connectDisposed = false;
+}
+
+bool virConnectWasDisposed(void)
+{
+    return connectDisposed;
+}
+
+void virAdmConnectWatchDispose(void)
+{
+    admConnectDisposed = false;
+}
+
+bool virAdmConnectWasDisposed(void)
+{
+    return admConnectDisposed;
+}
+
 /**
  * virConnectDispose:
  * @obj: the hypervisor connection to release
@@ -145,6 +169,7 @@ virConnectDispose(void *obj)
 {
     virConnectPtr conn = obj;
 
+    connectDisposed = true;
     if (conn->driver)
         conn->driver->connectClose(conn);
 
@@ -1092,6 +1117,7 @@ virAdmConnectDispose(void *obj)
 {
     virAdmConnectPtr conn = obj;
 
+    admConnectDisposed = true;
     if (conn->privateDataFreeFunc)
         conn->privateDataFreeFunc(conn);
 

diff --git a/src/datatypes.h b/src/datatypes.h
index 2d0407f7ecbfb762f9da499b6bb1011721e81de5..d58429ad6c5a25323f5e04ce1f1e9c42c5fd8942 100644 (file)
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -843,6 +843,12 @@ virAdmClientPtr virAdmGetClient(virAdmServerPtr srv,
                                 unsigned long long timestamp,
                                 unsigned int transport);
 
+/* Thread local to watch if an ObjectUnref causes a Dispoe */
+void virConnectWatchDispose(void);
+bool virConnectWasDisposed(void);
+void virAdmConnectWatchDispose(void);
+bool virAdmConnectWasDisposed(void);
+
 virConnectCloseCallbackDataPtr virNewConnectCloseCallbackData(void);
 void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close,
                                          virConnectPtr conn,

diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c
index dd0c1481d9dbed934c0ea7e94e8732107ed638f6..f30829442dbf7cb12b14ef3c2c113bf650bdb1d2 100644 (file)
--- a/src/interface/interface_backend_netcf.c
+++ b/src/interface/interface_backend_netcf.c
@@ -148,12 +148,7 @@ netcfStateCleanup(void)
     if (!driver)
         return -1;
 
-    if (virObjectUnref(driver)) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Attempt to close netcf state driver "
-                         "with open connections"));
-        return -1;
-    }
+    virObjectUnref(driver);
     driver = NULL;
     return 0;
 }

diff --git a/src/libvirt.c b/src/libvirt.c
index 76bf1fa6772dea12a645e895bcb6c59a64a5f727..b2d0ba3d232a81d3f505a2dc0dbbdd1fc3e7b24d 100644 (file)
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1277,7 +1277,9 @@ virConnectClose(virConnectPtr conn)
 
     virCheckConnectReturn(conn, -1);
 
-    if (!virObjectUnref(conn))
+    virConnectWatchDispose();
+    virObjectUnref(conn);
+    if (virConnectWasDisposed())
         return 0;
     return 1;
 }

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d5e3d1a3cc960b2240f55f965896b26832cffe29..10f16e9b3b8f101501ee14878d7d221fa0e6b26d 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6691,8 +6691,10 @@ qemuDomainObjExitMonitorInternal(virQEMUDriverPtr driver,
     qemuDomainObjPrivatePtr priv = obj->privateData;
     bool hasRefs;
 
-    hasRefs = virObjectUnref(priv->mon);
+    qemuMonitorWatchDispose();
+    virObjectUnref(priv->mon);
 
+    hasRefs = !qemuMonitorWasDisposed();
     if (hasRefs)
         virObjectUnlock(priv->mon);
 

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2911307f0e8aef3aced5d280650899837497d262..b88eac96a1966f4b18651db34dba370097350416 100644 (file)
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -146,6 +146,7 @@ struct _qemuMonitor {
     QEMU_CHECK_MONITOR_FULL(mon, goto label)
 
 static virClassPtr qemuMonitorClass;
+static __thread bool qemuMonitorDisposed;
 static void qemuMonitorDispose(void *obj);
 
 static int qemuMonitorOnceInit(void)
@@ -222,6 +223,7 @@ qemuMonitorDispose(void *obj)
     qemuMonitorPtr mon = obj;
 
     VIR_DEBUG("mon=%p", mon);
+    qemuMonitorDisposed = true;
     if (mon->cb && mon->cb->destroy)
         (mon->cb->destroy)(mon, mon->vm, mon->callbackOpaque);
     virObjectUnref(mon->vm);
@@ -799,6 +801,18 @@ qemuMonitorOpen(virDomainObjPtr vm,
 }
 
 
+void qemuMonitorWatchDispose(void)
+{
+    qemuMonitorDisposed = false;
+}
+
+
+bool qemuMonitorWasDisposed(void)
+{
+    return qemuMonitorDisposed;
+}
+
+
 /**
  * qemuMonitorRegister:
  * @mon: QEMU monitor

diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index c61c05cb9f2463ada5b82466b26d3956d00b75ce..50a337715dffff9d8a71395600b222dac3ff708b 100644 (file)
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -387,6 +387,9 @@ qemuMonitorPtr qemuMonitorOpen(virDomainObjPtr vm,
                                void *opaque)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);
 
+void qemuMonitorWatchDispose(void);
+bool qemuMonitorWasDisposed(void);
+
 void qemuMonitorRegister(qemuMonitorPtr mon)
     ATTRIBUTE_NONNULL(1);
 void qemuMonitorUnregister(qemuMonitorPtr mon)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 0506147888e759b464e22834f7379276b8633caf..3a085003e2563a8a5415dc1eb9b3acfbc547a2b1 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -128,6 +128,7 @@ static testDriverPtr defaultPrivconn;
 static virMutex defaultLock = VIR_MUTEX_INITIALIZER;
 
 static virClassPtr testDriverClass;
+static __thread bool testDriverDisposed;
 static void testDriverDispose(void *obj);
 static int testDriverOnceInit(void)
 {
@@ -171,6 +172,8 @@ testDriverDispose(void *obj)
         g_free(driver->auths[i].password);
     }
     g_free(driver->auths);
+
+    testDriverDisposed = true;
 }
 
 typedef struct _testDomainNamespaceDef testDomainNamespaceDef;
@@ -1446,8 +1449,9 @@ static void
 testDriverCloseInternal(testDriverPtr driver)
 {
     virMutexLock(&defaultLock);
-    bool disposed = !virObjectUnref(driver);
-    if (disposed && driver == defaultPrivconn)
+    testDriverDisposed = false;
+    virObjectUnref(driver);
+    if (testDriverDisposed && driver == defaultPrivconn)
         defaultPrivconn = NULL;
     virMutexUnlock(&defaultLock);
 }

diff --git a/src/util/virfdstream.c b/src/util/virfdstream.c
index 111e451f8c9d38dccd9e5d5dc189508a361f0de9..1c32be47a9b30bba285dc43cbd729c701c223945 100644 (file)
--- a/src/util/virfdstream.c
+++ b/src/util/virfdstream.c
@@ -112,6 +112,7 @@ struct virFDStreamData {
 };
 
 static virClassPtr virFDStreamDataClass;
+static __thread bool virFDStreamDataDisposed;
 
 static void virFDStreamMsgQueueFree(virFDStreamMsgPtr *queue);
 
@@ -121,6 +122,7 @@ virFDStreamDataDispose(void *obj)
     virFDStreamDataPtr fdst = obj;
 
     VIR_DEBUG("obj=%p", fdst);
+    virFDStreamDataDisposed = true;
     virFreeError(fdst->threadErr);
     virFDStreamMsgQueueFree(&fdst->msg);
 }
@@ -631,7 +633,9 @@ virFDStreamThread(void *opaque)
  cleanup:
     fdst->threadQuit = true;
     virObjectUnlock(fdst);
-    if (!virObjectUnref(fdst))
+    virFDStreamDataDisposed = false;
+    virObjectUnref(fdst);
+    if (virFDStreamDataDisposed)
         st->privateData = NULL;
     VIR_FORCE_CLOSE(fdin);
     VIR_FORCE_CLOSE(fdout);

diff --git a/src/util/virobject.c b/src/util/virobject.c
index c71781550f482d1d60b68bbf325efe7d1971ec4c..4060d7307b01e3e47f1fe129e524e166fb9b4af6 100644 (file)
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -331,17 +331,14 @@ virObjectRWLockableDispose(void *anyobj)
  * it hits zero, runs the "dispose" callbacks associated
  * with the object class and its parents before freeing
  * @anyobj.
- *
- * Returns true if the remaining reference count is
- * non-zero, false if the object was disposed of
  */
-bool
+void
 virObjectUnref(void *anyobj)
 {
     virObjectPtr obj = anyobj;
 
     if (VIR_OBJECT_NOTVALID(obj))
-        return false;
+        return;
 
     bool lastRef = !!g_atomic_int_dec_and_test(&obj->u.s.refs);
     PROBE(OBJECT_UNREF, "obj=%p", obj);
@@ -360,8 +357,6 @@ virObjectUnref(void *anyobj)
         obj->klass = (void*)0xDEADBEEF;
         VIR_FREE(obj);
     }
-
-    return !lastRef;
 }
 
 

diff --git a/src/util/virobject.h b/src/util/virobject.h
index 62a8a3d1323d9e0c50284065a30a21271d8cbafb..cfedb19b13cf614a883a7f57f3c7160d880ce76b 100644 (file)
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -106,7 +106,7 @@ void *
 virObjectNew(virClassPtr klass)
     ATTRIBUTE_NONNULL(1);
 
-bool
+void
 virObjectUnref(void *obj);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virObject, virObjectUnref);

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index e98ae04ec04b4b47760912779df9de1a51f384c3..a834a971f0d8ffff535896062255db4cbde75b50 100644 (file)
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -61,6 +61,7 @@ static virClassPtr vboxDriverClass;
 static virMutex vbox_driver_lock = VIR_MUTEX_INITIALIZER;
 static vboxDriverPtr vbox_driver;
 static vboxDriverPtr vboxDriverObjNew(void);
+static __thread bool vboxDriverDisposed;
 
 static int
 vboxDomainDevicesDefPostParse(virDomainDeviceDefPtr dev G_GNUC_UNUSED,
@@ -124,6 +125,7 @@ vboxDriverDispose(void *obj)
 {
     vboxDriverPtr driver = obj;
 
+    vboxDriverDisposed = true;
     virObjectUnref(driver->caps);
     virObjectUnref(driver->xmlopt);
 }
@@ -250,7 +252,9 @@ vboxGetDriverConnection(void)
     if (vboxSdkInitialize() < 0 || vboxExtractVersion() < 0) {
         gVBoxAPI.UPFN.Uninitialize(vbox_driver);
         /* make sure to clear the pointer when last reference was released */
-        if (!virObjectUnref(vbox_driver))
+        vboxDriverDisposed = false;
+        virObjectUnref(vbox_driver);
+        if (vboxDriverDisposed)
             vbox_driver = NULL;
 
         virMutexUnlock(&vbox_driver_lock);
@@ -277,7 +281,9 @@ vboxDestroyDriverConnection(void)
 
     vboxSdkUninitialize();
 
-    if (!virObjectUnref(vbox_driver))
+    vboxDriverDisposed = false;
+    virObjectUnref(vbox_driver);
+    if (vboxDriverDisposed)
         vbox_driver = NULL;
 
  cleanup: