From nobody Sun Feb 8 21:26:30 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 207.211.31.81 as permitted sender) client-ip=207.211.31.81; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 207.211.31.81 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1589910492; cv=none; d=zohomail.com; s=zohoarc; b=S5YHYkKD48QYzU6Xx9lxM6L5220jiVRCIcctsWSVDhVFy2bWwneQAfrHHCjhbKb3PKd/XXMrytYsBmgI499U8C1ftbvRbYhixhVIzPgvE7OjzdLMpZeL9hLLcKcT/JkfRgGpkQhyBQ5FNy5uRNzFfBfiYM3ONNrbmEJy8mk70NM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1589910492; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=YBLlfQ2hG4SGUrUHkoK1o/nPTc70Sbcxuv3WtzFuJV4=; b=T4nkUAahTqX7LNI7Ooqtp5Hxelgp2BgegE8JQo4T6i35kys0aUdG8eshLU799leFnbifLxKViCxuXxS8IbtGfAfucc3EJsvHHoThk3ZhN+PyZUoq4FnCToRB9Q/pNq+rDm7FwC4afnycNHSmkJaWu/XAKOgUXK4iUugj5iql0h0= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 207.211.31.81 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by mx.zohomail.com with SMTPS id 1589910492273652.4118262354709; Tue, 19 May 2020 10:48:12 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-41-YMEqFUB6M72dVqwOom-apA-1; Tue, 19 May 2020 13:48:08 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E5B6A8015CE; Tue, 19 May 2020 17:48:02 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C0A1160BE1; Tue, 19 May 2020 17:48:02 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 8E2C74ED99; Tue, 19 May 2020 17:48:02 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 04JHlg56014487 for ; Tue, 19 May 2020 13:47:42 -0400 Received: by smtp.corp.redhat.com (Postfix) id 2C9485D9DD; Tue, 19 May 2020 17:47:42 +0000 (UTC) Received: from catbus.gsslab.fab.redhat.com (mustard.gsslab.fab.redhat.com [10.33.8.112]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8EFFF5D9C5; Tue, 19 May 2020 17:47:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1589910491; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=YBLlfQ2hG4SGUrUHkoK1o/nPTc70Sbcxuv3WtzFuJV4=; b=WvXxJ8OI0Cap/BIt44L6TIemq8fn+nusUfMfmz3p0IRBpXiCzOd0lnmtGhAwsNlcqyQKQ3 If1VuPWnlIax7DvlX+XGlH2UhGjewRDDRmwNkB6vU3z/axETcThGFdntuAwmjPxGlqiW2Z 1AN5PpIp5Wiei4jYT2dMC6qW1rgrqok= X-MC-Unique: YMEqFUB6M72dVqwOom-apA-1 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [libvirt PATCH 2/6] src: make virObjectUnref return void Date: Tue, 19 May 2020 18:41:27 +0100 Message-Id: <20200519174131.91783-3-berrange@redhat.com> In-Reply-To: <20200519174131.91783-1-berrange@redhat.com> References: <20200519174131.91783-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) 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=C3=A9 --- 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) =20 virCheckAdmConnectReturn(conn, -1); =20 - 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); =20 +static __thread bool connectDisposed; +static __thread bool admConnectDisposed; + static int virDataTypesOnceInit(void) { @@ -133,6 +136,27 @@ virGetConnect(void) return virObjectLockableNew(virConnectClass); } =20 + +void virConnectWatchDispose(void) +{ + connectDisposed =3D false; +} + +bool virConnectWasDisposed(void) +{ + return connectDisposed; +} + +void virAdmConnectWatchDispose(void) +{ + admConnectDisposed =3D false; +} + +bool virAdmConnectWasDisposed(void) +{ + return admConnectDisposed; +} + /** * virConnectDispose: * @obj: the hypervisor connection to release @@ -145,6 +169,7 @@ virConnectDispose(void *obj) { virConnectPtr conn =3D obj; =20 + connectDisposed =3D true; if (conn->driver) conn->driver->connectClose(conn); =20 @@ -1092,6 +1117,7 @@ virAdmConnectDispose(void *obj) { virAdmConnectPtr conn =3D obj; =20 + admConnectDisposed =3D true; if (conn->privateDataFreeFunc) conn->privateDataFreeFunc(conn); =20 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); =20 +/* 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 cl= ose, virConnectPtr conn, diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interf= ace_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; =20 - if (virObjectUnref(driver)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Attempt to close netcf state driver " - "with open connections")); - return -1; - } + virObjectUnref(driver); driver =3D 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) =20 virCheckConnectReturn(conn, -1); =20 - 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 dr= iver, qemuDomainObjPrivatePtr priv =3D obj->privateData; bool hasRefs; =20 - hasRefs =3D virObjectUnref(priv->mon); + qemuMonitorWatchDispose(); + virObjectUnref(priv->mon); =20 + hasRefs =3D !qemuMonitorWasDisposed(); if (hasRefs) virObjectUnlock(priv->mon); =20 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) =20 static virClassPtr qemuMonitorClass; +static __thread bool qemuMonitorDisposed; static void qemuMonitorDispose(void *obj); =20 static int qemuMonitorOnceInit(void) @@ -222,6 +223,7 @@ qemuMonitorDispose(void *obj) qemuMonitorPtr mon =3D obj; =20 VIR_DEBUG("mon=3D%p", mon); + qemuMonitorDisposed =3D 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, } =20 =20 +void qemuMonitorWatchDispose(void) +{ + qemuMonitorDisposed =3D 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); =20 +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 =3D VIR_MUTEX_INITIALIZER; =20 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 =3D true; } =20 typedef struct _testDomainNamespaceDef testDomainNamespaceDef; @@ -1446,8 +1449,9 @@ static void testDriverCloseInternal(testDriverPtr driver) { virMutexLock(&defaultLock); - bool disposed =3D !virObjectUnref(driver); - if (disposed && driver =3D=3D defaultPrivconn) + testDriverDisposed =3D false; + virObjectUnref(driver); + if (testDriverDisposed && driver =3D=3D defaultPrivconn) defaultPrivconn =3D 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 { }; =20 static virClassPtr virFDStreamDataClass; +static __thread bool virFDStreamDataDisposed; =20 static void virFDStreamMsgQueueFree(virFDStreamMsgPtr *queue); =20 @@ -121,6 +122,7 @@ virFDStreamDataDispose(void *obj) virFDStreamDataPtr fdst =3D obj; =20 VIR_DEBUG("obj=3D%p", fdst); + virFDStreamDataDisposed =3D true; virFreeError(fdst->threadErr); virFDStreamMsgQueueFree(&fdst->msg); } @@ -631,7 +633,9 @@ virFDStreamThread(void *opaque) cleanup: fdst->threadQuit =3D true; virObjectUnlock(fdst); - if (!virObjectUnref(fdst)) + virFDStreamDataDisposed =3D false; + virObjectUnref(fdst); + if (virFDStreamDataDisposed) st->privateData =3D 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 =3D anyobj; =20 if (VIR_OBJECT_NOTVALID(obj)) - return false; + return; =20 bool lastRef =3D !!g_atomic_int_dec_and_test(&obj->u.s.refs); PROBE(OBJECT_UNREF, "obj=3D%p", obj); @@ -360,8 +357,6 @@ virObjectUnref(void *anyobj) obj->klass =3D (void*)0xDEADBEEF; VIR_FREE(obj); } - - return !lastRef; } =20 =20 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); =20 -bool +void virObjectUnref(void *obj); =20 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 =3D VIR_MUTEX_INITIALIZER; static vboxDriverPtr vbox_driver; static vboxDriverPtr vboxDriverObjNew(void); +static __thread bool vboxDriverDisposed; =20 static int vboxDomainDevicesDefPostParse(virDomainDeviceDefPtr dev G_GNUC_UNUSED, @@ -124,6 +125,7 @@ vboxDriverDispose(void *obj) { vboxDriverPtr driver =3D obj; =20 + vboxDriverDisposed =3D 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 =3D false; + virObjectUnref(vbox_driver); + if (vboxDriverDisposed) vbox_driver =3D NULL; =20 virMutexUnlock(&vbox_driver_lock); @@ -277,7 +281,9 @@ vboxDestroyDriverConnection(void) =20 vboxSdkUninitialize(); =20 - if (!virObjectUnref(vbox_driver)) + vboxDriverDisposed =3D false; + virObjectUnref(vbox_driver); + if (vboxDriverDisposed) vbox_driver =3D NULL; =20 cleanup: --=20 2.24.1