[libvirt] [PATCH v3 00/10] Introduce a virObject module for reference counting
A followup to: https://www.redhat.com/archives/libvir-list/2012-July/msg01733.html New in v3: - Introduce virObjectFreeCallback - Remove bogus ATTRIBUTE_NONNULL annotations - Add virObjectUnref/FreeCallback to useless-if-before-free checks - Other misc issues pointed out - Merged patches already ACKd
From: "Daniel P. Berrange" <berrange@redhat.com> This introduces a fairly basic reference counted virObject type and an associated virClass type, that use atomic operations for ref counting. In a global initializer (recommended to be invoked using the virOnceInit API), a virClass type must be allocated for each object type. This requires a class name, a "dispose" callback which will be invoked to free memory associated with the object's fields, and the size in bytes of the object struct. eg, virClassPtr connclass = virClassNew("virConnect", sizeof(virConnect), virConnectDispose); The struct for the object, must include 'virObject' as its first member eg struct _virConnect { virObject object; virURIPtr uri; }; The 'dispose' callback is only responsible for freeing fields in the object, not the object itself. eg a suitable impl for the above struct would be void virConnectDispose(void *obj) { virConnectPtr conn = obj; virURIFree(conn->uri); } There is no need to reset fields to 'NULL' or '0' in the dispose callback, since the entire object will be memset to 0, and the klass pointer & magic integer fields will be poisoned with 0xDEADBEEF before being free()d When creating an instance of an object, one needs simply pass the virClassPtr eg virConnectPtr conn = virObjectNew(connclass); if (!conn) return NULL; conn->uri = virURIParse("foo:///bar") Object references can be manipulated with virObjectRef(conn) virObjectUnref(conn) The latter returns a true value, if the object has been freed (ie its ref count hit zero) Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- cfg.mk | 2 + src/Makefile.am | 1 + src/libvirt_private.syms | 10 +++ src/libvirt_probes.d | 7 ++ src/util/virobject.c | 216 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virobject.h | 60 +++++++++++++ 6 files changed, 296 insertions(+) create mode 100644 src/util/virobject.c create mode 100644 src/util/virobject.h diff --git a/cfg.mk b/cfg.mk index e2af2bb..ccff146 100644 --- a/cfg.mk +++ b/cfg.mk @@ -171,6 +171,8 @@ useless_free_options = \ --name=virNetworkObjFree \ --name=virNodeDeviceDefFree \ --name=virNodeDeviceObjFree \ + --name=virObjectUnref \ + --name=virObjectFreeCallback \ --name=virSecretDefFree \ --name=virStorageEncryptionFree \ --name=virStorageEncryptionSecretFree \ diff --git a/src/Makefile.am b/src/Makefile.am index 49bcf50..6ed4a41 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -85,6 +85,7 @@ UTIL_SOURCES = \ util/virauthconfig.c util/virauthconfig.h \ util/virfile.c util/virfile.h \ util/virnodesuspend.c util/virnodesuspend.h \ + util/virobject.c util/virobject.h \ util/virpidfile.c util/virpidfile.h \ util/virtypedparam.c util/virtypedparam.h \ util/xml.c util/xml.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 44b6652..9ed6a8d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1644,6 +1644,16 @@ nodeSuspendForDuration; virNodeSuspendGetTargetMask; +# virobject.h +virClassName; +virClassNew; +virObjectFreeCallback; +virObjectIsClass; +virObjectNew; +virObjectRef; +virObjectUnref; + + # virpidfile.h virPidFileAcquire; virPidFileAcquirePath; diff --git a/src/libvirt_probes.d b/src/libvirt_probes.d index 0dac8f3..ceb3caa 100644 --- a/src/libvirt_probes.d +++ b/src/libvirt_probes.d @@ -16,6 +16,13 @@ provider libvirt { probe event_poll_run(int nfds, int timeout); + # file: src/util/virobject.c + # prefix: object + probe object_new(void *obj, const char *klassname); + probe object_ref(void *obj); + probe object_unref(void *obj); + probe object_dispose(void *obj); + # file: src/rpc/virnetsocket.c # prefix: rpc probe rpc_socket_new(void *sock, int refs, int fd, int errfd, pid_t pid, const char *localAddr, const char *remoteAddr); diff --git a/src/util/virobject.c b/src/util/virobject.c new file mode 100644 index 0000000..80a1f40 --- /dev/null +++ b/src/util/virobject.c @@ -0,0 +1,216 @@ +/* + * virobject.c: libvirt reference counted object + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "virobject.h" +#include "threads.h" +#include "memory.h" +#include "viratomic.h" +#include "virterror_internal.h" +#include "logging.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +static unsigned int magicCounter = 0xCAFE0000; + +struct _virClass { + unsigned int magic; + const char *name; + size_t objectSize; + + virObjectDisposeCallback dispose; +}; + + +/** + * virClassNew: + * @name: the class name + * @objectSize: total size of the object struct + * @dispose: callback to run to free object fields + * + * Register a new object class with @name. The @objectSize + * should give the total size of the object struct, which + * is expected to have a 'virObject object;' field as its + * first member. When the last reference on the object is + * released, the @dispose callback will be invoked to free + * memory of the object fields + * + * Returns a new class instance + */ +virClassPtr virClassNew(const char *name, + size_t objectSize, + virObjectDisposeCallback dispose) +{ + virClassPtr klass; + + if (VIR_ALLOC(klass) < 0) { + virReportOOMError(); + return NULL; + } + + if (!(klass->name = strdup(name))) + goto no_memory; + klass->magic = virAtomicIntInc(&magicCounter); + klass->objectSize = objectSize; + klass->dispose = dispose; + + return klass; + +no_memory: + VIR_FREE(klass); + virReportOOMError(); + return NULL; +} + + +/** + * virObjectNew: + * @klass: the klass of object to create + * + * Allocates a new object of type @klass. The returned + * object will be an instance of "virObjectPtr", which + * can be cast to the struct associated with @klass. + * + * The initial reference count of the object will be 1. + * + * Returns the new object + */ +void *virObjectNew(virClassPtr klass) +{ + virObjectPtr obj = NULL; + char *somebytes; + + if (VIR_ALLOC_N(somebytes, klass->objectSize) < 0) { + virReportOOMError(); + return NULL; + } + obj = (virObjectPtr)somebytes; + + obj->magic = klass->magic; + obj->klass = klass; + virAtomicIntSet(&obj->refs, 1); + + PROBE(OBJECT_NEW, "obj=%p classname=%s", obj, obj->klass->name); + + return obj; +} + + +/** + * virObjectUnref: + * @anyobj: any instance of virObjectPtr + * + * Decrement the reference count on @anyobj and if + * it hits zero, runs the "dispose" callback associated + * with the object class and frees @anyobj. + * + * Returns true if the remaining reference count is + * non-zero, false if the object was disposed of + */ +bool virObjectUnref(void *anyobj) +{ + virObjectPtr obj = anyobj; + + if (!obj) + return false; + + bool lastRef = virAtomicIntDecAndTest(&obj->refs); + PROBE(OBJECT_UNREF, "obj=%p", obj); + if (lastRef) { + PROBE(OBJECT_DISPOSE, "obj=%p", obj); + if (obj->klass->dispose) + obj->klass->dispose(obj); + + /* Clear & poison object */ + memset(obj, 0, obj->klass->objectSize); + obj->magic = 0xDEADBEEF; + obj->klass = (void*)0xDEADBEEF; + VIR_FREE(obj); + } + + return !lastRef; +} + + +/** + * virObjectRef: + * @anyobj: any instance of virObjectPtr + * + * Increment the reference count on @anyobj and return + * the same pointer + * + * Returns @anyobj + */ +void *virObjectRef(void *anyobj) +{ + virObjectPtr obj = anyobj; + + if (!obj) + return NULL; + virAtomicIntInc(&obj->refs); + PROBE(OBJECT_REF, "obj=%p", obj); + return anyobj; +} + + +/** + * virObjectIsClass: + * @anyobj: any instance of virObjectPtr + * @klass: the class to check + * + * Checks whether @anyobj is an instance of + * @klass + * + * Returns true if @anyobj is an instance of @klass + */ +bool virObjectIsClass(void *anyobj, + virClassPtr klass) +{ + virObjectPtr obj = anyobj; + return obj != NULL && (obj->magic == klass->magic) && (obj->klass == klass); +} + + +/** + * virClassName: + * @klass: the object class + * + * Returns the name of @klass + */ +const char *virClassName(virClassPtr klass) +{ + return klass->name; +} + + +/** + * virObjectFreeCallback: + * @opaque: a pointer to a virObject instance + * + * Provides identical functionality to virObjectUnref, + * but with the signature maching the virFreeCallback + * typedef. + */ +void virObjectFreeCallback(void *opaque) +{ + virObjectUnref(opaque); +} diff --git a/src/util/virobject.h b/src/util/virobject.h new file mode 100644 index 0000000..2581cb5 --- /dev/null +++ b/src/util/virobject.h @@ -0,0 +1,60 @@ +/* + * virobject.h: libvirt reference counted object + * + * Copyright (C) 2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __VIR_OBJECT_H__ +# define __VIR_OBJECT_H__ + +# include "internal.h" + +typedef struct _virClass virClass; +typedef virClass *virClassPtr; + +typedef struct _virObject virObject; +typedef virObject *virObjectPtr; + +typedef void (*virObjectDisposeCallback)(void *obj); + +struct _virObject { + unsigned int magic; + int refs; + virClassPtr klass; +}; + +virClassPtr virClassNew(const char *name, + size_t objectSize, + virObjectDisposeCallback dispose) + ATTRIBUTE_NONNULL(1); + +const char *virClassName(virClassPtr klass) + ATTRIBUTE_NONNULL(1); + +void *virObjectNew(virClassPtr klass) + ATTRIBUTE_NONNULL(1); +bool virObjectUnref(void *obj); +void *virObjectRef(void *obj); + +bool virObjectIsClass(void *obj, + virClassPtr klass) + ATTRIBUTE_NONNULL(2); + +void virObjectFreeCallback(void *opaque); + +#endif /* __VIR_OBJECT_H */ -- 1.7.10.4
On 08/06/2012 05:52 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This introduces a fairly basic reference counted virObject type and an associated virClass type, that use atomic operations for ref counting.
+virClassPtr virClassNew(const char *name, + size_t objectSize, + virObjectDisposeCallback dispose) +{ + virClassPtr klass; + + if (VIR_ALLOC(klass) < 0) { + virReportOOMError(); + return NULL; + }
This could be simplified to: if (VIR_ALLOC(klass) < 0) goto no_memory;
+/** + * virObjectFreeCallback: + * @opaque: a pointer to a virObject instance + * + * Provides identical functionality to virObjectUnref, + * but with the signature maching the virFreeCallback
s/maching/matching/ ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> This converts the following public API datatypes to use the virObject infrastructure: virConnectPtr virDomainPtr virDomainSnapshotPtr virInterfacePtr virNetworkPtr virNodeDevicePtr virNWFilterPtr virSecretPtr virStreamPtr virStorageVolPtr virStoragePoolPtr The code is significantly simplified, since the mutex in the virConnectPtr object now only needs to be held when accessing the per-connection virError object instance. All other operations are completely lock free. * src/datatypes.c, src/datatypes.h, src/libvirt.c: Convert public datatypes to use virObject * src/conf/domain_event.c, src/phyp/phyp_driver.c, src/qemu/qemu_command.c, src/qemu/qemu_migration.c, src/qemu/qemu_process.c, src/storage/storage_driver.c, src/vbox/vbox_tmpl.c, src/xen/xend_internal.c, tests/qemuxml2argvtest.c, tests/qemuxmlnstest.c, tests/sexpr2xmltest.c, tests/xmconfigtest.c: Convert to use virObjectUnref/virObjectRef Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/conf/domain_event.c | 8 +- src/datatypes.c | 1025 ++++++++++---------------------------- src/datatypes.h | 277 ++++------ src/libvirt.c | 145 ++---- src/libvirt_private.syms | 17 +- src/parallels/parallels_driver.c | 4 +- src/phyp/phyp_driver.c | 18 +- src/qemu/qemu_command.c | 3 +- src/qemu/qemu_migration.c | 10 +- src/qemu/qemu_process.c | 2 +- src/storage/storage_driver.c | 6 +- src/vbox/vbox_tmpl.c | 2 +- src/xen/xend_internal.c | 5 +- tests/qemuxml2argvtest.c | 21 +- tests/qemuxmlnstest.c | 2 +- tests/sexpr2xmltest.c | 3 +- tests/xmconfigtest.c | 4 +- 17 files changed, 445 insertions(+), 1107 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index f72bc8c..43ecdcf 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -169,7 +169,7 @@ virDomainEventCallbackListRemove(virConnectPtr conn, virFreeCallback freecb = cbList->callbacks[i]->freecb; if (freecb) (*freecb)(cbList->callbacks[i]->opaque); - virUnrefConnect(cbList->callbacks[i]->conn); + virObjectUnref(cbList->callbacks[i]->conn); VIR_FREE(cbList->callbacks[i]); if (i < (cbList->count - 1)) @@ -219,7 +219,7 @@ virDomainEventCallbackListRemoveID(virConnectPtr conn, virFreeCallback freecb = cbList->callbacks[i]->freecb; if (freecb) (*freecb)(cbList->callbacks[i]->opaque); - virUnrefConnect(cbList->callbacks[i]->conn); + virObjectUnref(cbList->callbacks[i]->conn); VIR_FREE(cbList->callbacks[i]); if (i < (cbList->count - 1)) @@ -309,7 +309,7 @@ virDomainEventCallbackListPurgeMarked(virDomainEventCallbackListPtr cbList) virFreeCallback freecb = cbList->callbacks[i]->freecb; if (freecb) (*freecb)(cbList->callbacks[i]->opaque); - virUnrefConnect(cbList->callbacks[i]->conn); + virObjectUnref(cbList->callbacks[i]->conn); VIR_FREE(cbList->callbacks[i]); if (i < (cbList->count - 1)) @@ -395,7 +395,7 @@ virDomainEventCallbackListAddID(virConnectPtr conn, if (VIR_REALLOC_N(cbList->callbacks, cbList->count + 1) < 0) goto no_memory; - event->conn->refs++; + virObjectRef(event->conn); cbList->callbacks[cbList->count] = event; cbList->count++; diff --git a/src/datatypes.c b/src/datatypes.c index 343ee0d..e827c2d 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -35,12 +35,58 @@ virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) -/************************************************************************ - * * - * Domain and Connections allocations * - * * - ************************************************************************/ +virClassPtr virConnectClass; +virClassPtr virDomainClass; +virClassPtr virDomainSnapshotClass; +virClassPtr virInterfaceClass; +virClassPtr virNetworkClass; +virClassPtr virNodeDeviceClass; +virClassPtr virNWFilterClass; +virClassPtr virSecretClass; +virClassPtr virStreamClass; +virClassPtr virStorageVolClass; +virClassPtr virStoragePoolClass; + +static void virConnectDispose(void *obj); +static void virDomainDispose(void *obj); +static void virDomainSnapshotDispose(void *obj); +static void virInterfaceDispose(void *obj); +static void virNetworkDispose(void *obj); +static void virNodeDeviceDispose(void *obj); +static void virNWFilterDispose(void *obj); +static void virSecretDispose(void *obj); +static void virStreamDispose(void *obj); +static void virStorageVolDispose(void *obj); +static void virStoragePoolDispose(void *obj); + +static int +virDataTypesOnceInit(void) +{ +#define DECLARE_CLASS(basename) \ + if (!(basename ## Class = virClassNew(#basename, \ + sizeof(basename), \ + basename ## Dispose))) \ + return -1; + + DECLARE_CLASS(virConnect); + DECLARE_CLASS(virDomain); + DECLARE_CLASS(virDomainSnapshot); + DECLARE_CLASS(virInterface); + DECLARE_CLASS(virNetwork); + DECLARE_CLASS(virNodeDevice); + DECLARE_CLASS(virNWFilter); + DECLARE_CLASS(virSecret); + DECLARE_CLASS(virStream); + DECLARE_CLASS(virStorageVol); + DECLARE_CLASS(virStoragePool); + +#undef DECLARE_CLASS + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virDataTypes) /** * virGetConnect: @@ -50,38 +96,26 @@ * Returns a new pointer or NULL in case of error. */ virConnectPtr -virGetConnect(void) { +virGetConnect(void) +{ virConnectPtr ret; - if (VIR_ALLOC(ret) < 0) { - virReportOOMError(); - goto failed; - } + if (virDataTypesInitialize() < 0) + return NULL; + + if (!(ret = virObjectNew(virConnectClass))) + return NULL; + if (virMutexInit(&ret->lock) < 0) { VIR_FREE(ret); - goto failed; + return NULL; } - ret->magic = VIR_CONNECT_MAGIC; - ret->driver = NULL; - ret->networkDriver = NULL; - ret->privateData = NULL; - ret->networkPrivateData = NULL; - ret->interfacePrivateData = NULL; - - ret->refs = 1; return ret; - -failed: - if (ret != NULL) { - virMutexDestroy(&ret->lock); - VIR_FREE(ret); - } - return NULL; } /** - * virReleaseConnect: + * virConnectDispose: * @conn: the hypervisor connection to release * * Unconditionally release all memory associated with a connection. @@ -90,13 +124,9 @@ failed: * be used once this method returns. */ static void -virReleaseConnect(virConnectPtr conn) { - VIR_DEBUG("release connection %p", conn); - - /* make sure to release the connection lock before we call the - * close callbacks, otherwise we will deadlock if an error - * is raised by any of the callbacks */ - virMutexUnlock(&conn->lock); +virConnectDispose(void *obj) +{ + virConnectPtr conn = obj; if (conn->networkDriver) conn->networkDriver->close(conn); @@ -127,35 +157,6 @@ virReleaseConnect(virConnectPtr conn) { VIR_FREE(conn); } -/** - * virUnrefConnect: - * @conn: the hypervisor connection to unreference - * - * Unreference the connection. If the use count drops to zero, the structure is - * actually freed. - * - * Returns the reference count or -1 in case of failure. - */ -int -virUnrefConnect(virConnectPtr conn) { - int refs; - - if ((!VIR_IS_CONNECT(conn))) { - virLibConnError(VIR_ERR_INVALID_CONN, "%s", _("no connection")); - return -1; - } - virMutexLock(&conn->lock); - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - refs = conn->refs; - if (refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return 0; - } - virMutexUnlock(&conn->lock); - return refs; -} /** * virGetDomain: @@ -166,14 +167,18 @@ virUnrefConnect(virConnectPtr conn) { * Lookup if the domain is already registered for that connection, * if yes return a new pointer to it, if no allocate a new structure, * and register it in the table. In any case a corresponding call to - * virUnrefDomain() is needed to not leak data. + * virObjectUnref() is needed to not leak data. * * Returns a pointer to the domain, or NULL in case of failure */ virDomainPtr -virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { +virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) +{ virDomainPtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, "%s", _("no connection")); return NULL; @@ -181,39 +186,26 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { virCheckNonNullArgReturn(name, NULL); virCheckNonNullArgReturn(uuid, NULL); - virMutexLock(&conn->lock); + if (!(ret = virObjectNew(virDomainClass))) + return NULL; - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_DOMAIN_MAGIC; - ret->conn = conn; + if (!(ret->name = strdup(name))) + goto no_memory; + + ret->conn = virObjectRef(conn); ret->id = -1; memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - conn->refs++; - ret->refs++; - virMutexUnlock(&conn->lock); return ret; - error: - if (ret != NULL) { - VIR_FREE(ret->name); - VIR_FREE(ret); - } +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } /** - * virReleaseDomain: + * virDomainDispose: * @domain: the domain to release * * Unconditionally release all memory associated with a domain. @@ -225,64 +217,20 @@ virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) { * which may also be released if its ref count hits zero. */ static void -virReleaseDomain(virDomainPtr domain) { - virConnectPtr conn = domain->conn; +virDomainDispose(void *obj) +{ + virDomainPtr domain = obj; char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(domain->uuid, uuidstr); VIR_DEBUG("release domain %p %s %s", domain, domain->name, uuidstr); - domain->magic = -1; - domain->id = -1; VIR_FREE(domain->name); - VIR_FREE(domain); - - if (conn) { - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; - } - virMutexUnlock(&conn->lock); - } + virObjectUnref(domain->conn); } /** - * virUnrefDomain: - * @domain: the domain to unreference - * - * Unreference the domain. If the use count drops to zero, the structure is - * actually freed. - * - * Returns the reference count or -1 in case of failure. - */ -int -virUnrefDomain(virDomainPtr domain) { - int refs; - - if (!VIR_IS_CONNECTED_DOMAIN(domain)) { - virLibConnError(VIR_ERR_INVALID_DOMAIN, "%s", - _("bad domain or no connection")); - return -1; - } - virMutexLock(&domain->conn->lock); - VIR_DEBUG("unref domain %p %s %d", domain, domain->name, domain->refs); - domain->refs--; - refs = domain->refs; - if (refs == 0) { - virReleaseDomain(domain); - /* Already unlocked mutex */ - return 0; - } - - virMutexUnlock(&domain->conn->lock); - return refs; -} - -/** * virGetNetwork: * @conn: the hypervisor connection * @name: pointer to the network name @@ -291,14 +239,18 @@ virUnrefDomain(virDomainPtr domain) { * Lookup if the network is already registered for that connection, * if yes return a new pointer to it, if no allocate a new structure, * and register it in the table. In any case a corresponding call to - * virUnrefNetwork() is needed to not leak data. + * virObjectUnref() is needed to not leak data. * * Returns a pointer to the network, or NULL in case of failure */ virNetworkPtr -virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) { +virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) +{ virNetworkPtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, "%s", _("no connection")); return NULL; @@ -306,38 +258,25 @@ virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) { virCheckNonNullArgReturn(name, NULL); virCheckNonNullArgReturn(uuid, NULL); - virMutexLock(&conn->lock); + if (!(ret = virObjectNew(virNetworkClass))) + return NULL; - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_NETWORK_MAGIC; - ret->conn = conn; + if (!(ret->name = strdup(name))) + goto no_memory; + + ret->conn = virObjectRef(conn); memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - conn->refs++; - ret->refs++; - virMutexUnlock(&conn->lock); return ret; - error: - if (ret != NULL) { - VIR_FREE(ret->name); - VIR_FREE(ret); - } +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } /** - * virReleaseNetwork: + * virNetworkDispose: * @network: the network to release * * Unconditionally release all memory associated with a network. @@ -349,60 +288,16 @@ virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) { * which may also be released if its ref count hits zero. */ static void -virReleaseNetwork(virNetworkPtr network) { - virConnectPtr conn = network->conn; +virNetworkDispose(void *obj) +{ + virNetworkPtr network = obj; char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(network->uuid, uuidstr); VIR_DEBUG("release network %p %s %s", network, network->name, uuidstr); - network->magic = -1; VIR_FREE(network->name); - VIR_FREE(network); - - if (conn) { - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; - } - virMutexUnlock(&conn->lock); - } -} - - -/** - * virUnrefNetwork: - * @network: the network to unreference - * - * Unreference the network. If the use count drops to zero, the structure is - * actually freed. - * - * Returns the reference count or -1 in case of failure. - */ -int -virUnrefNetwork(virNetworkPtr network) { - int refs; - - if (!VIR_IS_CONNECTED_NETWORK(network)) { - virLibConnError(VIR_ERR_INVALID_NETWORK, "%s", - _("bad network or no connection")); - return -1; - } - virMutexLock(&network->conn->lock); - VIR_DEBUG("unref network %p %s %d", network, network->name, network->refs); - network->refs--; - refs = network->refs; - if (refs == 0) { - virReleaseNetwork(network); - /* Already unlocked mutex */ - return 0; - } - - virMutexUnlock(&network->conn->lock); - return refs; + virObjectUnref(network->conn); } @@ -415,15 +310,19 @@ virUnrefNetwork(virNetworkPtr network) { * Lookup if the interface is already registered for that connection, * if yes return a new pointer to it (possibly updating the MAC * address), if no allocate a new structure, and register it in the - * table. In any case a corresponding call to virUnrefInterface() is + * table. In any case a corresponding call to virObjectUnref() is * needed to not leak data. * * Returns a pointer to the interface, or NULL in case of failure */ virInterfacePtr -virGetInterface(virConnectPtr conn, const char *name, const char *mac) { +virGetInterface(virConnectPtr conn, const char *name, const char *mac) +{ virInterfacePtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, "%s", _("no connection")); return NULL; @@ -434,45 +333,26 @@ virGetInterface(virConnectPtr conn, const char *name, const char *mac) { if (mac == NULL) mac = ""; - virMutexLock(&conn->lock); + if (!(ret = virObjectNew(virInterfaceClass))) + return NULL; - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->mac = strdup(mac); - if (ret->mac == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } + if (!(ret->name = strdup(name))) + goto no_memory; + if (!(ret->mac = strdup(mac))) + goto no_memory; - ret->magic = VIR_INTERFACE_MAGIC; - ret->conn = conn; + ret->conn = virObjectRef(conn); - conn->refs++; - ret->refs++; - virMutexUnlock(&conn->lock); return ret; - error: - if (ret != NULL) { - VIR_FREE(ret->name); - VIR_FREE(ret->mac); - VIR_FREE(ret); - } +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } /** - * virReleaseInterface: + * virInterfaceDispose: * @interface: the interface to release * * Unconditionally release all memory associated with an interface. @@ -484,58 +364,14 @@ virGetInterface(virConnectPtr conn, const char *name, const char *mac) { * which may also be released if its ref count hits zero. */ static void -virReleaseInterface(virInterfacePtr iface) { - virConnectPtr conn = iface->conn; +virInterfaceDispose(void *obj) +{ + virInterfacePtr iface = obj; VIR_DEBUG("release interface %p %s", iface, iface->name); - iface->magic = -1; VIR_FREE(iface->name); VIR_FREE(iface->mac); - VIR_FREE(iface); - - if (conn) { - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; - } - virMutexUnlock(&conn->lock); - } -} - - -/** - * virUnrefInterface: - * @interface: the interface to unreference - * - * Unreference the interface. If the use count drops to zero, the structure is - * actually freed. - * - * Returns the reference count or -1 in case of failure. - */ -int -virUnrefInterface(virInterfacePtr iface) { - int refs; - - if (!VIR_IS_CONNECTED_INTERFACE(iface)) { - virLibConnError(VIR_ERR_INVALID_INTERFACE, "%s", - _("bad interface or no connection")); - return -1; - } - virMutexLock(&iface->conn->lock); - VIR_DEBUG("unref interface %p %s %d", iface, iface->name, iface->refs); - iface->refs--; - refs = iface->refs; - if (refs == 0) { - virReleaseInterface(iface); - /* Already unlocked mutex */ - return 0; - } - - virMutexUnlock(&iface->conn->lock); - return refs; + virObjectUnref(iface->conn); } @@ -548,15 +384,19 @@ virUnrefInterface(virInterfacePtr iface) { * Lookup if the storage pool is already registered for that connection, * if yes return a new pointer to it, if no allocate a new structure, * and register it in the table. In any case a corresponding call to - * virUnrefStoragePool() is needed to not leak data. + * virObjectUnref() is needed to not leak data. * * Returns a pointer to the network, or NULL in case of failure */ virStoragePoolPtr virGetStoragePool(virConnectPtr conn, const char *name, - const unsigned char *uuid) { + const unsigned char *uuid) +{ virStoragePoolPtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, "%s", _("no connection")); return NULL; @@ -564,39 +404,26 @@ virGetStoragePool(virConnectPtr conn, const char *name, virCheckNonNullArgReturn(name, NULL); virCheckNonNullArgReturn(uuid, NULL); - virMutexLock(&conn->lock); + if (!(ret = virObjectNew(virStoragePoolClass))) + return NULL; - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_STORAGE_POOL_MAGIC; - ret->conn = conn; + if (!(ret->name = strdup(name))) + goto no_memory; + + ret->conn = virObjectRef(conn); memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - conn->refs++; - ret->refs++; - virMutexUnlock(&conn->lock); return ret; -error: - if (ret != NULL) { - VIR_FREE(ret->name); - VIR_FREE(ret); - } +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } /** - * virReleaseStoragePool: + * virStoragePoolDispose: * @pool: the pool to release * * Unconditionally release all memory associated with a pool. @@ -608,60 +435,16 @@ error: * which may also be released if its ref count hits zero. */ static void -virReleaseStoragePool(virStoragePoolPtr pool) { - virConnectPtr conn = pool->conn; +virStoragePoolDispose(void *obj) +{ + virStoragePoolPtr pool = obj; char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(pool->uuid, uuidstr); VIR_DEBUG("release pool %p %s %s", pool, pool->name, uuidstr); - pool->magic = -1; VIR_FREE(pool->name); - VIR_FREE(pool); - - if (conn) { - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; - } - virMutexUnlock(&conn->lock); - } -} - - -/** - * virUnrefStoragePool: - * @pool: the pool to unreference - * - * Unreference the pool. If the use count drops to zero, the structure is - * actually freed. - * - * Returns the reference count or -1 in case of failure. - */ -int -virUnrefStoragePool(virStoragePoolPtr pool) { - int refs; - - if (!VIR_IS_CONNECTED_STORAGE_POOL(pool)) { - virLibConnError(VIR_ERR_INVALID_STORAGE_POOL, "%s", - _("bad storage pool or no connection")); - return -1; - } - virMutexLock(&pool->conn->lock); - VIR_DEBUG("unref pool %p %s %d", pool, pool->name, pool->refs); - pool->refs--; - refs = pool->refs; - if (refs == 0) { - virReleaseStoragePool(pool); - /* Already unlocked mutex */ - return 0; - } - - virMutexUnlock(&pool->conn->lock); - return refs; + virObjectUnref(pool->conn); } @@ -675,15 +458,19 @@ virUnrefStoragePool(virStoragePoolPtr pool) { * Lookup if the storage vol is already registered for that connection, * if yes return a new pointer to it, if no allocate a new structure, * and register it in the table. In any case a corresponding call to - * virUnrefStorageVol() is needed to not leak data. + * virObjectUnref() is needed to not leak data. * * Returns a pointer to the storage vol, or NULL in case of failure */ virStorageVolPtr virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, - const char *key) { + const char *key) +{ virStorageVolPtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, "%s", _("no connection")); return NULL; @@ -691,52 +478,29 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, virCheckNonNullArgReturn(name, NULL); virCheckNonNullArgReturn(key, NULL); - virMutexLock(&conn->lock); + if (!(ret = virObjectNew(virStorageVolClass))) + return NULL; - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->pool = strdup(pool); - if (ret->pool == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->key = strdup(key); - if (ret->key == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_STORAGE_VOL_MAGIC; - ret->conn = conn; + if (!(ret->pool = strdup(pool))) + goto no_memory; + if (!(ret->name = strdup(name))) + goto no_memory; + if (!(ret->key = strdup(key))) + goto no_memory; + + ret->conn = virObjectRef(conn); - conn->refs++; - ret->refs++; - virMutexUnlock(&conn->lock); return ret; -error: - if (ret != NULL) { - VIR_FREE(ret->key); - VIR_FREE(ret->name); - VIR_FREE(ret->pool); - VIR_FREE(ret); - } +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } /** - * virReleaseStorageVol: + * virStorageVolDispose: * @vol: the vol to release * * Unconditionally release all memory associated with a vol. @@ -748,59 +512,15 @@ error: * which may also be released if its ref count hits zero. */ static void -virReleaseStorageVol(virStorageVolPtr vol) { - virConnectPtr conn = vol->conn; +virStorageVolDispose(void *obj) +{ + virStorageVolPtr vol = obj; VIR_DEBUG("release vol %p %s", vol, vol->name); - vol->magic = -1; VIR_FREE(vol->key); VIR_FREE(vol->name); VIR_FREE(vol->pool); - VIR_FREE(vol); - - if (conn) { - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; - } - virMutexUnlock(&conn->lock); - } -} - - -/** - * virUnrefStorageVol: - * @vol: the vol to unreference - * - * Unreference the vol. If the use count drops to zero, the structure is - * actually freed. - * - * Returns the reference count or -1 in case of failure. - */ -int -virUnrefStorageVol(virStorageVolPtr vol) { - int refs; - - if (!VIR_IS_CONNECTED_STORAGE_VOL(vol)) { - virLibConnError(VIR_ERR_INVALID_STORAGE_VOL, "%s", - _("bad storage volume or no connection")); - return -1; - } - virMutexLock(&vol->conn->lock); - VIR_DEBUG("unref vol %p %s %d", vol, vol->name, vol->refs); - vol->refs--; - refs = vol->refs; - if (refs == 0) { - virReleaseStorageVol(vol); - /* Already unlocked mutex */ - return 0; - } - - virMutexUnlock(&vol->conn->lock); - return refs; + virObjectUnref(vol->conn); } @@ -812,7 +532,7 @@ virUnrefStorageVol(virStorageVolPtr vol) { * Lookup if the device is already registered for that connection, * if yes return a new pointer to it, if no allocate a new structure, * and register it in the table. In any case a corresponding call to - * virUnrefNodeDevice() is needed to not leak data. + * virObjectUnref() is needed to not leak data. * * Returns a pointer to the node device, or NULL in case of failure */ @@ -821,44 +541,32 @@ virGetNodeDevice(virConnectPtr conn, const char *name) { virNodeDevicePtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, "%s", _("no connection")); return NULL; } virCheckNonNullArgReturn(name, NULL); - virMutexLock(&conn->lock); + if (!(ret = virObjectNew(virNodeDeviceClass))) + return NULL; - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_NODE_DEVICE_MAGIC; - ret->conn = conn; - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } + if (!(ret->name = strdup(name))) + goto no_memory; - conn->refs++; - ret->refs++; - virMutexUnlock(&conn->lock); + ret->conn = virObjectRef(conn); return ret; - -error: - if (ret != NULL) { - VIR_FREE(ret->name); - VIR_FREE(ret); - } +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } /** - * virReleaseNodeDevice: + * virNodeDeviceDispose: * @dev: the dev to release * * Unconditionally release all memory associated with a dev. @@ -870,53 +578,15 @@ error: * which may also be released if its ref count hits zero. */ static void -virReleaseNodeDevice(virNodeDevicePtr dev) { - virConnectPtr conn = dev->conn; +virNodeDeviceDispose(void *obj) +{ + virNodeDevicePtr dev = obj; VIR_DEBUG("release dev %p %s", dev, dev->name); - dev->magic = -1; VIR_FREE(dev->name); VIR_FREE(dev->parent); - VIR_FREE(dev); - - if (conn) { - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; - } - virMutexUnlock(&conn->lock); - } -} - -/** - * virUnrefNodeDevice: - * @dev: the dev to unreference - * - * Unreference the dev. If the use count drops to zero, the structure is - * actually freed. - * - * Returns the reference count or -1 in case of failure. - */ -int -virUnrefNodeDevice(virNodeDevicePtr dev) { - int refs; - - virMutexLock(&dev->conn->lock); - VIR_DEBUG("unref dev %p %s %d", dev, dev->name, dev->refs); - dev->refs--; - refs = dev->refs; - if (refs == 0) { - virReleaseNodeDevice(dev); - /* Already unlocked mutex */ - return 0; - } - - virMutexUnlock(&dev->conn->lock); - return refs; + virObjectUnref(dev->conn); } @@ -927,7 +597,7 @@ virUnrefNodeDevice(virNodeDevicePtr dev) { * * Lookup if the secret is already registered for that connection, if so return * a pointer to it, otherwise allocate a new structure, and register it in the - * table. In any case a corresponding call to virUnrefSecret() is needed to not + * table. In any case a corresponding call to virObjectUnref() is needed to not * leak data. * * Returns a pointer to the secret, or NULL in case of failure @@ -938,6 +608,9 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid, { virSecretPtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, "%s", _("no connection")); return NULL; @@ -945,37 +618,25 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid, virCheckNonNullArgReturn(uuid, NULL); virCheckNonNullArgReturn(usageID, NULL); - virMutexLock(&conn->lock); + if (!(ret = virObjectNew(virSecretClass))) + return NULL; - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_SECRET_MAGIC; - ret->conn = conn; memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); ret->usageType = usageType; - if (!(ret->usageID = strdup(usageID))) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - conn->refs++; - ret->refs++; - virMutexUnlock(&conn->lock); - return ret; + if (!(ret->usageID = strdup(usageID))) + goto no_memory; -error: - if (ret != NULL) { - VIR_FREE(ret->usageID); - VIR_FREE(ret); - } + ret->conn = virObjectRef(conn); + + return ret; +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } /** - * virReleaseSecret: + * virSecretDispose: * @secret: the secret to release * * Unconditionally release all memory associated with a secret. The conn.lock @@ -986,117 +647,42 @@ error: * released if its ref count hits zero. */ static void -virReleaseSecret(virSecretPtr secret) { - virConnectPtr conn = secret->conn; +virSecretDispose(void *obj) +{ + virSecretPtr secret = obj; char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(secret->uuid, uuidstr); VIR_DEBUG("release secret %p %s", secret, uuidstr); VIR_FREE(secret->usageID); - secret->magic = -1; - VIR_FREE(secret); - - if (conn) { - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; - } - virMutexUnlock(&conn->lock); - } + virObjectUnref(secret->conn); } -/** - * virUnrefSecret: - * @secret: the secret to unreference - * - * Unreference the secret. If the use count drops to zero, the structure is - * actually freed. - * - * Returns the reference count or -1 in case of failure. - */ -int -virUnrefSecret(virSecretPtr secret) { - int refs; - if (!VIR_IS_CONNECTED_SECRET(secret)) { - virLibConnError(VIR_ERR_INVALID_SECRET, "%s", - _("bad secret or no connection")); - return -1; - } - virMutexLock(&secret->conn->lock); - VIR_DEBUG("unref secret %p %p %d", secret, secret->uuid, secret->refs); - secret->refs--; - refs = secret->refs; - if (refs == 0) { - virReleaseSecret(secret); - /* Already unlocked mutex */ - return 0; - } +virStreamPtr +virGetStream(virConnectPtr conn) +{ + virStreamPtr ret = NULL; - virMutexUnlock(&secret->conn->lock); - return refs; -} + if (virDataTypesInitialize() < 0) + return NULL; -virStreamPtr virGetStream(virConnectPtr conn) { - virStreamPtr ret = NULL; + if (!(ret = virObjectNew(virStreamClass))) + return NULL; - virMutexLock(&conn->lock); + ret->conn = virObjectRef(conn); - if (VIR_ALLOC(ret) < 0) { - virReportOOMError(); - goto error; - } - ret->magic = VIR_STREAM_MAGIC; - ret->conn = conn; - conn->refs++; - ret->refs++; - virMutexUnlock(&conn->lock); return ret; - -error: - virMutexUnlock(&conn->lock); - VIR_FREE(ret); - return NULL; } static void -virReleaseStream(virStreamPtr st) { - virConnectPtr conn = st->conn; +virStreamDispose(void *obj) +{ + virStreamPtr st = obj; VIR_DEBUG("release dev %p", st); - st->magic = -1; - VIR_FREE(st); - - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; - } - - virMutexUnlock(&conn->lock); -} - -int virUnrefStream(virStreamPtr st) { - int refs; - - virMutexLock(&st->conn->lock); - VIR_DEBUG("unref stream %p %d", st, st->refs); - st->refs--; - refs = st->refs; - if (refs == 0) { - virReleaseStream(st); - /* Already unlocked mutex */ - return 0; - } - - virMutexUnlock(&st->conn->lock); - return refs; + virObjectUnref(st->conn); } @@ -1109,14 +695,19 @@ int virUnrefStream(virStreamPtr st) { * Lookup if the network filter is already registered for that connection, * if yes return a new pointer to it, if no allocate a new structure, * and register it in the table. In any case a corresponding call to - * virUnrefNWFilter() is needed to not leak data. + * virObjectUnref() is needed to not leak data. * * Returns a pointer to the network, or NULL in case of failure */ virNWFilterPtr -virGetNWFilter(virConnectPtr conn, const char *name, const unsigned char *uuid) { +virGetNWFilter(virConnectPtr conn, const char *name, + const unsigned char *uuid) +{ virNWFilterPtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, "%s", _("no connection")); return NULL; @@ -1124,39 +715,26 @@ virGetNWFilter(virConnectPtr conn, const char *name, const unsigned char *uuid) virCheckNonNullArgReturn(name, NULL); virCheckNonNullArgReturn(uuid, NULL); - virMutexLock(&conn->lock); + if (!(ret = virObjectNew(virNWFilterClass))) + return NULL; + + if (!(ret->name = strdup(name))) + goto no_memory; - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_NWFILTER_MAGIC; - ret->conn = conn; memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); - conn->refs++; - ret->refs++; - virMutexUnlock(&conn->lock); - return ret; + ret->conn = virObjectRef(conn); -error: - if (ret != NULL) { - VIR_FREE(ret->name); - VIR_FREE(ret); - } + return ret; +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } /** - * virReleaseNWFilter: + * virNWFilterDispose: * @nwfilter: the nwfilter to release * * Unconditionally release all memory associated with a nwfilter. @@ -1168,63 +746,16 @@ error: * which may also be released if its ref count hits zero. */ static void -virReleaseNWFilter(virNWFilterPtr nwfilter) +virNWFilterDispose(void *obj) { - virConnectPtr conn = nwfilter->conn; + virNWFilterPtr nwfilter = obj; char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(nwfilter->uuid, uuidstr); VIR_DEBUG("release nwfilter %p %s %s", nwfilter, nwfilter->name, uuidstr); - nwfilter->magic = -1; VIR_FREE(nwfilter->name); - VIR_FREE(nwfilter); - - if (conn) { - VIR_DEBUG("unref connection %p %d", conn, conn->refs); - conn->refs--; - if (conn->refs == 0) { - virReleaseConnect(conn); - /* Already unlocked mutex */ - return; - } - virMutexUnlock(&conn->lock); - } -} - - -/** - * virUnrefNWFilter: - * @nwfilter: the nwfilter to unreference - * - * Unreference the networkf itler. If the use count drops to zero, the - * structure is actually freed. - * - * Returns the reference count or -1 in case of failure. - */ -int -virUnrefNWFilter(virNWFilterPtr nwfilter) -{ - int refs; - - if (!VIR_IS_CONNECTED_NWFILTER(nwfilter)) { - virLibConnError(VIR_ERR_INVALID_NWFILTER, "%s", - _("bad nwfilter or no connection")); - return -1; - } - virMutexLock(&nwfilter->conn->lock); - VIR_DEBUG("unref nwfilter %p %s %d", nwfilter, nwfilter->name, - nwfilter->refs); - nwfilter->refs--; - refs = nwfilter->refs; - if (refs == 0) { - virReleaseNWFilter(nwfilter); - /* Already unlocked mutex */ - return 0; - } - - virMutexUnlock(&nwfilter->conn->lock); - return refs; + virObjectUnref(nwfilter->conn); } @@ -1233,85 +764,35 @@ virGetDomainSnapshot(virDomainPtr domain, const char *name) { virDomainSnapshotPtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_DOMAIN(domain)) { virLibConnError(VIR_ERR_INVALID_DOMAIN, "%s", _("bad domain")); return NULL; } virCheckNonNullArgReturn(name, NULL); - virMutexLock(&domain->conn->lock); - - if (VIR_ALLOC(ret) < 0) { - virMutexUnlock(&domain->conn->lock); - virReportOOMError(); - goto error; - } - ret->name = strdup(name); - if (ret->name == NULL) { - virMutexUnlock(&domain->conn->lock); - virReportOOMError(); - goto error; - } - ret->magic = VIR_SNAPSHOT_MAGIC; - ret->domain = domain; + if (!(ret = virObjectNew(virDomainSnapshotClass))) + return NULL; + if (!(ret->name = strdup(name))) + goto no_memory; + ret->domain = virObjectRef(domain); - domain->refs++; - ret->refs++; - virMutexUnlock(&domain->conn->lock); return ret; - - error: - if (ret != NULL) { - VIR_FREE(ret->name); - VIR_FREE(ret); - } +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } static void -virReleaseDomainSnapshot(virDomainSnapshotPtr snapshot) +virDomainSnapshotDispose(void *obj) { - virDomainPtr domain = snapshot->domain; + virDomainSnapshotPtr snapshot = obj; VIR_DEBUG("release snapshot %p %s", snapshot, snapshot->name); - snapshot->magic = -1; VIR_FREE(snapshot->name); - VIR_FREE(snapshot); - - if (domain) { - VIR_DEBUG("unref domain %p %d", domain, domain->refs); - domain->refs--; - if (domain->refs == 0) { - virReleaseDomain(domain); - /* Already unlocked mutex */ - return; - } - virMutexUnlock(&domain->conn->lock); - } -} - -int -virUnrefDomainSnapshot(virDomainSnapshotPtr snapshot) -{ - int refs; - - if (!VIR_IS_DOMAIN_SNAPSHOT(snapshot)) { - virLibConnError(VIR_ERR_INVALID_DOMAIN_SNAPSHOT, "%s", - _("not a snapshot")); - return -1; - } - - virMutexLock(&snapshot->domain->conn->lock); - VIR_DEBUG("unref snapshot %p %s %d", snapshot, snapshot->name, snapshot->refs); - snapshot->refs--; - refs = snapshot->refs; - if (refs == 0) { - virReleaseDomainSnapshot(snapshot); - /* Already unlocked mutex */ - return 0; - } - - virMutexUnlock(&snapshot->domain->conn->lock); - return refs; + virObjectUnref(snapshot->domain); } diff --git a/src/datatypes.h b/src/datatypes.h index cd17410..0ec2b56 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -19,125 +19,79 @@ * */ -#ifndef __VIRT_DATATYPES_H_ -# define __VIRT_DATATYPES_H_ +#ifndef __VIR_DATATYPES_H_ +# define __VIR_DATATYPES_H_ # include "internal.h" # include "driver.h" # include "threads.h" - -/** - * VIR_CONNECT_MAGIC: - * - * magic value used to protect the API when pointers to connection structures - * are passed down by the users. - */ -# define VIR_CONNECT_MAGIC 0x4F23DEAD -# define VIR_IS_CONNECT(obj) ((obj) && (obj)->magic==VIR_CONNECT_MAGIC) - - -/** - * VIR_DOMAIN_MAGIC: - * - * magic value used to protect the API when pointers to domain structures - * are passed down by the users. - */ -# define VIR_DOMAIN_MAGIC 0xDEAD4321 -# define VIR_IS_DOMAIN(obj) ((obj) && (obj)->magic==VIR_DOMAIN_MAGIC) -# define VIR_IS_CONNECTED_DOMAIN(obj) (VIR_IS_DOMAIN(obj) && VIR_IS_CONNECT((obj)->conn)) - -/** - * VIR_NETWORK_MAGIC: - * - * magic value used to protect the API when pointers to network structures - * are passed down by the users. - */ -# define VIR_NETWORK_MAGIC 0xDEAD1234 -# define VIR_IS_NETWORK(obj) ((obj) && (obj)->magic==VIR_NETWORK_MAGIC) -# define VIR_IS_CONNECTED_NETWORK(obj) (VIR_IS_NETWORK(obj) && VIR_IS_CONNECT((obj)->conn)) - -/** - * VIR_INTERFACE_MAGIC: - * - * magic value used to protect the API when pointers to interface structures - * are passed down by the users. - */ -# define VIR_INTERFACE_MAGIC 0xDEAD5309 -# define VIR_IS_INTERFACE(obj) ((obj) && (obj)->magic==VIR_INTERFACE_MAGIC) -# define VIR_IS_CONNECTED_INTERFACE(obj) (VIR_IS_INTERFACE(obj) && VIR_IS_CONNECT((obj)->conn)) - -/** - * VIR_STORAGE_POOL_MAGIC: - * - * magic value used to protect the API when pointers to storage pool structures - * are passed down by the users. - */ -# define VIR_STORAGE_POOL_MAGIC 0xDEAD5678 -# define VIR_IS_STORAGE_POOL(obj) ((obj) && (obj)->magic==VIR_STORAGE_POOL_MAGIC) -# define VIR_IS_CONNECTED_STORAGE_POOL(obj) (VIR_IS_STORAGE_POOL(obj) && VIR_IS_CONNECT((obj)->conn)) - -/** - * VIR_STORAGE_VOL_MAGIC: - * - * magic value used to protect the API when pointers to storage vol structures - * are passed down by the users. - */ -# define VIR_STORAGE_VOL_MAGIC 0xDEAD8765 -# define VIR_IS_STORAGE_VOL(obj) ((obj) && (obj)->magic==VIR_STORAGE_VOL_MAGIC) -# define VIR_IS_CONNECTED_STORAGE_VOL(obj) (VIR_IS_STORAGE_VOL(obj) && VIR_IS_CONNECT((obj)->conn)) - -/** - * VIR_NODE_DEVICE_MAGIC: - * - * magic value used to protect the API when pointers to storage vol structures - * are passed down by the users. - */ -# define VIR_NODE_DEVICE_MAGIC 0xDEAD5679 -# define VIR_IS_NODE_DEVICE(obj) ((obj) && (obj)->magic==VIR_NODE_DEVICE_MAGIC) -# define VIR_IS_CONNECTED_NODE_DEVICE(obj) (VIR_IS_NODE_DEVICE(obj) && VIR_IS_CONNECT((obj)->conn)) - -/** - * VIR_SECRET_MAGIC: - * - * magic value used to protect the API when pointers to secret structures are - * passed down by the users. - */ -# define VIR_SECRET_MAGIC 0x5678DEAD -# define VIR_IS_SECRET(obj) ((obj) && (obj)->magic==VIR_SECRET_MAGIC) -# define VIR_IS_CONNECTED_SECRET(obj) (VIR_IS_SECRET(obj) && VIR_IS_CONNECT((obj)->conn)) - - -/** - * VIR_STREAM_MAGIC: - * - * magic value used to protect the API when pointers to stream structures - * are passed down by the users. - */ -# define VIR_STREAM_MAGIC 0x1DEAD666 -# define VIR_IS_STREAM(obj) ((obj) && (obj)->magic==VIR_STREAM_MAGIC) -# define VIR_IS_CONNECTED_STREAM(obj) (VIR_IS_STREAM(obj) && VIR_IS_CONNECT((obj)->conn)) - - -/** - * VIR_NWFILTER_MAGIC: - * - * magic value used to protect the API when pointers to network filter - * pool structures are passed down by the users. - */ -# define VIR_NWFILTER_MAGIC 0xDEAD7777 -# define VIR_IS_NWFILTER(obj) ((obj) && (obj)->magic==VIR_NWFILTER_MAGIC) -# define VIR_IS_CONNECTED_NWFILTER(obj) (VIR_IS_NWFILTER(obj) && VIR_IS_CONNECT((obj)->conn)) - -/** - * VIR_SNAPSHOT_MAGIC: - * - * magic value used to protect the API when pointers to snapshot structures - * are passed down by the users. - */ -# define VIR_SNAPSHOT_MAGIC 0x6666DEAD -# define VIR_IS_SNAPSHOT(obj) ((obj) && (obj)->magic==VIR_SNAPSHOT_MAGIC) -# define VIR_IS_DOMAIN_SNAPSHOT(obj) (VIR_IS_SNAPSHOT(obj) && VIR_IS_DOMAIN((obj)->domain)) +# include "virobject.h" + +extern virClassPtr virConnectClass; +extern virClassPtr virDomainClass; +extern virClassPtr virDomainSnapshotClass; +extern virClassPtr virInterfaceClass; +extern virClassPtr virNetworkClass; +extern virClassPtr virNodeDeviceClass; +extern virClassPtr virNWFilterClass; +extern virClassPtr virSecretClass; +extern virClassPtr virStreamClass; +extern virClassPtr virStorageVolClass; +extern virClassPtr virStoragePoolClass; + +# define VIR_IS_CONNECT(obj) \ + (virObjectIsClass((obj), virConnectClass)) + +# define VIR_IS_DOMAIN(obj) \ + (virObjectIsClass((obj), virDomainClass)) +# define VIR_IS_CONNECTED_DOMAIN(obj) \ + (VIR_IS_DOMAIN(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_NETWORK(obj) \ + (virObjectIsClass((obj), virNetworkClass)) +# define VIR_IS_CONNECTED_NETWORK(obj) \ + (VIR_IS_NETWORK(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_INTERFACE(obj) \ + (virObjectIsClass((obj), virInterfaceClass)) +# define VIR_IS_CONNECTED_INTERFACE(obj) \ + (VIR_IS_INTERFACE(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_STORAGE_POOL(obj) \ + (virObjectIsClass((obj), virStoragePoolClass)) +# define VIR_IS_CONNECTED_STORAGE_POOL(obj) \ + (VIR_IS_STORAGE_POOL(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_STORAGE_VOL(obj) \ + (virObjectIsClass((obj), virStorageVolClass)) +# define VIR_IS_CONNECTED_STORAGE_VOL(obj) \ + (VIR_IS_STORAGE_VOL(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_NODE_DEVICE(obj) \ + (virObjectIsClass((obj), virNodeDeviceClass)) +# define VIR_IS_CONNECTED_NODE_DEVICE(obj) \ + (VIR_IS_NODE_DEVICE(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_SECRET(obj) \ + (virObjectIsClass((obj), virSecretClass)) +# define VIR_IS_CONNECTED_SECRET(obj) \ + (VIR_IS_SECRET(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_STREAM(obj) \ + (virObjectIsClass((obj), virStreamClass)) +# define VIR_IS_CONNECTED_STREAM(obj) \ + (VIR_IS_STREAM(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_NWFILTER(obj) \ + (virObjectIsClass((obj), virNWFilterClass)) +# define VIR_IS_CONNECTED_NWFILTER(obj) \ + (VIR_IS_NWFILTER(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_SNAPSHOT(obj) \ + (virObjectIsClass((obj), virDomainSnapshotClass)) +# define VIR_IS_DOMAIN_SNAPSHOT(obj) \ + (VIR_IS_SNAPSHOT(obj) && VIR_IS_DOMAIN((obj)->domain)) /** * _virConnect: @@ -145,11 +99,11 @@ * Internal structure associated to a connection */ struct _virConnect { + virObject object; /* All the variables from here, until the 'lock' declaration * are setup at time of connection open, and never changed * since. Thus no need to lock when accessing them */ - unsigned int magic; /* specific value to check */ unsigned int flags; /* a set of connection flags */ virURIPtr uri; /* connection URI */ @@ -193,8 +147,6 @@ struct _virConnect { virFreeCallback closeFreeCallback; bool closeDispatch; unsigned closeUnregisterCount; - - int refs; /* reference count */ }; /** @@ -203,8 +155,7 @@ struct _virConnect { * Internal structure associated to a domain */ struct _virDomain { - unsigned int magic; /* specific value to check */ - int refs; /* reference count */ + virObject object; virConnectPtr conn; /* pointer back to the connection */ char *name; /* the domain external name */ int id; /* the domain ID */ @@ -217,8 +168,7 @@ struct _virDomain { * Internal structure associated to a domain */ struct _virNetwork { - unsigned int magic; /* specific value to check */ - int refs; /* reference count */ + virObject object; virConnectPtr conn; /* pointer back to the connection */ char *name; /* the network external name */ unsigned char uuid[VIR_UUID_BUFLEN]; /* the network unique identifier */ @@ -230,8 +180,7 @@ struct _virNetwork { * Internal structure associated to a physical host interface */ struct _virInterface { - unsigned int magic; /* specific value to check */ - int refs; /* reference count */ + virObject object; virConnectPtr conn; /* pointer back to the connection */ char *name; /* the network external name */ char *mac; /* the interface MAC address */ @@ -243,8 +192,7 @@ struct _virInterface { * Internal structure associated to a storage pool */ struct _virStoragePool { - unsigned int magic; /* specific value to check */ - int refs; /* reference count */ + virObject object; virConnectPtr conn; /* pointer back to the connection */ char *name; /* the storage pool external name */ unsigned char uuid[VIR_UUID_BUFLEN]; /* the storage pool unique identifier */ @@ -256,8 +204,7 @@ struct _virStoragePool { * Internal structure associated to a storage volume */ struct _virStorageVol { - unsigned int magic; /* specific value to check */ - int refs; /* reference count */ + virObject object; virConnectPtr conn; /* pointer back to the connection */ char *pool; /* Pool name of owner */ char *name; /* the storage vol external name */ @@ -270,8 +217,7 @@ struct _virStorageVol { * Internal structure associated with a node device */ struct _virNodeDevice { - unsigned int magic; /* specific value to check */ - int refs; /* reference count */ + virObject object; virConnectPtr conn; /* pointer back to the connection */ char *name; /* device name (unique on node) */ char *parent; /* parent device name */ @@ -283,8 +229,7 @@ struct _virNodeDevice { * Internal structure associated with a secret */ struct _virSecret { - unsigned int magic; /* specific value to check */ - int refs; /* reference count */ + virObject object; virConnectPtr conn; /* pointer back to the connection */ unsigned char uuid[VIR_UUID_BUFLEN]; /* the domain unique identifier */ int usageType; /* the type of usage */ @@ -301,9 +246,8 @@ typedef int (*virStreamFinishFunc)(virStreamPtr, void *opaque); * Internal structure associated with an input stream */ struct _virStream { - unsigned int magic; + virObject object; virConnectPtr conn; - int refs; unsigned int flags; virStreamDriverPtr driver; @@ -316,77 +260,56 @@ struct _virStream { * Internal structure associated with a domain snapshot */ struct _virDomainSnapshot { - unsigned int magic; - int refs; + virObject object; char *name; virDomainPtr domain; }; -/************************************************************************ - * * - * API for domain/connections (de)allocations and lookups * - * * - ************************************************************************/ +/** +* _virNWFilter: +* +* Internal structure associated to a network filter +*/ +struct _virNWFilter { + virObject object; + virConnectPtr conn; /* pointer back to the connection */ + char *name; /* the network filter external name */ + unsigned char uuid[VIR_UUID_BUFLEN]; /* the network filter unique identifier */ +}; + + +/* + * Helper APIs for allocating new object instances + */ virConnectPtr virGetConnect(void); -int virUnrefConnect(virConnectPtr conn); virDomainPtr virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid); -int virUnrefDomain(virDomainPtr domain); virNetworkPtr virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid); -int virUnrefNetwork(virNetworkPtr network); - virInterfacePtr virGetInterface(virConnectPtr conn, const char *name, const char *mac); -int virUnrefInterface(virInterfacePtr iface); - virStoragePoolPtr virGetStoragePool(virConnectPtr conn, - const char *name, - const unsigned char *uuid); -int virUnrefStoragePool(virStoragePoolPtr pool); + const char *name, + const unsigned char *uuid); virStorageVolPtr virGetStorageVol(virConnectPtr conn, - const char *pool, - const char *name, - const char *key); -int virUnrefStorageVol(virStorageVolPtr vol); - + const char *pool, + const char *name, + const char *key); virNodeDevicePtr virGetNodeDevice(virConnectPtr conn, const char *name); -int virUnrefNodeDevice(virNodeDevicePtr dev); - virSecretPtr virGetSecret(virConnectPtr conn, const unsigned char *uuid, int usageType, const char *usageID); -int virUnrefSecret(virSecretPtr secret); - virStreamPtr virGetStream(virConnectPtr conn); -int virUnrefStream(virStreamPtr st); - -/** -* _virNWFilter: -* -* Internal structure associated to a network filter -*/ -struct _virNWFilter { - unsigned int magic; /* specific value to check */ - int refs; /* reference count */ - virConnectPtr conn; /* pointer back to the connection */ - char *name; /* the network filter external name */ - unsigned char uuid[VIR_UUID_BUFLEN]; /* the network filter unique identifier */ -}; - virNWFilterPtr virGetNWFilter(virConnectPtr conn, const char *name, const unsigned char *uuid); -int virUnrefNWFilter(virNWFilterPtr nwfilter); - virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, const char *name); -int virUnrefDomainSnapshot(virDomainSnapshotPtr snapshot); -#endif +#endif /* __VIR_DATATYPES_H__ */ diff --git a/src/libvirt.c b/src/libvirt.c index 3c4bf8c..1064e2e 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1294,7 +1294,7 @@ do_open (const char *name, failed: virConfFree(conf); - virUnrefConnect(ret); + virObjectUnref(ret); return NULL; } @@ -1429,14 +1429,16 @@ error: * matching virConnectClose, and all other references will be released * after the corresponding operation completes. * - * Returns the number of remaining references on success - * (positive implies that some other call still has a reference open, - * 0 implies that no references remain and the connection is closed), - * or -1 on failure. It is possible for the last virConnectClose to - * return a positive value if some other object still has a temporary - * reference to the connection, but the application should not try to - * further use a connection after the virConnectClose that matches the - * initial open. + * Returns a positive number if at least 1 reference remains on + * success. The returned value should not be assumed to be the total + * reference count. A return of 0 implies no references remain and + * the connection is closed & memory has been freed. A return of -1 + * implies a failure. + * + * It is possible for the last virConnectClose to return a positive + * value if some other object still has a temporary reference to the + * connection, but the application should not try to further use a + * connection after the virConnectClose that matches the initial open. */ int virConnectClose(virConnectPtr conn) @@ -1451,10 +1453,9 @@ virConnectClose(virConnectPtr conn) goto error; } - ret = virUnrefConnect(conn); - if (ret < 0) - goto error; - return ret; + if (!virObjectUnref(conn)) + return 0; + return 1; error: virDispatchError(NULL); @@ -1486,10 +1487,8 @@ virConnectRef(virConnectPtr conn) virDispatchError(NULL); return -1; } - virMutexLock(&conn->lock); - VIR_DEBUG("conn=%p refs=%d", conn, conn->refs); - conn->refs++; - virMutexUnlock(&conn->lock); + VIR_DEBUG("conn=%p refs=%d", conn, conn->object.refs); + virObjectRef(conn); return 0; } @@ -2286,10 +2285,7 @@ virDomainFree(virDomainPtr domain) virDispatchError(NULL); return -1; } - if (virUnrefDomain(domain) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(domain); return 0; } @@ -2318,10 +2314,9 @@ virDomainRef(virDomainPtr domain) virDispatchError(NULL); return -1; } - virMutexLock(&domain->conn->lock); - VIR_DOMAIN_DEBUG(domain, "refs=%d", domain->refs); - domain->refs++; - virMutexUnlock(&domain->conn->lock); + + VIR_DOMAIN_DEBUG(domain, "refs=%d", domain->object.refs); + virObjectRef(domain); return 0; } @@ -10118,10 +10113,7 @@ virNetworkFree(virNetworkPtr network) virDispatchError(NULL); return -1; } - if (virUnrefNetwork(network) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(network); return 0; } @@ -10150,10 +10142,8 @@ virNetworkRef(virNetworkPtr network) virDispatchError(NULL); return -1; } - virMutexLock(&network->conn->lock); - VIR_DEBUG("network=%p refs=%d", network, network->refs); - network->refs++; - virMutexUnlock(&network->conn->lock); + VIR_DEBUG("network=%p refs=%d", network, network->object.refs); + virObjectRef(network); return 0; } @@ -11023,10 +11013,8 @@ virInterfaceRef(virInterfacePtr iface) virDispatchError(NULL); return -1; } - virMutexLock(&iface->conn->lock); - VIR_DEBUG("iface=%p refs=%d", iface, iface->refs); - iface->refs++; - virMutexUnlock(&iface->conn->lock); + VIR_DEBUG("iface=%p refs=%d", iface, iface->object.refs); + virObjectRef(iface); return 0; } @@ -11051,10 +11039,7 @@ virInterfaceFree(virInterfacePtr iface) virDispatchError(NULL); return -1; } - if (virUnrefInterface(iface) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(iface); return 0; } @@ -11960,10 +11945,7 @@ virStoragePoolFree(virStoragePoolPtr pool) virDispatchError(NULL); return -1; } - if (virUnrefStoragePool(pool) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(pool); return 0; } @@ -11994,10 +11976,8 @@ virStoragePoolRef(virStoragePoolPtr pool) virDispatchError(NULL); return -1; } - virMutexLock(&pool->conn->lock); - VIR_DEBUG("pool=%p refs=%d", pool, pool->refs); - pool->refs++; - virMutexUnlock(&pool->conn->lock); + VIR_DEBUG("pool=%p refs=%d", pool, pool->object.refs); + virObjectRef(pool); return 0; } @@ -13026,10 +13006,7 @@ virStorageVolFree(virStorageVolPtr vol) virDispatchError(NULL); return -1; } - if (virUnrefStorageVol(vol) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(vol); return 0; } @@ -13059,10 +13036,8 @@ virStorageVolRef(virStorageVolPtr vol) virDispatchError(NULL); return -1; } - virMutexLock(&vol->conn->lock); - VIR_DEBUG("vol=%p refs=%d", vol, vol->refs); - vol->refs++; - virMutexUnlock(&vol->conn->lock); + VIR_DEBUG("vol=%p refs=%d", vol, vol->object.refs); + virObjectRef(vol); return 0; } @@ -13601,10 +13576,7 @@ int virNodeDeviceFree(virNodeDevicePtr dev) virDispatchError(NULL); return -1; } - if (virUnrefNodeDevice(dev) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(dev); return 0; } @@ -13634,10 +13606,8 @@ virNodeDeviceRef(virNodeDevicePtr dev) virDispatchError(NULL); return -1; } - virMutexLock(&dev->conn->lock); - VIR_DEBUG("dev=%p refs=%d", dev, dev->refs); - dev->refs++; - virMutexUnlock(&dev->conn->lock); + VIR_DEBUG("dev=%p refs=%d", dev, dev->object.refs); + virObjectRef(dev); return 0; } @@ -14607,10 +14577,8 @@ virSecretRef(virSecretPtr secret) virDispatchError(NULL); return -1; } - virMutexLock(&secret->conn->lock); - VIR_DEBUG("secret=%p refs=%d", secret, secret->refs); - secret->refs++; - virMutexUnlock(&secret->conn->lock); + VIR_DEBUG("secret=%p refs=%d", secret, secret->object.refs); + virObjectRef(secret); return 0; } @@ -14634,10 +14602,7 @@ virSecretFree(virSecretPtr secret) virDispatchError(NULL); return -1; } - if (virUnrefSecret(secret) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(secret); return 0; } @@ -14706,10 +14671,8 @@ virStreamRef(virStreamPtr stream) virDispatchError(NULL); return -1; } - virMutexLock(&stream->conn->lock); - VIR_DEBUG("stream=%p refs=%d", stream, stream->refs); - stream->refs++; - virMutexUnlock(&stream->conn->lock); + VIR_DEBUG("stream=%p refs=%d", stream, stream->object.refs); + virObjectRef(stream); return 0; } @@ -15350,10 +15313,7 @@ int virStreamFree(virStreamPtr stream) /* XXX Enforce shutdown before free'ing resources ? */ - if (virUnrefStream(stream) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(stream); return 0; } @@ -15814,10 +15774,8 @@ virNWFilterFree(virNWFilterPtr nwfilter) virDispatchError(NULL); return -1; } - if (virUnrefNWFilter(nwfilter) < 0) { - virDispatchError(NULL); - return -1; - } + + virObjectUnref(nwfilter); return 0; } @@ -16073,10 +16031,8 @@ virNWFilterRef(virNWFilterPtr nwfilter) virDispatchError(NULL); return -1; } - virMutexLock(&nwfilter->conn->lock); - VIR_DEBUG("nwfilter=%p refs=%d", nwfilter, nwfilter->refs); - nwfilter->refs++; - virMutexUnlock(&nwfilter->conn->lock); + VIR_DEBUG("nwfilter=%p refs=%d", nwfilter, nwfilter->object.refs); + virObjectRef(nwfilter); return 0; } @@ -17960,10 +17916,8 @@ virDomainSnapshotRef(virDomainSnapshotPtr snapshot) virDispatchError(NULL); return -1; } - virMutexLock(&snapshot->domain->conn->lock); - VIR_DEBUG("snapshot=%p, refs=%d", snapshot, snapshot->refs); - snapshot->refs++; - virMutexUnlock(&snapshot->domain->conn->lock); + VIR_DEBUG("snapshot=%p, refs=%d", snapshot, snapshot->object.refs); + virObjectRef(snapshot); return 0; } @@ -17989,10 +17943,7 @@ virDomainSnapshotFree(virDomainSnapshotPtr snapshot) virDispatchError(NULL); return -1; } - if (virUnrefDomainSnapshot(snapshot) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(snapshot); return 0; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9ed6a8d..e6884d8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -194,6 +194,9 @@ virCPUModeTypeToString; # datatypes.h +virConnectClass; +virDomainClass; +virDomainSnapshotClass; virGetConnect; virGetDomain; virGetDomainSnapshot; @@ -205,12 +208,14 @@ virGetSecret; virGetStoragePool; virGetStorageVol; virGetStream; -virUnrefConnect; -virUnrefDomain; -virUnrefNWFilter; -virUnrefSecret; -virUnrefStorageVol; -virUnrefStream; +virInterfaceClass; +virNetworkClass; +virNodeDeviceClass; +virNWFilterClass; +virSecretClass; +virStoragePoolClass; +virStorageVolClass; +virStreamClass; # dnsmasq.h diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index c26ea25..05db54f 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1600,14 +1600,14 @@ parallelsCreateVm(virConnectPtr conn, virDomainDefPtr def) goto error2; virStoragePoolObjUnlock(pool); - virUnrefStorageVol(vol); + virObjectUnref(vol); return 0; error2: virStoragePoolObjUnlock(pool); error: - virUnrefStorageVol(vol); + virObjectUnref(vol); return -1; } diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 0ebdfff..2707a4d 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -2085,7 +2085,7 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, /* checking if this name already exists on this system */ if ((dup_vol = phypVolumeLookupByName(pool, voldef->name)) != NULL) { VIR_ERROR(_("StoragePool name already exists.")); - virUnrefStorageVol(dup_vol); + virObjectUnref(dup_vol); goto err; } @@ -2121,8 +2121,7 @@ err: VIR_FREE(key); virStorageVolDefFree(voldef); virStoragePoolDefFree(spdef); - if (vol) - virUnrefStorageVol(vol); + virObjectUnref(vol); return NULL; } @@ -2321,8 +2320,7 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) VIR_FREE(voldef.key); cleanup: - if (sp) - virUnrefStoragePool(sp); + virObjectUnref(sp); return xml; } @@ -2718,14 +2716,14 @@ phypStoragePoolCreateXML(virConnectPtr conn, /* checking if this name already exists on this system */ if ((dup_sp = phypStoragePoolLookupByName(conn, def->name)) != NULL) { VIR_WARN("StoragePool name already exists."); - virUnrefStoragePool(dup_sp); + virObjectUnref(dup_sp); goto err; } /* checking if ID or UUID already exists on this system */ if ((dup_sp = phypGetStoragePoolLookUpByUUID(conn, def->uuid)) != NULL) { VIR_WARN("StoragePool uuid already exists."); - virUnrefStoragePool(dup_sp); + virObjectUnref(dup_sp); goto err; } @@ -2739,8 +2737,7 @@ phypStoragePoolCreateXML(virConnectPtr conn, err: virStoragePoolDefFree(def); - if (sp) - virUnrefStoragePool(sp); + virObjectUnref(sp); return NULL; } @@ -3685,8 +3682,7 @@ phypDomainCreateAndStart(virConnectPtr conn, err: virDomainDefFree(def); - if (dom) - virUnrefDomain(dom); + virObjectUnref(dom); return NULL; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6ad65a6..b52d13e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1896,8 +1896,7 @@ qemuBuildRBDString(virConnectPtr conn, cleanup: VIR_FREE(secret); - if (sec) - virUnrefSecret(sec); + virObjectUnref(sec); return ret; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7663e5e..003c399 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2285,14 +2285,13 @@ finish: cleanup: if (ddomain) { - virUnrefDomain(ddomain); + virObjectUnref(ddomain); ret = 0; } else { ret = -1; } - if (st) - virUnrefStream(st); + virObjectUnref(st); if (orig_err) { virSetError(orig_err); @@ -2479,14 +2478,13 @@ finish: cleanup: if (ddomain) { - virUnrefDomain(ddomain); + virObjectUnref(ddomain); ret = 0; } else { ret = -1; } - if (st) - virUnrefStream(st); + virObjectUnref(st); if (orig_err) { virSetError(orig_err); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 685ea7c..0d4f5ae 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -447,7 +447,7 @@ qemuProcessGetVolumeQcowPassphrase(virConnectPtr conn, goto cleanup; data = conn->secretDriver->getValue(secret, &size, 0, VIR_SECRET_GET_VALUE_INTERNAL_CALL); - virUnrefSecret(secret); + virObjectUnref(secret); if (data == NULL) goto cleanup; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6d7dd2b..3dc66db 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1398,8 +1398,7 @@ storageVolumeCreateXML(virStoragePoolPtr obj, voldef = NULL; cleanup: - if (volobj) - virUnrefStorageVol(volobj); + virObjectUnref(volobj); virStorageVolDefFree(voldef); if (pool) virStoragePoolObjUnlock(pool); @@ -1560,8 +1559,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, volobj = NULL; cleanup: - if (volobj) - virUnrefStorageVol(volobj); + virObjectUnref(volobj); virStorageVolDefFree(newvol); if (pool) virStoragePoolObjUnlock(pool); diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index b672b24..8185c3a 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1204,7 +1204,7 @@ static virDomainPtr vboxDomainCreateXML(virConnectPtr conn, const char *xml, if (vboxDomainCreate(dom) < 0) { vboxDomainUndefineFlags(dom, 0); - virUnrefDomain(dom); + virObjectUnref(dom); return NULL; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 9c27933..892d0e5 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1236,8 +1236,7 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) error: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse Xend domain information")); - if (ret != NULL) - virUnrefDomain(ret); + virObjectUnref(ret); return NULL; } @@ -2600,7 +2599,7 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, /* Make sure we don't leave a still-born domain around */ if (dom != NULL) { xenDaemonDomainDestroyFlags(dom, 0); - virUnrefDomain(dom); + virObjectUnref(dom); } virDomainDefFree(def); return NULL; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 39fcd9f..1adba78 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -40,23 +40,12 @@ fakeSecretLookupByUsage(virConnectPtr conn, int usageType ATTRIBUTE_UNUSED, const char *usageID) { - virSecretPtr ret = NULL; - int err; + unsigned char uuid[VIR_UUID_BUFLEN]; if (STRNEQ(usageID, "mycluster_myname")) return NULL; - err = VIR_ALLOC(ret); - if (err < 0) - return NULL; - ret->magic = VIR_SECRET_MAGIC; - ret->refs = 1; - ret->usageID = strdup(usageID); - if (!ret->usageID) { - VIR_FREE(ret); - return NULL; - } - ret->conn = conn; - conn->refs++; - return ret; + + virUUIDGenerate(uuid); + return virGetSecret(conn, uuid, usageType, usageID); } static int @@ -239,7 +228,7 @@ out: VIR_FREE(actualargv); virCommandFree(cmd); virDomainDefFree(vmdef); - virUnrefConnect(conn); + virObjectUnref(conn); return ret; } diff --git a/tests/qemuxmlnstest.c b/tests/qemuxmlnstest.c index 0bc821d..253c89e 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -157,7 +157,7 @@ static int testCompareXMLToArgvFiles(const char *xml, VIR_FREE(actualargv); virCommandFree(cmd); virDomainDefFree(vmdef); - virUnrefConnect(conn); + virObjectUnref(conn); return ret; } diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c index 37e3749..9b9b09d 100644 --- a/tests/sexpr2xmltest.c +++ b/tests/sexpr2xmltest.c @@ -71,8 +71,7 @@ testCompareFiles(const char *xml, const char *sexpr, int xendConfigVersion) VIR_FREE(sexprData); VIR_FREE(gotxml); virDomainDefFree(def); - if (conn) - virUnrefConnect(conn); + virObjectUnref(conn); return ret; } diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index 3a0937e..826bed8 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -93,7 +93,7 @@ testCompareParseXML(const char *xmcfg, const char *xml, int xendConfigVersion) if (conf) virConfFree(conf); virDomainDefFree(def); - virUnrefConnect(conn); + virObjectUnref(conn); return ret; } @@ -147,7 +147,7 @@ testCompareFormatXML(const char *xmcfg, const char *xml, int xendConfigVersion) VIR_FREE(xmcfgData); VIR_FREE(gotxml); virDomainDefFree(def); - virUnrefConnect(conn); + virObjectUnref(conn); return ret; } -- 1.7.10.4
On 08/06/2012 05:52 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This converts the following public API datatypes to use the virObject infrastructure:
+virGetConnect(void) +{ virConnectPtr ret;
- if (VIR_ALLOC(ret) < 0) { - virReportOOMError(); - goto failed; - } + if (virDataTypesInitialize() < 0) + return NULL; + + if (!(ret = virObjectNew(virConnectClass))) + return NULL; + if (virMutexInit(&ret->lock) < 0) { VIR_FREE(ret); - goto failed; + return NULL;
Theoretically, we have left non-poisoned memory on the heap, and careless memory management could happen to revive the pointer and then operate on it; but as that would imply a chain of multiple bugs, I'm not too worried about the use of VIR_FREE(ret) here instead. Poisoning before free() is only a debug aid, after all.
+# define VIR_IS_CONNECT(obj) \ + (virObjectIsClass((obj), virConnectClass))
Parens around 'obj' are redundant in this particular context, but it's not a bad habit to have in general when writing macros, so I don't care if you leave them.
+ +# define VIR_IS_DOMAIN(obj) \ + (virObjectIsClass((obj), virDomainClass)) +# define VIR_IS_CONNECTED_DOMAIN(obj) \ + (VIR_IS_DOMAIN(obj) && VIR_IS_CONNECT((obj)->conn))
For example, in _this_ macro, the parens around the second 'obj' are mandatory.
@@ -1429,14 +1429,16 @@ error: * matching virConnectClose, and all other references will be released * after the corresponding operation completes. * - * Returns the number of remaining references on success - * (positive implies that some other call still has a reference open, - * 0 implies that no references remain and the connection is closed), - * or -1 on failure. It is possible for the last virConnectClose to - * return a positive value if some other object still has a temporary - * reference to the connection, but the application should not try to - * further use a connection after the virConnectClose that matches the - * initial open. + * Returns a positive number if at least 1 reference remains on + * success. The returned value should not be assumed to be the total + * reference count. A return of 0 implies no references remain and + * the connection is closed & memory has been freed. A return of -1
s/&/and/ - no need to abbreviate in formal documentation. ACK with the doc fix. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/06/2012 10:28 AM, Eric Blake wrote:
On 08/06/2012 05:52 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This converts the following public API datatypes to use the virObject infrastructure:
ACK with the doc fix.
I spoke too soon, 'make check' fails: GEN check-symfile Expected symbol virConnectClass is not in ELF library Expected symbol virDomainClass is not in ELF library Expected symbol virDomainSnapshotClass is not in ELF library Expected symbol virInterfaceClass is not in ELF library Expected symbol virNetworkClass is not in ELF library Expected symbol virNodeDeviceClass is not in ELF library Expected symbol virNWFilterClass is not in ELF library Expected symbol virSecretClass is not in ELF library Expected symbol virStoragePoolClass is not in ELF library Expected symbol virStorageVolClass is not in ELF library Expected symbol virStreamClass is not in ELF library make[3]: *** [check-symfile] Error 1 -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/06/2012 10:29 AM, Eric Blake wrote:
On 08/06/2012 10:28 AM, Eric Blake wrote:
On 08/06/2012 05:52 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This converts the following public API datatypes to use the virObject infrastructure:
ACK with the doc fix.
I spoke too soon, 'make check' fails:
GEN check-symfile Expected symbol virConnectClass is not in ELF library Expected symbol virDomainClass is not in ELF library Expected symbol virDomainSnapshotClass is not in ELF library Expected symbol virInterfaceClass is not in ELF library Expected symbol virNetworkClass is not in ELF library Expected symbol virNodeDeviceClass is not in ELF library Expected symbol virNWFilterClass is not in ELF library Expected symbol virSecretClass is not in ELF library Expected symbol virStoragePoolClass is not in ELF library Expected symbol virStorageVolClass is not in ELF library Expected symbol virStreamClass is not in ELF library make[3]: *** [check-symfile] Error 1
Squash this in to fix the problem. diff --git i/src/check-symfile.pl w/src/check-symfile.pl index 73cdfcd..ab187ee 100755 --- i/src/check-symfile.pl +++ w/src/check-symfile.pl @@ -34,7 +34,7 @@ foreach my $elflib (@elflibs) { open NM, "-|", "nm", $elflib or die "cannot run 'nm $elflib': $!"; while (<NM>) { - next unless /^\S+\s(?:T|D)\s(\S+)\s*$/; + next unless /^\S+\s(?:[TDB])\s(\S+)\s*$/; $gotsyms{$1} = 1; } -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Aug 06, 2012 at 10:44:11AM -0600, Eric Blake wrote:
On 08/06/2012 10:29 AM, Eric Blake wrote:
On 08/06/2012 10:28 AM, Eric Blake wrote:
On 08/06/2012 05:52 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This converts the following public API datatypes to use the virObject infrastructure:
ACK with the doc fix.
I spoke too soon, 'make check' fails:
GEN check-symfile Expected symbol virConnectClass is not in ELF library Expected symbol virDomainClass is not in ELF library Expected symbol virDomainSnapshotClass is not in ELF library Expected symbol virInterfaceClass is not in ELF library Expected symbol virNetworkClass is not in ELF library Expected symbol virNodeDeviceClass is not in ELF library Expected symbol virNWFilterClass is not in ELF library Expected symbol virSecretClass is not in ELF library Expected symbol virStoragePoolClass is not in ELF library Expected symbol virStorageVolClass is not in ELF library Expected symbol virStreamClass is not in ELF library make[3]: *** [check-symfile] Error 1
Squash this in to fix the problem.
diff --git i/src/check-symfile.pl w/src/check-symfile.pl index 73cdfcd..ab187ee 100755 --- i/src/check-symfile.pl +++ w/src/check-symfile.pl @@ -34,7 +34,7 @@ foreach my $elflib (@elflibs) { open NM, "-|", "nm", $elflib or die "cannot run 'nm $elflib': $!";
while (<NM>) { - next unless /^\S+\s(?:T|D)\s(\S+)\s*$/; + next unless /^\S+\s(?:[TDB])\s(\S+)\s*$/;
$gotsyms{$1} = 1; }
Ahh cool. I see 'nm' shows the difference between B and D "B" "b" The symbol is in the uninitialized data section (known as BSS). "D" "d" The symbol is in the initialized data section. So that addition makes sense. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/06/2012 05:52 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This converts the following public API datatypes to use the virObject infrastructure:
/** - * virReleaseConnect: + * virConnectDispose: * @conn: the hypervisor connection to release * * Unconditionally release all memory associated with a connection. @@ -90,13 +124,9 @@ failed: * be used once this method returns. */ static void -virReleaseConnect(virConnectPtr conn) { - VIR_DEBUG("release connection %p", conn); - - /* make sure to release the connection lock before we call the - * close callbacks, otherwise we will deadlock if an error - * is raised by any of the callbacks */ - virMutexUnlock(&conn->lock); +virConnectDispose(void *obj) +{ + virConnectPtr conn = obj;
if (conn->networkDriver) conn->networkDriver->close(conn); @@ -127,35 +157,6 @@ virReleaseConnect(virConnectPtr conn) { VIR_FREE(conn);
Ouch. I missed this bug, which causes 'make check' to segfault due to a double-free of each virConnectPtr. Squash this in. diff --git i/src/datatypes.c w/src/datatypes.c index e827c2d..d65eec0 100644 --- i/src/datatypes.c +++ w/src/datatypes.c @@ -154,7 +154,6 @@ virConnectDispose(void *obj) virMutexUnlock(&conn->lock); virMutexDestroy(&conn->lock); - VIR_FREE(conn); } -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Aug 06, 2012 at 10:57:41AM -0600, Eric Blake wrote:
On 08/06/2012 05:52 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This converts the following public API datatypes to use the virObject infrastructure:
/** - * virReleaseConnect: + * virConnectDispose: * @conn: the hypervisor connection to release * * Unconditionally release all memory associated with a connection. @@ -90,13 +124,9 @@ failed: * be used once this method returns. */ static void -virReleaseConnect(virConnectPtr conn) { - VIR_DEBUG("release connection %p", conn); - - /* make sure to release the connection lock before we call the - * close callbacks, otherwise we will deadlock if an error - * is raised by any of the callbacks */ - virMutexUnlock(&conn->lock); +virConnectDispose(void *obj) +{ + virConnectPtr conn = obj;
if (conn->networkDriver) conn->networkDriver->close(conn); @@ -127,35 +157,6 @@ virReleaseConnect(virConnectPtr conn) { VIR_FREE(conn);
Ouch. I missed this bug, which causes 'make check' to segfault due to a double-free of each virConnectPtr. Squash this in.
diff --git i/src/datatypes.c w/src/datatypes.c index e827c2d..d65eec0 100644 --- i/src/datatypes.c +++ w/src/datatypes.c @@ -154,7 +154,6 @@ virConnectDispose(void *obj)
virMutexUnlock(&conn->lock); virMutexDestroy(&conn->lock); - VIR_FREE(conn); }
Opps, this was a rebase mistake - I had giant conflicts in this file to sort out Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
From: "Daniel P. Berrange" <berrange@redhat.com> Switch virDomainObjPtr to use the virObject APIs for reference counting. The main change is that virObjectUnref does not return the reference count, merely a bool indicating whether the object still has any refs left. Checking the return value is also not mandatory. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/conf/domain_conf.c | 60 ++++++++++++++++++++------------------------- src/conf/domain_conf.h | 8 +++--- src/libvirt_private.syms | 3 +-- src/libxl/libxl_driver.c | 6 ++--- src/lxc/lxc_process.c | 6 ++--- src/openvz/openvz_conf.c | 19 +++----------- src/qemu/qemu_domain.c | 27 +++++++++----------- src/qemu/qemu_domain.h | 8 +++--- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 22 ++++++++--------- src/qemu/qemu_migration.h | 4 +-- src/qemu/qemu_process.c | 50 +++++++++++++++++-------------------- src/vmware/vmware_conf.c | 4 +-- 13 files changed, 93 insertions(+), 126 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58603a3..d8c0969 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -660,6 +660,21 @@ VIR_ENUM_IMPL(virDomainNumatuneMemPlacementMode, #define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE #define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE +static virClassPtr virDomainObjClass; +static void virDomainObjDispose(void *obj); + +static int virDomainObjOnceInit(void) +{ + if (!(virDomainObjClass = virClassNew("virDomainObj", + sizeof(virDomainObj), + virDomainObjDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virDomainObj) + void virBlkioDeviceWeightArrayClear(virBlkioDeviceWeightPtr deviceWeights, int ndevices) @@ -725,7 +740,7 @@ virDomainObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED) { virDomainObjPtr obj = payload; virDomainObjLock(obj); - if (virDomainObjUnref(obj) > 0) + if (virObjectUnref(obj)) virDomainObjUnlock(obj); } @@ -1639,10 +1654,10 @@ void virDomainDefFree(virDomainDefPtr def) } static void virDomainSnapshotObjListDeinit(virDomainSnapshotObjListPtr snapshots); -static void virDomainObjFree(virDomainObjPtr dom) + +static void virDomainObjDispose(void *obj) { - if (!dom) - return; + virDomainObjPtr dom = obj; VIR_DEBUG("obj=%p", dom); virDomainDefFree(dom->def); @@ -1654,37 +1669,18 @@ static void virDomainObjFree(virDomainObjPtr dom) virMutexDestroy(&dom->lock); virDomainSnapshotObjListDeinit(&dom->snapshots); - - VIR_FREE(dom); -} - -void virDomainObjRef(virDomainObjPtr dom) -{ - dom->refs++; - VIR_DEBUG("obj=%p refs=%d", dom, dom->refs); } -int virDomainObjUnref(virDomainObjPtr dom) -{ - dom->refs--; - VIR_DEBUG("obj=%p refs=%d", dom, dom->refs); - if (dom->refs == 0) { - virDomainObjUnlock(dom); - virDomainObjFree(dom); - return 0; - } - return dom->refs; -} - -static virDomainObjPtr virDomainObjNew(virCapsPtr caps) +virDomainObjPtr virDomainObjNew(virCapsPtr caps) { virDomainObjPtr domain; - if (VIR_ALLOC(domain) < 0) { - virReportOOMError(); + if (virDomainObjInitialize() < 0) + return NULL; + + if (!(domain = virObjectNew(virDomainObjClass))) return NULL; - } if (caps->privateDataAllocFunc && !(domain->privateData = (caps->privateDataAllocFunc)())) { @@ -1706,7 +1702,6 @@ static virDomainObjPtr virDomainObjNew(virCapsPtr caps) virDomainObjLock(domain); virDomainObjSetState(domain, VIR_DOMAIN_SHUTOFF, VIR_DOMAIN_SHUTOFF_UNKNOWN); - domain->refs = 1; virDomainSnapshotObjListInit(&domain->snapshots); @@ -9417,8 +9412,7 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps, return obj; error: - /* obj was never shared, so unref should return 0 */ - ignore_value(virDomainObjUnref(obj)); + virObjectUnref(obj); VIR_FREE(nodes); return NULL; } @@ -13527,9 +13521,7 @@ static virDomainObjPtr virDomainLoadStatus(virCapsPtr caps, return obj; error: - /* obj was never shared, so unref should return 0 */ - if (obj) - ignore_value(virDomainObjUnref(obj)); + virObjectUnref(obj); VIR_FREE(statusFile); return NULL; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f4c43c6..3f25ad2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -43,6 +43,7 @@ # include "virnetdevvportprofile.h" # include "virnetdevopenvswitch.h" # include "virnetdevbandwidth.h" +# include "virobject.h" /* forward declarations of all device types, required by * virDomainDeviceDef @@ -1809,8 +1810,9 @@ struct _virDomainStateReason { typedef struct _virDomainObj virDomainObj; typedef virDomainObj *virDomainObjPtr; struct _virDomainObj { + virObject object; + virMutex lock; - int refs; pid_t pid; virDomainStateReason state; @@ -1847,6 +1849,7 @@ virDomainObjIsActive(virDomainObjPtr dom) return dom->def->id != -1; } +virDomainObjPtr virDomainObjNew(virCapsPtr caps); int virDomainObjListInit(virDomainObjListPtr objs); void virDomainObjListDeinit(virDomainObjListPtr objs); @@ -1909,9 +1912,6 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, void *opaque); void virDomainDefFree(virDomainDefPtr vm); -void virDomainObjRef(virDomainObjPtr vm); -/* Returns 1 if the object was freed, 0 if more refs exist */ -int virDomainObjUnref(virDomainObjPtr vm) ATTRIBUTE_RETURN_CHECK; virDomainChrDefPtr virDomainChrDefNew(void); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e6884d8..3fb10d2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -428,12 +428,11 @@ virDomainObjListGetInactiveNames; virDomainObjListInit; virDomainObjListNumOfDomains; virDomainObjLock; -virDomainObjRef; +virDomainObjNew; virDomainObjSetDefTransient; virDomainObjSetState; virDomainObjTaint; virDomainObjUnlock; -virDomainObjUnref; virDomainPausedReasonTypeFromString; virDomainPausedReasonTypeToString; virDomainPciRombarModeTypeFromString; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 873f973..150900f 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -134,7 +134,7 @@ libxlDomainObjUnref(void *data) { virDomainObjPtr vm = data; - ignore_value(virDomainObjUnref(vm)); + virObjectUnref(vm); } static void @@ -484,13 +484,13 @@ libxlCreateDomEvents(virDomainObjPtr vm) /* Add a reference to the domain object while it is injected in * the event loop. */ - virDomainObjRef(vm); + virObjectRef(vm); if ((priv->eventHdl = virEventAddHandle( fd, VIR_EVENT_HANDLE_READABLE | VIR_EVENT_HANDLE_ERROR, libxlEventHandler, vm, libxlDomainObjUnref)) < 0) { - ignore_value(virDomainObjUnref(vm)); + virObjectUnref(vm); goto error; } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 65b463f..046ed86 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -565,7 +565,7 @@ static void virLXCProcessMonitorDestroy(virLXCMonitorPtr mon, priv = vm->privateData; if (priv->monitor == mon) priv->monitor = NULL; - if (virDomainObjUnref(vm) > 0) + if (virObjectUnref(vm)) virDomainObjUnlock(vm); } @@ -666,12 +666,12 @@ static virLXCMonitorPtr virLXCProcessConnectMonitor(virLXCDriverPtr driver, /* Hold an extra reference because we can't allow 'vm' to be * deleted while the monitor is active */ - virDomainObjRef(vm); + virObjectRef(vm); monitor = virLXCMonitorNew(vm, driver->stateDir, &monitorCallbacks); if (monitor == NULL) - ignore_value(virDomainObjUnref(vm)); + virObjectUnref(vm); if (virSecurityManagerClearSocketLabel(driver->securityManager, vm->def) < 0) { if (monitor) { diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 5dc071c..e62bf8c 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -598,17 +598,8 @@ int openvzLoadDomains(struct openvz_driver *driver) { } *line++ = '\0'; - if (VIR_ALLOC(dom) < 0) - goto no_memory; - - if (virMutexInit(&dom->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot initialize mutex")); - VIR_FREE(dom); - goto cleanup; - } - - virDomainObjLock(dom); + if (!(dom = virDomainObjNew(driver->caps))) + goto cleanup; if (VIR_ALLOC(dom->def) < 0) goto no_memory; @@ -623,7 +614,6 @@ int openvzLoadDomains(struct openvz_driver *driver) { VIR_DOMAIN_RUNNING_UNKNOWN); } - dom->refs = 1; dom->pid = veid; if (virDomainObjGetState(dom, NULL) == VIR_DOMAIN_SHUTOFF) dom->def->id = -1; @@ -683,7 +673,6 @@ int openvzLoadDomains(struct openvz_driver *driver) { goto cleanup; } - virDomainObjUnlock(dom); dom = NULL; } @@ -700,9 +689,7 @@ int openvzLoadDomains(struct openvz_driver *driver) { virCommandFree(cmd); VIR_FREE(temp); VIR_FREE(outbuf); - /* dom hasn't been shared yet, so unref should return 0 */ - if (dom) - ignore_value(virDomainObjUnref(dom)); + virObjectUnref(dom); return -1; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 86f0265..4acbb20 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -774,7 +774,7 @@ qemuDomainObjBeginJobInternal(struct qemud_driver *driver, return -1; then = now + QEMU_JOB_WAIT_TIME; - virDomainObjRef(obj); + virObjectRef(obj); if (driver_locked) qemuDriverUnlock(driver); @@ -854,8 +854,7 @@ error: qemuDriverLock(driver); virDomainObjLock(obj); } - /* Safe to ignore value since ref count was incremented above */ - ignore_value(virDomainObjUnref(obj)); + virObjectUnref(obj); return -1; } @@ -922,10 +921,10 @@ int qemuDomainObjBeginAsyncJobWithDriver(struct qemud_driver *driver, * To be called after completing the work associated with the * earlier qemuDomainBeginJob() call * - * Returns remaining refcount on 'obj', maybe 0 to indicated it - * was deleted + * Returns true if @obj was still referenced, false if it was + * disposed of. */ -int qemuDomainObjEndJob(struct qemud_driver *driver, virDomainObjPtr obj) +bool qemuDomainObjEndJob(struct qemud_driver *driver, virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; enum qemuDomainJob job = priv->job.active; @@ -941,10 +940,10 @@ int qemuDomainObjEndJob(struct qemud_driver *driver, virDomainObjPtr obj) qemuDomainObjSaveJob(driver, obj); virCondSignal(&priv->job.cond); - return virDomainObjUnref(obj); + return virObjectUnref(obj); } -int +bool qemuDomainObjEndAsyncJob(struct qemud_driver *driver, virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; @@ -958,7 +957,7 @@ qemuDomainObjEndAsyncJob(struct qemud_driver *driver, virDomainObjPtr obj) qemuDomainObjSaveJob(driver, obj); virCondBroadcast(&priv->job.asyncCond); - return virDomainObjUnref(obj); + return virObjectUnref(obj); } static int @@ -1031,9 +1030,7 @@ qemuDomainObjExitMonitorInternal(struct qemud_driver *driver, qemuDomainObjSaveJob(driver, obj); virCondSignal(&priv->job.cond); - /* safe to ignore since the surrounding async job increased - * the reference counter as well */ - ignore_value(virDomainObjUnref(obj)); + virObjectUnref(obj); } } @@ -1207,7 +1204,7 @@ void qemuDomainObjExitAgentWithDriver(struct qemud_driver *driver, void qemuDomainObjEnterRemoteWithDriver(struct qemud_driver *driver, virDomainObjPtr obj) { - virDomainObjRef(obj); + virObjectRef(obj); virDomainObjUnlock(obj); qemuDriverUnlock(driver); } @@ -1217,9 +1214,7 @@ void qemuDomainObjExitRemoteWithDriver(struct qemud_driver *driver, { qemuDriverLock(driver); virDomainObjLock(obj); - /* Safe to ignore value, since we incremented ref in - * qemuDomainObjEnterRemoteWithDriver */ - ignore_value(virDomainObjUnref(obj)); + virObjectUnref(obj); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9f9467d..d5ea33d 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -193,11 +193,11 @@ int qemuDomainObjBeginAsyncJobWithDriver(struct qemud_driver *driver, enum qemuDomainAsyncJob asyncJob) ATTRIBUTE_RETURN_CHECK; -int qemuDomainObjEndJob(struct qemud_driver *driver, - virDomainObjPtr obj) +bool qemuDomainObjEndJob(struct qemud_driver *driver, + virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; -int qemuDomainObjEndAsyncJob(struct qemud_driver *driver, - virDomainObjPtr obj) +bool qemuDomainObjEndAsyncJob(struct qemud_driver *driver, + virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK; void qemuDomainObjSetJobPhase(struct qemud_driver *driver, virDomainObjPtr obj, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 369e8ed..dee1268 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3402,7 +3402,7 @@ endjob: ignore_value(qemuDomainObjEndAsyncJob(driver, wdEvent->vm)); unlock: - if (virDomainObjUnref(wdEvent->vm) > 0) + if (virObjectUnref(wdEvent->vm)) virDomainObjUnlock(wdEvent->vm); qemuDriverUnlock(driver); VIR_FREE(wdEvent); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 003c399..912ba58 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1369,7 +1369,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver, * This prevents any other APIs being invoked while incoming * migration is taking place. */ - if (qemuMigrationJobContinue(vm) == 0) { + if (!qemuMigrationJobContinue(vm)) { vm = NULL; virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("domain disappeared")); @@ -1396,7 +1396,7 @@ cleanup: return ret; endjob: - if (qemuMigrationJobFinish(driver, vm) == 0) { + if (!qemuMigrationJobFinish(driver, vm)) { vm = NULL; } goto cleanup; @@ -2680,7 +2680,7 @@ endjob: VIR_DOMAIN_EVENT_RESUMED_MIGRATED); } - if (qemuMigrationJobFinish(driver, vm) == 0) { + if (!qemuMigrationJobFinish(driver, vm)) { vm = NULL; } else if (!virDomainObjIsActive(vm) && (!vm->persistent || @@ -2722,7 +2722,7 @@ qemuMigrationPerformPhase(struct qemud_driver *driver, virDomainEventPtr event = NULL; int ret = -1; bool resume; - int refs; + bool hasrefs; /* If we didn't start the job in the begin phase, start it now. */ if (!(flags & VIR_MIGRATE_CHANGE_PROTECTION)) { @@ -2770,10 +2770,10 @@ qemuMigrationPerformPhase(struct qemud_driver *driver, endjob: if (ret < 0) - refs = qemuMigrationJobFinish(driver, vm); + hasrefs = qemuMigrationJobFinish(driver, vm); else - refs = qemuMigrationJobContinue(vm); - if (refs == 0) { + hasrefs = qemuMigrationJobContinue(vm); + if (!hasrefs) { vm = NULL; } else if (!virDomainObjIsActive(vm) && !vm->persistent) { qemuDomainRemoveInactive(driver, vm); @@ -3374,15 +3374,15 @@ qemuMigrationJobStartPhase(struct qemud_driver *driver, virDomainObjPtr vm, enum qemuMigrationJobPhase phase) { - virDomainObjRef(vm); + virObjectRef(vm); qemuMigrationJobSetPhase(driver, vm, phase); } -int +bool qemuMigrationJobContinue(virDomainObjPtr vm) { qemuDomainObjReleaseAsyncJob(vm); - return virDomainObjUnref(vm); + return virObjectUnref(vm); } bool @@ -3405,7 +3405,7 @@ qemuMigrationJobIsActive(virDomainObjPtr vm, return true; } -int +bool qemuMigrationJobFinish(struct qemud_driver *driver, virDomainObjPtr vm) { return qemuDomainObjEndAsyncJob(driver, vm); diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index e6ca215..1740204 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -66,12 +66,12 @@ void qemuMigrationJobStartPhase(struct qemud_driver *driver, virDomainObjPtr vm, enum qemuMigrationJobPhase phase) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int qemuMigrationJobContinue(virDomainObjPtr obj) +bool qemuMigrationJobContinue(virDomainObjPtr obj) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; bool qemuMigrationJobIsActive(virDomainObjPtr vm, enum qemuDomainAsyncJob job) ATTRIBUTE_NONNULL(1); -int qemuMigrationJobFinish(struct qemud_driver *driver, virDomainObjPtr obj) +bool qemuMigrationJobFinish(struct qemud_driver *driver, virDomainObjPtr obj) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int qemuMigrationSetOffline(struct qemud_driver *driver, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0d4f5ae..3a08c5b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -173,7 +173,7 @@ static void qemuProcessHandleAgentDestroy(qemuAgentPtr agent, priv = vm->privateData; if (priv->agent == agent) priv->agent = NULL; - if (virDomainObjUnref(vm) > 0) + if (virObjectUnref(vm)) virDomainObjUnlock(vm); } @@ -225,7 +225,7 @@ qemuConnectAgent(struct qemud_driver *driver, virDomainObjPtr vm) /* Hold an extra reference because we can't allow 'vm' to be * deleted while the agent is active */ - virDomainObjRef(vm); + virObjectRef(vm); ignore_value(virTimeMillisNow(&priv->agentStart)); virDomainObjUnlock(vm); @@ -246,9 +246,8 @@ qemuConnectAgent(struct qemud_driver *driver, virDomainObjPtr vm) goto cleanup; } - /* Safe to ignore value since ref count was incremented above */ if (agent == NULL) - ignore_value(virDomainObjUnref(vm)); + virObjectUnref(vm); if (!virDomainObjIsActive(vm)) { qemuAgentClose(agent); @@ -584,7 +583,7 @@ qemuProcessFakeReboot(void *opaque) ret = 0; endjob: - if (qemuDomainObjEndJob(driver, vm) == 0) + if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; cleanup: @@ -593,7 +592,7 @@ cleanup: ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE)); } - if (virDomainObjUnref(vm) > 0) + if (virObjectUnref(vm)) virDomainObjUnlock(vm); } if (event) @@ -610,7 +609,7 @@ qemuProcessShutdownOrReboot(struct qemud_driver *driver, if (priv->fakeReboot) { qemuDomainSetFakeReboot(driver, vm, false); - virDomainObjRef(vm); + virObjectRef(vm); virThread th; if (virThreadCreate(&th, false, @@ -619,8 +618,7 @@ qemuProcessShutdownOrReboot(struct qemud_driver *driver, VIR_ERROR(_("Failed to create reboot thread, killing domain")); ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_NOWAIT)); - /* Safe to ignore value since ref count was incremented above */ - ignore_value(virDomainObjUnref(vm)); + virObjectUnref(vm); } } else { ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_NOWAIT)); @@ -801,9 +799,9 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon ATTRIBUTE_UNUSED, /* Hold an extra reference because we can't allow 'vm' to be * deleted before handling watchdog event is finished. */ - virDomainObjRef(vm); + virObjectRef(vm); if (virThreadPoolSendJob(driver->workerPool, 0, wdEvent) < 0) { - if (virDomainObjUnref(vm) == 0) + if (!virObjectUnref(vm)) vm = NULL; VIR_FREE(wdEvent); } @@ -1022,7 +1020,7 @@ static void qemuProcessHandleMonitorDestroy(qemuMonitorPtr mon, priv = vm->privateData; if (priv->mon == mon) priv->mon = NULL; - if (virDomainObjUnref(vm) > 0) + if (virObjectUnref(vm)) virDomainObjUnlock(vm); } @@ -1213,7 +1211,7 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) /* Hold an extra reference because we can't allow 'vm' to be * deleted while the monitor is active */ - virDomainObjRef(vm); + virObjectRef(vm); ignore_value(virTimeMillisNow(&priv->monStart)); virDomainObjUnlock(vm); @@ -1228,9 +1226,8 @@ qemuConnectMonitor(struct qemud_driver *driver, virDomainObjPtr vm) virDomainObjLock(vm); priv->monStart = 0; - /* Safe to ignore value since ref count was incremented above */ if (mon == NULL) - ignore_value(virDomainObjUnref(vm)); + virObjectUnref(vm); if (!virDomainObjIsActive(vm)) { qemuMonitorClose(mon); @@ -3068,7 +3065,7 @@ qemuProcessReconnect(void *opaque) /* Hold an extra reference because we can't allow 'vm' to be * deleted if qemuConnectMonitor() failed */ - virDomainObjRef(obj); + virObjectRef(obj); /* XXX check PID liveliness & EXE path */ if (qemuConnectMonitor(driver, obj) < 0) @@ -3165,10 +3162,10 @@ qemuProcessReconnect(void *opaque) driver->nextvmid = obj->def->id + 1; endjob: - if (qemuDomainObjEndJob(driver, obj) == 0) + if (!qemuDomainObjEndJob(driver, obj)) obj = NULL; - if (obj && virDomainObjUnref(obj) > 0) + if (obj && virObjectUnref(obj)) virDomainObjUnlock(obj); qemuDriverUnlock(driver); @@ -3178,18 +3175,18 @@ endjob: return; error: - if (qemuDomainObjEndJob(driver, obj) == 0) + if (!qemuDomainObjEndJob(driver, obj)) obj = NULL; if (obj) { if (!virDomainObjIsActive(obj)) { - if (virDomainObjUnref(obj) > 0) + if (virObjectUnref(obj)) virDomainObjUnlock(obj); qemuDriverUnlock(driver); return; } - if (virDomainObjUnref(obj) > 0) { + if (virObjectUnref(obj)) { /* We can't get the monitor back, so must kill the VM * to remove danger of it ending up running twice if * user tries to start it again later @@ -3277,9 +3274,9 @@ qemuProcessReconnectHelper(void *payload, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not create thread. QEMU initialization " "might be incomplete")); - if (qemuDomainObjEndJob(src->driver, obj) == 0) { + if (!qemuDomainObjEndJob(src->driver, obj)) { obj = NULL; - } else if (virDomainObjUnref(obj) > 0) { + } else if (virObjectUnref(obj)) { /* We can't spawn a thread and thus connect to monitor. * Kill qemu */ qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, 0); @@ -3950,12 +3947,11 @@ cleanup: * a case, but there are too many to maintain certainty, so we * will do this as a precaution). */ - virDomainObjRef(vm); + virObjectRef(vm); virDomainObjUnlock(vm); qemuDriverLock(driver); virDomainObjLock(vm); - /* Safe to ignore value since ref count was incremented above */ - ignore_value(virDomainObjUnref(vm)); + virObjectUnref(vm); } return ret; } @@ -4407,7 +4403,7 @@ qemuProcessAutoDestroy(struct qemud_driver *driver, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - if (qemuDomainObjEndJob(driver, dom) == 0) + if (!qemuDomainObjEndJob(driver, dom)) dom = NULL; if (dom && !dom->persistent) qemuDomainRemoveInactive(driver, dom); diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 9fb95c4..b52c002 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -213,9 +213,7 @@ cleanup: VIR_FREE(directoryName); VIR_FREE(fileName); VIR_FREE(vmx); - /* any non-NULL vm here has not been shared, so unref will return 0 */ - if (vm) - ignore_value(virDomainObjUnref(vm)); + virObjectUnref(vm); return ret; } -- 1.7.10.4
On 08/06/2012 05:52 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Switch virDomainObjPtr to use the virObject APIs for reference counting. The main change is that virObjectUnref does not return the reference count, merely a bool indicating whether the object still has any refs left. Checking the return value is also not mandatory.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
13 files changed, 93 insertions(+), 126 deletions(-)
This whole series has a lot of nice diffstats.
@@ -1909,9 +1912,6 @@ int virDomainDeviceInfoIterate(virDomainDefPtr def, void *opaque);
void virDomainDefFree(virDomainDefPtr vm); -void virDomainObjRef(virDomainObjPtr vm); -/* Returns 1 if the object was freed, 0 if more refs exist */ -int virDomainObjUnref(virDomainObjPtr vm) ATTRIBUTE_RETURN_CHECK;
Oh my - that deleted comment was a bold-faced lie - we used to return 0 on no more refs, > 0 if still in use. I did a double-take on whether you had inverted logic (since virObjectUnref returns non-zero when still in use), and thankfully the code was correct and just the comment wrong.
@@ -3165,10 +3162,10 @@ qemuProcessReconnect(void *opaque) driver->nextvmid = obj->def->id + 1;
endjob: - if (qemuDomainObjEndJob(driver, obj) == 0) + if (!qemuDomainObjEndJob(driver, obj)) obj = NULL;
- if (obj && virDomainObjUnref(obj) > 0) + if (obj && virObjectUnref(obj))
Straight translation; but a future patch could simplify this to: if (virObjectUnref(obj)) since virObjectUnref is safe to call on NULL. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Make qemuAgentPtr and qemuMonitorPtr types use the virObject APIs for reference counting Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu_agent.c | 86 ++++++++++++++++++----------------------- src/qemu/qemu_agent.h | 3 -- src/qemu/qemu_domain.c | 22 +++++------ src/qemu/qemu_monitor.c | 97 ++++++++++++++++++++--------------------------- src/qemu/qemu_monitor.h | 3 -- 5 files changed, 89 insertions(+), 122 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 14bf11b..15af758 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -40,6 +40,7 @@ #include "json.h" #include "virfile.h" #include "virtime.h" +#include "virobject.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -80,11 +81,11 @@ struct _qemuAgentMessage { struct _qemuAgent { + virObject object; + virMutex lock; /* also used to protect fd */ virCond notify; - int refs; - int fd; int watch; @@ -114,6 +115,22 @@ struct _qemuAgent { qemuAgentEvent await_event; }; +static virClassPtr qemuAgentClass; +static void qemuAgentDispose(void *obj); + +static int qemuAgentOnceInit(void) +{ + if (!(qemuAgentClass = virClassNew("qemuAgent", + sizeof(qemuAgent), + qemuAgentDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(qemuAgent) + + #if DEBUG_RAW_IO # include <c-ctype.h> static char * @@ -146,45 +163,15 @@ void qemuAgentUnlock(qemuAgentPtr mon) } -static void qemuAgentFree(qemuAgentPtr mon) +static void qemuAgentDispose(void *obj) { + qemuAgentPtr mon = obj; VIR_DEBUG("mon=%p", mon); if (mon->cb && mon->cb->destroy) (mon->cb->destroy)(mon, mon->vm); ignore_value(virCondDestroy(&mon->notify)); virMutexDestroy(&mon->lock); VIR_FREE(mon->buffer); - VIR_FREE(mon); -} - -int qemuAgentRef(qemuAgentPtr mon) -{ - mon->refs++; - VIR_DEBUG("%d", mon->refs); - return mon->refs; -} - -int qemuAgentUnref(qemuAgentPtr mon) -{ - mon->refs--; - VIR_DEBUG("%d", mon->refs); - if (mon->refs == 0) { - qemuAgentUnlock(mon); - qemuAgentFree(mon); - return 0; - } - - return mon->refs; -} - -static void -qemuAgentUnwatch(void *monitor) -{ - qemuAgentPtr mon = monitor; - - qemuAgentLock(mon); - if (qemuAgentUnref(mon) > 0) - qemuAgentUnlock(mon); } static int @@ -599,9 +586,9 @@ qemuAgentIO(int watch, int fd, int events, void *opaque) { bool error = false; bool eof = false; + virObjectRef(mon); /* lock access to the monitor and protect fd */ qemuAgentLock(mon); - qemuAgentRef(mon); #if DEBUG_IO VIR_DEBUG("Agent %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif @@ -704,8 +691,8 @@ qemuAgentIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - if (qemuAgentUnref(mon) > 0) - qemuAgentUnlock(mon); + qemuAgentUnlock(mon); + virObjectUnref(mon); VIR_DEBUG("Triggering EOF callback"); (eofNotify)(mon, vm); } else if (error) { @@ -715,13 +702,13 @@ qemuAgentIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - if (qemuAgentUnref(mon) > 0) - qemuAgentUnlock(mon); + qemuAgentUnlock(mon); + virObjectUnref(mon); VIR_DEBUG("Triggering error callback"); (errorNotify)(mon, vm); } else { - if (qemuAgentUnref(mon) > 0) - qemuAgentUnlock(mon); + qemuAgentUnlock(mon); + virObjectUnref(mon); } } @@ -739,10 +726,11 @@ qemuAgentOpen(virDomainObjPtr vm, return NULL; } - if (VIR_ALLOC(mon) < 0) { - virReportOOMError(); + if (qemuAgentInitialize() < 0) + return NULL; + + if (!(mon = virObjectNew(qemuAgentClass))) return NULL; - } if (virMutexInit(&mon->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -758,7 +746,6 @@ qemuAgentOpen(virDomainObjPtr vm, return NULL; } mon->fd = -1; - mon->refs = 1; mon->vm = vm; mon->cb = cb; qemuAgentLock(mon); @@ -791,12 +778,13 @@ qemuAgentOpen(virDomainObjPtr vm, VIR_EVENT_HANDLE_WRITABLE : 0), qemuAgentIO, - mon, qemuAgentUnwatch)) < 0) { + mon, + virObjectFreeCallback)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to register monitor events")); goto cleanup; } - qemuAgentRef(mon); + virObjectRef(mon); VIR_DEBUG("New mon %p fd =%d watch=%d", mon, mon->fd, mon->watch); qemuAgentUnlock(mon); @@ -837,9 +825,9 @@ void qemuAgentClose(qemuAgentPtr mon) mon->msg->finished = 1; virCondSignal(&mon->notify); } + qemuAgentUnlock(mon); - if (qemuAgentUnref(mon) > 0) - qemuAgentUnlock(mon); + virObjectUnref(mon); } #define QEMU_AGENT_WAIT_TIME (1000ull * 5) diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 860e7e5..2fdebb2 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -50,9 +50,6 @@ qemuAgentPtr qemuAgentOpen(virDomainObjPtr vm, void qemuAgentLock(qemuAgentPtr mon); void qemuAgentUnlock(qemuAgentPtr mon); -int qemuAgentRef(qemuAgentPtr mon); -int qemuAgentUnref(qemuAgentPtr mon) ATTRIBUTE_RETURN_CHECK; - void qemuAgentClose(qemuAgentPtr mon); typedef enum { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 4acbb20..f7cbe7f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -994,7 +994,7 @@ qemuDomainObjEnterMonitorInternal(struct qemud_driver *driver, } qemuMonitorLock(priv->mon); - qemuMonitorRef(priv->mon); + virObjectRef(priv->mon); ignore_value(virTimeMillisNow(&priv->monStart)); virDomainObjUnlock(obj); if (driver_locked) @@ -1009,11 +1009,11 @@ qemuDomainObjExitMonitorInternal(struct qemud_driver *driver, virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - int refs; + bool hasRefs; - refs = qemuMonitorUnref(priv->mon); + hasRefs = virObjectUnref(priv->mon); - if (refs > 0) + if (hasRefs) qemuMonitorUnlock(priv->mon); if (driver_locked) @@ -1021,9 +1021,8 @@ qemuDomainObjExitMonitorInternal(struct qemud_driver *driver, virDomainObjLock(obj); priv->monStart = 0; - if (refs == 0) { + if (!hasRefs) priv->mon = NULL; - } if (priv->job.active == QEMU_JOB_ASYNC_NESTED) { qemuDomainObjResetJob(priv); @@ -1118,7 +1117,7 @@ qemuDomainObjEnterAgentInternal(struct qemud_driver *driver, qemuDomainObjPrivatePtr priv = obj->privateData; qemuAgentLock(priv->agent); - qemuAgentRef(priv->agent); + virObjectRef(priv->agent); ignore_value(virTimeMillisNow(&priv->agentStart)); virDomainObjUnlock(obj); if (driver_locked) @@ -1133,11 +1132,11 @@ qemuDomainObjExitAgentInternal(struct qemud_driver *driver, virDomainObjPtr obj) { qemuDomainObjPrivatePtr priv = obj->privateData; - int refs; + bool hasRefs; - refs = qemuAgentUnref(priv->agent); + hasRefs = virObjectUnref(priv->agent); - if (refs > 0) + if (hasRefs) qemuAgentUnlock(priv->agent); if (driver_locked) @@ -1145,9 +1144,8 @@ qemuDomainObjExitAgentInternal(struct qemud_driver *driver, virDomainObjLock(obj); priv->agentStart = 0; - if (refs == 0) { + if (!hasRefs) priv->agent = NULL; - } } /* diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 20395b0..b0f3bb6 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -36,6 +36,7 @@ #include "memory.h" #include "logging.h" #include "virfile.h" +#include "virobject.h" #ifdef WITH_DTRACE_PROBES # include "libvirt_qemu_probes.h" @@ -47,11 +48,11 @@ #define DEBUG_RAW_IO 0 struct _qemuMonitor { + virObject object; + virMutex lock; /* also used to protect fd */ virCond notify; - int refs; - int fd; int watch; int hasSendFD; @@ -80,6 +81,21 @@ struct _qemuMonitor { unsigned json_hmp: 1; }; +static virClassPtr qemuMonitorClass; +static void qemuMonitorDispose(void *obj); + +static int qemuMonitorOnceInit(void) +{ + if (!(qemuMonitorClass = virClassNew("qemuMonitor", + sizeof(qemuMonitor), + qemuMonitorDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(qemuMonitor) + VIR_ENUM_IMPL(qemuMonitorMigrationStatus, QEMU_MONITOR_MIGRATION_STATUS_LAST, @@ -224,8 +240,10 @@ void qemuMonitorUnlock(qemuMonitorPtr mon) } -static void qemuMonitorFree(qemuMonitorPtr mon) +static void qemuMonitorDispose(void *obj) { + qemuMonitorPtr mon = obj; + VIR_DEBUG("mon=%p", mon); if (mon->cb && mon->cb->destroy) (mon->cb->destroy)(mon, mon->vm); @@ -233,41 +251,8 @@ static void qemuMonitorFree(qemuMonitorPtr mon) {} virMutexDestroy(&mon->lock); VIR_FREE(mon->buffer); - VIR_FREE(mon); -} - -int qemuMonitorRef(qemuMonitorPtr mon) -{ - mon->refs++; - PROBE(QEMU_MONITOR_REF, - "mon=%p refs=%d", mon, mon->refs); - return mon->refs; -} - -int qemuMonitorUnref(qemuMonitorPtr mon) -{ - mon->refs--; - - PROBE(QEMU_MONITOR_UNREF, - "mon=%p refs=%d", mon, mon->refs); - if (mon->refs == 0) { - qemuMonitorUnlock(mon); - qemuMonitorFree(mon); - return 0; - } - - return mon->refs; } -static void -qemuMonitorUnwatch(void *monitor) -{ - qemuMonitorPtr mon = monitor; - - qemuMonitorLock(mon); - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); -} static int qemuMonitorOpenUnix(const char *monitor, pid_t cpid) @@ -567,9 +552,10 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { bool error = false; bool eof = false; + virObjectRef(mon); + /* lock access to the monitor and protect fd */ qemuMonitorLock(mon); - qemuMonitorRef(mon); #if DEBUG_IO VIR_DEBUG("Monitor %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif @@ -667,8 +653,8 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); + virObjectUnref(mon); VIR_DEBUG("Triggering EOF callback"); (eofNotify)(mon, vm); } else if (error) { @@ -678,13 +664,13 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) { /* Make sure anyone waiting wakes up now */ virCondSignal(&mon->notify); - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); + virObjectUnref(mon); VIR_DEBUG("Triggering error callback"); (errorNotify)(mon, vm); } else { - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); + virObjectUnref(mon); } } @@ -703,10 +689,11 @@ qemuMonitorOpen(virDomainObjPtr vm, return NULL; } - if (VIR_ALLOC(mon) < 0) { - virReportOOMError(); + if (qemuMonitorInitialize() < 0) + return NULL; + + if (!(mon = virObjectNew(qemuMonitorClass))) return NULL; - } if (virMutexInit(&mon->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -722,7 +709,6 @@ qemuMonitorOpen(virDomainObjPtr vm, return NULL; } mon->fd = -1; - mon->refs = 1; mon->vm = vm; mon->json = json; mon->cb = cb; @@ -764,16 +750,17 @@ qemuMonitorOpen(virDomainObjPtr vm, VIR_EVENT_HANDLE_ERROR | VIR_EVENT_HANDLE_READABLE, qemuMonitorIO, - mon, qemuMonitorUnwatch)) < 0) { + mon, + virObjectFreeCallback)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to register monitor events")); goto cleanup; } - qemuMonitorRef(mon); + virObjectRef(mon); PROBE(QEMU_MONITOR_NEW, "mon=%p refs=%d fd=%d", - mon, mon->refs, mon->fd); + mon, mon->object.refs, mon->fd); qemuMonitorUnlock(mon); return mon; @@ -798,7 +785,7 @@ void qemuMonitorClose(qemuMonitorPtr mon) qemuMonitorLock(mon); PROBE(QEMU_MONITOR_CLOSE, - "mon=%p refs=%d", mon, mon->refs); + "mon=%p refs=%d", mon, mon->object.refs); if (mon->fd >= 0) { if (mon->watch) @@ -827,8 +814,8 @@ void qemuMonitorClose(qemuMonitorPtr mon) virCondSignal(&mon->notify); } - if (qemuMonitorUnref(mon) > 0) - qemuMonitorUnlock(mon); + qemuMonitorUnlock(mon); + virObjectUnref(mon); } @@ -919,12 +906,12 @@ cleanup: /* Ensure proper locking around callbacks. */ #define QEMU_MONITOR_CALLBACK(mon, ret, callback, ...) \ do { \ - qemuMonitorRef(mon); \ + virObjectRef(mon); \ qemuMonitorUnlock(mon); \ if ((mon)->cb && (mon)->cb->callback) \ (ret) = ((mon)->cb->callback)(mon, __VA_ARGS__); \ qemuMonitorLock(mon); \ - ignore_value(qemuMonitorUnref(mon)); \ + virObjectUnref(mon); \ } while (0) int qemuMonitorGetDiskSecret(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 995948b..42f33d1 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -156,9 +156,6 @@ int qemuMonitorCheckHMP(qemuMonitorPtr mon, const char *cmd); void qemuMonitorLock(qemuMonitorPtr mon); void qemuMonitorUnlock(qemuMonitorPtr mon); -int qemuMonitorRef(qemuMonitorPtr mon); -int qemuMonitorUnref(qemuMonitorPtr mon) ATTRIBUTE_RETURN_CHECK; - int qemuMonitorSetLink(qemuMonitorPtr mon, const char *name, enum virDomainNetInterfaceLinkState state) ; -- 1.7.10.4
On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Make qemuAgentPtr and qemuMonitorPtr types use the virObject APIs for reference counting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Make virNetTLSContext and virNetTLSSession use the virObject APIs for reference counting Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 4 +- src/libvirt_private.syms | 2 - src/libvirt_probes.d | 8 +-- src/remote/remote_driver.c | 2 +- src/rpc/virnetclient.c | 6 +-- src/rpc/virnetserver.c | 3 +- src/rpc/virnetserverclient.c | 11 ++--- src/rpc/virnetserverservice.c | 10 ++-- src/rpc/virnetsocket.c | 7 ++- src/rpc/virnettlscontext.c | 110 +++++++++++++++-------------------------- src/rpc/virnettlscontext.h | 10 +--- tests/virnettlscontexttest.c | 10 ++-- 12 files changed, 66 insertions(+), 117 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 49b69ef..7dd7d5c 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -541,7 +541,7 @@ static int daemonSetupNetworking(virNetServerPtr srv, false, config->max_client_requests, ctxt))) { - virNetTLSContextFree(ctxt); + virObjectUnref(ctxt); goto error; } if (virNetServerAddService(srv, svcTLS, @@ -549,7 +549,7 @@ static int daemonSetupNetworking(virNetServerPtr srv, !config->listen_tcp ? "_libvirt._tcp" : NULL) < 0) goto error; - virNetTLSContextFree(ctxt); + virObjectUnref(ctxt); } } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3fb10d2..336fdb7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1625,14 +1625,12 @@ virNetSocketWrite; # virnettlscontext.h virNetTLSContextCheckCertificate; -virNetTLSContextFree; virNetTLSContextNewClient; virNetTLSContextNewClientPath; virNetTLSContextNewServer; virNetTLSContextNewServerPath; virNetTLSContextRef; virNetTLSInit; -virNetTLSSessionFree; virNetTLSSessionGetHandshakeStatus; virNetTLSSessionGetKeySize; virNetTLSSessionHandshake; diff --git a/src/libvirt_probes.d b/src/libvirt_probes.d index ceb3caa..3b138a9 100644 --- a/src/libvirt_probes.d +++ b/src/libvirt_probes.d @@ -61,19 +61,15 @@ provider libvirt { # file: src/rpc/virnettlscontext.c # prefix: rpc - probe rpc_tls_context_new(void *ctxt, int refs, const char *cacert, const char *cacrl, + probe rpc_tls_context_new(void *ctxt, const char *cacert, const char *cacrl, const char *cert, const char *key, int sanityCheckCert, int requireValidCert, int isServer); - probe rpc_tls_context_ref(void *ctxt, int refs); - probe rpc_tls_context_free(void *ctxt, int refs); probe rpc_tls_context_session_allow(void *ctxt, void *sess, const char *dname); probe rpc_tls_context_session_deny(void *ctxt, void *sess, const char *dname); probe rpc_tls_context_session_fail(void *ctxt, void *sess); - probe rpc_tls_session_new(void *sess, void *ctxt, int refs, const char *hostname, int isServer); - probe rpc_tls_session_ref(void *sess, int refs); - probe rpc_tls_session_free(void *sess, int refs); + probe rpc_tls_session_new(void *sess, void *ctxt, const char *hostname, int isServer); probe rpc_tls_session_handshake_pass(void *sess); probe rpc_tls_session_handshake_fail(void *sess); diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index afd367b..5116080 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -943,7 +943,7 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv) (xdrproc_t) xdr_void, (char *) NULL) == -1) ret = -1; - virNetTLSContextFree(priv->tls); + virObjectUnref(priv->tls); priv->tls = NULL; virNetClientClose(priv->client); virNetClientFree(priv->client); diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index cb373b6..72f55a1 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -495,7 +495,7 @@ void virNetClientFree(virNetClientPtr client) if (client->sock) virNetSocketRemoveIOCallback(client->sock); virNetSocketFree(client->sock); - virNetTLSSessionFree(client->tls); + virObjectUnref(client->tls); #if HAVE_SASL virNetSASLSessionFree(client->sasl); #endif @@ -532,7 +532,7 @@ virNetClientCloseLocked(virNetClientPtr client) virNetSocketFree(client->sock); client->sock = NULL; - virNetTLSSessionFree(client->tls); + virObjectUnref(client->tls); client->tls = NULL; #if HAVE_SASL virNetSASLSessionFree(client->sasl); @@ -712,7 +712,7 @@ int virNetClientSetTLSSession(virNetClientPtr client, return 0; error: - virNetTLSSessionFree(client->tls); + virObjectUnref(client->tls); client->tls = NULL; virNetClientUnlock(client); return -1; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 248ad9f..e9f500e 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -652,8 +652,7 @@ no_memory: int virNetServerSetTLSContext(virNetServerPtr srv, virNetTLSContextPtr tls) { - srv->tls = tls; - virNetTLSContextRef(tls); + srv->tls = virObjectRef(tls); return 0; } diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index d0a144c..c419e74 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -346,7 +346,7 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, client->sock = sock; client->auth = auth; client->readonly = readonly; - client->tlsCtxt = tls; + client->tlsCtxt = virObjectRef(tls); client->nrequests_max = nrequests_max; client->sockTimer = virEventAddTimeout(-1, virNetServerClientSockTimerFunc, @@ -354,9 +354,6 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, if (client->sockTimer < 0) goto error; - if (tls) - virNetTLSContextRef(tls); - /* Prepare one for packet receive */ if (!(client->rx = virNetMessageNew(true))) goto error; @@ -598,8 +595,8 @@ void virNetServerClientFree(virNetServerClientPtr client) #endif if (client->sockTimer > 0) virEventRemoveTimeout(client->sockTimer); - virNetTLSSessionFree(client->tls); - virNetTLSContextFree(client->tlsCtxt); + virObjectUnref(client->tls); + virObjectUnref(client->tlsCtxt); virNetSocketFree(client->sock); virNetServerClientUnlock(client); virMutexDestroy(&client->lock); @@ -654,7 +651,7 @@ void virNetServerClientClose(virNetServerClientPtr client) virNetSocketRemoveIOCallback(client->sock); if (client->tls) { - virNetTLSSessionFree(client->tls); + virObjectUnref(client->tls); client->tls = NULL; } client->wantClose = true; diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 2880df3..60fe89f 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -116,9 +116,7 @@ virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, svc->auth = auth; svc->readonly = readonly; svc->nrequests_client_max = nrequests_client_max; - svc->tls = tls; - if (tls) - virNetTLSContextRef(tls); + svc->tls = virObjectRef(tls); if (virNetSocketNewListenTCP(nodename, service, @@ -172,9 +170,7 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, svc->auth = auth; svc->readonly = readonly; svc->nrequests_client_max = nrequests_client_max; - svc->tls = tls; - if (tls) - virNetTLSContextRef(tls); + svc->tls = virObjectRef(tls); svc->nsocks = 1; if (VIR_ALLOC_N(svc->socks, svc->nsocks) < 0) @@ -265,7 +261,7 @@ void virNetServerServiceFree(virNetServerServicePtr svc) virNetSocketFree(svc->socks[i]); VIR_FREE(svc->socks); - virNetTLSContextFree(svc->tls); + virObjectUnref(svc->tls); VIR_FREE(svc); } diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 88e5525..bca78b5 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -748,7 +748,7 @@ void virNetSocketFree(virNetSocketPtr sock) /* Make sure it can't send any more I/O during shutdown */ if (sock->tlsSession) virNetTLSSessionSetIOCallbacks(sock->tlsSession, NULL, NULL, NULL); - virNetTLSSessionFree(sock->tlsSession); + virObjectUnref(sock->tlsSession); #if HAVE_SASL virNetSASLSessionFree(sock->saslSession); #endif @@ -909,13 +909,12 @@ void virNetSocketSetTLSSession(virNetSocketPtr sock, virNetTLSSessionPtr sess) { virMutexLock(&sock->lock); - virNetTLSSessionFree(sock->tlsSession); - sock->tlsSession = sess; + virObjectUnref(sock->tlsSession); + sock->tlsSession = virObjectRef(sess); virNetTLSSessionSetIOCallbacks(sess, virNetSocketTLSSessionWrite, virNetSocketTLSSessionRead, sock); - virNetTLSSessionRef(sess); virMutexUnlock(&sock->lock); } diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 5ae22f2..9fe6eb1 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -50,8 +50,9 @@ #define VIR_FROM_THIS VIR_FROM_RPC struct _virNetTLSContext { + virObject object; + virMutex lock; - int refs; gnutls_certificate_credentials_t x509cred; gnutls_dh_params_t dhParams; @@ -62,9 +63,9 @@ struct _virNetTLSContext { }; struct _virNetTLSSession { - virMutex lock; + virObject object; - int refs; + virMutex lock; bool handshakeComplete; @@ -76,6 +77,29 @@ struct _virNetTLSSession { void *opaque; }; +static virClassPtr virNetTLSContextClass; +static virClassPtr virNetTLSSessionClass; +static void virNetTLSContextDispose(void *obj); +static void virNetTLSSessionDispose(void *obj); + + +static int virNetTLSContextOnceInit(void) +{ + if (!(virNetTLSContextClass = virClassNew("virNetTLSContext", + sizeof(virNetTLSContext), + virNetTLSContextDispose))) + return -1; + + if (!(virNetTLSSessionClass = virClassNew("virNetTLSSession", + sizeof(virNetTLSSession), + virNetTLSSessionDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetTLSContext) + static int virNetTLSContextCheckCertFile(const char *type, const char *file, bool allowMissing) @@ -647,10 +671,11 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, char *gnutlsdebug; int err; - if (VIR_ALLOC(ctxt) < 0) { - virReportOOMError(); + if (virNetTLSContextInitialize() < 0) + return NULL; + + if (!(ctxt = virObjectNew(virNetTLSContextClass))) return NULL; - } if (virMutexInit(&ctxt->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -659,8 +684,6 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, return NULL; } - ctxt->refs = 1; - if ((gnutlsdebug = getenv("LIBVIRT_GNUTLS_DEBUG")) != NULL) { int val; if (virStrToLong_i(gnutlsdebug, NULL, 10, &val) < 0) @@ -716,8 +739,8 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert, ctxt->isServer = isServer; PROBE(RPC_TLS_CONTEXT_NEW, - "ctxt=%p refs=%d cacert=%s cacrl=%s cert=%s key=%s sanityCheckCert=%d requireValidCert=%d isServer=%d", - ctxt, ctxt->refs, cacert, NULLSTR(cacrl), cert, key, sanityCheckCert, requireValidCert, isServer); + "ctxt=%p cacert=%s cacrl=%s cert=%s key=%s sanityCheckCert=%d requireValidCert=%d isServer=%d", + ctxt, cacert, NULLSTR(cacrl), cert, key, sanityCheckCert, requireValidCert, isServer); return ctxt; @@ -927,17 +950,6 @@ virNetTLSContextPtr virNetTLSContextNewClient(const char *cacert, } -void virNetTLSContextRef(virNetTLSContextPtr ctxt) -{ - virMutexLock(&ctxt->lock); - ctxt->refs++; - PROBE(RPC_TLS_CONTEXT_REF, - "ctxt=%p refs=%d", - ctxt, ctxt->refs); - virMutexUnlock(&ctxt->lock); -} - - static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, virNetTLSSessionPtr sess) { @@ -1106,30 +1118,16 @@ cleanup: return ret; } -void virNetTLSContextFree(virNetTLSContextPtr ctxt) +void virNetTLSContextDispose(void *obj) { - if (!ctxt) - return; - - virMutexLock(&ctxt->lock); - PROBE(RPC_TLS_CONTEXT_FREE, - "ctxt=%p refs=%d", - ctxt, ctxt->refs); - ctxt->refs--; - if (ctxt->refs > 0) { - virMutexUnlock(&ctxt->lock); - return; - } + virNetTLSContextPtr ctxt = obj; gnutls_dh_params_deinit(ctxt->dhParams); gnutls_certificate_free_credentials(ctxt->x509cred); - virMutexUnlock(&ctxt->lock); virMutexDestroy(&ctxt->lock); - VIR_FREE(ctxt); } - static ssize_t virNetTLSSessionPush(void *opaque, const void *buf, size_t len) { @@ -1167,10 +1165,8 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt, VIR_DEBUG("ctxt=%p hostname=%s isServer=%d", ctxt, NULLSTR(hostname), ctxt->isServer); - if (VIR_ALLOC(sess) < 0) { - virReportOOMError(); + if (!(sess = virObjectNew(virNetTLSSessionClass))) return NULL; - } if (virMutexInit(&sess->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1179,7 +1175,6 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt, return NULL; } - sess->refs = 1; if (hostname && !(sess->hostname = strdup(hostname))) { virReportOOMError(); @@ -1230,27 +1225,17 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt, sess->isServer = ctxt->isServer; PROBE(RPC_TLS_SESSION_NEW, - "sess=%p refs=%d ctxt=%p hostname=%s isServer=%d", - sess, sess->refs, ctxt, hostname, sess->isServer); + "sess=%p ctxt=%p hostname=%s isServer=%d", + sess, ctxt, hostname, sess->isServer); return sess; error: - virNetTLSSessionFree(sess); + virObjectUnref(sess); return NULL; } -void virNetTLSSessionRef(virNetTLSSessionPtr sess) -{ - virMutexLock(&sess->lock); - sess->refs++; - PROBE(RPC_TLS_SESSION_REF, - "sess=%p refs=%d", - sess, sess->refs); - virMutexUnlock(&sess->lock); -} - void virNetTLSSessionSetIOCallbacks(virNetTLSSessionPtr sess, virNetTLSSessionWriteFunc writeFunc, virNetTLSSessionReadFunc readFunc, @@ -1393,26 +1378,13 @@ cleanup: } -void virNetTLSSessionFree(virNetTLSSessionPtr sess) +void virNetTLSSessionDispose(void *obj) { - if (!sess) - return; - - virMutexLock(&sess->lock); - PROBE(RPC_TLS_SESSION_FREE, - "sess=%p refs=%d", - sess, sess->refs); - sess->refs--; - if (sess->refs > 0) { - virMutexUnlock(&sess->lock); - return; - } + virNetTLSSessionPtr sess = obj; VIR_FREE(sess->hostname); gnutls_deinit(sess->session); - virMutexUnlock(&sess->lock); virMutexDestroy(&sess->lock); - VIR_FREE(sess); } /* diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h index 8893da9..e47c3c0 100644 --- a/src/rpc/virnettlscontext.h +++ b/src/rpc/virnettlscontext.h @@ -22,6 +22,7 @@ # define __VIR_NET_TLS_CONTEXT_H__ # include "internal.h" +# include "virobject.h" typedef struct _virNetTLSContext virNetTLSContext; typedef virNetTLSContext *virNetTLSContextPtr; @@ -58,13 +59,9 @@ virNetTLSContextPtr virNetTLSContextNewClient(const char *cacert, bool sanityCheckCert, bool requireValidCert); -void virNetTLSContextRef(virNetTLSContextPtr ctxt); - int virNetTLSContextCheckCertificate(virNetTLSContextPtr ctxt, virNetTLSSessionPtr sess); -void virNetTLSContextFree(virNetTLSContextPtr ctxt); - typedef ssize_t (*virNetTLSSessionWriteFunc)(const char *buf, size_t len, void *opaque); @@ -79,8 +76,6 @@ void virNetTLSSessionSetIOCallbacks(virNetTLSSessionPtr sess, virNetTLSSessionReadFunc readFunc, void *opaque); -void virNetTLSSessionRef(virNetTLSSessionPtr sess); - ssize_t virNetTLSSessionWrite(virNetTLSSessionPtr sess, const char *buf, size_t len); ssize_t virNetTLSSessionRead(virNetTLSSessionPtr sess, @@ -99,7 +94,4 @@ virNetTLSSessionGetHandshakeStatus(virNetTLSSessionPtr sess); int virNetTLSSessionGetKeySize(virNetTLSSessionPtr sess); -void virNetTLSSessionFree(virNetTLSSessionPtr sess); - - #endif diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index 0dfaa23..397c967 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -496,7 +496,7 @@ static int testTLSContextInit(const void *opaque) ret = 0; cleanup: - virNetTLSContextFree(ctxt); + virObjectUnref(ctxt); gnutls_x509_crt_deinit(data->careq.crt); gnutls_x509_crt_deinit(data->certreq.crt); data->careq.crt = data->certreq.crt = NULL; @@ -710,10 +710,10 @@ static int testTLSSessionInit(const void *opaque) ret = 0; cleanup: - virNetTLSContextFree(serverCtxt); - virNetTLSContextFree(clientCtxt); - virNetTLSSessionFree(serverSess); - virNetTLSSessionFree(clientSess); + virObjectUnref(serverCtxt); + virObjectUnref(clientCtxt); + virObjectUnref(serverSess); + virObjectUnref(clientSess); gnutls_x509_crt_deinit(data->careq.crt); if (data->othercareq.filename) gnutls_x509_crt_deinit(data->othercareq.crt); -- 1.7.10.4
On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Make virNetTLSContext and virNetTLSSession use the virObject APIs for reference counting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
'make check' failed: GEN check-symfile Expected symbol virNetTLSContextRef is not in ELF library Expected symbol virNetTLSSessionRef is not in ELF library make[3]: *** [check-symfile] Error 1 ACK with this squashed in: diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms index 3bd4aeb..acaa6f3 100644 --- i/src/libvirt_private.syms +++ w/src/libvirt_private.syms @@ -1629,14 +1629,12 @@ virNetTLSContextNewClient; virNetTLSContextNewClientPath; virNetTLSContextNewServer; virNetTLSContextNewServerPath; -virNetTLSContextRef; virNetTLSInit; virNetTLSSessionGetHandshakeStatus; virNetTLSSessionGetKeySize; virNetTLSSessionHandshake; virNetTLSSessionNew; virNetTLSSessionRead; -virNetTLSSessionRef; virNetTLSSessionSetIOCallbacks; virNetTLSSessionWrite; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Make virNetSASLContext and virNetSASLSession use virObject APIs for reference counting Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/remote.c | 8 ++-- src/remote/remote_driver.c | 4 +- src/rpc/virnetclient.c | 7 ++- src/rpc/virnetsaslcontext.c | 106 ++++++++++++++++++------------------------ src/rpc/virnetsaslcontext.h | 8 +--- src/rpc/virnetserverclient.c | 7 ++- src/rpc/virnetsocket.c | 7 ++- 7 files changed, 61 insertions(+), 86 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index d25717c..832307e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2325,7 +2325,7 @@ authfail: PROBE(RPC_SERVER_CLIENT_AUTH_FAIL, "client=%p auth=%d", client, REMOTE_AUTH_SASL); - virNetSASLSessionFree(sasl); + virObjectUnref(sasl); virMutexUnlock(&priv->lock); return -1; } @@ -2369,7 +2369,7 @@ remoteSASLFinish(virNetServerClientPtr client) "client=%p auth=%d identity=%s", client, REMOTE_AUTH_SASL, identity); - virNetSASLSessionFree(priv->sasl); + virObjectUnref(priv->sasl); priv->sasl = NULL; return 0; @@ -2467,7 +2467,7 @@ authdeny: goto error; error: - virNetSASLSessionFree(priv->sasl); + virObjectUnref(priv->sasl); priv->sasl = NULL; virResetLastError(); virReportError(VIR_ERR_AUTH_FAILED, "%s", @@ -2565,7 +2565,7 @@ authdeny: goto error; error: - virNetSASLSessionFree(priv->sasl); + virObjectUnref(priv->sasl); priv->sasl = NULL; virResetLastError(); virReportError(VIR_ERR_AUTH_FAILED, "%s", diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 5116080..38b11e1 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3397,8 +3397,8 @@ remoteAuthSASL (virConnectPtr conn, struct private_data *priv, remoteAuthInteractStateClear(&state, true); VIR_FREE(saslcb); - virNetSASLSessionFree(sasl); - virNetSASLContextFree(saslCtxt); + virObjectUnref(sasl); + virObjectUnref(saslCtxt); return ret; } diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 72f55a1..58c80e2 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -497,7 +497,7 @@ void virNetClientFree(virNetClientPtr client) virNetSocketFree(client->sock); virObjectUnref(client->tls); #if HAVE_SASL - virNetSASLSessionFree(client->sasl); + virObjectUnref(client->sasl); #endif virNetMessageClear(&client->msg); @@ -535,7 +535,7 @@ virNetClientCloseLocked(virNetClientPtr client) virObjectUnref(client->tls); client->tls = NULL; #if HAVE_SASL - virNetSASLSessionFree(client->sasl); + virObjectUnref(client->sasl); client->sasl = NULL; #endif ka = client->keepalive; @@ -607,8 +607,7 @@ void virNetClientSetSASLSession(virNetClientPtr client, virNetSASLSessionPtr sasl) { virNetClientLock(client); - client->sasl = sasl; - virNetSASLSessionRef(sasl); + client->sasl = virObjectRef(sasl); virNetSocketSetSASLSession(client->sock, client->sasl); virNetClientUnlock(client); } diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c index af6e237..2feb9a9 100644 --- a/src/rpc/virnetsaslcontext.c +++ b/src/rpc/virnetsaslcontext.c @@ -33,24 +33,52 @@ #define VIR_FROM_THIS VIR_FROM_RPC struct _virNetSASLContext { + virObject object; + virMutex lock; const char *const*usernameWhitelist; - int refs; }; struct _virNetSASLSession { + virObject object; + virMutex lock; sasl_conn_t *conn; - int refs; size_t maxbufsize; }; +static virClassPtr virNetSASLContextClass; +static virClassPtr virNetSASLSessionClass; +static void virNetSASLContextDispose(void *obj); +static void virNetSASLSessionDispose(void *obj); + +static int virNetSASLContextOnceInit(void) +{ + if (!(virNetSASLContextClass = virClassNew("virNetSASLContext", + sizeof(virNetSASLContext), + virNetSASLContextDispose))) + return -1; + + if (!(virNetSASLSessionClass = virClassNew("virNetSASLSession", + sizeof(virNetSASLSession), + virNetSASLSessionDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetSASLContext) + + virNetSASLContextPtr virNetSASLContextNewClient(void) { virNetSASLContextPtr ctxt; int err; + if (virNetSASLContextInitialize() < 0) + return NULL; + err = sasl_client_init(NULL); if (err != SASL_OK) { virReportError(VIR_ERR_AUTH_FAILED, @@ -59,10 +87,8 @@ virNetSASLContextPtr virNetSASLContextNewClient(void) return NULL; } - if (VIR_ALLOC(ctxt) < 0) { - virReportOOMError(); + if (!(ctxt = virObjectNew(virNetSASLContextClass))) return NULL; - } if (virMutexInit(&ctxt->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -71,8 +97,6 @@ virNetSASLContextPtr virNetSASLContextNewClient(void) return NULL; } - ctxt->refs = 1; - return ctxt; } @@ -81,6 +105,9 @@ virNetSASLContextPtr virNetSASLContextNewServer(const char *const*usernameWhitel virNetSASLContextPtr ctxt; int err; + if (virNetSASLContextInitialize() < 0) + return NULL; + err = sasl_server_init(NULL, "libvirt"); if (err != SASL_OK) { virReportError(VIR_ERR_AUTH_FAILED, @@ -89,10 +116,8 @@ virNetSASLContextPtr virNetSASLContextNewServer(const char *const*usernameWhitel return NULL; } - if (VIR_ALLOC(ctxt) < 0) { - virReportOOMError(); + if (!(ctxt = virObjectNew(virNetSASLContextClass))) return NULL; - } if (virMutexInit(&ctxt->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -102,7 +127,6 @@ virNetSASLContextPtr virNetSASLContextNewServer(const char *const*usernameWhitel } ctxt->usernameWhitelist = usernameWhitelist; - ctxt->refs = 1; return ctxt; } @@ -152,28 +176,11 @@ cleanup: } -void virNetSASLContextRef(virNetSASLContextPtr ctxt) -{ - virMutexLock(&ctxt->lock); - ctxt->refs++; - virMutexUnlock(&ctxt->lock); -} - -void virNetSASLContextFree(virNetSASLContextPtr ctxt) +void virNetSASLContextDispose(void *obj) { - if (!ctxt) - return; - - virMutexLock(&ctxt->lock); - ctxt->refs--; - if (ctxt->refs > 0) { - virMutexUnlock(&ctxt->lock); - return; - } + virNetSASLContextPtr ctxt = obj; - virMutexUnlock(&ctxt->lock); virMutexDestroy(&ctxt->lock); - VIR_FREE(ctxt); } virNetSASLSessionPtr virNetSASLSessionNewClient(virNetSASLContextPtr ctxt ATTRIBUTE_UNUSED, @@ -186,10 +193,8 @@ virNetSASLSessionPtr virNetSASLSessionNewClient(virNetSASLContextPtr ctxt ATTRIB virNetSASLSessionPtr sasl = NULL; int err; - if (VIR_ALLOC(sasl) < 0) { - virReportOOMError(); - goto cleanup; - } + if (!(sasl = virObjectNew(virNetSASLSessionClass))) + return NULL; if (virMutexInit(&sasl->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -198,7 +203,6 @@ virNetSASLSessionPtr virNetSASLSessionNewClient(virNetSASLContextPtr ctxt ATTRIB return NULL; } - sasl->refs = 1; /* Arbitrary size for amount of data we can encode in a single block */ sasl->maxbufsize = 1 << 16; @@ -219,7 +223,7 @@ virNetSASLSessionPtr virNetSASLSessionNewClient(virNetSASLContextPtr ctxt ATTRIB return sasl; cleanup: - virNetSASLSessionFree(sasl); + virObjectUnref(sasl); return NULL; } @@ -231,10 +235,8 @@ virNetSASLSessionPtr virNetSASLSessionNewServer(virNetSASLContextPtr ctxt ATTRIB virNetSASLSessionPtr sasl = NULL; int err; - if (VIR_ALLOC(sasl) < 0) { - virReportOOMError(); - goto cleanup; - } + if (!(sasl = virObjectNew(virNetSASLSessionClass))) + return NULL; if (virMutexInit(&sasl->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -243,7 +245,6 @@ virNetSASLSessionPtr virNetSASLSessionNewServer(virNetSASLContextPtr ctxt ATTRIB return NULL; } - sasl->refs = 1; /* Arbitrary size for amount of data we can encode in a single block */ sasl->maxbufsize = 1 << 16; @@ -265,17 +266,10 @@ virNetSASLSessionPtr virNetSASLSessionNewServer(virNetSASLContextPtr ctxt ATTRIB return sasl; cleanup: - virNetSASLSessionFree(sasl); + virObjectUnref(sasl); return NULL; } -void virNetSASLSessionRef(virNetSASLSessionPtr sasl) -{ - virMutexLock(&sasl->lock); - sasl->refs++; - virMutexUnlock(&sasl->lock); -} - int virNetSASLSessionExtKeySize(virNetSASLSessionPtr sasl, int ssf) { @@ -712,22 +706,12 @@ cleanup: return ret; } -void virNetSASLSessionFree(virNetSASLSessionPtr sasl) +void virNetSASLSessionDispose(void *obj) { - if (!sasl) - return; - - virMutexLock(&sasl->lock); - sasl->refs--; - if (sasl->refs > 0) { - virMutexUnlock(&sasl->lock); - return; - } + virNetSASLSessionPtr sasl = obj; if (sasl->conn) sasl_dispose(&sasl->conn); - virMutexUnlock(&sasl->lock); virMutexDestroy(&sasl->lock); - VIR_FREE(sasl); } diff --git a/src/rpc/virnetsaslcontext.h b/src/rpc/virnetsaslcontext.h index 914c45c..8e322d8 100644 --- a/src/rpc/virnetsaslcontext.h +++ b/src/rpc/virnetsaslcontext.h @@ -24,6 +24,7 @@ # include <sasl/sasl.h> # include "internal.h" +# include "virobject.h" typedef struct _virNetSASLContext virNetSASLContext; typedef virNetSASLContext *virNetSASLContextPtr; @@ -43,9 +44,6 @@ virNetSASLContextPtr virNetSASLContextNewServer(const char *const*usernameWhitel int virNetSASLContextCheckIdentity(virNetSASLContextPtr ctxt, const char *identity); -void virNetSASLContextRef(virNetSASLContextPtr sasl); -void virNetSASLContextFree(virNetSASLContextPtr sasl); - virNetSASLSessionPtr virNetSASLSessionNewClient(virNetSASLContextPtr ctxt, const char *service, const char *hostname, @@ -59,8 +57,6 @@ virNetSASLSessionPtr virNetSASLSessionNewServer(virNetSASLContextPtr ctxt, char *virNetSASLSessionListMechanisms(virNetSASLSessionPtr sasl); -void virNetSASLSessionRef(virNetSASLSessionPtr sasl); - int virNetSASLSessionExtKeySize(virNetSASLSessionPtr sasl, int ssf); @@ -114,6 +110,4 @@ ssize_t virNetSASLSessionDecode(virNetSASLSessionPtr sasl, const char **output, size_t *outputlen); -void virNetSASLSessionFree(virNetSASLSessionPtr sasl); - #endif /* __VIR_NET_CLIENT_SASL_CONTEXT_H__ */ diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index c419e74..471cca0 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -474,8 +474,7 @@ void virNetServerClientSetSASLSession(virNetServerClientPtr client, * operation do we switch to SASL mode */ virNetServerClientLock(client); - client->sasl = sasl; - virNetSASLSessionRef(sasl); + client->sasl = virObjectRef(sasl); virNetServerClientUnlock(client); } #endif @@ -591,7 +590,7 @@ void virNetServerClientFree(virNetServerClientPtr client) VIR_FREE(client->identity); #if HAVE_SASL - virNetSASLSessionFree(client->sasl); + virObjectUnref(client->sasl); #endif if (client->sockTimer > 0) virEventRemoveTimeout(client->sockTimer); @@ -1009,7 +1008,7 @@ virNetServerClientDispatchWrite(virNetServerClientPtr client) */ if (client->sasl) { virNetSocketSetSASLSession(client->sock, client->sasl); - virNetSASLSessionFree(client->sasl); + virObjectUnref(client->sasl); client->sasl = NULL; } #endif diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index bca78b5..b6bb211 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -750,7 +750,7 @@ void virNetSocketFree(virNetSocketPtr sock) virNetTLSSessionSetIOCallbacks(sock->tlsSession, NULL, NULL, NULL); virObjectUnref(sock->tlsSession); #if HAVE_SASL - virNetSASLSessionFree(sock->saslSession); + virObjectUnref(sock->saslSession); #endif VIR_FORCE_CLOSE(sock->fd); @@ -924,9 +924,8 @@ void virNetSocketSetSASLSession(virNetSocketPtr sock, virNetSASLSessionPtr sess) { virMutexLock(&sock->lock); - virNetSASLSessionFree(sock->saslSession); - sock->saslSession = sess; - virNetSASLSessionRef(sess); + virObjectUnref(sock->saslSession); + sock->saslSession = virObjectRef(sess); virMutexUnlock(&sock->lock); } #endif -- 1.7.10.4
On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Make virNetSASLContext and virNetSASLSession use virObject APIs for reference counting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/remote.c | 8 ++-- src/remote/remote_driver.c | 4 +- src/rpc/virnetclient.c | 7 ++- src/rpc/virnetsaslcontext.c | 106 ++++++++++++++++++------------------------ src/rpc/virnetsaslcontext.h | 8 +--- src/rpc/virnetserverclient.c | 7 ++- src/rpc/virnetsocket.c | 7 ++- 7 files changed, 61 insertions(+), 86 deletions(-)
Not as dramatic, but still a net reduction. Fails 'make check', and missing changes to cfg.mk to clean up the cruft.
+++ b/src/rpc/virnetclient.c @@ -497,7 +497,7 @@ void virNetClientFree(virNetClientPtr client) virNetSocketFree(client->sock); virObjectUnref(client->tls); #if HAVE_SASL - virNetSASLSessionFree(client->sasl); + virObjectUnref(client->sasl); #endif
A followup patch could probably remove this #ifdef protection, if client->sasl exists but is NULL when HAVE_SASL is not present.
@@ -152,28 +176,11 @@ cleanup: }
-void virNetSASLContextRef(virNetSASLContextPtr ctxt) -{ - virMutexLock(&ctxt->lock); - ctxt->refs++; - virMutexUnlock(&ctxt->lock); -} - -void virNetSASLContextFree(virNetSASLContextPtr ctxt) +void virNetSASLContextDispose(void *obj)
For consistency with the prototype earlier in the file, I would have marked this static, but what you have works.
@@ -186,10 +193,8 @@ virNetSASLSessionPtr virNetSASLSessionNewClient(virNetSASLContextPtr ctxt ATTRIB virNetSASLSessionPtr sasl = NULL; int err;
- if (VIR_ALLOC(sasl) < 0) { - virReportOOMError(); - goto cleanup; - } + if (!(sasl = virObjectNew(virNetSASLSessionClass))) + return NULL;
I suppose that since this function takes a virNetSASLContextPtr, and creating one of those already calls virNetSASLContextInitialize(), that you are safe not calling the one-shot initialization here after all.
@@ -231,10 +235,8 @@ virNetSASLSessionPtr virNetSASLSessionNewServer(virNetSASLContextPtr ctxt ATTRIB virNetSASLSessionPtr sasl = NULL; int err;
- if (VIR_ALLOC(sasl) < 0) { - virReportOOMError(); - goto cleanup; - } + if (!(sasl = virObjectNew(virNetSASLSessionClass))) + return NULL;
Another instance of the same comment. ACK with this squashed in (oh, one of those cfg.mk tweaks actually belongs in 5/10, and you may want to factor out the duplication cleanup between libvirt_{private,sasl}.syms into a separate patch): diff --git i/cfg.mk w/cfg.mk index ccff146..dc39646 100644 --- i/cfg.mk +++ w/cfg.mk @@ -156,9 +156,6 @@ useless_free_options = \ --name=virNetServerProgramFree \ --name=virNetServerServiceFree \ --name=virNetSocketFree \ - --name=virNetSASLContextFree \ - --name=virNetSASLSessionFree \ - --name=virNetTLSSessionFree \ --name=virNWFilterDefFree \ --name=virNWFilterEntryFree \ --name=virNWFilterHashTableFree \ diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms index acaa6f3..c0bb5a5 100644 --- i/src/libvirt_private.syms +++ w/src/libvirt_private.syms @@ -1470,27 +1470,13 @@ xdr_virNetMessageError; # virnetsaslcontext.h -virNetSASLContextCheckIdentity; -virNetSASLContextFree; virNetSASLContextNewClient; -virNetSASLContextNewServer; -virNetSASLContextRef; virNetSASLSessionClientStart; virNetSASLSessionClientStep; virNetSASLSessionDecode; virNetSASLSessionEncode; -virNetSASLSessionExtKeySize; -virNetSASLSessionFree; -virNetSASLSessionGetIdentity; -virNetSASLSessionGetKeySize; virNetSASLSessionGetMaxBufSize; -virNetSASLSessionListMechanisms; virNetSASLSessionNewClient; -virNetSASLSessionNewServer; -virNetSASLSessionRef; -virNetSASLSessionSecProps; -virNetSASLSessionServerStart; -virNetSASLSessionServerStep; # virnetserver.h @@ -1542,7 +1528,6 @@ virNetServerClientSetCloseHook; virNetServerClientSetDispatcher; virNetServerClientSetIdentity; virNetServerClientSetPrivateData; -virNetServerClientSetSASLSession; virNetServerClientStartKeepAlive; virNetServerClientWantClose; diff --git i/src/libvirt_sasl.syms w/src/libvirt_sasl.syms index 2c278c8..cc46c0d 100644 --- i/src/libvirt_sasl.syms +++ w/src/libvirt_sasl.syms @@ -6,7 +6,6 @@ virNetSASLContextCheckIdentity; virNetSASLContextNewServer; virNetSASLSessionExtKeySize; -virNetSASLSessionFree; virNetSASLSessionGetIdentity; virNetSASLSessionGetKeySize; virNetSASLSessionListMechanisms; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Make virKeepAlive use the virObject APIs for reference counting Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_probes.d | 4 +-- src/rpc/virkeepalive.c | 73 +++++++++++++++++------------------------- src/rpc/virkeepalive.h | 4 +-- src/rpc/virnetclient.c | 4 +-- src/rpc/virnetserverclient.c | 4 +-- 5 files changed, 35 insertions(+), 54 deletions(-) diff --git a/src/libvirt_probes.d b/src/libvirt_probes.d index 3b138a9..807239f 100644 --- a/src/libvirt_probes.d +++ b/src/libvirt_probes.d @@ -77,9 +77,7 @@ provider libvirt { # file: src/rpc/virkeepalive.c # prefix: rpc - probe rpc_keepalive_new(void *ka, void *client, int refs); - probe rpc_keepalive_ref(void *ka, void *client, int refs); - probe rpc_keepalive_free(void *ka, void *client, int refs); + probe rpc_keepalive_new(void *ka, void *client); probe rpc_keepalive_start(void *ka, void *client, int interval, int count); probe rpc_keepalive_stop(void *ka, void *client); probe rpc_keepalive_send(void *ka, void *client, int prog, int vers, int proc); diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c index ffb059e..7ff125e 100644 --- a/src/rpc/virkeepalive.c +++ b/src/rpc/virkeepalive.c @@ -35,7 +35,8 @@ #define VIR_FROM_THIS VIR_FROM_RPC struct _virKeepAlive { - int refs; + virObject object; + virMutex lock; int interval; @@ -52,6 +53,21 @@ struct _virKeepAlive { }; +static virClassPtr virKeepAliveClass; +static void virKeepAliveDispose(void *obj); + +static int virKeepAliveOnceInit(void) +{ + if (!(virKeepAliveClass = virClassNew("virKeepAlive", + sizeof(virKeepAlive), + virKeepAliveDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virKeepAlive) + static void virKeepAliveLock(virKeepAlivePtr ka) { @@ -165,7 +181,7 @@ virKeepAliveTimer(int timer ATTRIBUTE_UNUSED, void *opaque) if (!dead && !msg) goto cleanup; - ka->refs++; + virObjectRef(ka); virKeepAliveUnlock(ka); if (dead) { @@ -176,20 +192,13 @@ virKeepAliveTimer(int timer ATTRIBUTE_UNUSED, void *opaque) } virKeepAliveLock(ka); - ka->refs--; + virObjectUnref(ka); cleanup: virKeepAliveUnlock(ka); } -static void -virKeepAliveTimerFree(void *opaque) -{ - virKeepAliveFree(opaque); -} - - virKeepAlivePtr virKeepAliveNew(int interval, unsigned int count, @@ -202,17 +211,17 @@ virKeepAliveNew(int interval, VIR_DEBUG("client=%p, interval=%d, count=%u", client, interval, count); - if (VIR_ALLOC(ka) < 0) { - virReportOOMError(); + if (virKeepAliveInitialize() < 0) + return NULL; + + if (!(ka = virObjectNew(virKeepAliveClass))) return NULL; - } if (virMutexInit(&ka->lock) < 0) { VIR_FREE(ka); return NULL; } - ka->refs = 1; ka->interval = interval; ka->count = count; ka->countToDeath = count; @@ -223,44 +232,20 @@ virKeepAliveNew(int interval, ka->freeCB = freeCB; PROBE(RPC_KEEPALIVE_NEW, - "ka=%p client=%p refs=%d", - ka, ka->client, ka->refs); + "ka=%p client=%p", + ka, ka->client); return ka; } void -virKeepAliveRef(virKeepAlivePtr ka) +virKeepAliveDispose(void *obj) { - virKeepAliveLock(ka); - ka->refs++; - PROBE(RPC_KEEPALIVE_REF, - "ka=%p client=%p refs=%d", - ka, ka->client, ka->refs); - virKeepAliveUnlock(ka); -} - - -void -virKeepAliveFree(virKeepAlivePtr ka) -{ - if (!ka) - return; - - virKeepAliveLock(ka); - PROBE(RPC_KEEPALIVE_FREE, - "ka=%p client=%p refs=%d", - ka, ka->client, ka->refs); - - if (--ka->refs > 0) { - virKeepAliveUnlock(ka); - return; - } + virKeepAlivePtr ka = obj; virMutexDestroy(&ka->lock); ka->freeCB(ka->client); - VIR_FREE(ka); } @@ -311,12 +296,12 @@ virKeepAliveStart(virKeepAlivePtr ka, timeout = ka->interval - delay; ka->intervalStart = now - (ka->interval - timeout); ka->timer = virEventAddTimeout(timeout * 1000, virKeepAliveTimer, - ka, virKeepAliveTimerFree); + ka, virObjectFreeCallback); if (ka->timer < 0) goto cleanup; /* the timer now has another reference to this object */ - ka->refs++; + virObjectRef(ka); ret = 0; cleanup: diff --git a/src/rpc/virkeepalive.h b/src/rpc/virkeepalive.h index 52e1d04..0f77f34 100644 --- a/src/rpc/virkeepalive.h +++ b/src/rpc/virkeepalive.h @@ -24,6 +24,7 @@ # define __VIR_KEEPALIVE_H__ # include "virnetmessage.h" +# include "virobject.h" typedef int (*virKeepAliveSendFunc)(void *client, virNetMessagePtr msg); typedef void (*virKeepAliveDeadFunc)(void *client); @@ -42,9 +43,6 @@ virKeepAlivePtr virKeepAliveNew(int interval, ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6); -void virKeepAliveRef(virKeepAlivePtr ka); -void virKeepAliveFree(virKeepAlivePtr ka); - int virKeepAliveStart(virKeepAlivePtr ka, int interval, unsigned int count); diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 58c80e2..6d8c132 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -361,7 +361,7 @@ error: VIR_FORCE_CLOSE(wakeupFD[1]); if (ka) { virKeepAliveStop(ka); - virKeepAliveFree(ka); + virObjectUnref(ka); } virNetClientFree(client); return NULL; @@ -551,7 +551,7 @@ virNetClientCloseLocked(virNetClientPtr client) if (ka) { virKeepAliveStop(ka); - virKeepAliveFree(ka); + virObjectUnref(ka); } if (closeCb) closeCb(client, closeReason, closeOpaque); diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 471cca0..3a6439d 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -629,7 +629,7 @@ void virNetServerClientClose(virNetServerClientPtr client) client->keepalive = NULL; client->refs++; virNetServerClientUnlock(client); - virKeepAliveFree(ka); + virObjectUnref(ka); virNetServerClientLock(client); client->refs--; } @@ -1199,7 +1199,7 @@ cleanup: virNetServerClientUnlock(client); if (ka) virKeepAliveStop(ka); - virKeepAliveFree(ka); + virObjectUnref(ka); return ret; } -- 1.7.10.4
On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Make virKeepAlive use the virObject APIs for reference counting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> ---
+++ b/src/rpc/virkeepalive.h @@ -24,6 +24,7 @@ # define __VIR_KEEPALIVE_H__
# include "virnetmessage.h" +# include "virobject.h"
Do we technically need this include here, since the virKeepAlive struct is opaque? Perhaps it belongs better in the .c. At any rate, ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Make virSocket use the virObject APIs for reference counting Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 1 - src/libvirt_probes.d | 4 +-- src/qemu/qemu_migration.c | 4 +-- src/rpc/virnetclient.c | 4 +-- src/rpc/virnetserverclient.c | 4 +-- src/rpc/virnetserverservice.c | 4 +-- src/rpc/virnetsocket.c | 71 ++++++++++++++++++----------------------- src/rpc/virnetsocket.h | 3 +- tests/virnetsockettest.c | 26 +++++++-------- 9 files changed, 54 insertions(+), 67 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 336fdb7..73a1ec1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1593,7 +1593,6 @@ virNetSocketAccept; virNetSocketAddIOCallback; virNetSocketClose; virNetSocketDupFD; -virNetSocketFree; virNetSocketGetFD; virNetSocketGetPort; virNetSocketGetUNIXIdentity; diff --git a/src/libvirt_probes.d b/src/libvirt_probes.d index 807239f..be1d938 100644 --- a/src/libvirt_probes.d +++ b/src/libvirt_probes.d @@ -25,11 +25,9 @@ provider libvirt { # file: src/rpc/virnetsocket.c # prefix: rpc - probe rpc_socket_new(void *sock, int refs, int fd, int errfd, pid_t pid, const char *localAddr, const char *remoteAddr); + probe rpc_socket_new(void *sock, int fd, int errfd, pid_t pid, const char *localAddr, const char *remoteAddr); probe rpc_socket_send_fd(void *sock, int fd); probe rpc_socket_recv_fd(void *sock, int fd); - probe rpc_socket_ref(void *sock, int refs); - probe rpc_socket_free(void *sock, int refs); # file: src/rpc/virnetserverclient.c diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 912ba58..57b639f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1805,7 +1805,7 @@ qemuMigrationConnect(struct qemud_driver *driver, goto cleanup; if (virNetSocketNewConnectTCP(host, port, &sock) == 0) { spec->dest.fd.qemu = virNetSocketDupFD(sock, true); - virNetSocketFree(sock); + virObjectUnref(sock); } if (virSecurityManagerClearSocketLabel(driver->securityManager, vm->def) < 0 || spec->dest.fd.qemu == -1) @@ -2157,7 +2157,7 @@ cleanup: VIR_FORCE_CLOSE(spec.dest.fd.qemu); VIR_FORCE_CLOSE(spec.dest.fd.local); } else { - virNetSocketFree(sock); + virObjectUnref(sock); VIR_FREE(spec.dest.unix_socket.file); } diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 6d8c132..9025221 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -494,7 +494,7 @@ void virNetClientFree(virNetClientPtr client) if (client->sock) virNetSocketRemoveIOCallback(client->sock); - virNetSocketFree(client->sock); + virObjectUnref(client->sock); virObjectUnref(client->tls); #if HAVE_SASL virObjectUnref(client->sasl); @@ -530,7 +530,7 @@ virNetClientCloseLocked(virNetClientPtr client) if (!client->sock) return; - virNetSocketFree(client->sock); + virObjectUnref(client->sock); client->sock = NULL; virObjectUnref(client->tls); client->tls = NULL; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 3a6439d..85f7c88 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -596,7 +596,7 @@ void virNetServerClientFree(virNetServerClientPtr client) virEventRemoveTimeout(client->sockTimer); virObjectUnref(client->tls); virObjectUnref(client->tlsCtxt); - virNetSocketFree(client->sock); + virObjectUnref(client->sock); virNetServerClientUnlock(client); virMutexDestroy(&client->lock); VIR_FREE(client); @@ -667,7 +667,7 @@ void virNetServerClientClose(virNetServerClientPtr client) } if (client->sock) { - virNetSocketFree(client->sock); + virObjectUnref(client->sock); client->sock = NULL; } diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 60fe89f..93c0574 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -86,7 +86,7 @@ error: virNetServerClientClose(client); virNetServerClientFree(client); } else { - virNetSocketFree(clientsock); + virObjectUnref(clientsock); } } @@ -258,7 +258,7 @@ void virNetServerServiceFree(virNetServerServicePtr svc) return; for (i = 0 ; i < svc->nsocks ; i++) - virNetSocketFree(svc->socks[i]); + virObjectUnref(svc->socks[i]); VIR_FREE(svc->socks); virObjectUnref(svc->tls); diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index b6bb211..b6f156b 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -51,8 +51,9 @@ struct _virNetSocket { + virObject object; + virMutex lock; - int refs; int fd; int watch; @@ -85,6 +86,22 @@ struct _virNetSocket { }; +static virClassPtr virNetSocketClass; +static void virNetSocketDispose(void *obj); + +static int virNetSocketOnceInit(void) +{ + if (!(virNetSocketClass = virClassNew("virNetSocket", + sizeof(virNetSocket), + virNetSocketDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetSocket) + + #ifndef WIN32 static int virNetSocketForkDaemon(const char *binary) { @@ -114,6 +131,9 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, virNetSocketPtr sock; int no_slow_start = 1; + if (virNetSocketInitialize() < 0) + return NULL; + VIR_DEBUG("localAddr=%p remoteAddr=%p fd=%d errfd=%d pid=%lld", localAddr, remoteAddr, fd, errfd, (long long) pid); @@ -129,10 +149,8 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, return NULL; } - if (VIR_ALLOC(sock) < 0) { - virReportOOMError(); + if (!(sock = virObjectNew(virNetSocketClass))) return NULL; - } if (virMutexInit(&sock->lock) < 0) { virReportSystemError(errno, "%s", @@ -140,7 +158,6 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, VIR_FREE(sock); return NULL; } - sock->refs = 1; if (localAddr) sock->localAddr = *localAddr; @@ -174,15 +191,15 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, sock->client = isClient; PROBE(RPC_SOCKET_NEW, - "sock=%p refs=%d fd=%d errfd=%d pid=%lld localAddr=%s, remoteAddr=%s", - sock, sock->refs, fd, errfd, (long long) pid, + "sock=%p fd=%d errfd=%d pid=%lld localAddr=%s, remoteAddr=%s", + sock, fd, errfd, (long long) pid, NULLSTR(sock->localAddrStr), NULLSTR(sock->remoteAddrStr)); return sock; error: sock->fd = sock->errfd = -1; /* Caller owns fd/errfd on failure */ - virNetSocketFree(sock); + virObjectUnref(sock); return NULL; } @@ -296,7 +313,7 @@ int virNetSocketNewListenTCP(const char *nodename, error: for (i = 0 ; i < nsocks ; i++) - virNetSocketFree(socks[i]); + virObjectUnref(socks[i]); VIR_FREE(socks); freeaddrinfo(ai); VIR_FORCE_CLOSE(fd); @@ -704,32 +721,9 @@ int virNetSocketNewConnectExternal(const char **cmdargv, } -void virNetSocketRef(virNetSocketPtr sock) -{ - virMutexLock(&sock->lock); - sock->refs++; - PROBE(RPC_SOCKET_REF, - "sock=%p refs=%d", - sock, sock->refs); - virMutexUnlock(&sock->lock); -} - - -void virNetSocketFree(virNetSocketPtr sock) +void virNetSocketDispose(void *obj) { - if (!sock) - return; - - virMutexLock(&sock->lock); - PROBE(RPC_SOCKET_FREE, - "sock=%p refs=%d", - sock, sock->refs); - - sock->refs--; - if (sock->refs > 0) { - virMutexUnlock(&sock->lock); - return; - } + virNetSocketPtr sock = obj; VIR_DEBUG("sock=%p fd=%d", sock, sock->fd); if (sock->watch > 0) { @@ -761,10 +755,7 @@ void virNetSocketFree(virNetSocketPtr sock) VIR_FREE(sock->localAddrStr); VIR_FREE(sock->remoteAddrStr); - virMutexUnlock(&sock->lock); virMutexDestroy(&sock->lock); - - VIR_FREE(sock); } @@ -1331,7 +1322,7 @@ static void virNetSocketEventFree(void *opaque) if (ff) ff(eopaque); - virNetSocketFree(sock); + virObjectUnref(sock); } int virNetSocketAddIOCallback(virNetSocketPtr sock, @@ -1342,7 +1333,7 @@ int virNetSocketAddIOCallback(virNetSocketPtr sock, { int ret = -1; - virNetSocketRef(sock); + virObjectRef(sock); virMutexLock(&sock->lock); if (sock->watch > 0) { VIR_DEBUG("Watch already registered on socket %p", sock); @@ -1366,7 +1357,7 @@ int virNetSocketAddIOCallback(virNetSocketPtr sock, cleanup: virMutexUnlock(&sock->lock); if (ret != 0) - virNetSocketFree(sock); + virObjectUnref(sock); return ret; } diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 6c8e77c..cc3f912 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -27,6 +27,7 @@ # include "virsocketaddr.h" # include "command.h" # include "virnettlscontext.h" +# include "virobject.h" # ifdef HAVE_SASL # include "virnetsaslcontext.h" # endif @@ -108,8 +109,6 @@ void virNetSocketSetSASLSession(virNetSocketPtr sock, # endif bool virNetSocketHasCachedData(virNetSocketPtr sock); bool virNetSocketHasPendingData(virNetSocketPtr sock); -void virNetSocketRef(virNetSocketPtr sock); -void virNetSocketFree(virNetSocketPtr sock); const char *virNetSocketLocalAddrString(virNetSocketPtr sock); const char *virNetSocketRemoteAddrString(virNetSocketPtr sock); diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 0f7bbad..0395601 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -172,7 +172,7 @@ static int testSocketTCPAccept(const void *opaque) if (virNetSocketNewConnectTCP(data->cnode, portstr, &csock) < 0) goto cleanup; - virNetSocketFree(csock); + virObjectUnref(csock); for (i = 0 ; i < nlsock ; i++) { if (virNetSocketAccept(lsock[i], &ssock) != -1 && ssock) { @@ -183,16 +183,16 @@ static int testSocketTCPAccept(const void *opaque) goto cleanup; } } - virNetSocketFree(ssock); + virObjectUnref(ssock); ssock = NULL; } ret = 0; cleanup: - virNetSocketFree(ssock); + virObjectUnref(ssock); for (i = 0 ; i < nlsock ; i++) - virNetSocketFree(lsock[i]); + virObjectUnref(lsock[i]); VIR_FREE(lsock); return ret; } @@ -228,7 +228,7 @@ static int testSocketUNIXAccept(const void *data ATTRIBUTE_UNUSED) if (virNetSocketNewConnectUNIX(path, false, NULL, &csock) < 0) goto cleanup; - virNetSocketFree(csock); + virObjectUnref(csock); if (virNetSocketAccept(lsock, &ssock) != -1) { char c = 'a'; @@ -242,8 +242,8 @@ static int testSocketUNIXAccept(const void *data ATTRIBUTE_UNUSED) cleanup: VIR_FREE(path); - virNetSocketFree(lsock); - virNetSocketFree(ssock); + virObjectUnref(lsock); + virObjectUnref(ssock); if (tmpdir) rmdir(tmpdir); return ret; @@ -320,9 +320,9 @@ static int testSocketUNIXAddrs(const void *data ATTRIBUTE_UNUSED) cleanup: VIR_FREE(path); - virNetSocketFree(lsock); - virNetSocketFree(ssock); - virNetSocketFree(csock); + virObjectUnref(lsock); + virObjectUnref(ssock); + virObjectUnref(csock); if (tmpdir) rmdir(tmpdir); return ret; @@ -352,7 +352,7 @@ static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) ret = 0; cleanup: - virNetSocketFree(csock); + virObjectUnref(csock); return ret; } @@ -375,7 +375,7 @@ static int testSocketCommandFail(const void *data ATTRIBUTE_UNUSED) ret = 0; cleanup: - virNetSocketFree(csock); + virObjectUnref(csock); return ret; } @@ -444,7 +444,7 @@ static int testSocketSSH(const void *opaque) ret = 0; cleanup: - virNetSocketFree(csock); + virObjectUnref(csock); return ret; } -- 1.7.10.4
On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Make virSocket use the virObject APIs for reference counting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_private.syms | 1 - src/libvirt_probes.d | 4 +-- src/qemu/qemu_migration.c | 4 +-- src/rpc/virnetclient.c | 4 +-- src/rpc/virnetserverclient.c | 4 +-- src/rpc/virnetserverservice.c | 4 +-- src/rpc/virnetsocket.c | 71 ++++++++++++++++++----------------------- src/rpc/virnetsocket.h | 3 +- tests/virnetsockettest.c | 26 +++++++-------- 9 files changed, 54 insertions(+), 67 deletions(-)
Missing two changes. ACK with this squashed in: diff --git i/cfg.mk w/cfg.mk index dc39646..535d67b 100644 --- i/cfg.mk +++ w/cfg.mk @@ -155,7 +155,6 @@ useless_free_options = \ --name=virNetServerMDNSGroupFree \ --name=virNetServerProgramFree \ --name=virNetServerServiceFree \ - --name=virNetSocketFree \ --name=virNWFilterDefFree \ --name=virNWFilterEntryFree \ --name=virNWFilterHashTableFree \ diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms index 5a58f3f..5ee4eba 100644 --- i/src/libvirt_private.syms +++ w/src/libvirt_private.syms @@ -1596,7 +1596,6 @@ virNetSocketNewListenTCP; virNetSocketNewListenUNIX; virNetSocketRead; virNetSocketRecvFD; -virNetSocketRef; virNetSocketRemoteAddrString; virNetSocketRemoveIOCallback; virNetSocketSendFD; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Make all the virNetServer* objects use the virObject APIs for reference counting Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 22 ++++----- daemon/stream.c | 19 ++------ src/libvirt_private.syms | 7 --- src/libvirt_probes.d | 4 +- src/lxc/lxc_controller.c | 8 +-- src/rpc/virnetserver.c | 79 ++++++++++++++---------------- src/rpc/virnetserver.h | 5 +- src/rpc/virnetserverclient.c | 108 ++++++++++++++++++----------------------- src/rpc/virnetserverclient.h | 5 +- src/rpc/virnetserverprogram.c | 49 ++++++++++--------- src/rpc/virnetserverprogram.h | 8 +-- src/rpc/virnetserverservice.c | 83 +++++++++++++++---------------- src/rpc/virnetserverservice.h | 5 +- 13 files changed, 172 insertions(+), 230 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 7dd7d5c..fa9e7e8 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -568,10 +568,10 @@ static int daemonSetupNetworking(virNetServerPtr srv, return 0; error: - virNetServerServiceFree(svcTLS); - virNetServerServiceFree(svcTCP); - virNetServerServiceFree(svc); - virNetServerServiceFree(svcRO); + virObjectUnref(svcTLS); + virObjectUnref(svcTCP); + virObjectUnref(svc); + virObjectUnref(svcRO); return -1; } @@ -759,21 +759,21 @@ static void daemonRunStateInit(void *opaque) VIR_ERROR(_("Driver state initialization failed")); /* Ensure the main event loop quits */ kill(getpid(), SIGTERM); - virNetServerFree(srv); + virObjectUnref(srv); return; } /* Only now accept clients from network */ virNetServerUpdateServices(srv, true); - virNetServerFree(srv); + virObjectUnref(srv); } static int daemonStateInit(virNetServerPtr srv) { virThread thr; - virNetServerRef(srv); + virObjectRef(srv); if (virThreadCreate(&thr, false, daemonRunStateInit, srv) < 0) { - virNetServerFree(srv); + virObjectUnref(srv); return -1; } return 0; @@ -1325,10 +1325,10 @@ int main(int argc, char **argv) { cleanup: virNetlinkEventServiceStop(); - virNetServerProgramFree(remoteProgram); - virNetServerProgramFree(qemuProgram); + virObjectUnref(remoteProgram); + virObjectUnref(qemuProgram); virNetServerClose(srv); - virNetServerFree(srv); + virObjectUnref(srv); virNetlinkShutdown(); if (statuswrite != -1) { if (ret != 0) { diff --git a/daemon/stream.c b/daemon/stream.c index 6f26ee5..fe5cae5 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -104,14 +104,6 @@ daemonStreamMessageFinished(virNetMessagePtr msg ATTRIBUTE_UNUSED, } -static void -daemonStreamEventFreeFunc(void *opaque) -{ - virNetServerClientPtr client = opaque; - - virNetServerClientFree(client); -} - /* * Callback that gets invoked when a stream becomes writable/readable */ @@ -332,14 +324,12 @@ daemonCreateClientStream(virNetServerClientPtr client, stream->refs = 1; stream->priv = priv; - stream->prog = prog; + stream->prog = virObjectRef(prog); stream->procedure = header->proc; stream->serial = header->serial; stream->filterID = -1; stream->st = st; - virNetServerProgramRef(prog); - return stream; } @@ -365,7 +355,7 @@ int daemonFreeClientStream(virNetServerClientPtr client, VIR_DEBUG("client=%p, proc=%d, serial=%d", client, stream->procedure, stream->serial); - virNetServerProgramFree(stream->prog); + virObjectUnref(stream->prog); msg = stream->rx; while (msg) { @@ -411,10 +401,11 @@ int daemonAddClientStream(virNetServerClientPtr client, if (virStreamEventAddCallback(stream->st, 0, daemonStreamEvent, client, - daemonStreamEventFreeFunc) < 0) + virObjectFreeCallback) < 0) return -1; - virNetServerClientRef(client); + virObjectRef(client); + if ((stream->filterID = virNetServerClientAddFilter(client, daemonStreamFilter, stream)) < 0) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 73a1ec1..a3f3f6b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1499,14 +1499,11 @@ virNetServerAddService; virNetServerAddSignalHandler; virNetServerAutoShutdown; virNetServerClose; -virNetServerFree; virNetServerIsPrivileged; virNetServerKeepAliveRequired; virNetServerNew; virNetServerQuit; -virNetServerRef; virNetServerRun; -virNetServerServiceFree; virNetServerServiceNewTCP; virNetServerServiceNewUNIX; virNetServerSetTLSContext; @@ -1517,7 +1514,6 @@ virNetServerUpdateServices; virNetServerClientAddFilter; virNetServerClientClose; virNetServerClientDelayedClose; -virNetServerClientFree; virNetServerClientGetAuth; virNetServerClientGetFD; virNetServerClientGetIdentity; @@ -1534,7 +1530,6 @@ virNetServerClientIsSecure; virNetServerClientLocalAddrString; virNetServerClientNeedAuth; virNetServerClientNew; -virNetServerClientRef; virNetServerClientRemoteAddrString; virNetServerClientRemoveFilter; virNetServerClientSendMessage; @@ -1562,13 +1557,11 @@ virNetServerMDNSStop; # virnetserverprogram.h virNetServerProgramDispatch; -virNetServerProgramFree; virNetServerProgramGetID; virNetServerProgramGetPriority; virNetServerProgramGetVersion; virNetServerProgramMatches; virNetServerProgramNew; -virNetServerProgramRef; virNetServerProgramSendReplyError; virNetServerProgramSendStreamData; virNetServerProgramSendStreamError; diff --git a/src/libvirt_probes.d b/src/libvirt_probes.d index be1d938..27f4e9a 100644 --- a/src/libvirt_probes.d +++ b/src/libvirt_probes.d @@ -32,9 +32,7 @@ provider libvirt { # file: src/rpc/virnetserverclient.c # prefix: rpc - probe rpc_server_client_new(void *client, int refs, void *sock); - probe rpc_server_client_ref(void *client, int refs); - probe rpc_server_client_free(void *client, int refs); + probe rpc_server_client_new(void *client, void *sock); probe rpc_server_client_msg_tx_queue(void *client, int len, int prog, int vers, int proc, int type, int status, int serial); probe rpc_server_client_msg_rx(void *client, int len, int prog, int vers, int proc, int type, int status, int serial); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 56ed7d3..9bc8603 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -264,7 +264,7 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) if (ctrl->timerShutdown != -1) virEventRemoveTimeout(ctrl->timerShutdown); - virNetServerFree(ctrl->server); + virObjectUnref(ctrl->server); VIR_FREE(ctrl); } @@ -620,7 +620,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) if (virNetServerAddService(ctrl->server, svc, NULL) < 0) goto error; - virNetServerServiceFree(svc); + virObjectUnref(svc); svc = NULL; if (!(ctrl->prog = virNetServerProgramNew(VIR_LXC_PROTOCOL_PROGRAM, @@ -635,9 +635,9 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) error: VIR_FREE(sockpath); - virNetServerFree(ctrl->server); + virObjectUnref(ctrl->server); ctrl->server = NULL; - virNetServerServiceFree(svc); + virObjectUnref(svc); return -1; } diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index e9f500e..ecab0e0 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -66,7 +66,7 @@ struct _virNetServerJob { }; struct _virNetServer { - int refs; + virObjectPtr object; virMutex lock; @@ -113,6 +113,22 @@ struct _virNetServer { }; +static virClassPtr virNetServerClass; +static void virNetServerDispose(void *obj); + +static int virNetServerOnceInit(void) +{ + if (!(virNetServerClass = virClassNew("virNetServer", + sizeof(virNetServer), + virNetServerDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetServer) + + static void virNetServerLock(virNetServerPtr srv) { virMutexLock(&srv->lock); @@ -179,18 +195,18 @@ static void virNetServerHandleJob(void *jobOpaque, void *opaque) goto error; virNetServerLock(srv); - virNetServerProgramFree(job->prog); + virObjectUnref(job->prog); virNetServerUnlock(srv); - virNetServerClientFree(job->client); + virObjectUnref(job->client); VIR_FREE(job); return; error: - virNetServerProgramFree(job->prog); + virObjectUnref(job->prog); virNetMessageFree(job->msg); virNetServerClientClose(job->client); - virNetServerClientFree(job->client); + virObjectUnref(job->client); VIR_FREE(job); } @@ -227,7 +243,7 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, job->msg = msg; if (prog) { - virNetServerProgramRef(prog); + virObjectRef(prog); job->prog = prog; priority = virNetServerProgramGetPriority(prog, msg->header.proc); } @@ -236,7 +252,7 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, if (ret < 0) { VIR_FREE(job); - virNetServerProgramFree(prog); + virObjectUnref(prog); } } else { ret = virNetServerProcessMsg(srv, client, prog, msg); @@ -276,7 +292,7 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc ATTRIBUTE_UN goto error; } srv->clients[srv->nclients-1] = client; - virNetServerClientRef(client); + virObjectRef(client); virNetServerClientSetDispatcher(client, virNetServerDispatchNewMessage, @@ -336,12 +352,11 @@ virNetServerPtr virNetServerNew(size_t min_workers, virNetServerPtr srv; struct sigaction sig_action; - if (VIR_ALLOC(srv) < 0) { - virReportOOMError(); + if (virNetServerInitialize() < 0) return NULL; - } - srv->refs = 1; + if (!(srv = virObjectNew(virNetServerClass))) + return NULL; if (max_workers && !(srv->workers = virThreadPoolNew(min_workers, max_workers, @@ -404,24 +419,14 @@ virNetServerPtr virNetServerNew(size_t min_workers, sigaction(SIGUSR2, &sig_action, NULL); #endif - VIR_DEBUG("srv=%p refs=%d", srv, srv->refs); return srv; error: - virNetServerFree(srv); + virObjectUnref(srv); return NULL; } -void virNetServerRef(virNetServerPtr srv) -{ - virNetServerLock(srv); - srv->refs++; - VIR_DEBUG("srv=%p refs=%d", srv, srv->refs); - virNetServerUnlock(srv); -} - - bool virNetServerIsPrivileged(virNetServerPtr srv) { bool priv; @@ -611,7 +616,7 @@ int virNetServerAddService(virNetServerPtr srv, #endif srv->services[srv->nservices-1] = svc; - virNetServerServiceRef(svc); + virObjectRef(svc); virNetServerServiceSetDispatcher(svc, virNetServerDispatchNewClient, @@ -637,8 +642,7 @@ int virNetServerAddProgram(virNetServerPtr srv, if (VIR_EXPAND_N(srv->programs, srv->nprograms, 1) < 0) goto no_memory; - srv->programs[srv->nprograms-1] = prog; - virNetServerProgramRef(prog); + srv->programs[srv->nprograms-1] = virObjectRef(prog); virNetServerUnlock(srv); return 0; @@ -749,7 +753,7 @@ void virNetServerRun(virNetServerPtr srv) if (virNetServerClientWantClose(srv->clients[i])) virNetServerClientClose(srv->clients[i]); if (virNetServerClientIsClosed(srv->clients[i])) { - virNetServerClientFree(srv->clients[i]); + virObjectUnref(srv->clients[i]); if (srv->nclients > 1) { memmove(srv->clients + i, srv->clients + i + 1, @@ -780,20 +784,10 @@ void virNetServerQuit(virNetServerPtr srv) virNetServerUnlock(srv); } -void virNetServerFree(virNetServerPtr srv) +void virNetServerDispose(void *obj) { + virNetServerPtr srv = obj; int i; - int refs; - - if (!srv) - return; - - virNetServerLock(srv); - VIR_DEBUG("srv=%p refs=%d", srv, srv->refs); - refs = --srv->refs; - virNetServerUnlock(srv); - if (refs > 0) - return; for (i = 0 ; i < srv->nservices ; i++) virNetServerServiceToggle(srv->services[i], false); @@ -811,16 +805,16 @@ void virNetServerFree(virNetServerPtr srv) virEventRemoveHandle(srv->sigwatch); for (i = 0 ; i < srv->nservices ; i++) - virNetServerServiceFree(srv->services[i]); + virObjectUnref(srv->services[i]); VIR_FREE(srv->services); for (i = 0 ; i < srv->nprograms ; i++) - virNetServerProgramFree(srv->programs[i]); + virObjectUnref(srv->programs[i]); VIR_FREE(srv->programs); for (i = 0 ; i < srv->nclients ; i++) { virNetServerClientClose(srv->clients[i]); - virNetServerClientFree(srv->clients[i]); + virObjectUnref(srv->clients[i]); } VIR_FREE(srv->clients); @@ -830,7 +824,6 @@ void virNetServerFree(virNetServerPtr srv) #endif virMutexDestroy(&srv->lock); - VIR_FREE(srv); } void virNetServerClose(virNetServerPtr srv) diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 92f741a..7dc52ca 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -30,6 +30,7 @@ # include "virnetserverprogram.h" # include "virnetserverclient.h" # include "virnetserverservice.h" +# include "virobject.h" typedef int (*virNetServerClientInitHook)(virNetServerPtr srv, virNetServerClientPtr client, @@ -48,8 +49,6 @@ virNetServerPtr virNetServerNew(size_t min_workers, typedef int (*virNetServerAutoShutdownFunc)(virNetServerPtr srv, void *opaque); -void virNetServerRef(virNetServerPtr srv); - bool virNetServerIsPrivileged(virNetServerPtr srv); void virNetServerAutoShutdown(virNetServerPtr srv, @@ -81,8 +80,6 @@ void virNetServerRun(virNetServerPtr srv); void virNetServerQuit(virNetServerPtr srv); -void virNetServerFree(virNetServerPtr srv); - void virNetServerClose(virNetServerPtr srv); bool virNetServerKeepAliveRequired(virNetServerPtr srv); diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 85f7c88..d135b0f 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -57,7 +57,8 @@ struct _virNetServerClientFilter { struct _virNetServerClient { - int refs; + virObject object; + bool wantClose; bool delayedClose; virMutex lock; @@ -103,6 +104,22 @@ struct _virNetServerClient }; +static virClassPtr virNetServerClientClass; +static void virNetServerClientDispose(void *obj); + +static int virNetServerClientOnceInit(void) +{ + if (!(virNetServerClientClass = virClassNew("virNetServerClient", + sizeof(virNetServerClient), + virNetServerClientDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetServerClient) + + static void virNetServerClientDispatchEvent(virNetSocketPtr sock, int events, void *opaque); static void virNetServerClientUpdateEvent(virNetServerClientPtr client); static void virNetServerClientDispatchRead(virNetServerClientPtr client); @@ -167,13 +184,6 @@ virNetServerClientCalculateHandleMode(virNetServerClientPtr client) { return mode; } -static void virNetServerClientEventFree(void *opaque) -{ - virNetServerClientPtr client = opaque; - - virNetServerClientFree(client); -} - /* * @server: a locked or unlocked server object * @client: a locked client object @@ -182,15 +192,17 @@ static int virNetServerClientRegisterEvent(virNetServerClientPtr client) { int mode = virNetServerClientCalculateHandleMode(client); - client->refs++; + if (!client->sock) + return -1; + + virObjectRef(client); VIR_DEBUG("Registering client event callback %d", mode); - if (!client->sock || - virNetSocketAddIOCallback(client->sock, + if (virNetSocketAddIOCallback(client->sock, mode, virNetServerClientDispatchEvent, client, - virNetServerClientEventFree) < 0) { - client->refs--; + virObjectFreeCallback) < 0) { + virObjectUnref(client); return -1; } @@ -334,15 +346,17 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, VIR_DEBUG("sock=%p auth=%d tls=%p", sock, auth, tls); - if (VIR_ALLOC(client) < 0) { - virReportOOMError(); + if (virNetServerClientInitialize() < 0) return NULL; - } - if (virMutexInit(&client->lock) < 0) - goto error; + if (!(client = virObjectNew(virNetServerClientClass))) + return NULL; + + if (virMutexInit(&client->lock) < 0) { + VIR_FREE(client); + return NULL; + } - client->refs = 1; client->sock = sock; client->auth = auth; client->readonly = readonly; @@ -365,28 +379,18 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, client->nrequests = 1; PROBE(RPC_SERVER_CLIENT_NEW, - "client=%p refs=%d sock=%p", - client, client->refs, client->sock); + "client=%p sock=%p", + client, client->sock); return client; error: /* XXX ref counting is better than this */ client->sock = NULL; /* Caller owns 'sock' upon failure */ - virNetServerClientFree(client); + virObjectUnref(client); return NULL; } -void virNetServerClientRef(virNetServerClientPtr client) -{ - virNetServerClientLock(client); - client->refs++; - PROBE(RPC_SERVER_CLIENT_REF, - "client=%p refs=%d", - client, client->refs); - virNetServerClientUnlock(client); -} - int virNetServerClientGetAuth(virNetServerClientPtr client) { @@ -568,21 +572,9 @@ const char *virNetServerClientRemoteAddrString(virNetServerClientPtr client) } -void virNetServerClientFree(virNetServerClientPtr client) +void virNetServerClientDispose(void *obj) { - if (!client) - return; - - virNetServerClientLock(client); - PROBE(RPC_SERVER_CLIENT_FREE, - "client=%p refs=%d", - client, client->refs); - - client->refs--; - if (client->refs > 0) { - virNetServerClientUnlock(client); - return; - } + virNetServerClientPtr client = obj; if (client->privateData && client->privateDataFreeFunc) @@ -599,7 +591,6 @@ void virNetServerClientFree(virNetServerClientPtr client) virObjectUnref(client->sock); virNetServerClientUnlock(client); virMutexDestroy(&client->lock); - VIR_FREE(client); } @@ -617,7 +608,7 @@ void virNetServerClientClose(virNetServerClientPtr client) virKeepAlivePtr ka; virNetServerClientLock(client); - VIR_DEBUG("client=%p refs=%d", client, client->refs); + VIR_DEBUG("client=%p", client); if (!client->sock) { virNetServerClientUnlock(client); return; @@ -627,20 +618,20 @@ void virNetServerClientClose(virNetServerClientPtr client) virKeepAliveStop(client->keepalive); ka = client->keepalive; client->keepalive = NULL; - client->refs++; + virObjectRef(client); virNetServerClientUnlock(client); virObjectUnref(ka); virNetServerClientLock(client); - client->refs--; + virObjectUnref(client); } if (client->privateDataCloseFunc) { cf = client->privateDataCloseFunc; - client->refs++; + virObjectRef(client); virNetServerClientUnlock(client); (cf)(client); virNetServerClientLock(client); - client->refs--; + virObjectUnref(client); } /* Do now, even though we don't close the socket @@ -904,12 +895,12 @@ readmore: /* Send off to for normal dispatch to workers */ if (msg) { - client->refs++; + virObjectRef(client); if (!client->dispatchFunc || client->dispatchFunc(client, msg, client->dispatchOpaque) < 0) { virNetMessageFree(msg); client->wantClose = true; - client->refs--; + virObjectUnref(client); return; } } @@ -1168,11 +1159,6 @@ virNetServerClientKeepAliveSendCB(void *opaque, return virNetServerClientSendMessage(opaque, msg); } -static void -virNetServerClientFreeCB(void *opaque) -{ - virNetServerClientFree(opaque); -} int virNetServerClientInitKeepAlive(virNetServerClientPtr client, @@ -1187,10 +1173,10 @@ virNetServerClientInitKeepAlive(virNetServerClientPtr client, if (!(ka = virKeepAliveNew(interval, count, client, virNetServerClientKeepAliveSendCB, virNetServerClientKeepAliveDeadCB, - virNetServerClientFreeCB))) + virObjectFreeCallback))) goto cleanup; /* keepalive object has a reference to client */ - client->refs++; + virObjectRef(client); client->keepalive = ka; ka = NULL; diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 606c428..a1ff19b 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -26,6 +26,7 @@ # include "virnetsocket.h" # include "virnetmessage.h" +# include "virobject.h" typedef struct _virNetServerClient virNetServerClient; typedef virNetServerClient *virNetServerClientPtr; @@ -73,8 +74,6 @@ const char *virNetServerClientGetIdentity(virNetServerClientPtr client); int virNetServerClientGetUNIXIdentity(virNetServerClientPtr client, uid_t *uid, gid_t *gid, pid_t *pid); -void virNetServerClientRef(virNetServerClientPtr client); - typedef void (*virNetServerClientFreeFunc)(void *data); void virNetServerClientSetPrivateData(virNetServerClientPtr client, @@ -114,7 +113,5 @@ int virNetServerClientSendMessage(virNetServerClientPtr client, bool virNetServerClientNeedAuth(virNetServerClientPtr client); -void virNetServerClientFree(virNetServerClientPtr client); - #endif /* __VIR_NET_SERVER_CLIENT_H__ */ diff --git a/src/rpc/virnetserverprogram.c b/src/rpc/virnetserverprogram.c index c083fb3..d13b621 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -30,11 +30,12 @@ #include "virterror_internal.h" #include "logging.h" #include "virfile.h" +#include "threads.h" #define VIR_FROM_THIS VIR_FROM_RPC struct _virNetServerProgram { - int refs; + virObject object; unsigned program; unsigned version; @@ -42,6 +43,23 @@ struct _virNetServerProgram { size_t nprocs; }; + +static virClassPtr virNetServerProgramClass; +static void virNetServerProgramDispose(void *obj); + +static int virNetServerProgramOnceInit(void) +{ + if (!(virNetServerProgramClass = virClassNew("virNetServerProgram", + sizeof(virNetServerProgram), + virNetServerProgramDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetServerProgram) + + virNetServerProgramPtr virNetServerProgramNew(unsigned program, unsigned version, virNetServerProgramProcPtr procs, @@ -49,18 +67,18 @@ virNetServerProgramPtr virNetServerProgramNew(unsigned program, { virNetServerProgramPtr prog; - if (VIR_ALLOC(prog) < 0) { - virReportOOMError(); + if (virNetServerProgramInitialize() < 0) + return NULL; + + if (!(prog = virObjectNew(virNetServerProgramClass))) return NULL; - } - prog->refs = 1; prog->program = program; prog->version = version; prog->procs = procs; prog->nprocs = nprocs; - VIR_DEBUG("prog=%p refs=%d", prog, prog->refs); + VIR_DEBUG("prog=%p", prog); return prog; } @@ -78,13 +96,6 @@ int virNetServerProgramGetVersion(virNetServerProgramPtr prog) } -void virNetServerProgramRef(virNetServerProgramPtr prog) -{ - prog->refs++; - VIR_DEBUG("prog=%p refs=%d", prog, prog->refs); -} - - int virNetServerProgramMatches(virNetServerProgramPtr prog, virNetMessagePtr msg) { @@ -516,16 +527,6 @@ int virNetServerProgramSendStreamData(virNetServerProgramPtr prog, } -void virNetServerProgramFree(virNetServerProgramPtr prog) +void virNetServerProgramDispose(void *obj ATTRIBUTE_UNUSED) { - if (!prog) - return; - - VIR_DEBUG("prog=%p refs=%d", prog, prog->refs); - - prog->refs--; - if (prog->refs > 0) - return; - - VIR_FREE(prog); } diff --git a/src/rpc/virnetserverprogram.h b/src/rpc/virnetserverprogram.h index 015ecef..99e1661 100644 --- a/src/rpc/virnetserverprogram.h +++ b/src/rpc/virnetserverprogram.h @@ -26,6 +26,7 @@ # include "virnetmessage.h" # include "virnetserverclient.h" +# include "virobject.h" typedef struct _virNetServer virNetServer; typedef virNetServer *virNetServerPtr; @@ -67,8 +68,6 @@ int virNetServerProgramGetVersion(virNetServerProgramPtr prog); unsigned int virNetServerProgramGetPriority(virNetServerProgramPtr prog, int procedure); -void virNetServerProgramRef(virNetServerProgramPtr prog); - int virNetServerProgramMatches(virNetServerProgramPtr prog, virNetMessagePtr msg); @@ -102,9 +101,4 @@ int virNetServerProgramSendStreamData(virNetServerProgramPtr prog, const char *data, size_t len); -void virNetServerProgramFree(virNetServerProgramPtr prog); - - - - #endif /* __VIR_NET_SERVER_PROGRAM_H__ */ diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 93c0574..000b5b6 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -27,12 +27,12 @@ #include "memory.h" #include "virterror_internal.h" - +#include "threads.h" #define VIR_FROM_THIS VIR_FROM_RPC struct _virNetServerService { - int refs; + virObject object; size_t nsocks; virNetSocketPtr *socks; @@ -48,6 +48,21 @@ struct _virNetServerService { }; +static virClassPtr virNetServerServiceClass; +static void virNetServerServiceDispose(void *obj); + +static int virNetServerServiceOnceInit(void) +{ + if (!(virNetServerServiceClass = virClassNew("virNetServerService", + sizeof(virNetServerService), + virNetServerServiceDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetServerService) + static void virNetServerServiceAccept(virNetSocketPtr sock, int events ATTRIBUTE_UNUSED, @@ -76,7 +91,7 @@ static void virNetServerServiceAccept(virNetSocketPtr sock, if (svc->dispatchFunc(svc, client, svc->dispatchOpaque) < 0) virNetServerClientClose(client); - virNetServerClientFree(client); + virObjectUnref(client); cleanup: return; @@ -84,21 +99,13 @@ cleanup: error: if (client) { virNetServerClientClose(client); - virNetServerClientFree(client); + virObjectUnref(client); } else { virObjectUnref(clientsock); } } -static void virNetServerServiceEventFree(void *opaque) -{ - virNetServerServicePtr svc = opaque; - - virNetServerServiceFree(svc); -} - - virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, const char *service, int auth, @@ -109,10 +116,12 @@ virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, virNetServerServicePtr svc; size_t i; - if (VIR_ALLOC(svc) < 0) - goto no_memory; + if (virNetServerServiceInitialize() < 0) + return NULL; + + if (!(svc = virObjectNew(virNetServerServiceClass))) + return NULL; - svc->refs = 1; svc->auth = auth; svc->readonly = readonly; svc->nrequests_client_max = nrequests_client_max; @@ -130,13 +139,13 @@ virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, /* IO callback is initially disabled, until we're ready * to deal with incoming clients */ - virNetServerServiceRef(svc); + virObjectRef(svc); if (virNetSocketAddIOCallback(svc->socks[i], 0, virNetServerServiceAccept, svc, - virNetServerServiceEventFree) < 0) { - virNetServerServiceFree(svc); + virObjectFreeCallback) < 0) { + virObjectUnref(svc); goto error; } } @@ -144,10 +153,8 @@ virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, return svc; -no_memory: - virReportOOMError(); error: - virNetServerServiceFree(svc); + virObjectUnref(svc); return NULL; } @@ -163,10 +170,12 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, virNetServerServicePtr svc; int i; - if (VIR_ALLOC(svc) < 0) - goto no_memory; + if (virNetServerServiceInitialize() < 0) + return NULL; + + if (!(svc = virObjectNew(virNetServerServiceClass))) + return NULL; - svc->refs = 1; svc->auth = auth; svc->readonly = readonly; svc->nrequests_client_max = nrequests_client_max; @@ -189,13 +198,13 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, /* IO callback is initially disabled, until we're ready * to deal with incoming clients */ - virNetServerServiceRef(svc); + virObjectRef(svc); if (virNetSocketAddIOCallback(svc->socks[i], 0, virNetServerServiceAccept, svc, - virNetServerServiceEventFree) < 0) { - virNetServerServiceFree(svc); + virObjectFreeCallback) < 0) { + virObjectUnref(svc); goto error; } } @@ -206,7 +215,7 @@ virNetServerServicePtr virNetServerServiceNewUNIX(const char *path, no_memory: virReportOOMError(); error: - virNetServerServiceFree(svc); + virObjectUnref(svc); return NULL; } @@ -231,12 +240,6 @@ bool virNetServerServiceIsReadonly(virNetServerServicePtr svc) } -void virNetServerServiceRef(virNetServerServicePtr svc) -{ - svc->refs++; -} - - void virNetServerServiceSetDispatcher(virNetServerServicePtr svc, virNetServerServiceDispatchFunc func, void *opaque) @@ -246,24 +249,16 @@ void virNetServerServiceSetDispatcher(virNetServerServicePtr svc, } -void virNetServerServiceFree(virNetServerServicePtr svc) +void virNetServerServiceDispose(void *obj) { + virNetServerServicePtr svc = obj; int i; - if (!svc) - return; - - svc->refs--; - if (svc->refs > 0) - return; - for (i = 0 ; i < svc->nsocks ; i++) virObjectUnref(svc->socks[i]); VIR_FREE(svc->socks); virObjectUnref(svc->tls); - - VIR_FREE(svc); } void virNetServerServiceToggle(virNetServerServicePtr svc, diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h index d6bf98b..98fd396 100644 --- a/src/rpc/virnetserverservice.h +++ b/src/rpc/virnetserverservice.h @@ -25,6 +25,7 @@ # define __VIR_NET_SERVER_SERVICE_H__ # include "virnetserverprogram.h" +# include "virobject.h" enum { VIR_NET_SERVER_SERVICE_AUTH_NONE = 0, @@ -55,14 +56,10 @@ int virNetServerServiceGetPort(virNetServerServicePtr svc); int virNetServerServiceGetAuth(virNetServerServicePtr svc); bool virNetServerServiceIsReadonly(virNetServerServicePtr svc); -void virNetServerServiceRef(virNetServerServicePtr svc); - void virNetServerServiceSetDispatcher(virNetServerServicePtr svc, virNetServerServiceDispatchFunc func, void *opaque); -void virNetServerServiceFree(virNetServerServicePtr svc); - void virNetServerServiceToggle(virNetServerServicePtr svc, bool enabled); -- 1.7.10.4
On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Make all the virNetServer* objects use the virObject APIs for reference counting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 22 ++++----- daemon/stream.c | 19 ++------ src/libvirt_private.syms | 7 --- src/libvirt_probes.d | 4 +- src/lxc/lxc_controller.c | 8 +-- src/rpc/virnetserver.c | 79 ++++++++++++++---------------- src/rpc/virnetserver.h | 5 +- src/rpc/virnetserverclient.c | 108 ++++++++++++++++++----------------------- src/rpc/virnetserverclient.h | 5 +- src/rpc/virnetserverprogram.c | 49 ++++++++++--------- src/rpc/virnetserverprogram.h | 8 +-- src/rpc/virnetserverservice.c | 83 +++++++++++++++---------------- src/rpc/virnetserverservice.h | 5 +- 13 files changed, 172 insertions(+), 230 deletions(-)
I hit a minor merge conflict when applying this, thanks to my recent HAVE_AVAHI stuff, but shouldn't be too hard for you to figure out.
@@ -227,7 +243,7 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, job->msg = msg;
if (prog) { - virNetServerProgramRef(prog); + virObjectRef(prog); job->prog = prog;
You could collapse these two lines into one if you want.
+++ b/src/rpc/virnetserverclient.h @@ -26,6 +26,7 @@
# include "virnetsocket.h" # include "virnetmessage.h" +# include "virobject.h"
Another place where I wonder if this would be better in the .c instead of the .h.
-void virNetServerProgramFree(virNetServerProgramPtr prog) +void virNetServerProgramDispose(void *obj ATTRIBUTE_UNUSED) { - if (!prog) - return; - - VIR_DEBUG("prog=%p refs=%d", prog, prog->refs); - - prog->refs--; - if (prog->refs > 0) - return; - - VIR_FREE(prog); }
This is a no-op; is it worth passing NULL instead of wasting space in the executable for the stub function?
+++ b/src/rpc/virnetserverservice.h @@ -25,6 +25,7 @@ # define __VIR_NET_SERVER_SERVICE_H__
# include "virnetserverprogram.h" +# include "virobject.h"
And another possible candidate for .c instead of .h. Meanwhile, I noticed a couple missing changes, some required for 'make check' to be happy; ACK once you squash this in: diff --git i/cfg.mk w/cfg.mk index 535d67b..64af1ee 100644 --- i/cfg.mk +++ w/cfg.mk @@ -148,13 +148,9 @@ useless_free_options = \ --name=virNetClientFree \ --name=virNetClientProgramFree \ --name=virNetClientStreamFree \ - --name=virNetServerFree \ - --name=virNetServerClientFree \ --name=virNetServerMDNSFree \ --name=virNetServerMDNSEntryFree \ --name=virNetServerMDNSGroupFree \ - --name=virNetServerProgramFree \ - --name=virNetServerServiceFree \ --name=virNWFilterDefFree \ --name=virNWFilterEntryFree \ --name=virNWFilterHashTableFree \ diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms index e071fe1..0543005 100644 --- i/src/libvirt_private.syms +++ w/src/libvirt_private.syms @@ -1555,13 +1555,11 @@ virNetServerProgramUnknownError; # virnetserverservice.h virNetServerServiceClose; -virNetServerServiceFree; virNetServerServiceGetAuth; virNetServerServiceGetPort; virNetServerServiceIsReadonly; virNetServerServiceNewTCP; virNetServerServiceNewUNIX; -virNetServerServiceRef; virNetServerServiceSetDispatcher; virNetServerServiceToggle; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> Make all the virNetClient* objects use virObject APIs for reference counting Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_probes.d | 4 +- src/lxc/lxc_monitor.c | 4 +- src/remote/remote_driver.c | 20 ++++----- src/rpc/gendispatch.pl | 4 +- src/rpc/virnetclient.c | 96 +++++++++++++++++------------------------ src/rpc/virnetclient.h | 4 +- src/rpc/virnetclientprogram.c | 43 +++++++++--------- src/rpc/virnetclientprogram.h | 5 +-- src/rpc/virnetclientstream.c | 65 +++++++++++++--------------- src/rpc/virnetclientstream.h | 5 +-- 10 files changed, 110 insertions(+), 140 deletions(-) diff --git a/src/libvirt_probes.d b/src/libvirt_probes.d index 27f4e9a..9343fa4 100644 --- a/src/libvirt_probes.d +++ b/src/libvirt_probes.d @@ -40,9 +40,7 @@ provider libvirt { # file: src/rpc/virnetclient.c # prefix: rpc - probe rpc_client_new(void *client, int refs, void *sock); - probe rpc_client_ref(void *client, int refs); - probe rpc_client_free(void *client, int refs); + probe rpc_client_new(void *client, void *sock); probe rpc_client_msg_tx_queue(void *client, int len, int prog, int vers, int proc, int type, int status, int serial); probe rpc_client_msg_rx(void *client, int len, int prog, int vers, int proc, int type, int status, int serial); diff --git a/src/lxc/lxc_monitor.c b/src/lxc/lxc_monitor.c index a2a2599..a81d0f7 100644 --- a/src/lxc/lxc_monitor.c +++ b/src/lxc/lxc_monitor.c @@ -170,7 +170,7 @@ static void virLXCMonitorFree(virLXCMonitorPtr mon) if (mon->cb && mon->cb->destroy) (mon->cb->destroy)(mon, mon->vm); virMutexDestroy(&mon->lock); - virNetClientProgramFree(mon->program); + virObjectUnref(mon->program); VIR_FREE(mon); } @@ -199,7 +199,7 @@ void virLXCMonitorClose(virLXCMonitorPtr mon) { if (mon->client) { virNetClientClose(mon->client); - virNetClientFree(mon->client); + virObjectUnref(mon->client); mon->client = NULL; } } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 38b11e1..353a153 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -792,10 +792,10 @@ doRemoteOpen (virConnectPtr conn, virReportOOMError(); failed: - virNetClientProgramFree(priv->remoteProgram); - virNetClientProgramFree(priv->qemuProgram); + virObjectUnref(priv->remoteProgram); + virObjectUnref(priv->qemuProgram); virNetClientClose(priv->client); - virNetClientFree(priv->client); + virObjectUnref(priv->client); priv->client = NULL; VIR_FREE(priv->hostname); @@ -946,10 +946,10 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv) virObjectUnref(priv->tls); priv->tls = NULL; virNetClientClose(priv->client); - virNetClientFree(priv->client); + virObjectUnref(priv->client); priv->client = NULL; - virNetClientProgramFree(priv->remoteProgram); - virNetClientProgramFree(priv->qemuProgram); + virObjectUnref(priv->remoteProgram); + virObjectUnref(priv->qemuProgram); priv->remoteProgram = priv->qemuProgram = NULL; /* Free hostname copy */ @@ -4171,7 +4171,7 @@ remoteStreamFinish(virStreamPtr st) cleanup: virNetClientRemoveStream(priv->client, privst); - virNetClientStreamFree(privst); + virObjectUnref(privst); st->privateData = NULL; st->driver = NULL; @@ -4206,7 +4206,7 @@ remoteStreamAbort(virStreamPtr st) cleanup: virNetClientRemoveStream(priv->client, privst); - virNetClientStreamFree(privst); + virObjectUnref(privst); st->privateData = NULL; st->driver = NULL; @@ -4507,7 +4507,7 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn, goto done; if (virNetClientAddStream(priv->client, netst) < 0) { - virNetClientStreamFree(netst); + virObjectUnref(netst); goto done; } @@ -4525,7 +4525,7 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn, (xdrproc_t) xdr_remote_domain_migrate_prepare_tunnel3_args, (char *) &args, (xdrproc_t) xdr_remote_domain_migrate_prepare_tunnel3_ret, (char *) &ret) == -1) { virNetClientRemoveStream(priv->client, netst); - virNetClientStreamFree(netst); + virObjectUnref(netst); goto done; } diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 3a66445..12023e9 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1450,7 +1450,7 @@ elsif ($opt_k) { print " goto done;\n"; print "\n"; print " if (virNetClientAddStream(priv->client, netst) < 0) {\n"; - print " virNetClientStreamFree(netst);\n"; + print " virObjectUnref(netst);\n"; print " goto done;\n"; print " }"; print "\n"; @@ -1526,7 +1526,7 @@ elsif ($opt_k) { if ($call->{streamflag} ne "none") { print " virNetClientRemoveStream(priv->client, netst);\n"; - print " virNetClientStreamFree(netst);\n"; + print " virObjectUnref(netst);\n"; print " st->driver = NULL;\n"; print " st->privateData = NULL;\n"; } diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 9025221..2530ffa 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -63,7 +63,7 @@ struct _virNetClientCall { struct _virNetClient { - int refs; + virObject object; virMutex lock; @@ -109,6 +109,21 @@ struct _virNetClient { }; +static virClassPtr virNetClientClass; +static void virNetClientDispose(void *obj); + +static int virNetClientOnceInit(void) +{ + if (!(virNetClientClass = virClassNew("virNetClient", + sizeof(virNetClient), + virNetClientDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetClient) + static void virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetClientCallPtr thiscall); static int virNetClientQueueNonBlocking(virNetClientPtr client, @@ -237,13 +252,6 @@ static bool virNetClientCallMatchPredicate(virNetClientCallPtr head, } -static void virNetClientEventFree(void *opaque) -{ - virNetClientPtr client = opaque; - - virNetClientFree(client); -} - bool virNetClientKeepAliveIsSupported(virNetClientPtr client) { @@ -303,19 +311,22 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr sock, int wakeupFD[2] = { -1, -1 }; virKeepAlivePtr ka = NULL; + if (virNetClientInitialize() < 0) + return NULL; + if (pipe2(wakeupFD, O_CLOEXEC) < 0) { virReportSystemError(errno, "%s", _("unable to make pipe")); goto error; } - if (VIR_ALLOC(client) < 0) - goto no_memory; - - client->refs = 1; + if (!(client = virObjectNew(virNetClientClass))) + goto error; - if (virMutexInit(&client->lock) < 0) + if (virMutexInit(&client->lock) < 0) { + VIR_FREE(client); goto error; + } client->sock = sock; client->wakeupReadFD = wakeupFD[0]; @@ -327,13 +338,13 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr sock, goto no_memory; /* Set up a callback to listen on the socket data */ - client->refs++; + virObjectRef(client); if (virNetSocketAddIOCallback(client->sock, VIR_EVENT_HANDLE_READABLE, virNetClientIncomingEvent, client, - virNetClientEventFree) < 0) { - client->refs--; + virObjectFreeCallback) < 0) { + virObjectUnref(client); VIR_DEBUG("Failed to add event watch, disabling events and support for" " keepalive messages"); } else { @@ -342,16 +353,16 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr sock, if (!(ka = virKeepAliveNew(-1, 0, client, virNetClientKeepAliveSendCB, virNetClientKeepAliveDeadCB, - virNetClientEventFree))) + virObjectFreeCallback))) goto error; /* keepalive object has a reference to client */ - client->refs++; + virObjectRef(client); } client->keepalive = ka; PROBE(RPC_CLIENT_NEW, - "client=%p refs=%d sock=%p", - client, client->refs, client->sock); + "client=%p sock=%p", + client, client->sock); return client; no_memory: @@ -363,7 +374,7 @@ error: virKeepAliveStop(ka); virObjectUnref(ka); } - virNetClientFree(client); + virObjectUnref(client); return NULL; } @@ -422,17 +433,6 @@ virNetClientPtr virNetClientNewExternal(const char **cmdargv) } -void virNetClientRef(virNetClientPtr client) -{ - virNetClientLock(client); - client->refs++; - PROBE(RPC_CLIENT_REF, - "client=%p refs=%d", - client, client->refs); - virNetClientUnlock(client); -} - - int virNetClientGetFD(virNetClientPtr client) { int fd; @@ -463,28 +463,16 @@ bool virNetClientHasPassFD(virNetClientPtr client) } -void virNetClientFree(virNetClientPtr client) +void virNetClientDispose(void *obj) { + virNetClientPtr client = obj; int i; - if (!client) - return; - - virNetClientLock(client); - PROBE(RPC_CLIENT_FREE, - "client=%p refs=%d", - client, client->refs); - client->refs--; - if (client->refs > 0) { - virNetClientUnlock(client); - return; - } - if (client->closeFf) client->closeFf(client->closeOpaque); for (i = 0 ; i < client->nprograms ; i++) - virNetClientProgramFree(client->programs[i]); + virObjectUnref(client->programs[i]); VIR_FREE(client->programs); VIR_FORCE_CLOSE(client->wakeupSendFD); @@ -504,8 +492,6 @@ void virNetClientFree(virNetClientPtr client) virNetClientUnlock(client); virMutexDestroy(&client->lock); - - VIR_FREE(client); } @@ -546,7 +532,7 @@ virNetClientCloseLocked(virNetClientPtr client) virNetClientCloseFunc closeCb = client->closeCb; void *closeOpaque = client->closeOpaque; int closeReason = client->closeReason; - client->refs++; + virObjectRef(client); virNetClientUnlock(client); if (ka) { @@ -557,7 +543,7 @@ virNetClientCloseLocked(virNetClientPtr client) closeCb(client, closeReason, closeOpaque); virNetClientLock(client); - client->refs--; + virObjectUnref(client); } } @@ -754,8 +740,7 @@ int virNetClientAddProgram(virNetClientPtr client, if (VIR_EXPAND_N(client->programs, client->nprograms, 1) < 0) goto no_memory; - client->programs[client->nprograms-1] = prog; - virNetClientProgramRef(prog); + client->programs[client->nprograms-1] = virObjectRef(prog); virNetClientUnlock(client); return 0; @@ -775,8 +760,7 @@ int virNetClientAddStream(virNetClientPtr client, if (VIR_EXPAND_N(client->streams, client->nstreams, 1) < 0) goto no_memory; - client->streams[client->nstreams-1] = st; - virNetClientStreamRef(st); + client->streams[client->nstreams-1] = virObjectRef(st); virNetClientUnlock(client); return 0; @@ -810,7 +794,7 @@ void virNetClientRemoveStream(virNetClientPtr client, VIR_FREE(client->streams); client->nstreams = 0; } - virNetClientStreamFree(st); + virObjectUnref(st); cleanup: virNetClientUnlock(client); diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index c939475..d6b9b3c 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -30,6 +30,7 @@ # endif # include "virnetclientprogram.h" # include "virnetclientstream.h" +# include "virobject.h" virNetClientPtr virNetClientNewUNIX(const char *path, @@ -60,8 +61,6 @@ void virNetClientSetCloseCallback(virNetClientPtr client, void *opaque, virFreeCallback ff); -void virNetClientRef(virNetClientPtr client); - int virNetClientGetFD(virNetClientPtr client); int virNetClientDupFD(virNetClientPtr client, bool cloexec); @@ -105,7 +104,6 @@ const char *virNetClientRemoteAddrString(virNetClientPtr client); int virNetClientGetTLSKeySize(virNetClientPtr client); -void virNetClientFree(virNetClientPtr client); void virNetClientClose(virNetClientPtr client); bool virNetClientKeepAliveIsSupported(virNetClientPtr client); diff --git a/src/rpc/virnetclientprogram.c b/src/rpc/virnetclientprogram.c index c7d2409..320b7d4 100644 --- a/src/rpc/virnetclientprogram.c +++ b/src/rpc/virnetclientprogram.c @@ -33,11 +33,12 @@ #include "logging.h" #include "util.h" #include "virfile.h" +#include "threads.h" #define VIR_FROM_THIS VIR_FROM_RPC struct _virNetClientProgram { - int refs; + virObject object; unsigned program; unsigned version; @@ -46,6 +47,22 @@ struct _virNetClientProgram { void *eventOpaque; }; +static virClassPtr virNetClientProgramClass; +static void virNetClientProgramDispose(void *obj); + +static int virNetClientProgramOnceInit(void) +{ + if (!(virNetClientProgramClass = virClassNew("virNetClientProgram", + sizeof(virNetClientProgram), + virNetClientProgramDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetClientProgram) + + virNetClientProgramPtr virNetClientProgramNew(unsigned program, unsigned version, virNetClientProgramEventPtr events, @@ -54,12 +71,12 @@ virNetClientProgramPtr virNetClientProgramNew(unsigned program, { virNetClientProgramPtr prog; - if (VIR_ALLOC(prog) < 0) { - virReportOOMError(); + if (virNetClientProgramInitialize() < 0) + return NULL; + + if (!(prog = virObjectNew(virNetClientProgramClass))) return NULL; - } - prog->refs = 1; prog->program = program; prog->version = version; prog->events = events; @@ -70,22 +87,8 @@ virNetClientProgramPtr virNetClientProgramNew(unsigned program, } -void virNetClientProgramRef(virNetClientProgramPtr prog) +void virNetClientProgramDispose(void *obj ATTRIBUTE_UNUSED) { - prog->refs++; -} - - -void virNetClientProgramFree(virNetClientProgramPtr prog) -{ - if (!prog) - return; - - prog->refs--; - if (prog->refs > 0) - return; - - VIR_FREE(prog); } diff --git a/src/rpc/virnetclientprogram.h b/src/rpc/virnetclientprogram.h index f799f2e..9e92a3c 100644 --- a/src/rpc/virnetclientprogram.h +++ b/src/rpc/virnetclientprogram.h @@ -27,6 +27,7 @@ # include <rpc/xdr.h> # include "virnetmessage.h" +# include "virobject.h" typedef struct _virNetClient virNetClient; typedef virNetClient *virNetClientPtr; @@ -62,10 +63,6 @@ virNetClientProgramPtr virNetClientProgramNew(unsigned program, unsigned virNetClientProgramGetProgram(virNetClientProgramPtr prog); unsigned virNetClientProgramGetVersion(virNetClientProgramPtr prog); -void virNetClientProgramRef(virNetClientProgramPtr prog); - -void virNetClientProgramFree(virNetClientProgramPtr prog); - int virNetClientProgramMatches(virNetClientProgramPtr prog, virNetMessagePtr msg); diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c index 8580e39..0a2bbea 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -33,12 +33,13 @@ #define VIR_FROM_THIS VIR_FROM_RPC struct _virNetClientStream { + virObject object; + virMutex lock; virNetClientProgramPtr prog; int proc; unsigned serial; - int refs; virError err; @@ -63,6 +64,22 @@ struct _virNetClientStream { }; +static virClassPtr virNetClientStreamClass; +static void virNetClientStreamDispose(void *obj); + +static int virNetClientStreamOnceInit(void) +{ + if (!(virNetClientStreamClass = virClassNew("virNetClientStream", + sizeof(virNetClientStream), + virNetClientStreamDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetClientStream) + + static void virNetClientStreamEventTimerUpdate(virNetClientStreamPtr st) { @@ -119,26 +136,18 @@ virNetClientStreamEventTimer(int timer ATTRIBUTE_UNUSED, void *opaque) } -static void -virNetClientStreamEventTimerFree(void *opaque) -{ - virNetClientStreamPtr st = opaque; - virNetClientStreamFree(st); -} - - virNetClientStreamPtr virNetClientStreamNew(virNetClientProgramPtr prog, int proc, unsigned serial) { virNetClientStreamPtr st; - if (VIR_ALLOC(st) < 0) { - virReportOOMError(); + if (virNetClientStreamInitialize() < 0) + return NULL; + + if (!(st = virObjectNew(virNetClientStreamClass))) return NULL; - } - st->refs = 1; st->prog = prog; st->proc = proc; st->serial = serial; @@ -150,35 +159,19 @@ virNetClientStreamPtr virNetClientStreamNew(virNetClientProgramPtr prog, return NULL; } - virNetClientProgramRef(prog); + virObjectRef(prog); return st; } - -void virNetClientStreamRef(virNetClientStreamPtr st) +void virNetClientStreamDispose(void *obj) { - virMutexLock(&st->lock); - st->refs++; - virMutexUnlock(&st->lock); -} - -void virNetClientStreamFree(virNetClientStreamPtr st) -{ - virMutexLock(&st->lock); - st->refs--; - if (st->refs > 0) { - virMutexUnlock(&st->lock); - return; - } - - virMutexUnlock(&st->lock); + virNetClientStreamPtr st = obj; virResetError(&st->err); VIR_FREE(st->incoming); virMutexDestroy(&st->lock); - virNetClientProgramFree(st->prog); - VIR_FREE(st); + virObjectUnref(st->prog); } bool virNetClientStreamMatches(virNetClientStreamPtr st, @@ -454,13 +447,13 @@ int virNetClientStreamEventAddCallback(virNetClientStreamPtr st, goto cleanup; } - st->refs++; + virObjectRef(st); if ((st->cbTimer = virEventAddTimeout(-1, virNetClientStreamEventTimer, st, - virNetClientStreamEventTimerFree)) < 0) { - st->refs--; + virObjectFreeCallback)) < 0) { + virObjectUnref(st); goto cleanup; } diff --git a/src/rpc/virnetclientstream.h b/src/rpc/virnetclientstream.h index 00d598a..68a9e99 100644 --- a/src/rpc/virnetclientstream.h +++ b/src/rpc/virnetclientstream.h @@ -24,6 +24,7 @@ # define __VIR_NET_CLIENT_STREAM_H__ # include "virnetclientprogram.h" +# include "virobject.h" typedef struct _virNetClientStream virNetClientStream; typedef virNetClientStream *virNetClientStreamPtr; @@ -35,10 +36,6 @@ virNetClientStreamPtr virNetClientStreamNew(virNetClientProgramPtr prog, int proc, unsigned serial); -void virNetClientStreamRef(virNetClientStreamPtr st); - -void virNetClientStreamFree(virNetClientStreamPtr st); - bool virNetClientStreamRaiseError(virNetClientStreamPtr st); int virNetClientStreamSetError(virNetClientStreamPtr st, -- 1.7.10.4
On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Make all the virNetClient* objects use virObject APIs for reference counting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_probes.d | 4 +- src/lxc/lxc_monitor.c | 4 +- src/remote/remote_driver.c | 20 ++++----- src/rpc/gendispatch.pl | 4 +- src/rpc/virnetclient.c | 96 +++++++++++++++++------------------------ src/rpc/virnetclient.h | 4 +- src/rpc/virnetclientprogram.c | 43 +++++++++--------- src/rpc/virnetclientprogram.h | 5 +-- src/rpc/virnetclientstream.c | 65 +++++++++++++--------------- src/rpc/virnetclientstream.h | 5 +-- 10 files changed, 110 insertions(+), 140 deletions(-)
+++ b/src/rpc/virnetclient.h @@ -30,6 +30,7 @@ # endif # include "virnetclientprogram.h" # include "virnetclientstream.h" +# include "virobject.h"
Same comments about .c instead of .h.
-void virNetClientProgramRef(virNetClientProgramPtr prog) +void virNetClientProgramDispose(void *obj ATTRIBUTE_UNUSED) { - prog->refs++; -} - - -void virNetClientProgramFree(virNetClientProgramPtr prog) -{ - if (!prog) - return; - - prog->refs--; - if (prog->refs > 0) - return; - - VIR_FREE(prog); }
And another no-op dispose where you could use NULL instead. ACK with this squashed in: diff --git i/cfg.mk w/cfg.mk index 64af1ee..c0457e7 100644 --- i/cfg.mk +++ w/cfg.mk @@ -145,9 +145,6 @@ useless_free_options = \ --name=virJSONValueFree \ --name=virLastErrFreeData \ --name=virNetMessageFree \ - --name=virNetClientFree \ - --name=virNetClientProgramFree \ - --name=virNetClientStreamFree \ --name=virNetServerMDNSFree \ --name=virNetServerMDNSEntryFree \ --name=virNetServerMDNSGroupFree \ diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms index 0543005..79b4a18 100644 --- i/src/libvirt_private.syms +++ w/src/libvirt_private.syms @@ -1304,7 +1304,6 @@ virNetClientAddProgram; virNetClientAddStream; virNetClientClose; virNetClientDupFD; -virNetClientFree; virNetClientGetFD; virNetClientGetTLSKeySize; virNetClientHasPassFD; @@ -1318,7 +1317,6 @@ virNetClientNewExternal; virNetClientNewSSH; virNetClientNewTCP; virNetClientNewUNIX; -virNetClientRef; virNetClientRemoteAddrString; virNetClientRemoveStream; virNetClientSendNoReply; @@ -1333,12 +1331,10 @@ virNetClientSetTLSSession; # virnetclientprogram.h virNetClientProgramCall; virNetClientProgramDispatch; -virNetClientProgramFree; virNetClientProgramGetProgram; virNetClientProgramGetVersion; virNetClientProgramMatches; virNetClientProgramNew; -virNetClientProgramRef; # virnetclientstream.h @@ -1346,13 +1342,11 @@ virNetClientStreamEOF; virNetClientStreamEventAddCallback; virNetClientStreamEventRemoveCallback; virNetClientStreamEventUpdateCallback; -virNetClientStreamFree; virNetClientStreamMatches; virNetClientStreamNew; virNetClientStreamQueuePacket; virNetClientStreamRaiseError; virNetClientStreamRecvPacket; -virNetClientStreamRef; virNetClientStreamSendPacket; virNetClientStreamSetError; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Aug 06, 2012 at 01:56:33PM -0600, Eric Blake wrote:
On 08/06/2012 05:53 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Make all the virNetClient* objects use virObject APIs for reference counting
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_probes.d | 4 +- src/lxc/lxc_monitor.c | 4 +- src/remote/remote_driver.c | 20 ++++----- src/rpc/gendispatch.pl | 4 +- src/rpc/virnetclient.c | 96 +++++++++++++++++------------------------ src/rpc/virnetclient.h | 4 +- src/rpc/virnetclientprogram.c | 43 +++++++++--------- src/rpc/virnetclientprogram.h | 5 +-- src/rpc/virnetclientstream.c | 65 +++++++++++++--------------- src/rpc/virnetclientstream.h | 5 +-- 10 files changed, 110 insertions(+), 140 deletions(-)
+++ b/src/rpc/virnetclient.h @@ -30,6 +30,7 @@ # endif # include "virnetclientprogram.h" # include "virnetclientstream.h" +# include "virobject.h"
Same comments about .c instead of .h.
-void virNetClientProgramRef(virNetClientProgramPtr prog) +void virNetClientProgramDispose(void *obj ATTRIBUTE_UNUSED) { - prog->refs++; -} - - -void virNetClientProgramFree(virNetClientProgramPtr prog) -{ - if (!prog) - return; - - prog->refs--; - if (prog->refs > 0) - return; - - VIR_FREE(prog); }
And another no-op dispose where you could use NULL instead.
ACK with this squashed in:
diff --git i/cfg.mk w/cfg.mk index 64af1ee..c0457e7 100644 --- i/cfg.mk +++ w/cfg.mk @@ -145,9 +145,6 @@ useless_free_options = \ --name=virJSONValueFree \ --name=virLastErrFreeData \ --name=virNetMessageFree \ - --name=virNetClientFree \ - --name=virNetClientProgramFree \ - --name=virNetClientStreamFree \ --name=virNetServerMDNSFree \ --name=virNetServerMDNSEntryFree \ --name=virNetServerMDNSGroupFree \ diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms index 0543005..79b4a18 100644 --- i/src/libvirt_private.syms +++ w/src/libvirt_private.syms @@ -1304,7 +1304,6 @@ virNetClientAddProgram; virNetClientAddStream; virNetClientClose; virNetClientDupFD; -virNetClientFree; virNetClientGetFD; virNetClientGetTLSKeySize; virNetClientHasPassFD; @@ -1318,7 +1317,6 @@ virNetClientNewExternal; virNetClientNewSSH; virNetClientNewTCP; virNetClientNewUNIX; -virNetClientRef; virNetClientRemoteAddrString; virNetClientRemoveStream; virNetClientSendNoReply; @@ -1333,12 +1331,10 @@ virNetClientSetTLSSession; # virnetclientprogram.h virNetClientProgramCall; virNetClientProgramDispatch; -virNetClientProgramFree; virNetClientProgramGetProgram; virNetClientProgramGetVersion; virNetClientProgramMatches; virNetClientProgramNew; -virNetClientProgramRef;
# virnetclientstream.h @@ -1346,13 +1342,11 @@ virNetClientStreamEOF; virNetClientStreamEventAddCallback; virNetClientStreamEventRemoveCallback; virNetClientStreamEventUpdateCallback; -virNetClientStreamFree; virNetClientStreamMatches; virNetClientStreamNew; virNetClientStreamQueuePacket; virNetClientStreamRaiseError; virNetClientStreamRecvPacket; -virNetClientStreamRef; virNetClientStreamSendPacket; virNetClientStreamSetError;
Thanks for all the reviews. This series is now merged, with the changes you suggested for cfg.mk and libvirt_private.syms Now we're in a position to consider removing locking from some places which only use it for the purpose of protecting ref counts.... Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Eric Blake