From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Using virOnce for global initialization is desirable since it ensures that initialization will always take place when it is needed, and guarantees it only occurs once. The problem is that the code to setup a global initializer with proper error propagation is tedious. This introduces VIR_ONCE_GLOBAL_INIT macro to simplify this. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/util/threads.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/util/threads.h b/src/util/threads.h index e5000ea..8f192cf 100644 --- a/src/util/threads.h +++ b/src/util/threads.h @@ -114,4 +114,53 @@ int virThreadLocalSet(virThreadLocalPtr l, void*) ATTRIBUTE_RETURN_CHECK; # error "Either pthreads or Win32 threads are required" # endif + +/** + * VIR_ONCE_GLOBAL_INIT: + * classname: base classname + * + * This macro simplifies the setup of a one-time only + * global file initializer. + * + * Assuming a class called "virMyObject", and a method + * implemented like: + * + * int virMyObjectOnceInit(void) { + * ...do init tasks... + * } + * + * Then invoking the macro: + * + * VIR_ONCE_GLOBAL_INIT(virMyObject) + * + * Will create a method + * + * int virMyObjectInitialize(void); + * + * Which will ensure that 'virMyObjectOnceInit' is + * guaranteed to be invoked exactly once. + */ +# define VIR_ONCE_GLOBAL_INIT(classname) \ + static virOnceControl classname ## OnceControl = VIR_ONCE_CONTROL_INITIALIZER; \ + static virErrorPtr classname ## OnceError = NULL; \ + \ + static void classname ## Once(void) \ + { \ + if (classname ## OnceInit() < 0) \ + classname ## OnceError = virSaveLastError(); \ + } \ + \ + static int classname ## Initialize(void) \ + { \ + if (virOnce(&classname ## OnceControl, classname ## Once) < 0) \ + return -1; \ + \ + if (classname ## OnceError) { \ + virSetError(classname ## OnceError); \ + return -1; \ + } \ + \ + return 0; \ + } + #endif -- 1.7.10.2

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Remove the use of a manually run virLogStartup and virNodeSuspendInitialize methods. Instead make sure they are automatically run using VIR_ONCE_GLOBAL_INIT Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- daemon/libvirtd.c | 1 - src/libvirt.c | 4 +--- src/libvirt_private.syms | 3 --- src/util/logging.c | 54 +++++++++++++-------------------------------- src/util/logging.h | 2 -- src/util/virnodesuspend.c | 25 +++++++++++---------- src/util/virnodesuspend.h | 1 - 7 files changed, 29 insertions(+), 61 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 8c434a0..79f37ae 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1331,7 +1331,6 @@ cleanup: VIR_FREE(run_dir); daemonConfigFree(config); - virLogShutdown(); return ret; } diff --git a/src/libvirt.c b/src/libvirt.c index db6ba15..c6640b2 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -41,7 +41,6 @@ #include "conf.h" #include "rpc/virnettlscontext.h" #include "command.h" -#include "virnodesuspend.h" #include "virrandom.h" #include "viruri.h" @@ -395,8 +394,7 @@ virInitialize(void) if (virThreadInitialize() < 0 || virErrorInitialize() < 0 || - virRandomInitialize(time(NULL) ^ getpid()) || - virNodeSuspendInit() < 0) + virRandomInitialize(time(NULL) ^ getpid())) return -1; gcry_control(GCRYCTL_SET_THREAD_CBS, &virTLSThreadImpl); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6625fc6..e22b133 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -756,8 +756,6 @@ virLogReset; virLogSetBufferSize; virLogSetDefaultPriority; virLogSetFromEnv; -virLogShutdown; -virLogStartup; virLogUnlock; @@ -1491,7 +1489,6 @@ virNetTLSSessionSetIOCallbacks; # virnodesuspend.h nodeSuspendForDuration; -virNodeSuspendInit; virNodeSuspendGetTargetMask; diff --git a/src/util/logging.c b/src/util/logging.c index f8233cd..59b455e 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -147,25 +147,14 @@ static const char *virLogPriorityString(virLogPriority lvl) { return "unknown"; } -static int virLogInitialized = 0; -/** - * virLogStartup: - * - * Initialize the logging module - * - * Returns 0 if successful, and -1 in case or error - */ -int virLogStartup(void) { +static int virLogOnceInit(void) +{ const char *pbm = NULL; - if (virLogInitialized) - return -1; - if (virMutexInit(&virLogMutex) < 0) return -1; - virLogInitialized = 1; virLogLock(); if (VIR_ALLOC_N(virLogBuffer, virLogSize + 1) < 0) { /* @@ -191,6 +180,8 @@ int virLogStartup(void) { return 0; } +VIR_ONCE_GLOBAL_INIT(virLog) + /** * virLogSetBufferSize: * @size: size of the buffer in kilobytes or <= 0 to deactivate @@ -211,7 +202,10 @@ virLogSetBufferSize(int size) { if (size < 0) size = 0; - if ((virLogInitialized == 0) || (size * 1024 == virLogSize)) + if (virLogInitialize() < 0) + return -1; + + if (size * 1024 == virLogSize) return ret; virLogLock(); @@ -253,8 +247,8 @@ error: * Returns 0 if successful, and -1 in case or error */ int virLogReset(void) { - if (!virLogInitialized) - return virLogStartup(); + if (virLogInitialize() < 0) + return -1; virLogLock(); virLogResetFilters(); @@ -266,25 +260,6 @@ int virLogReset(void) { virLogUnlock(); return 0; } -/** - * virLogShutdown: - * - * Shutdown the logging module - */ -void virLogShutdown(void) { - if (!virLogInitialized) - return; - virLogLock(); - virLogResetFilters(); - virLogResetOutputs(); - virLogLen = 0; - virLogStart = 0; - virLogEnd = 0; - VIR_FREE(virLogBuffer); - virLogUnlock(); - virMutexDestroy(&virLogMutex); - virLogInitialized = 0; -} /* * Store a string in the ring buffer @@ -450,8 +425,9 @@ int virLogSetDefaultPriority(int priority) { VIR_WARN("Ignoring invalid log level setting."); return -1; } - if (!virLogInitialized) - virLogStartup(); + if (virLogInitialize() < 0) + return -1; + virLogDefaultPriority = priority; return 0; } @@ -723,8 +699,8 @@ void virLogVMessage(const char *category, int priority, const char *funcname, int emit = 1; unsigned int filterflags = 0; - if (!virLogInitialized) - virLogStartup(); + if (virLogInitialize() < 0) + return; if (fmt == NULL) goto cleanup; diff --git a/src/util/logging.h b/src/util/logging.h index 70318d0..d02cda7 100644 --- a/src/util/logging.h +++ b/src/util/logging.h @@ -125,9 +125,7 @@ extern int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c, void *data, extern void virLogLock(void); extern void virLogUnlock(void); -extern int virLogStartup(void); extern int virLogReset(void); -extern void virLogShutdown(void); extern int virLogParseDefaultPriority(const char *priority); extern int virLogParseFilters(const char *filters); extern int virLogParseOutputs(const char *output); diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c index 7e37118..d7a67c0 100644 --- a/src/util/virnodesuspend.c +++ b/src/util/virnodesuspend.c @@ -64,24 +64,19 @@ static void virNodeSuspendUnlock(void) } -/** - * virNodeSuspendInit: - * - * Get the system-wide sleep states supported by the host, such as - * Suspend-to-RAM, Suspend-to-Disk, or Hybrid-Suspend, so that a request - * to suspend/hibernate the host can be handled appropriately based on - * this information. - * - * Returns 0 if successful, and -1 in case of error. - */ -int virNodeSuspendInit(void) +static int virNodeSuspendOnceInit(void) { - if (virMutexInit(&virNodeSuspendMutex) < 0) + if (virMutexInit(&virNodeSuspendMutex) < 0) { + virNodeSuspendError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize mutex")); return -1; + } return 0; } +VIR_ONCE_GLOBAL_INIT(virNodeSuspend) + /** * virNodeSuspendSetNodeWakeup: @@ -187,6 +182,9 @@ int nodeSuspendForDuration(virConnectPtr conn ATTRIBUTE_UNUSED, virCheckFlags(0, -1); + if (virNodeSuspendInitialize() < 0) + return -1; + if (virNodeSuspendGetTargetMask(&supported) < 0) return -1; @@ -273,6 +271,9 @@ virNodeSuspendSupportsTarget(unsigned int target, bool *supported) int status; int ret = -1; + if (virNodeSuspendInitialize() < 0) + return -1; + *supported = false; switch (target) { diff --git a/src/util/virnodesuspend.h b/src/util/virnodesuspend.h index 5debde2..f1ee4bc 100644 --- a/src/util/virnodesuspend.h +++ b/src/util/virnodesuspend.h @@ -30,7 +30,6 @@ int nodeSuspendForDuration(virConnectPtr conn, unsigned long long duration, unsigned int flags); -int virNodeSuspendInit(void); int virNodeSuspendGetTargetMask(unsigned int *bitmask); #endif /* __VIR_NODE_SUSPEND_H__ */ -- 1.7.10.2

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; There are a few issues with the current virAtomic APIs - They require use of a virAtomicInt struct instead of a plain int type - Several of the methods do not implement memory barriers - The methods do not implement compiler re-ordering barriers - There is no Win32 native impl The GLib library has a nice LGPLv2+ licensed impl of atomic ops that works with GCC, Win32, or pthreads.h that addresses all these problems. The main downside to their code is that the pthreads impl uses a single global mutex, instead of a per-variable mutex. Given that it does have a Win32 impl though, we don't expect anyone to seriously use the pthread.h impl, so this downside is not significant. * .gitignore: Ignore test case * configure.ac: Check for which atomic ops impl to use * src/Makefile.am: Add viratomic.c * src/nwfilter/nwfilter_dhcpsnoop.c: Switch to new atomic ops APIs and plain int datatype * src/util/viratomic.h: inline impls of all atomic ops for GCC, Win32 and pthreads * src/util/viratomic.c: Global pthreads mutex for atomic ops * tests/viratomictest.c: Test validate to validate safety of atomic ops. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- .gitignore | 1 + configure.ac | 59 +++++- src/Makefile.am | 6 +- src/libvirt_atomic.syms | 3 + src/nwfilter/nwfilter_dhcpsnoop.c | 48 ++--- src/util/viratomic.c | 35 +++ src/util/viratomic.h | 424 ++++++++++++++++++++++++++++++------- src/util/virfile.c | 4 +- tests/Makefile.am | 5 + tests/viratomictest.c | 180 ++++++++++++++++ 10 files changed, 663 insertions(+), 102 deletions(-) create mode 100644 src/libvirt_atomic.syms create mode 100644 src/util/viratomic.c create mode 100644 tests/viratomictest.c diff --git a/.gitignore b/.gitignore index e5d5db9..3fe9486 100644 --- a/.gitignore +++ b/.gitignore @@ -146,6 +146,7 @@ /tests/ssh /tests/statstest /tests/utiltest +/tests/viratomictest /tests/virauthconfigtest /tests/virbuftest /tests/virdrivermoduletest diff --git a/configure.ac b/configure.ac index d45f4f1..2ac17a4 100644 --- a/configure.ac +++ b/configure.ac @@ -157,7 +157,64 @@ dnl Availability of various common headers (non-fatal if missing). AC_CHECK_HEADERS([pwd.h paths.h regex.h sys/un.h \ sys/poll.h syslog.h mntent.h net/ethernet.h linux/magic.h \ sys/un.h sys/syscall.h netinet/tcp.h ifaddrs.h libtasn1.h \ - net/if.h execinfo.h]) + net/if.h execinfo.h pthread.h]) + +dnl We need to decide at configure time if libvirt will use real atomic +dnl operations ("lock free") or emulated ones with a mutex. + +dnl Note that the atomic ops are only available with GCC on x86 when +dnl using -march=i486 or higher. If we detect that the atomic ops are +dnl not available but would be available given the right flags, we want +dnl to abort and advise the user to fix their CFLAGS. It's better to do +dnl that then to silently fall back on emulated atomic ops just because +dnl the user had the wrong build environment. + +atomic_ops= + +AC_MSG_CHECKING([for atomic ops implementation]) + +AC_TRY_COMPILE([], [__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4;],[ + atomic_ops=gcc +],[]) + +if test "$atomic_ops" = "" ; then + SAVE_CFLAGS="${CFLAGS}" + CFLAGS="-march=i486" + AC_TRY_COMPILE([], + [__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4;], + [AC_MSG_ERROR([Libvirt must be build with -march=i486 or later.])], + []) + CFLAGS="${SAVE_CFLAGS}" + + case "$host" in + *-*-mingw* | *-*-cygwin* | *-*-msvc* ) + atomic_ops=win32 + ;; + *) + if test "$ac_cv_header_pthread_h" = "yes" ; then + atomic_ops=pthread + else + AC_MSG_ERROR([Libvirt must be built with GCC or have pthread.h on non-Win32 platforms]) + fi + ;; + esac +fi + +case "$atomic_ops" in + gcc) + AC_DEFINE([VIR_ATOMIC_OPS_GCC],[1],[Use GCC atomic ops]) + ;; + win32) + AC_DEFINE([VIR_ATOMIC_OPS_WIN32],[1],[Use Win32 atomic ops]) + ;; + pthread) + AC_DEFINE([VIR_ATOMIC_OPS_PTHREAD],[1],[Use pthread atomic ops emulation]) + ;; +esac +AM_CONDITIONAL([WITH_ATOMIC_OPS_PTHREAD],[test "$atomic_ops" = "pthread"]) +AC_MSG_RESULT([$atomic_ops]) + + AC_MSG_CHECKING([for struct ifreq in net/if.h]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM( diff --git a/src/Makefile.am b/src/Makefile.am index 6c3eaa7..76d5eb7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -79,7 +79,7 @@ UTIL_SOURCES = \ util/threadpool.c util/threadpool.h \ util/uuid.c util/uuid.h \ util/util.c util/util.h \ - util/viratomic.h \ + util/viratomic.h util/viratomic.c \ util/viraudit.c util/viraudit.h \ util/virauth.c util/virauth.h \ util/virauthconfig.c util/virauthconfig.h \ @@ -1233,6 +1233,10 @@ if HAVE_SASL USED_SYM_FILES += libvirt_sasl.syms endif +if WITH_ATOMIC_OPS_PTHREAD +USED_SYM_FILES += libvirt_atomic.syms +endif + EXTRA_DIST += \ libvirt_public.syms \ libvirt_private.syms \ diff --git a/src/libvirt_atomic.syms b/src/libvirt_atomic.syms new file mode 100644 index 0000000..afaaf70 --- /dev/null +++ b/src/libvirt_atomic.syms @@ -0,0 +1,3 @@ + +# viratomic.h +virAtomicLock; diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 5dffaad..b4b4fae 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -70,16 +70,15 @@ #define VIR_FROM_THIS VIR_FROM_NWFILTER #ifdef HAVE_LIBPCAP - # define LEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.leases" # define TMPLEASEFILE LOCALSTATEDIR "/run/libvirt/network/nwfilter.ltmp" struct virNWFilterSnoopState { /* lease file */ int leaseFD; - virAtomicInt nLeases; /* number of active leases */ - virAtomicInt wLeases; /* number of written leases */ - virAtomicInt nThreads; /* number of running threads */ + int nLeases; /* number of active leases */ + int wLeases; /* number of written leases */ + int nThreads; /* number of running threads */ /* thread management */ virHashTablePtr snoopReqs; virHashTablePtr ifnameToKey; @@ -125,7 +124,7 @@ struct _virNWFilterSnoopReq { * publicSnoopReqs hash, the refctr may only * be modified with the SnoopLock held */ - virAtomicInt refctr; + int refctr; virNWFilterTechDriverPtr techdriver; char *ifname; @@ -239,7 +238,7 @@ struct _virNWFilterDHCPDecodeJob { unsigned char packet[PCAP_PBUFSIZE]; int caplen; bool fromVM; - virAtomicIntPtr qCtr; + int *qCtr; }; # define DHCP_PKT_RATE 10 /* pkts/sec */ @@ -270,7 +269,7 @@ struct _virNWFilterSnoopPcapConf { const pcap_direction_t dir; const char *filter; virNWFilterSnoopRateLimitConf rateLimit; /* indep. rate limiters */ - virAtomicInt qCtr; /* number of jobs in the worker's queue */ + int qCtr; /* number of jobs in the worker's queue */ const unsigned int maxQSize; unsigned long long penaltyTimeoutAbs; }; @@ -587,8 +586,7 @@ virNWFilterSnoopReqNew(const char *ifkey) req->threadStatus = THREAD_STATUS_NONE; - if (virAtomicIntInit(&req->refctr) < 0 || - virStrcpyStatic(req->ifkey, ifkey) == NULL || + if (virStrcpyStatic(req->ifkey, ifkey) == NULL || virMutexInitRecursive(&req->lock) < 0) goto err_free_req; @@ -621,7 +619,7 @@ virNWFilterSnoopReqFree(virNWFilterSnoopReqPtr req) if (!req) return; - if (virAtomicIntRead(&req->refctr) != 0) + if (virAtomicIntGet(&req->refctr) != 0) return; /* free all leases */ @@ -714,7 +712,7 @@ virNWFilterSnoopReqPut(virNWFilterSnoopReqPtr req) virNWFilterSnoopLock(); - if (virAtomicIntDec(&req->refctr) == 0) { + if (virAtomicIntDecAndTest(&req->refctr)) { /* * delete the request: * - if we don't find req on the global list anymore @@ -896,7 +894,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req, skip_instantiate: VIR_FREE(ipl); - virAtomicIntDec(&virNWFilterSnoopState.nLeases); + virAtomicIntDecAndTest(&virNWFilterSnoopState.nLeases); lease_not_found: VIR_FREE(ipstr); @@ -1158,7 +1156,7 @@ static void virNWFilterDHCPDecodeWorker(void *jobdata, void *opaque) _("Instantiation of rules failed on " "interface '%s'"), req->ifname); } - virAtomicIntDec(job->qCtr); + virAtomicIntDecAndTest(job->qCtr); VIR_FREE(job); } @@ -1169,7 +1167,7 @@ static int virNWFilterSnoopDHCPDecodeJobSubmit(virThreadPoolPtr pool, virNWFilterSnoopEthHdrPtr pep, int len, pcap_direction_t dir, - virAtomicIntPtr qCtr) + int *qCtr) { virNWFilterDHCPDecodeJobPtr job; int ret; @@ -1375,8 +1373,7 @@ virNWFilterDHCPSnoopThread(void *req0) virNWFilterSnoopDHCPOpen(req->ifname, &req->macaddr, pcapConf[i].filter, pcapConf[i].dir); - if (!pcapConf[i].handle || - virAtomicIntInit(&pcapConf[i].qCtr) < 0) { + if (!pcapConf[i].handle) { error = true; break; } @@ -1487,7 +1484,7 @@ virNWFilterDHCPSnoopThread(void *req0) unsigned int diff; /* submit packet to worker thread */ - if (virAtomicIntRead(&pcapConf[i].qCtr) > + if (virAtomicIntGet(&pcapConf[i].qCtr) > pcapConf[i].maxQSize) { if (last_displayed_queue - time(0) > 10) { last_displayed_queue = time(0); @@ -1553,7 +1550,7 @@ exit: pcap_close(pcapConf[i].handle); } - virAtomicIntDec(&virNWFilterSnoopState.nThreads); + virAtomicIntDecAndTest(&virNWFilterSnoopState.nThreads); return; } @@ -1802,8 +1799,8 @@ virNWFilterSnoopLeaseFileSave(virNWFilterSnoopIPLeasePtr ipl) goto err_exit; /* keep dead leases at < ~95% of file size */ - if (virAtomicIntInc(&virNWFilterSnoopState.wLeases) >= - virAtomicIntRead(&virNWFilterSnoopState.nLeases) * 20) + if (virAtomicIntAdd(&virNWFilterSnoopState.wLeases, 1) >= + virAtomicIntGet(&virNWFilterSnoopState.nLeases) * 20) virNWFilterSnoopLeaseFileLoad(); /* load & refresh lease file */ err_exit: @@ -1834,7 +1831,7 @@ virNWFilterSnoopPruneIter(const void *payload, /* * have the entry removed if it has no leases and no one holds a ref */ - del_req = ((req->start == NULL) && (virAtomicIntRead(&req->refctr) == 0)); + del_req = ((req->start == NULL) && (virAtomicIntGet(&req->refctr) == 0)); virNWFilterSnoopReqUnlock(req); @@ -1992,9 +1989,9 @@ virNWFilterSnoopLeaseFileLoad(void) static void virNWFilterSnoopJoinThreads(void) { - while (virAtomicIntRead(&virNWFilterSnoopState.nThreads) != 0) { + while (virAtomicIntGet(&virNWFilterSnoopState.nThreads) != 0) { VIR_WARN("Waiting for snooping threads to terminate: %u\n", - virAtomicIntRead(&virNWFilterSnoopState.nThreads)); + virAtomicIntGet(&virNWFilterSnoopState.nThreads)); usleep(1000 * 1000); } } @@ -2059,10 +2056,7 @@ virNWFilterDHCPSnoopInit(void) VIR_DEBUG("Initializing DHCP snooping"); if (virMutexInitRecursive(&virNWFilterSnoopState.snoopLock) < 0 || - virMutexInit(&virNWFilterSnoopState.activeLock) < 0 || - virAtomicIntInit(&virNWFilterSnoopState.nLeases) < 0 || - virAtomicIntInit(&virNWFilterSnoopState.wLeases) < 0 || - virAtomicIntInit(&virNWFilterSnoopState.nThreads) < 0) + virMutexInit(&virNWFilterSnoopState.activeLock) < 0) return -1; virNWFilterSnoopState.ifnameToKey = virHashCreate(0, NULL); diff --git a/src/util/viratomic.c b/src/util/viratomic.c new file mode 100644 index 0000000..af846ff --- /dev/null +++ b/src/util/viratomic.c @@ -0,0 +1,35 @@ +/* + * viratomic.h: atomic integer operations + * + * Copyright (C) 2012 Red Hat, Inc. + * + * Based on code taken from GLib 2.32, under the LGPLv2+ + * + * Copyright (C) 2011 Ryan Lortie + * + * 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <config.h> + +#include "viratomic.h" + + +#ifdef VIR_ATOMIC_OPS_PTHREAD + +pthread_mutex_t virAtomicLock = PTHREAD_MUTEX_INITIALIZER; + +#endif diff --git a/src/util/viratomic.h b/src/util/viratomic.h index 5fee35b..d344ea5 100644 --- a/src/util/viratomic.h +++ b/src/util/viratomic.h @@ -1,10 +1,11 @@ /* * viratomic.h: atomic integer operations * - * Copyright (C) 2012 IBM Corporation + * Copyright (C) 2012 Red Hat, Inc. * - * Authors: - * Stefan Berger <stefanb(a)linux.vnet.ibm.com&gt; + * Based on code taken from GLib 2.32, under the LGPLv2+ + * + * Copyright (C) 2011 Ryan Lortie * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -25,130 +26,411 @@ #ifndef __VIR_ATOMIC_H__ # define __VIR_ATOMIC_H__ -# include "threads.h" +# include "internal.h" -typedef struct _virAtomicInt virAtomicInt; -typedef virAtomicInt *virAtomicIntPtr; +/** + * virAtomicIntGet: + * Gets the current value of atomic. + * + * This call acts as a full compiler and hardware memory barrier + * (before the get) + */ +int virAtomicIntGet(volatile int *atomic) + ATTRIBUTE_NONNULL(1); -# define __VIR_ATOMIC_USES_LOCK +/** + * virAtomicIntSet: + * Sets the value of atomic to newval. + * + * This call acts as a full compiler and hardware memory barrier + * (after the set) + */ +void virAtomicIntSet(volatile int *atomic, + int newval) + ATTRIBUTE_NONNULL(1); -# if ((__GNUC__ == 4) && (__GNUC_MINOR__ >= 1)) || (__GNUC__ > 4) -# undef __VIR_ATOMIC_USES_LOCK -# endif +/** + * virAtomicIntInc: + * Increments the value of atomic by 1. + * + * Think of this operation as an atomic version of { *atomic += 1; } + * + * This call acts as a full compiler and hardware memory barrier. + */ +void virAtomicIntInc(volatile int *atomic) + ATTRIBUTE_NONNULL(1); -static inline int virAtomicIntInit(virAtomicIntPtr vaip) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; -static inline int virAtomicIntRead(virAtomicIntPtr vaip) +/** + * virAtomicIntDecAndTest: + * Decrements the value of atomic by 1. + * + * Think of this operation as an atomic version of + * { *atomic -= 1; return *atomic == 0; } + * + * This call acts as a full compiler and hardware memory barrier. + */ +bool virAtomicIntDecAndTest(volatile int *atomic) ATTRIBUTE_NONNULL(1); -static inline void virAtomicIntSet(virAtomicIntPtr vaip, int val) + +/** + * virAtomicIntCompareExchange: + * Compares atomic to oldval and, if equal, sets it to newval. If + * atomic was not equal to oldval then no change occurs. + * + * This compare and exchange is done atomically. + * + * Think of this operation as an atomic version of + * { if (*atomic == oldval) { *atomic = newval; return TRUE; } + * else return FALSE; } + * + * This call acts as a full compiler and hardware memory barrier. + */ +bool virAtomicIntCompareExchange(volatile int *atomic, + int oldval, + int newval) ATTRIBUTE_NONNULL(1); -static inline int virAtomicIntAdd(virAtomicIntPtr vaip, int add) + +/** + * virAtomicIntAdd: + * Atomically adds val to the value of atomic. + * + * Think of this operation as an atomic version of + * { tmp = *atomic; *atomic += val; return tmp; } + * + * This call acts as a full compiler and hardware memory barrier. + */ +int virAtomicIntAdd(volatile int *atomic, + int val) ATTRIBUTE_NONNULL(1); -static inline int virAtomicIntSub(virAtomicIntPtr vaip, int add) + +/** + * virAtomicIntAnd: + * Performs an atomic bitwise 'and' of the value of atomic + * and val, storing the result back in atomic. + * + * This call acts as a full compiler and hardware memory barrier. + * + * Think of this operation as an atomic version of + * { tmp = *atomic; *atomic &= val; return tmp; } + */ +unsigned int virAtomicIntAnd(volatile unsigned int *atomic, + unsigned int val) ATTRIBUTE_NONNULL(1); -static inline int virAtomicIntInc(virAtomicIntPtr vaip) + +/** + * virAtomicIntOr: + * Performs an atomic bitwise 'or' of the value of atomic + * and val, storing the result back in atomic. + * + * Think of this operation as an atomic version of + * { tmp = *atomic; *atomic |= val; return tmp; } + * + * This call acts as a full compiler and hardware memory barrier. + */ +unsigned int virAtomicIntOr(volatile unsigned int *atomic, + unsigned int val) ATTRIBUTE_NONNULL(1); -static inline int virAtomicIntDec(virAtomicIntPtr vaip) + +/** + * virAtomicIntXor: + * Performs an atomic bitwise 'xor' of the value of atomic + * and val, storing the result back in atomic. + * + * Think of this operation as an atomic version of + * { tmp = *atomic; *atomic ^= val; return tmp; } + * + * This call acts as a full compiler and hardware memory barrier. + */ +unsigned int virAtomicIntXor(volatile unsigned int *atomic, + unsigned int val) ATTRIBUTE_NONNULL(1); -# ifdef __VIR_ATOMIC_USES_LOCK -struct _virAtomicInt { - virMutex lock; - int value; -}; +# ifdef VIR_ATOMIC_OPS_GCC + +# define virAtomicIntGet(atomic) \ + (__extension__ ({ \ + verify (sizeof(*(atomic)) == sizeof(int)); \ + (void) (0 ? *(atomic) ^ *(atomic) : 0); \ + __sync_synchronize (); \ + (int) *(atomic); \ + })) +# define virAtomicIntSet(atomic, newval) \ + (__extension__ ({ \ + verify (sizeof(*(atomic)) == sizeof(int)); \ + (void) (0 ? *(atomic) ^ (newval) : 0); \ + *(atomic) = (newval); \ + __sync_synchronize (); \ + })) +# define virAtomicIntInc(atomic) \ + (__extension__ ({ \ + verify (sizeof(*(atomic)) == sizeof(int)); \ + (void) (0 ? *(atomic) ^ *(atomic) : 0); \ + (void) __sync_fetch_and_add ((atomic), 1); \ + })) +# define virAtomicIntDecAndTest(atomic) \ + (__extension__ ({ \ + verify (sizeof(*(atomic)) == sizeof(int)); \ + (void) (0 ? *(atomic) ^ *(atomic) : 0); \ + __sync_fetch_and_sub ((atomic), 1) == 1; \ + })) +# define virAtomicIntCompareExchange(atomic, oldval, newval) \ + (__extension__ ({ \ + verify (sizeof(*(atomic)) == sizeof(int)); \ + (void) (0 ? *(atomic) ^ (newval) ^ (oldval) : 0); \ + (bool) __sync_bool_compare_and_swap ((atomic), (oldval), (newval)); \ + })) +# define virAtomicIntAdd(atomic, val) \ + (__extension__ ({ \ + verify (sizeof(*(atomic)) == sizeof(int)); \ + (void) (0 ? *(atomic) ^ (val) : 0); \ + (int) __sync_fetch_and_add ((atomic), (val)); \ + })) +# define virAtomicIntAnd(atomic, val) \ + (__extension__ ({ \ + verify (sizeof(*(atomic)) == sizeof(int)); \ + (void) (0 ? *(atomic) ^ (val) : 0); \ + (unsigned int) __sync_fetch_and_and ((atomic), (val)); \ + })) +# define virAtomicIntOr(atomic, val) \ + (__extension__ ({ \ + verify (sizeof(*(atomic)) == sizeof(int)); \ + (void) (0 ? *(atomic) ^ (val) : 0); \ + (unsigned int) __sync_fetch_and_or ((atomic), (val)); \ + })) +# define virAtomicIntXor(atomic, val) \ + (__extension__ ({ \ + verify (sizeof(*(atomic)) == sizeof(int)); \ + (void) (0 ? *(atomic) ^ (val) : 0); \ + (unsigned int) __sync_fetch_and_xor ((atomic), (val)); \ + })) + + +# else + +# ifdef VIR_ATOMIC_OPS_WIN32 + +# include <winsock2.h> +# include <windows.h> +# include <intrin.h> +# if !defined(_M_AMD64) && !defined (_M_IA64) && !defined(_M_X64) +# define InterlockedAnd _InterlockedAnd +# define InterlockedOr _InterlockedOr +# define InterlockedXor _InterlockedXor +# endif -static inline int -virAtomicIntInit(virAtomicIntPtr vaip) +/* + * http://msdn.microsoft.com/en-us/library/ms684122(v=vs.85).aspx + */ +inline int +virAtomicIntGet(volatile int *atomic) { - vaip->value = 0; - return virMutexInit(&vaip->lock); + MemoryBarrier(); + return *atomic; } -static inline int -virAtomicIntAdd(virAtomicIntPtr vaip, int add) +inline void +virAtomicIntSet(volatile int *atomic, + int newval) { - int ret; + *atomic = newval; + MemoryBarrier(); +} - virMutexLock(&vaip->lock); +inline void +virAtomicIntInc(volatile int *atomic) +{ + InterlockedIncrement((volatile LONG *)atomic); +} - vaip->value += add; - ret = vaip->value; +inline bool +virAtomicIntDecAndTest(volatile int *atomic) +{ + return InterlockedDecrement((volatile LONG *)atomic) == 0; +} - virMutexUnlock(&vaip->lock); +inline bool +virAtomicIntCompareExchange(volatile int *atomic, + int oldval, + int newval) +{ + return InterlockedCompareExchange((volatile LONG *)atomic, newval, oldval) == oldval; +} - return ret; +inline int +virAtomicIntAdd(volatile int *atomic, + int val) +{ + return InterlockedExchangeAdd((volatile LONG *)atomic, val); } -static inline int -virAtomicIntSub(virAtomicIntPtr vaip, int sub) +inline unsigned int +virAtomicIntAnd(volatile unsigned int *atomic, + unsigned int val) { - int ret; + return InterlockedAnd((volatile LONG *)atomic, val); +} - virMutexLock(&vaip->lock); +inline unsigned int +virAtomicIntOr(volatile unsigned int *atomic, + unsigned int val) +{ + return InterlockedOr((volatile LONG *)atomic, val); +} - vaip->value -= sub; - ret = vaip->value; +inline unsigned int +virAtomicIntXor(volatile unsigned int *atomic, + unsigned int val) +{ + return InterlockedXor((volatile LONG *)atomic, val); +} - virMutexUnlock(&vaip->lock); - return ret; -} +# else +# ifdef VIR_ATOMIC_OPS_PTHREAD +# include <pthread.h> -# else /* __VIR_ATOMIC_USES_LOCK */ +extern pthread_mutex_t virAtomicLock; -struct _virAtomicInt { +inline int +virAtomicIntGet(volatile int *atomic) +{ int value; -}; -static inline int -virAtomicIntInit(virAtomicIntPtr vaip) + pthread_mutex_lock(&virAtomicLock); + value = *atomic; + pthread_mutex_unlock(&virAtomicLock); + + return value; +} + +inline void +virAtomicIntSet(volatile int *atomic, + int value) { - vaip->value = 0; - return 0; + pthread_mutex_lock(&virAtomicLock); + *atomic = value; + pthread_mutex_unlock(&virAtomicLock); } -static inline int -virAtomicIntAdd(virAtomicIntPtr vaip, int add) +inline void +virAtomicIntInc(volatile int *atomic) { - return __sync_add_and_fetch(&vaip->value, add); + pthread_mutex_lock(&virAtomicLock); + (*atomic)++; + pthread_mutex_unlock(&virAtomicLock); } -static inline int -virAtomicIntSub(virAtomicIntPtr vaip, int sub) +inline bool +virAtomicIntDecAndTest(volatile int *atomic) { - return __sync_sub_and_fetch(&vaip->value, sub); + bool is_zero; + + pthread_mutex_lock(&virAtomicLock); + is_zero = --(*atomic) == 0; + pthread_mutex_unlock(&virAtomicLock); + + return is_zero; } -# endif /* __VIR_ATOMIC_USES_LOCK */ +inline bool +virAtomicIntCompareExchange(volatile int *atomic, + int oldval, + int newval) +{ + bool success; + pthread_mutex_lock(&virAtomicLock); + if ((success = (*atomic == oldval))) + *atomic = newval; -/* common operations that need no locking or build on others */ + pthread_mutex_unlock(&virAtomicLock); + return success; +} -static inline void -virAtomicIntSet(virAtomicIntPtr vaip, int value) +inline int +virAtomicIntAdd(volatile int *atomic, + int val) { - vaip->value = value; + int oldval; + + pthread_mutex_lock(&virAtomicLock); + oldval = *atomic; + *atomic = oldval + val; + pthread_mutex_unlock(&virAtomicLock); + + return oldval; } -static inline int -virAtomicIntRead(virAtomicIntPtr vaip) +inline unsigned int +virAtomicIntAnd(volatile unsigned int *atomic, + unsigned int val) { - return *(volatile int *)&vaip->value; + unsigned int oldval; + + pthread_mutex_lock(&virAtomicLock); + oldval = *atomic; + *atomic = oldval & val; + pthread_mutex_unlock(&virAtomicLock); + + return oldval; } -static inline int -virAtomicIntInc(virAtomicIntPtr vaip) +inline unsigned int +virAtomicIntOr(volatile unsigned int *atomic, + unsigned int val) { - return virAtomicIntAdd(vaip, 1); + unsigned int oldval; + + pthread_mutex_lock(&virAtomicLock); + oldval = *atomic; + *atomic = oldval | val; + pthread_mutex_unlock(&virAtomicLock); + + return oldval; } -static inline int -virAtomicIntDec(virAtomicIntPtr vaip) +inline unsigned int +virAtomicIntXor(volatile unsigned int *atomic, + unsigned int val) { - return virAtomicIntSub(vaip, 1); + unsigned int oldval; + + pthread_mutex_lock(&virAtomicLock); + oldval = *atomic; + *atomic = oldval ^ val; + pthread_mutex_unlock(&virAtomicLock); + + return oldval; } + +# else +# error "No atomic integer impl for this platform" +# endif +# endif + +# define virAtomicIntGet(atomic) \ + virAtomicIntGet((int *)atomic) +# define virAtomicIntSet(atomic, val) \ + virAtomicIntSet((int *)atomic, val) +# define virAtomicIntInc(atomic) \ + virAtomicIntInc((int *)atomic) +# define virAtomicIntDecAndTest(atomic) \ + virAtomicIntDecAndTest((int *)atomic) +# define virAtomicIntCompareExchange(atomic, oldval, newval) \ + virAtomicIntCompareExchange((int *)atomic, oldval, newval) +# define virAtomicIntAdd(atomic, val) \ + virAtomicIntAdd((int *)atomic, val) +# define virAtomicIntAnd(atomic, val) \ + virAtomicIntAnd((unsigned int *)atomic, val) +# define virAtomicIntOr(atomic, val) \ + virAtomicIntOr((unsigned int *)atomic, val) +# define virAtomicIntXor(atomic, val) \ + virAtomicIntXor((unsigned int *)atomic, val) + +# endif + #endif /* __VIR_ATOMIC_H */ diff --git a/src/util/virfile.c b/src/util/virfile.c index 8387ae9..0ea9194 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -622,12 +622,12 @@ cleanup: #else /* __linux__ */ int virFileLoopDeviceAssociate(const char *file, - char **dev) + char **dev ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, _("Unable to associate file %s with loop device"), file); - return -1;m + return -1; } #endif /* __linux__ */ diff --git a/tests/Makefile.am b/tests/Makefile.am index e3bd6d1..49a25e2 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -89,6 +89,7 @@ test_programs = virshtest sockettest \ nodeinfotest virbuftest \ commandtest seclabeltest \ virhashtest virnetmessagetest virnetsockettest \ + viratomictest \ utiltest virnettlscontexttest shunloadtest \ virtimetest viruritest virkeyfiletest \ virauthconfigtest @@ -516,6 +517,10 @@ virhashtest_SOURCES = \ virhashtest.c virhashdata.h testutils.h testutils.c virhashtest_LDADD = $(LDADDS) +viratomictest_SOURCES = \ + viratomictest.c viratomicdata.h testutils.h testutils.c +viratomictest_LDADD = $(LDADDS) + jsontest_SOURCES = \ jsontest.c testutils.h testutils.c jsontest_LDADD = $(LDADDS) diff --git a/tests/viratomictest.c b/tests/viratomictest.c new file mode 100644 index 0000000..a5ae5cd --- /dev/null +++ b/tests/viratomictest.c @@ -0,0 +1,180 @@ +/* + * Copyright (C) 2011-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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <config.h> + +#include <time.h> +#include <sched.h> + +#include "testutils.h" + +#include "viratomic.h" +#include "virrandom.h" +#include "threads.h" + +static int +testTypes(const void *data ATTRIBUTE_UNUSED) +{ + unsigned int u, u2; + int s, s2; + bool res; + +#define virAssertCmpInt(a, op, b) \ + if (!((a) op (b))) \ + return -1; + virAtomicIntSet(&u, 5); + u2 = virAtomicIntGet(&u); + virAssertCmpInt(u2, ==, 5); + + res = virAtomicIntCompareExchange(&u, 6, 7); + if (res) + return -1; + virAssertCmpInt(u, ==, 5); + + virAtomicIntAdd(&u, 1); + virAssertCmpInt(u, ==, 6); + + virAtomicIntInc(&u); + virAssertCmpInt(u, ==, 7); + + res = virAtomicIntDecAndTest(&u); + if (res) + return -1; + virAssertCmpInt(u, ==, 6); + + u2 = virAtomicIntAnd(&u, 5); + virAssertCmpInt(u2, ==, 6); + virAssertCmpInt(u, ==, 4); + + u2 = virAtomicIntOr(&u, 8); + virAssertCmpInt(u2, ==, 4); + virAssertCmpInt(u, ==, 12); + + u2 = virAtomicIntXor(&u, 4); + virAssertCmpInt(u2, ==, 12); + virAssertCmpInt(u, ==, 8); + + virAtomicIntSet(&s, 5); + s2 = virAtomicIntGet(&s); + virAssertCmpInt(s2, ==, 5); + + res = virAtomicIntCompareExchange(&s, 6, 7); + if (res) + return -1; + virAssertCmpInt(s, ==, 5); + + virAtomicIntAdd(&s, 1); + virAssertCmpInt(s, ==, 6); + + virAtomicIntInc(&s); + virAssertCmpInt(s, ==, 7); + + res = virAtomicIntDecAndTest(&s); + if (res) + return -1; + virAssertCmpInt(s, ==, 6); + + s2 = virAtomicIntAnd(&s, 5); + virAssertCmpInt(s2, ==, 6); + virAssertCmpInt(s, ==, 4); + + s2 = virAtomicIntOr(&s, 8); + virAssertCmpInt(s2, ==, 4); + virAssertCmpInt(s, ==, 12); + + s2 = virAtomicIntXor(&s, 4); + virAssertCmpInt(s2, ==, 12); + virAssertCmpInt(s, ==, 8); + + return 0; +} + +#define THREADS 10 +#define ROUNDS 10000 + +volatile int bucket[THREADS]; +volatile int atomic; + +static void +thread_func(void *data) +{ + int idx = (int)(long)data; + int i; + int d; + + for(i = 0; i < ROUNDS; i++) { + d = virRandomBits(7); + bucket[idx] += d; + virAtomicIntAdd(&atomic, d); +#ifdef WIN32 + SleepEx (0,0); +#else + sched_yield(); +#endif + } +} + +static int +testThreads(const void *data ATTRIBUTE_UNUSED) +{ + int sum; + int i; + virThread threads[THREADS]; + + atomic = 0; + for(i = 0; i < THREADS; i++) + bucket[i] = 0; + + for(i = 0; i < THREADS; i++) { + if (virThreadCreate(&(threads[i]), true, thread_func, (void*)(long)i) < 0) + return -1; + } + + for(i = 0; i < THREADS; i++) + virThreadJoin(&threads[i]); + + sum = 0; + for(i = 0; i < THREADS; i++) + sum += bucket[i]; + + if (sum != atomic) + return -1; + + return 0; +} + +static int +mymain(void) +{ + int ret = 0; + + if (virRandomInitialize(time(NULL)) < 0) + return -1; + if (virThreadInitialize() < 0) + return -1; + + if (virtTestRun("types", 1, testTypes, NULL) < 0) + ret = -1; + if (virtTestRun("threads", 1, testThreads, NULL) < 0) + ret = -1; + + return ret; +} + +VIRT_TEST_MAIN(mymain) -- 1.7.10.2

On Fri, Jul 13, 2012 at 03:44:03PM -0600, Eric Blake wrote: No, I meant Win32 impl - both codebases have a GCC impl - I was pointing out that we gain a Win32 impl, which is the only non-GCC scenario we hugely care about. Hmm, I wonder why I didn't see HAVE_PTHREAD_H in config.h before. I'll investigate. Ok, I'll separate this out. Yeah I think so. IntInc is void(), so I had to use IntAdd. That said this code is not too pleasant so I'll check into this. The reason for returning the OLD value is that this is the only thing you can really work with, race free. If you are trying to do something based on the new value then your code is quitely probably racy, since any other thread can have further changed the new value at any time. Opps, yes. With the inline GCC macros we can pass either 'int' or 'unsigned int' with no warnings. When we use the WIn32/pthread helper functions though, we would get compiler warnings from mixed-sign ints. These macro casts get around that. 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(a)redhat.com&gt; 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 connklass = 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 When creating an instance of an object, one needs simply pass the virClassPtr eg virConnectPtr conn = virObjectNew(connklass); 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(a)redhat.com&gt; --- src/Makefile.am | 1 + src/libvirt_private.syms | 9 ++ src/libvirt_probes.d | 7 ++ src/util/virobject.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++ src/util/virobject.h | 60 ++++++++++++++ 5 files changed, 280 insertions(+) create mode 100644 src/util/virobject.c create mode 100644 src/util/virobject.h diff --git a/src/Makefile.am b/src/Makefile.am index 76d5eb7..2e596d5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -84,6 +84,7 @@ UTIL_SOURCES = \ util/virauth.c util/virauth.h \ util/virauthconfig.c util/virauthconfig.h \ util/virfile.c util/virfile.h \ + util/virobject.c util/virobject.h \ util/virnodesuspend.c util/virnodesuspend.h \ util/virpidfile.c util/virpidfile.h \ util/virtypedparam.c util/virtypedparam.h \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e22b133..1ff6735 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1492,6 +1492,15 @@ nodeSuspendForDuration; virNodeSuspendGetTargetMask; +# virobject.h +virClassNew; +virClassName; +virObjectNew; +virObjectRef; +virObjectUnref; +virObjectIsClass; + + # 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..002196e --- /dev/null +++ b/src/util/virobject.c @@ -0,0 +1,203 @@ +/* + * 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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#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 + +#define virObjectError(code, ...) \ + virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +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) + return NULL; + + if (!(klass->name = strdup(name))) + goto no_memory; + klass->magic = virAtomicIntAdd(&magicCounter, 1); + 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 = (void*)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. + * + * 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); + 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; +} diff --git a/src/util/virobject.h b/src/util/virobject.h new file mode 100644 index 0000000..235fb24 --- /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, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#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; + virClassPtr klass; + int refs; +}; + +virClassPtr virClassNew(const char *name, + size_t objectSize, + virObjectDisposeCallback dispose) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + +const char *virClassName(virClassPtr klass) + ATTRIBUTE_NONNULL(1); + +void *virObjectNew(virClassPtr klass) + ATTRIBUTE_NONNULL(1); +bool virObjectUnref(void *obj) + ATTRIBUTE_NONNULL(1); +void *virObjectRef(void *obj) + ATTRIBUTE_NONNULL(1); + +bool virObjectIsClass(void *obj, + virClassPtr klass) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +#endif /* __VIR_OBJECT_H */ -- 1.7.10.2

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; 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(a)redhat.com&gt; --- src/conf/domain_event.c | 8 +- src/datatypes.c | 999 ++++++++++-------------------------------- src/datatypes.h | 264 ++++------- src/libvirt.c | 127 ++---- src/libvirt_private.syms | 17 +- src/phyp/phyp_driver.c | 14 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_migration.c | 8 +- src/qemu/qemu_process.c | 2 +- src/storage/storage_driver.c | 4 +- src/vbox/vbox_tmpl.c | 2 +- src/xen/xend_internal.c | 4 +- tests/qemuxml2argvtest.c | 21 +- tests/qemuxmlnstest.c | 2 +- tests/sexpr2xmltest.c | 2 +- tests/xmconfigtest.c | 4 +- 16 files changed, 410 insertions(+), 1070 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 3cfd940..be7ccbd 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 d718170..8cac540 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -35,12 +35,55 @@ 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); + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virDataTypes) /** * virGetConnect: @@ -53,35 +96,28 @@ virConnectPtr 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. @@ -89,14 +125,8 @@ failed: * be released prior to this returning. The connection obj must not * 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); +static void virConnectDispose(void *obj) { + virConnectPtr conn = obj; if (conn->networkDriver) conn->networkDriver->close(conn); @@ -121,38 +151,8 @@ virReleaseConnect(virConnectPtr conn) { virMutexUnlock(&conn->lock); virMutexDestroy(&conn->lock); - 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, _("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: @@ -163,14 +163,16 @@ 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) { virDomainPtr ret = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (virDataTypesInitialize() < 0) + return NULL; if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, _("no connection")); @@ -179,41 +181,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; - virUUIDFormat(uuid, uuidstr); + 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_DOMAIN_MAGIC; - ret->conn = conn; + 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,62 +212,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); - } -} - - -/** - * 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, _("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; + if (domain->conn) + virObjectUnref(domain->conn); } + /** * virGetNetwork: * @conn: the hypervisor connection @@ -290,14 +235,16 @@ 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) { virNetworkPtr ret = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (virDataTypesInitialize() < 0) + return NULL; if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, _("no connection")); @@ -306,40 +253,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; - virUUIDFormat(uuid, uuidstr); + 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_NETWORK_MAGIC; - ret->conn = conn; + 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. @@ -351,63 +283,21 @@ 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, - _("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; + if (network->conn) + virObjectUnref(network->conn); } + /** * virGetInterface: * @conn: the hypervisor connection @@ -417,7 +307,7 @@ 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 @@ -426,6 +316,9 @@ virInterfacePtr 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, _("no connection")); return NULL; @@ -436,45 +329,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. @@ -486,58 +360,15 @@ 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, - _("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; + if (iface->conn) + virObjectUnref(iface->conn); } @@ -550,7 +381,7 @@ 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 */ @@ -558,7 +389,9 @@ virStoragePoolPtr virGetStoragePool(virConnectPtr conn, const char *name, const unsigned char *uuid) { virStoragePoolPtr ret = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (virDataTypesInitialize() < 0) + return NULL; if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, _("no connection")); @@ -567,41 +400,26 @@ virGetStoragePool(virConnectPtr conn, const char *name, virCheckNonNullArgReturn(name, NULL); virCheckNonNullArgReturn(uuid, NULL); - virMutexLock(&conn->lock); + if (!(ret = virObjectNew(virStoragePoolClass))) + return NULL; - virUUIDFormat(uuid, uuidstr); + 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_STORAGE_POOL_MAGIC; - ret->conn = conn; + 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. @@ -613,60 +431,17 @@ 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, - _("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; + if (pool->conn) + virObjectUnref(pool->conn); } @@ -680,7 +455,7 @@ 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 */ @@ -689,6 +464,9 @@ virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const char *key) { virStorageVolPtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, _("no connection")); return NULL; @@ -696,52 +474,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. @@ -753,59 +508,16 @@ 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, - _("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; + if (vol->conn) + virObjectUnref(vol->conn); } @@ -817,7 +529,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 */ @@ -826,44 +538,33 @@ virGetNodeDevice(virConnectPtr conn, const char *name) { virNodeDevicePtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, _("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); - 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; } /** - * virReleaseNodeDevice: + * virNodeDeviceDispose: * @dev: the dev to release * * Unconditionally release all memory associated with a dev. @@ -875,53 +576,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; + if (dev->conn) + virObjectUnref(dev->conn); } @@ -932,7 +595,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 @@ -942,7 +605,9 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid, int usageType, const char *usageID) { virSecretPtr ret = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (virDataTypesInitialize() < 0) + return NULL; if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, _("no connection")); @@ -951,39 +616,25 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid, virCheckNonNullArgReturn(uuid, NULL); virCheckNonNullArgReturn(usageID, NULL); - virMutexLock(&conn->lock); - - virUUIDFormat(uuid, uuidstr); + 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 @@ -994,116 +645,41 @@ 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); - } -} - -/** - * 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, _("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; - } - virMutexUnlock(&secret->conn->lock); - return refs; + if (secret->conn) + virObjectUnref(secret->conn); } + virStreamPtr virGetStream(virConnectPtr conn) { virStreamPtr ret = NULL; - virMutexLock(&conn->lock); + if (virDataTypesInitialize() < 0) + return NULL; - 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; + if (!(ret = virObjectNew(virStreamClass))) + return NULL; -error: - virMutexUnlock(&conn->lock); - VIR_FREE(ret); - return NULL; + ret->conn = virObjectRef(conn); + + return ret; } 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; + if (st->conn) + virObjectUnref(st->conn); } @@ -1116,14 +692,16 @@ 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) { virNWFilterPtr ret = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (virDataTypesInitialize() < 0) + return NULL; if (!VIR_IS_CONNECT(conn)) { virLibConnError(VIR_ERR_INVALID_CONN, _("no connection")); @@ -1132,41 +710,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; - virUUIDFormat(uuid, uuidstr); + 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. @@ -1178,63 +741,17 @@ error: * which may also be released if its ref count hits zero. */ static void -virReleaseNWFilter(virNWFilterPtr nwfilter) -{ - virConnectPtr conn = nwfilter->conn; +virNWFilterDispose(void *obj) { + 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, - _("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; + if (nwfilter->conn) + virObjectUnref(nwfilter->conn); } @@ -1243,84 +760,38 @@ virGetDomainSnapshot(virDomainPtr domain, const char *name) { virDomainSnapshotPtr ret = NULL; + if (virDataTypesInitialize() < 0) + return NULL; + if (!VIR_IS_DOMAIN(domain)) { virLibConnError(VIR_ERR_INVALID_DOMAIN, _("bad domain")); return NULL; } virCheckNonNullArgReturn(name, NULL); - virMutexLock(&domain->conn->lock); + if (!(ret = virObjectNew(virDomainSnapshotClass))) + return NULL; - 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->name = strdup(name))) + goto no_memory; - domain->refs++; - ret->refs++; - virMutexUnlock(&domain->conn->lock); - return ret; + ret->domain = virObjectRef(domain); - error: - if (ret != NULL) { - VIR_FREE(ret->name); - VIR_FREE(ret); - } + return ret; +no_memory: + virReportOOMError(); + virObjectUnref(ret); return NULL; } static void -virReleaseDomainSnapshot(virDomainSnapshotPtr snapshot) -{ - virDomainPtr domain = snapshot->domain; +virDomainSnapshotDispose(void *obj) { + 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, _("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; + if (snapshot->domain) + virObjectUnref(snapshot->domain); } diff --git a/src/datatypes.h b/src/datatypes.h index fc284d2..343c807 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -26,118 +26,72 @@ # 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((virObjectPtr)obj, virConnectClass)) + +# define VIR_IS_DOMAIN(obj) \ + (virObjectIsClass((virObjectPtr)obj, virDomainClass)) +# define VIR_IS_CONNECTED_DOMAIN(obj) \ + (VIR_IS_DOMAIN(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_NETWORK(obj) \ + (virObjectIsClass((virObjectPtr)obj, virNetworkClass)) +# define VIR_IS_CONNECTED_NETWORK(obj) \ + (VIR_IS_NETWORK(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_INTERFACE(obj) \ + (virObjectIsClass((virObjectPtr)obj, virInterfaceClass)) +# define VIR_IS_CONNECTED_INTERFACE(obj) \ + (VIR_IS_INTERFACE(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_STORAGE_POOL(obj) \ + (virObjectIsClass((virObjectPtr)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((virObjectPtr)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((virObjectPtr)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((virObjectPtr)obj, virSecretClass)) +# define VIR_IS_CONNECTED_SECRET(obj) \ + (VIR_IS_SECRET(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_STREAM(obj) \ + (virObjectIsClass((virObjectPtr)obj, virStreamClass)) +# define VIR_IS_CONNECTED_STREAM(obj) \ + (VIR_IS_STREAM(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_NWFILTER(obj) \ + (virObjectIsClass((virObjectPtr)obj, virNWFilterClass)) +# define VIR_IS_CONNECTED_NWFILTER(obj) \ + (VIR_IS_NWFILTER(obj) && VIR_IS_CONNECT((obj)->conn)) + +# define VIR_IS_SNAPSHOT(obj) \ + (virObjectIsClass((virObjectPtr)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 */ @@ -186,8 +140,6 @@ struct _virConnect { virError err; /* the last error */ virErrorFunc handler; /* associated handlet */ void *userData; /* the user data */ - - int refs; /* reference count */ }; /** @@ -196,8 +148,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 */ @@ -210,8 +161,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 */ @@ -223,8 +173,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 */ @@ -236,8 +185,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 */ @@ -249,8 +197,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 */ @@ -263,8 +210,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 */ @@ -276,8 +222,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 */ @@ -294,9 +239,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; @@ -309,77 +253,59 @@ 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); +virNetworkPtr virGetNetwork(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); virStorageVolPtr virGetStorageVol(virConnectPtr conn, const char *pool, const char *name, const char *key); -int virUnrefStorageVol(virStorageVolPtr vol); 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 diff --git a/src/libvirt.c b/src/libvirt.c index c6640b2..c16459c 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1273,7 +1273,7 @@ do_open (const char *name, failed: virConfFree(conf); - virUnrefConnect(ret); + virObjectUnref(ret); return NULL; } @@ -1430,10 +1430,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); @@ -1465,10 +1464,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; } @@ -2265,10 +2262,7 @@ virDomainFree(virDomainPtr domain) virDispatchError(NULL); return -1; } - if (virUnrefDomain(domain) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(domain); return 0; } @@ -2297,10 +2291,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; } @@ -10093,10 +10086,7 @@ virNetworkFree(virNetworkPtr network) virDispatchError(NULL); return -1; } - if (virUnrefNetwork(network) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(network); return 0; } @@ -10125,10 +10115,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; } @@ -10998,10 +10986,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; } @@ -11026,10 +11012,7 @@ virInterfaceFree(virInterfacePtr iface) virDispatchError(NULL); return -1; } - if (virUnrefInterface(iface) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(iface); return 0; } @@ -11935,10 +11918,7 @@ virStoragePoolFree(virStoragePoolPtr pool) virDispatchError(NULL); return -1; } - if (virUnrefStoragePool(pool) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(pool); return 0; } @@ -11969,10 +11949,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; } @@ -13001,10 +12979,7 @@ virStorageVolFree(virStorageVolPtr vol) virDispatchError(NULL); return -1; } - if (virUnrefStorageVol(vol) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(vol); return 0; } @@ -13034,10 +13009,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; } @@ -13576,10 +13549,7 @@ int virNodeDeviceFree(virNodeDevicePtr dev) virDispatchError(NULL); return -1; } - if (virUnrefNodeDevice(dev) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(dev); return 0; } @@ -13609,10 +13579,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; } @@ -14582,10 +14550,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; } @@ -14609,10 +14575,7 @@ virSecretFree(virSecretPtr secret) virDispatchError(NULL); return -1; } - if (virUnrefSecret(secret) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(secret); return 0; } @@ -14681,10 +14644,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; } @@ -15325,10 +15286,7 @@ int virStreamFree(virStreamPtr stream) /* XXX Enforce shutdown before free'ing resources ? */ - if (virUnrefStream(stream) < 0) { - virDispatchError(NULL); - return -1; - } + virObjectUnref(stream); return 0; } @@ -15789,10 +15747,8 @@ virNWFilterFree(virNWFilterPtr nwfilter) virDispatchError(NULL); return -1; } - if (virUnrefNWFilter(nwfilter) < 0) { - virDispatchError(NULL); - return -1; - } + + virObjectUnref(nwfilter); return 0; } @@ -16048,10 +16004,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; } @@ -17935,10 +17889,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; } @@ -17964,10 +17916,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 1ff6735..9327d9a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -202,12 +202,17 @@ virGetSecret; virGetStoragePool; virGetStorageVol; virGetStream; -virUnrefConnect; -virUnrefDomain; -virUnrefNWFilter; -virUnrefSecret; -virUnrefStorageVol; -virUnrefStream; +virConnectClass; +virDomainClass; +virDomainSnapshotClass; +virInterfaceClass; +virNetworkClass; +virNodeDeviceClass; +virNWFilterClass; +virSecretClass; +virStreamClass; +virStorageVolClass; +virStoragePoolClass; # dnsmasq.h diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 5136fbc..c349491 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -2089,7 +2089,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; } @@ -2126,7 +2126,7 @@ err: virStorageVolDefFree(voldef); virStoragePoolDefFree(spdef); if (vol) - virUnrefStorageVol(vol); + virObjectUnref(vol); return NULL; } @@ -2326,7 +2326,7 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) cleanup: if (sp) - virUnrefStoragePool(sp); + virObjectUnref(sp); return xml; } @@ -2722,14 +2722,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; } @@ -2744,7 +2744,7 @@ phypStoragePoolCreateXML(virConnectPtr conn, err: virStoragePoolDefFree(def); if (sp) - virUnrefStoragePool(sp); + virObjectUnref(sp); return NULL; } @@ -3690,7 +3690,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 94b2919..97697be 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1805,7 +1805,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 f51c99a..558f064 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2285,14 +2285,14 @@ 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 +2479,14 @@ 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 c5140c3..2aa6af1 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 fbc630d..5b00d75 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1399,7 +1399,7 @@ storageVolumeCreateXML(virStoragePoolPtr obj, cleanup: if (volobj) - virUnrefStorageVol(volobj); + virObjectUnref(volobj); virStorageVolDefFree(voldef); if (pool) virStoragePoolObjUnlock(pool); @@ -1561,7 +1561,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj, 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 ba7552c..4cdc580 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1208,7 +1208,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 f1aa9b6..8751189 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1243,7 +1243,7 @@ error: virXendError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse Xend domain information")); if (ret != NULL) - virUnrefDomain(ret); + virObjectUnref(ret); return NULL; } @@ -2606,7 +2606,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 7b00ea2..6449952 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 @@ -241,7 +230,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 8eca466..451b168 100644 --- a/tests/qemuxmlnstest.c +++ b/tests/qemuxmlnstest.c @@ -166,7 +166,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..93049dd 100644 --- a/tests/sexpr2xmltest.c +++ b/tests/sexpr2xmltest.c @@ -72,7 +72,7 @@ testCompareFiles(const char *xml, const char *sexpr, int xendConfigVersion) VIR_FREE(gotxml); virDomainDefFree(def); if (conn) - virUnrefConnect(conn); + virObjectUnref(conn); return ret; } diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index 9956bf2..75ced8b 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.2

On 07/11/2012 07:35 AM, Daniel P. Berrange wrote: Resuming where I left off... Another instance where we should either remove the uuidstr independently, or start logging it to make the logs more useful. As long as you are touching this area of code, is it worth giving this function consistent formatting? virStreamPtr virGetStream(virConnectPtr conn) { Technically under-parenthesized, should be: (virObjectIsClass((virObjectPtr)(obj), virNetworkClass)) in case 'obj' expands to a non-trivial expression. (Then again, our likelihood of using this macro on a non-trivial expression is practically zero). Same goes for all the other VIR_IS_* macros wrapping virObjectIsClass. Here is where you are changing the return value; you need to update the documentation to match. Not quite sorted. Mostly mechanical, and overall a nice patch, but I did point out one or two issues worth fixing for v2 (at which point, I will just review the interdiff). -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 07/11/2012 07:35 AM, Daniel P. Berrange wrote: This didn't apply cleanly for me, but I was able to get past the merge conflict (I'm sure you'll hit and get past the same issues when you rebase to latest). Backwards logic. You want to unlock if any other reference remains (a true return), not if you were the last reference. virObjectUnref() gracefully handles a NULL input argument, so you can get rid of the if. In fact, it might be worth adding virObjectUnref() to the list of free-like functions in cfg.mk, so our syntax-check will automatically detect these cases. Other than that, this looks okay. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 07/11/2012 07:35 AM, Daniel P. Berrange wrote: I think you're treading on thin water with regards to C99 definedness, but in practice, I'm guessing it is safe to cast a function returning bool to a function returning void (that is, the person calling this callback thinks it is calling a void function, so it will ignore the fact that a value was actually returned). The alternative is to write a one-line wrapper around virObjectUnref that sanitizes the return value. ACK. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Make virSocket use the virObject APIs for reference counting Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- 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 035658e..ee2f581 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1459,7 +1459,6 @@ virNetServerProgramSendStreamError; # virnetsocket.h virNetSocketAccept; virNetSocketDupFD; -virNetSocketFree; virNetSocketGetFD; virNetSocketHasPassFD; virNetSocketIsLocal; 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 61d3d68..8409bc1 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 45f3309..4da5082 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -474,7 +474,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); @@ -497,7 +497,7 @@ virNetClientCloseLocked(virNetClientPtr client) return; virNetSocketRemoveIOCallback(client->sock); - 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 b2ab12f..58b56e1 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -598,7 +598,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); @@ -669,7 +669,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 b4689b4..46cef3a 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 1a64cc0..93ff9dd 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -54,8 +54,9 @@ struct _virNetSocket { + virObject object; + virMutex lock; - int refs; int fd; int watch; @@ -88,6 +89,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); } @@ -1330,7 +1321,7 @@ static void virNetSocketEventFree(void *opaque) if (ff) ff(eopaque); - virNetSocketFree(sock); + virObjectUnref(sock); } int virNetSocketAddIOCallback(virNetSocketPtr sock, @@ -1341,7 +1332,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); @@ -1365,7 +1356,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 5ba7c8f..a42ee3a 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 204113e..080aa9f 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -167,7 +167,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) { @@ -178,16 +178,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; } @@ -223,7 +223,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'; @@ -237,8 +237,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; @@ -315,9 +315,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; @@ -347,7 +347,7 @@ static int testSocketCommandNormal(const void *data ATTRIBUTE_UNUSED) ret = 0; cleanup: - virNetSocketFree(csock); + virObjectUnref(csock); return ret; } @@ -370,7 +370,7 @@ static int testSocketCommandFail(const void *data ATTRIBUTE_UNUSED) ret = 0; cleanup: - virNetSocketFree(csock); + virObjectUnref(csock); return ret; } @@ -439,7 +439,7 @@ static int testSocketSSH(const void *opaque) ret = 0; cleanup: - virNetSocketFree(csock); + virObjectUnref(csock); return ret; } -- 1.7.10.2

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Make all the virNetServer* objects use the virObject APIs for reference countign Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- 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 211a4bc..bed5aa5 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; @@ -1306,10 +1306,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 bb66f75..024a8c9 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -108,14 +108,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 */ @@ -336,14 +328,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; } @@ -369,7 +359,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) { @@ -415,10 +405,11 @@ int daemonAddClientStream(virNetServerClientPtr client, if (virStreamEventAddCallback(stream->st, 0, daemonStreamEvent, client, - daemonStreamEventFreeFunc) < 0) + (virFreeCallback)virObjectUnref) < 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 ee2f581..a129cd5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1406,14 +1406,11 @@ virNetServerAddService; virNetServerAddSignalHandler; virNetServerAutoShutdown; virNetServerClose; -virNetServerFree; virNetServerIsPrivileged; virNetServerKeepAliveRequired; virNetServerNew; virNetServerQuit; -virNetServerRef; virNetServerRun; -virNetServerServiceFree; virNetServerServiceNewTCP; virNetServerServiceNewUNIX; virNetServerUpdateServices; @@ -1423,7 +1420,6 @@ virNetServerUpdateServices; virNetServerClientAddFilter; virNetServerClientClose; virNetServerClientDelayedClose; -virNetServerClientFree; virNetServerClientGetAuth; virNetServerClientGetFD; virNetServerClientGetPrivateData; @@ -1434,7 +1430,6 @@ virNetServerClientHasTLSSession; virNetServerClientImmediateClose; virNetServerClientIsSecure; virNetServerClientLocalAddrString; -virNetServerClientRef; virNetServerClientRemoteAddrString; virNetServerClientRemoveFilter; virNetServerClientSendMessage; @@ -1445,12 +1440,10 @@ virNetServerClientStartKeepAlive; # virnetserverprogram.h -virNetServerProgramFree; virNetServerProgramGetID; 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 a4874ea..40ac06d 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -248,7 +248,7 @@ static void virLXCControllerFree(virLXCControllerPtr ctrl) virDomainDefFree(ctrl->def); VIR_FREE(ctrl->name); - virNetServerFree(ctrl->server); + virObjectUnref(ctrl->server); VIR_FREE(ctrl); } @@ -806,7 +806,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) if (virNetServerAddService(ctrl->server, svc, NULL) < 0) goto error; - virNetServerServiceFree(svc); + virObjectUnref(svc); svc = NULL; virNetServerUpdateServices(ctrl->server, true); @@ -815,9 +815,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 17da40c..acfbeaa 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -69,7 +69,7 @@ struct _virNetServerJob { }; struct _virNetServer { - int refs; + virObjectPtr object; virMutex lock; @@ -116,6 +116,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); @@ -182,18 +198,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); } @@ -230,7 +246,7 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, job->msg = msg; if (prog) { - virNetServerProgramRef(prog); + virObjectRef(prog); job->prog = prog; priority = virNetServerProgramGetPriority(prog, msg->header.proc); } @@ -239,7 +255,7 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client, if (ret < 0) { VIR_FREE(job); - virNetServerProgramFree(prog); + virObjectUnref(prog); } } else { ret = virNetServerProcessMsg(srv, client, prog, msg); @@ -279,7 +295,7 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc ATTRIBUTE_UN goto error; } srv->clients[srv->nclients-1] = client; - virNetServerClientRef(client); + virObjectRef(client); virNetServerClientSetDispatcher(client, virNetServerDispatchNewMessage, @@ -339,12 +355,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, @@ -407,24 +422,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; @@ -614,7 +619,7 @@ int virNetServerAddService(virNetServerPtr srv, #endif srv->services[srv->nservices-1] = svc; - virNetServerServiceRef(svc); + virObjectRef(svc); virNetServerServiceSetDispatcher(svc, virNetServerDispatchNewClient, @@ -640,8 +645,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; @@ -750,7 +754,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, @@ -781,20 +785,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); @@ -812,16 +806,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); @@ -831,7 +825,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 438f524..25c1baa 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 58b56e1..a3b2e74 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -60,7 +60,8 @@ struct _virNetServerClientFilter { struct _virNetServerClient { - int refs; + virObject object; + bool wantClose; bool delayedClose; virMutex lock; @@ -106,6 +107,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); @@ -169,13 +186,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 @@ -184,15 +194,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--; + (virFreeCallback)virObjectUnref) < 0) { + virObjectUnref(client); return -1; } @@ -336,15 +348,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; @@ -367,28 +381,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) { @@ -570,21 +574,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) @@ -601,7 +593,6 @@ void virNetServerClientFree(virNetServerClientPtr client) virObjectUnref(client->sock); virNetServerClientUnlock(client); virMutexDestroy(&client->lock); - VIR_FREE(client); } @@ -619,7 +610,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; @@ -629,20 +620,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 @@ -906,12 +897,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; } } @@ -1170,11 +1161,6 @@ virNetServerClientKeepAliveSendCB(void *opaque, return virNetServerClientSendMessage(opaque, msg); } -static void -virNetServerClientFreeCB(void *opaque) -{ - virNetServerClientFree(opaque); -} int virNetServerClientInitKeepAlive(virNetServerClientPtr client, @@ -1189,10 +1175,10 @@ virNetServerClientInitKeepAlive(virNetServerClientPtr client, if (!(ka = virKeepAliveNew(interval, count, client, virNetServerClientKeepAliveSendCB, virNetServerClientKeepAliveDeadCB, - virNetServerClientFreeCB))) + (virFreeCallback)virObjectUnref))) 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 154a160..428c916 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 7f589c8..db79fa2 100644 --- a/src/rpc/virnetserverprogram.c +++ b/src/rpc/virnetserverprogram.c @@ -30,6 +30,7 @@ #include "virterror_internal.h" #include "logging.h" #include "virfile.h" +#include "threads.h" #define VIR_FROM_THIS VIR_FROM_RPC #define virNetError(code, ...) \ @@ -37,7 +38,7 @@ __FUNCTION__, __LINE__, __VA_ARGS__) struct _virNetServerProgram { - int refs; + virObject object; unsigned program; unsigned version; @@ -45,6 +46,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, @@ -52,18 +70,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; } @@ -81,13 +99,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) { @@ -519,16 +530,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 aa9f3cf..7bc9985 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 46cef3a..ffda655 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); + (virFreeCallback)virObjectUnref) < 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); + (virFreeCallback)virObjectUnref) < 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 8540bd9..67227a9 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.2

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; Make all the virNetClient* objects use virObject APIs for reference counting Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/libvirt_probes.d | 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 +-- 9 files changed, 108 insertions(+), 138 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/remote/remote_driver.c b/src/remote/remote_driver.c index f2600a8..3420ff8 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -757,10 +757,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); @@ -911,10 +911,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 */ @@ -4115,7 +4115,7 @@ remoteStreamFinish(virStreamPtr st) cleanup: virNetClientRemoveStream(priv->client, privst); - virNetClientStreamFree(privst); + virObjectUnref(privst); st->privateData = NULL; st->driver = NULL; @@ -4144,7 +4144,7 @@ remoteStreamAbort(virStreamPtr st) cleanup: virNetClientRemoveStream(priv->client, privst); - virNetClientStreamFree(privst); + virObjectUnref(privst); st->privateData = NULL; st->driver = NULL; @@ -4445,7 +4445,7 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn, goto done; if (virNetClientAddStream(priv->client, netst) < 0) { - virNetClientStreamFree(netst); + virObjectUnref(netst); goto done; } @@ -4463,7 +4463,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 f161ee0..ea31ae9 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -1404,7 +1404,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"; @@ -1480,7 +1480,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 4da5082..9be0592 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -66,7 +66,7 @@ struct _virNetClientCall { struct _virNetClient { - int refs; + virObject object; virMutex lock; @@ -107,6 +107,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, @@ -220,13 +235,6 @@ static bool virNetClientCallMatchPredicate(virNetClientCallPtr head, } -static void virNetClientEventFree(void *opaque) -{ - virNetClientPtr client = opaque; - - virNetClientFree(client); -} - bool virNetClientKeepAliveIsSupported(virNetClientPtr client) { @@ -286,19 +294,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]; @@ -310,13 +321,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--; + (virFreeCallback)virObjectUnref) < 0) { + virObjectUnref(client); VIR_DEBUG("Failed to add event watch, disabling events and support for" " keepalive messages"); } else { @@ -325,16 +336,16 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr sock, if (!(ka = virKeepAliveNew(-1, 0, client, virNetClientKeepAliveSendCB, virNetClientKeepAliveDeadCB, - virNetClientEventFree))) + (virFreeCallback)virObjectUnref))) 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: @@ -346,7 +357,7 @@ error: virKeepAliveStop(ka); virObjectUnref(ka); } - virNetClientFree(client); + virObjectUnref(client); return NULL; } @@ -405,17 +416,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; @@ -446,25 +446,13 @@ 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; - } - for (i = 0 ; i < client->nprograms ; i++) - virNetClientProgramFree(client->programs[i]); + virObjectUnref(client->programs[i]); VIR_FREE(client->programs); VIR_FORCE_CLOSE(client->wakeupSendFD); @@ -481,8 +469,6 @@ void virNetClientFree(virNetClientPtr client) #endif virNetClientUnlock(client); virMutexDestroy(&client->lock); - - VIR_FREE(client); } @@ -510,14 +496,14 @@ virNetClientCloseLocked(virNetClientPtr client) client->wantClose = false; if (ka) { - client->refs++; + virObjectRef(client); virNetClientUnlock(client); virKeepAliveStop(ka); virObjectUnref(ka); virNetClientLock(client); - client->refs--; + virObjectUnref(client); } } @@ -703,8 +689,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; @@ -724,8 +709,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; @@ -759,7 +743,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 13b4f96..6e41e75 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, @@ -51,8 +52,6 @@ virNetClientPtr virNetClientNewSSH(const char *nodename, virNetClientPtr virNetClientNewExternal(const char **cmdargv); -void virNetClientRef(virNetClientPtr client); - int virNetClientGetFD(virNetClientPtr client); int virNetClientDupFD(virNetClientPtr client, bool cloexec); @@ -96,7 +95,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 e1e8846..f6d2173 100644 --- a/src/rpc/virnetclientprogram.c +++ b/src/rpc/virnetclientprogram.c @@ -33,6 +33,7 @@ #include "logging.h" #include "util.h" #include "virfile.h" +#include "threads.h" #define VIR_FROM_THIS VIR_FROM_RPC #define virNetError(code, ...) \ @@ -40,7 +41,7 @@ __FUNCTION__, __LINE__, __VA_ARGS__) struct _virNetClientProgram { - int refs; + virObject object; unsigned program; unsigned version; @@ -49,6 +50,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, @@ -57,12 +74,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; @@ -73,22 +90,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 14a4c96..3fde3bf 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 de61a62..acdb67c 100644 --- a/src/rpc/virnetclientstream.c +++ b/src/rpc/virnetclientstream.c @@ -36,12 +36,13 @@ __FUNCTION__, __LINE__, __VA_ARGS__) struct _virNetClientStream { + virObject object; + virMutex lock; virNetClientProgramPtr prog; int proc; unsigned serial; - int refs; virError err; @@ -66,6 +67,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) { @@ -122,26 +139,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; @@ -153,35 +162,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, @@ -457,13 +450,13 @@ int virNetClientStreamEventAddCallback(virNetClientStreamPtr st, goto cleanup; } - st->refs++; + virObjectRef(st); if ((st->cbTimer = virEventAddTimeout(-1, virNetClientStreamEventTimer, st, - virNetClientStreamEventTimerFree)) < 0) { - st->refs--; + (virFreeCallback)virObjectUnref)) < 0) { + virObjectUnref(st); goto cleanup; } diff --git a/src/rpc/virnetclientstream.h b/src/rpc/virnetclientstream.h index fd7a2ee..9e22adb 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.2

On Tue, Jul 17, 2012 at 02:36:18PM -0600, Eric Blake wrote: FYI, this will be temporary nastiness. I intend to actually add a virMutex to the virObjectPtr base object in a future patch series, so subclasses won't need to maintain their own mutexes. So we'll easily be able todo the right thing wrt to poisoning then. 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 :|