12:41 p.m.
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.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/admin/libvirt-admin.c | 4 +++-
src/datatypes.c | 26 +++++++++++++++++++++++++
src/datatypes.h | 6 ++++++
src/interface/interface_backend_netcf.c | 7 +------
src/libvirt.c | 4 +++-
src/qemu/qemu_domain.c | 4 +++-
src/qemu/qemu_monitor.c | 14 +++++++++++++
src/qemu/qemu_monitor.h | 3 +++
src/test/test_driver.c | 8 ++++++--
src/util/virfdstream.c | 6 +++++-
src/util/virobject.c | 9 ++-------
src/util/virobject.h | 2 +-
src/vbox/vbox_common.c | 10 ++++++++--
13 files changed, 81 insertions(+), 22 deletions(-)
diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c
index 835b5560d2..1d4ac51296 100644
--- 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 552115c7a3..1db38c5aa6 100644
--- 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 2d0407f7ec..d58429ad6c 100644
--- 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 dd0c1481d9..f30829442d 100644
--- 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 76bf1fa677..b2d0ba3d23 100644
--- 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 d0528dbfe0..bb77cd38d3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6681,8 +6681,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 9c853ccb93..ff7d66eee5 100644
--- 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 2e35d94bda..99c73e14af 100644
--- 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 0506147888..3a085003e2 100644
--- 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 111e451f8c..1c32be47a9 100644
--- 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 c71781550f..4060d7307b 100644
--- 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 62a8a3d132..cfedb19b13 100644
--- 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 e98ae04ec0..a834a971f0 100644
--- 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:
--
2.24.1