Some, but not all, of the monitor event handlers check the virObjectUnref return value to see if the domain was disposed. It should not be possible for this to happen, since the functional ready holds a lock on the domain and has only just acquired an extra reference on the domain a few lines earlier. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/qemu/qemu_process.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f7f6793113..51a086031d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -307,7 +307,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon, processEvent->vm = virObjectRef(vm); if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { - ignore_value(virObjectUnref(vm)); + virObjectUnref(vm); qemuProcessEventFree(processEvent); goto cleanup; } @@ -840,15 +840,13 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon G_GNUC_UNUSED, */ processEvent->vm = virObjectRef(vm); if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { - if (!virObjectUnref(vm)) - vm = NULL; + virObjectUnref(vm); qemuProcessEventFree(processEvent); } } } - if (vm) - virObjectUnlock(vm); + virObjectUnlock(vm); virObjectEventStateQueue(driver->domainEventState, watchdogEvent); virObjectEventStateQueue(driver->domainEventState, lifecycleEvent); @@ -977,7 +975,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon G_GNUC_UNUSED, processEvent->status = status; if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { - ignore_value(virObjectUnref(vm)); + virObjectUnref(vm); goto cleanup; } @@ -1039,7 +1037,7 @@ qemuProcessHandleJobStatusChange(qemuMonitorPtr mon G_GNUC_UNUSED, processEvent->data = virObjectRef(job); if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { - ignore_value(virObjectUnref(vm)); + virObjectUnref(vm); goto cleanup; } @@ -1342,14 +1340,12 @@ qemuProcessHandleGuestPanic(qemuMonitorPtr mon G_GNUC_UNUSED, processEvent->vm = virObjectRef(vm); if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { - if (!virObjectUnref(vm)) - vm = NULL; + virObjectUnref(vm); qemuProcessEventFree(processEvent); } cleanup: - if (vm) - virObjectUnlock(vm); + virObjectUnlock(vm); return 0; } @@ -1383,7 +1379,7 @@ qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon G_GNUC_UNUSED, processEvent->vm = virObjectRef(vm); if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { - ignore_value(virObjectUnref(vm)); + virObjectUnref(vm); goto error; } @@ -1554,7 +1550,7 @@ qemuProcessHandleNicRxFilterChanged(qemuMonitorPtr mon G_GNUC_UNUSED, processEvent->vm = virObjectRef(vm); if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { - ignore_value(virObjectUnref(vm)); + virObjectUnref(vm); goto error; } @@ -1593,7 +1589,7 @@ qemuProcessHandleSerialChanged(qemuMonitorPtr mon G_GNUC_UNUSED, processEvent->vm = virObjectRef(vm); if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { - ignore_value(virObjectUnref(vm)); + virObjectUnref(vm); goto error; } @@ -1873,14 +1869,12 @@ qemuProcessHandleGuestCrashloaded(qemuMonitorPtr mon G_GNUC_UNUSED, processEvent->vm = virObjectRef(vm); if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { - if (!virObjectUnref(vm)) - vm = NULL; + virObjectUnref(vm); qemuProcessEventFree(processEvent); } cleanup: - if (vm) - virObjectUnlock(vm); + virObjectUnlock(vm); return 0; } -- 2.24.1

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&gt; --- 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

The ref count will be private to the GObject base class and we must not peek at it, even for debugging messages. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/admin/libvirt-admin.c | 3 +-- src/libvirt-domain-checkpoint.c | 3 +-- src/libvirt-domain-snapshot.c | 3 +-- src/libvirt-domain.c | 2 +- src/libvirt-host.c | 2 +- src/libvirt-interface.c | 2 +- src/libvirt-network.c | 6 ++---- src/libvirt-nodedev.c | 2 +- src/libvirt-nwfilter.c | 6 ++---- src/libvirt-secret.c | 3 +-- src/libvirt-storage.c | 4 ++-- src/libvirt-stream.c | 3 +-- src/libvirt_qemu_probes.d | 8 ++++---- src/qemu/qemu_monitor.c | 7 ++----- 14 files changed, 21 insertions(+), 33 deletions(-) diff --git a/src/admin/libvirt-admin.c b/src/admin/libvirt-admin.c index 1d4ac51296..b7f21275cb 100644 --- a/src/admin/libvirt-admin.c +++ b/src/admin/libvirt-admin.c @@ -322,8 +322,7 @@ virAdmConnectClose(virAdmConnectPtr conn) int virAdmConnectRef(virAdmConnectPtr conn) { - VIR_DEBUG("conn=%p refs=%d", conn, - conn ? conn->parent.parent.u.s.refs : 0); + VIR_DEBUG("conn=%p", conn); virResetLastError(); virCheckAdmConnectReturn(conn, -1); diff --git a/src/libvirt-domain-checkpoint.c b/src/libvirt-domain-checkpoint.c index 432c2d5a52..50627c486c 100644 --- a/src/libvirt-domain-checkpoint.c +++ b/src/libvirt-domain-checkpoint.c @@ -535,8 +535,7 @@ virDomainCheckpointDelete(virDomainCheckpointPtr checkpoint, int virDomainCheckpointRef(virDomainCheckpointPtr checkpoint) { - VIR_DEBUG("checkpoint=%p, refs=%d", checkpoint, - checkpoint ? checkpoint->parent.u.s.refs : 0); + VIR_DEBUG("checkpoint=%p", checkpoint); virResetLastError(); diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c index 33593e11e9..f856e5b9b8 100644 --- a/src/libvirt-domain-snapshot.c +++ b/src/libvirt-domain-snapshot.c @@ -1080,8 +1080,7 @@ virDomainSnapshotDelete(virDomainSnapshotPtr snapshot, int virDomainSnapshotRef(virDomainSnapshotPtr snapshot) { - VIR_DEBUG("snapshot=%p, refs=%d", snapshot, - snapshot ? snapshot->parent.u.s.refs : 0); + VIR_DEBUG("snapshot=%p", snapshot); virResetLastError(); diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 37f864b7b0..60b5e65fc3 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -590,7 +590,7 @@ virDomainFree(virDomainPtr domain) int virDomainRef(virDomainPtr domain) { - VIR_DOMAIN_DEBUG(domain, "refs=%d", domain ? domain->parent.u.s.refs : 0); + VIR_DOMAIN_DEBUG(domain); virResetLastError(); diff --git a/src/libvirt-host.c b/src/libvirt-host.c index bc3d1d2803..07d13585f4 100644 --- a/src/libvirt-host.c +++ b/src/libvirt-host.c @@ -51,7 +51,7 @@ VIR_LOG_INIT("libvirt.host"); int virConnectRef(virConnectPtr conn) { - VIR_DEBUG("conn=%p refs=%d", conn, conn ? conn->parent.parent.u.s.refs : 0); + VIR_DEBUG("conn=%p", conn); virResetLastError(); diff --git a/src/libvirt-interface.c b/src/libvirt-interface.c index 2d2df68131..5eb5980483 100644 --- a/src/libvirt-interface.c +++ b/src/libvirt-interface.c @@ -642,7 +642,7 @@ virInterfaceDestroy(virInterfacePtr iface, unsigned int flags) int virInterfaceRef(virInterfacePtr iface) { - VIR_DEBUG("iface=%p refs=%d", iface, iface ? iface->parent.u.s.refs : 0); + VIR_DEBUG("iface=%p", iface); virResetLastError(); diff --git a/src/libvirt-network.c b/src/libvirt-network.c index 09e24fb0a8..f691b672c7 100644 --- a/src/libvirt-network.c +++ b/src/libvirt-network.c @@ -679,8 +679,7 @@ virNetworkFree(virNetworkPtr network) int virNetworkRef(virNetworkPtr network) { - VIR_DEBUG("network=%p refs=%d", network, - network ? network->parent.u.s.refs : 0); + VIR_DEBUG("network=%p", network); virResetLastError(); @@ -1714,8 +1713,7 @@ virNetworkPortFree(virNetworkPortPtr port) int virNetworkPortRef(virNetworkPortPtr port) { - VIR_DEBUG("port=%p refs=%d", port, - port ? port->parent.u.s.refs : 0); + VIR_DEBUG("port=%p", port); virResetLastError(); diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index dce46b7181..cdec123568 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -477,7 +477,7 @@ virNodeDeviceFree(virNodeDevicePtr dev) int virNodeDeviceRef(virNodeDevicePtr dev) { - VIR_DEBUG("dev=%p refs=%d", dev, dev ? dev->parent.u.s.refs : 0); + VIR_DEBUG("dev=%p", dev); virResetLastError(); diff --git a/src/libvirt-nwfilter.c b/src/libvirt-nwfilter.c index d28220db8a..e299385895 100644 --- a/src/libvirt-nwfilter.c +++ b/src/libvirt-nwfilter.c @@ -503,8 +503,7 @@ virNWFilterGetXMLDesc(virNWFilterPtr nwfilter, unsigned int flags) int virNWFilterRef(virNWFilterPtr nwfilter) { - VIR_DEBUG("nwfilter=%p refs=%d", nwfilter, - nwfilter ? nwfilter->parent.u.s.refs : 0); + VIR_DEBUG("nwfilter=%p", nwfilter); virResetLastError(); @@ -820,8 +819,7 @@ virNWFilterBindingGetXMLDesc(virNWFilterBindingPtr binding, unsigned int flags) int virNWFilterBindingRef(virNWFilterBindingPtr binding) { - VIR_DEBUG("binding=%p refs=%d", binding, - binding ? binding->parent.u.s.refs : 0); + VIR_DEBUG("binding=%p", binding); virResetLastError(); diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c index 33cbdd7b0b..75d40f53dc 100644 --- a/src/libvirt-secret.c +++ b/src/libvirt-secret.c @@ -658,8 +658,7 @@ virSecretUndefine(virSecretPtr secret) int virSecretRef(virSecretPtr secret) { - VIR_DEBUG("secret=%p refs=%d", secret, - secret ? secret->parent.u.s.refs : 0); + VIR_DEBUG("secret=%p", secret); virResetLastError(); diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c index 0406fe84d3..a45c8b98c1 100644 --- a/src/libvirt-storage.c +++ b/src/libvirt-storage.c @@ -872,7 +872,7 @@ virStoragePoolFree(virStoragePoolPtr pool) int virStoragePoolRef(virStoragePoolPtr pool) { - VIR_DEBUG("pool=%p refs=%d", pool, pool ? pool->parent.u.s.refs : 0); + VIR_DEBUG("pool=%p", pool); virResetLastError(); @@ -1909,7 +1909,7 @@ virStorageVolFree(virStorageVolPtr vol) int virStorageVolRef(virStorageVolPtr vol) { - VIR_DEBUG("vol=%p refs=%d", vol, vol ? vol->parent.u.s.refs : 0); + VIR_DEBUG("vol=%p", vol); virResetLastError(); diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c index 41b9cc1445..aeb7562018 100644 --- a/src/libvirt-stream.c +++ b/src/libvirt-stream.c @@ -85,8 +85,7 @@ virStreamNew(virConnectPtr conn, int virStreamRef(virStreamPtr stream) { - VIR_DEBUG("stream=%p refs=%d", stream, - stream ? stream->parent.u.s.refs : 0); + VIR_DEBUG("stream=%p", stream); virResetLastError(); diff --git a/src/libvirt_qemu_probes.d b/src/libvirt_qemu_probes.d index e4449a9922..5bcac7667b 100644 --- a/src/libvirt_qemu_probes.d +++ b/src/libvirt_qemu_probes.d @@ -4,10 +4,10 @@ provider libvirt { # binary: libvirtd # module: libvirt/connection-driver/libvirt_driver_qemu.so # Monitor lifecycle - probe qemu_monitor_new(void *mon, int refs, int fd); - probe qemu_monitor_ref(void *mon, int refs); - probe qemu_monitor_unref(void *mon, int refs); - probe qemu_monitor_close(void *monm, int refs); + probe qemu_monitor_new(void *mon, int fd); + probe qemu_monitor_ref(void *mon); + probe qemu_monitor_unref(void *mon); + probe qemu_monitor_close(void *monm); # High level monitor message processing probe qemu_monitor_send_msg(void *mon, const char *msg, int fd); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index ff7d66eee5..94fae66708 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -714,9 +714,7 @@ qemuMonitorOpenInternal(virDomainObjPtr vm, virObjectLock(mon); qemuMonitorRegister(mon); - PROBE(QEMU_MONITOR_NEW, - "mon=%p refs=%d fd=%d", - mon, mon->parent.parent.u.s.refs, mon->fd); + PROBE(QEMU_MONITOR_NEW, "mon=%p fd=%d", mon, mon->fd); virObjectUnlock(mon); return mon; @@ -865,8 +863,7 @@ qemuMonitorClose(qemuMonitorPtr mon) return; virObjectLock(mon); - PROBE(QEMU_MONITOR_CLOSE, - "mon=%p refs=%d", mon, mon->parent.parent.u.s.refs); + PROBE(QEMU_MONITOR_CLOSE, "mon=%p", mon); qemuMonitorSetDomainLogLocked(mon, NULL, NULL, NULL); -- 2.24.1

On 5/19/20 7:41 PM, Daniel P. Berrangé wrote: The unref call needs to go exactly here, where you remove these lines, because at the point we jump onto the error label, @ctxt has exactly one reference. And if you decrease it, the object is freed and the subsequent call to gnutls_whatever() would deref invalid pointer. Michal