提交 97bed42f 编写于 作者: D Daniel P. Berrange

Fix & refactor ref counting cleanup code

上级 c9b7eae9
Mon Jan 21 11:18:04 EST 2008 Daniel P. Berrange <berrange@redhat.com>
* src/hash.c: Split the virFree* functions into virUnref*
and virRelease*. Switch to use standard pthread_mutex_t type.
* src/internal.h: Rename virFree* to virUnref*
* src/qemu_driver.c: Remove bogus release of object in
virDomainDestroy and virNetworkDestroy.
* src/xend_internal.c, src/libvirt.c: Update to call virUnref*
* src/xs_internal.c: Remove dead code & mark functions static
Mon Jan 21 10:52:04 EST 2008 Daniel P. Berrange <berrange@redhat.com>
* python/generator.py: Abort with non-zero status if any functions
......
......@@ -25,9 +25,13 @@
#include <libxml/threads.h>
#include "internal.h"
#include "hash.h"
#include <pthread.h>
#define MAX_HASH_LEN 8
#define DEBUG(fmt,...) VIR_DEBUG(__FILE__, fmt, __VA_ARGS__)
#define DEBUG0(msg) VIR_DEBUG(__FILE__, "%s", msg)
/* #define DEBUG_GROW */
/*
......@@ -677,60 +681,77 @@ virGetConnect(void) {
ret->networks = virHashCreate(20);
if (ret->networks == NULL)
goto failed;
ret->hashes_mux = xmlNewMutex();
if (ret->hashes_mux == NULL)
goto failed;
ret->uses = 1;
pthread_mutex_init(&ret->lock, NULL);
ret->refs = 1;
return(ret);
failed:
if (ret != NULL) {
if (ret->domains != NULL)
virHashFree(ret->domains, (virHashDeallocator) virDomainFreeName);
if (ret->networks != NULL)
virHashFree(ret->networks, (virHashDeallocator) virNetworkFreeName);
if (ret->hashes_mux != NULL)
xmlFreeMutex(ret->hashes_mux);
if (ret->domains != NULL)
virHashFree(ret->domains, (virHashDeallocator) virDomainFreeName);
if (ret->networks != NULL)
virHashFree(ret->networks, (virHashDeallocator) virNetworkFreeName);
pthread_mutex_destroy(&ret->lock);
free(ret);
}
return(NULL);
}
/**
* virFreeConnect:
* @conn: the hypervisor connection
* virReleaseConnect:
* @conn: the hypervisor connection to release
*
* Release the connection. if the use count drops to zero, the structure is
* Unconditionally release all memory associated with a connection.
* The conn.lock mutex must be held prior to calling this, and will
* be released prior to this returning. The connection obj must not
* be used once this method returns.
*/
static void
virReleaseConnect(virConnectPtr conn) {
DEBUG("release connection %p %s", conn, conn->name);
if (conn->domains != NULL)
virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName);
if (conn->networks != NULL)
virHashFree(conn->networks, (virHashDeallocator) virNetworkFreeName);
virResetError(&conn->err);
free(conn->name);
pthread_mutex_unlock(&conn->lock);
pthread_mutex_destroy(&conn->lock);
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
virFreeConnect(virConnectPtr conn) {
int ret;
int
virUnrefConnect(virConnectPtr conn) {
int refs;
if ((!VIR_IS_CONNECT(conn)) || (conn->hashes_mux == NULL)) {
if ((!VIR_IS_CONNECT(conn))) {
virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(-1);
}
xmlMutexLock(conn->hashes_mux);
conn->uses--;
ret = conn->uses;
if (ret > 0) {
xmlMutexUnlock(conn->hashes_mux);
return(ret);
pthread_mutex_lock(&conn->lock);
DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs);
conn->refs--;
refs = conn->refs;
if (refs == 0) {
virReleaseConnect(conn);
/* Already unlocked mutex */
return (0);
}
if (conn->domains != NULL)
virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName);
if (conn->networks != NULL)
virHashFree(conn->networks, (virHashDeallocator) virNetworkFreeName);
if (conn->hashes_mux != NULL)
xmlFreeMutex(conn->hashes_mux);
virResetError(&conn->err);
free(conn);
return(0);
pthread_mutex_unlock(&conn->lock);
return (refs);
}
/**
......@@ -742,7 +763,7 @@ virFreeConnect(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
* virFreeDomain() is needed to not leak data.
* virUnrefDomain() is needed to not leak data.
*
* Returns a pointer to the domain, or NULL in case of failure
*/
......@@ -750,120 +771,122 @@ virDomainPtr
__virGetDomain(virConnectPtr conn, const char *name, const unsigned char *uuid) {
virDomainPtr ret = NULL;
if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL) ||
(conn->hashes_mux == NULL)) {
if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) {
virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(NULL);
}
xmlMutexLock(conn->hashes_mux);
pthread_mutex_lock(&conn->lock);
/* TODO search by UUID first as they are better differenciators */
ret = (virDomainPtr) virHashLookup(conn->domains, name);
if (ret != NULL) {
/* TODO check the UUID */
goto done;
}
/*
* not found, allocate a new one
*/
ret = calloc(1, sizeof(*ret));
/* TODO check the UUID */
if (ret == NULL) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain"));
goto error;
}
ret->name = strdup(name);
if (ret->name == NULL) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain"));
goto error;
}
ret->magic = VIR_DOMAIN_MAGIC;
ret->conn = conn;
ret->id = -1;
if (uuid != NULL)
memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
if (virHashAddEntry(conn->domains, name, ret) < 0) {
virHashError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to add domain to connection hash table"));
goto error;
ret = (virDomainPtr) calloc(1, sizeof(*ret));
if (ret == NULL) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain"));
goto error;
}
ret->name = strdup(name);
if (ret->name == NULL) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating domain"));
goto error;
}
ret->magic = VIR_DOMAIN_MAGIC;
ret->conn = conn;
ret->id = -1;
if (uuid != NULL)
memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
if (virHashAddEntry(conn->domains, name, ret) < 0) {
virHashError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to add domain to connection hash table"));
goto error;
}
conn->refs++;
}
conn->uses++;
done:
ret->uses++;
xmlMutexUnlock(conn->hashes_mux);
ret->refs++;
pthread_mutex_unlock(&conn->lock);
return(ret);
error:
xmlMutexUnlock(conn->hashes_mux);
error:
pthread_mutex_unlock(&conn->lock);
if (ret != NULL) {
if (ret->name != NULL)
free(ret->name );
free(ret);
if (ret->name != NULL)
free(ret->name );
free(ret);
}
return(NULL);
}
/**
* virFreeDomain:
* @conn: the hypervisor connection
* virReleaseDomain:
* @domain: the domain to release
*
* Release the given domain, if the reference count drops to zero, then
* the domain is really freed.
* Unconditionally release all memory associated with a domain.
* The conn.lock mutex must be held prior to calling this, and will
* be released prior to this returning. The domain obj must not
* be used once this method returns.
*
* Returns the reference count or -1 in case of failure.
* It will also unreference the associated connection object,
* which may also be released if its ref count hits zero.
*/
int
virFreeDomain(virConnectPtr conn, virDomainPtr domain) {
int ret = 0;
if ((!VIR_IS_CONNECT(conn)) || (!VIR_IS_CONNECTED_DOMAIN(domain)) ||
(domain->conn != conn) || (conn->hashes_mux == NULL)) {
virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(-1);
}
xmlMutexLock(conn->hashes_mux);
/*
* decrement the count for the domain
*/
domain->uses--;
ret = domain->uses;
if (ret > 0)
goto done;
static void
virReleaseDomain(virDomainPtr domain) {
virConnectPtr conn = domain->conn;
DEBUG("release domain %p %s", domain, domain->name);
/* TODO search by UUID first as they are better differenciators */
if (virHashRemoveEntry(conn->domains, domain->name, NULL) < 0) {
if (virHashRemoveEntry(conn->domains, domain->name, NULL) < 0)
virHashError(conn, VIR_ERR_INTERNAL_ERROR,
_("domain missing from connection hash table"));
goto done;
}
_("domain missing from connection hash table"));
domain->magic = -1;
domain->id = -1;
if (domain->name)
free(domain->name);
free(domain->name);
free(domain);
/*
* decrement the count for the connection
*/
conn->uses--;
if (conn->uses > 0)
goto done;
if (conn->domains != NULL)
virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName);
if (conn->hashes_mux != NULL)
xmlFreeMutex(conn->hashes_mux);
free(conn);
return(0);
DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs);
conn->refs--;
if (conn->refs == 0) {
virReleaseConnect(conn);
/* Already unlocked mutex */
return;
}
done:
xmlMutexUnlock(conn->hashes_mux);
return(ret);
pthread_mutex_unlock(&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)) {
virHashError(domain->conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(-1);
}
pthread_mutex_lock(&domain->conn->lock);
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);
}
pthread_mutex_lock(&domain->conn->lock);
return (refs);
}
/**
......@@ -875,7 +898,7 @@ done:
* 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
* virFreeNetwork() is needed to not leak data.
* virUnrefNetwork() is needed to not leak data.
*
* Returns a pointer to the network, or NULL in case of failure
*/
......@@ -883,120 +906,127 @@ virNetworkPtr
__virGetNetwork(virConnectPtr conn, const char *name, const unsigned char *uuid) {
virNetworkPtr ret = NULL;
if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL) ||
(conn->hashes_mux == NULL)) {
if ((!VIR_IS_CONNECT(conn)) || (name == NULL) || (uuid == NULL)) {
virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(NULL);
}
xmlMutexLock(conn->hashes_mux);
pthread_mutex_lock(&conn->lock);
/* TODO search by UUID first as they are better differenciators */
ret = (virNetworkPtr) virHashLookup(conn->networks, name);
if (ret != NULL) {
/* TODO check the UUID */
goto done;
}
/*
* not found, allocate a new one
*/
ret = calloc(1, sizeof(*ret));
/* TODO check the UUID */
if (ret == NULL) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network"));
goto error;
}
ret->name = strdup(name);
if (ret->name == NULL) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network"));
goto error;
}
ret->magic = VIR_NETWORK_MAGIC;
ret->conn = conn;
if (uuid != NULL)
memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
if (virHashAddEntry(conn->networks, name, ret) < 0) {
virHashError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to add network to connection hash table"));
goto error;
ret = (virNetworkPtr) calloc(1, sizeof(*ret));
if (ret == NULL) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network"));
goto error;
}
ret->name = strdup(name);
if (ret->name == NULL) {
virHashError(conn, VIR_ERR_NO_MEMORY, _("allocating network"));
goto error;
}
ret->magic = VIR_NETWORK_MAGIC;
ret->conn = conn;
if (uuid != NULL)
memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN);
if (virHashAddEntry(conn->networks, name, ret) < 0) {
virHashError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to add network to connection hash table"));
goto error;
}
conn->refs++;
}
conn->uses++;
done:
ret->uses++;
xmlMutexUnlock(conn->hashes_mux);
ret->refs++;
pthread_mutex_unlock(&conn->lock);
return(ret);
error:
xmlMutexUnlock(conn->hashes_mux);
error:
pthread_mutex_unlock(&conn->lock);
if (ret != NULL) {
if (ret->name != NULL)
free(ret->name );
free(ret);
if (ret->name != NULL)
free(ret->name );
free(ret);
}
return(NULL);
}
/**
* virFreeNetwork:
* @conn: the hypervisor connection
* virReleaseNetwork:
* @network: the network to release
*
* Release the given network, if the reference count drops to zero, then
* the network is really freed.
* Unconditionally release all memory associated with a network.
* The conn.lock mutex must be held prior to calling this, and will
* be released prior to this returning. The network obj must not
* be used once this method returns.
*
* Returns the reference count or -1 in case of failure.
* It will also unreference the associated connection object,
* which may also be released if its ref count hits zero.
*/
int
virFreeNetwork(virConnectPtr conn, virNetworkPtr network) {
int ret = 0;
if ((!VIR_IS_CONNECT(conn)) || (!VIR_IS_CONNECTED_NETWORK(network)) ||
(network->conn != conn) || (conn->hashes_mux == NULL)) {
virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(-1);
}
xmlMutexLock(conn->hashes_mux);
/*
* decrement the count for the network
*/
network->uses--;
ret = network->uses;
if (ret > 0)
goto done;
static void
virReleaseNetwork(virNetworkPtr network) {
virConnectPtr conn = network->conn;
DEBUG("release network %p %s", network, network->name);
/* TODO search by UUID first as they are better differenciators */
if (virHashRemoveEntry(conn->networks, network->name, NULL) < 0) {
if (virHashRemoveEntry(conn->networks, network->name, NULL) < 0)
virHashError(conn, VIR_ERR_INTERNAL_ERROR,
_("network missing from connection hash table"));
goto done;
}
_("network missing from connection hash table"));
network->magic = -1;
if (network->name)
free(network->name);
free(network->name);
free(network);
/*
* decrement the count for the connection
*/
conn->uses--;
if (conn->uses > 0)
goto done;
DEBUG("unref connection %p %s %d", conn, conn->name, conn->refs);
conn->refs--;
if (conn->refs == 0) {
virReleaseConnect(conn);
/* Already unlocked mutex */
return;
}
if (conn->networks != NULL)
virHashFree(conn->networks, (virHashDeallocator) virNetworkFreeName);
if (conn->hashes_mux != NULL)
xmlFreeMutex(conn->hashes_mux);
free(conn);
return(0);
pthread_mutex_unlock(&conn->lock);
}
done:
xmlMutexUnlock(conn->hashes_mux);
return(ret);
/**
* 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)) {
virHashError(network->conn, VIR_ERR_INVALID_ARG, __FUNCTION__);
return(-1);
}
pthread_mutex_lock(&network->conn->lock);
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);
}
pthread_mutex_lock(&network->conn->lock);
return (refs);
}
/*
* vim: set tabstop=4:
* vim: set shiftwidth=4:
* vim: set expandtab:
*/
/*
* Local variables:
* indent-tabs-mode: nil
......
......@@ -149,8 +149,8 @@ extern int debugFlag;
*/
struct _virConnect {
unsigned int magic; /* specific value to check */
int uses; /* reference count */
int flags; /* a set of connection flags */
char *name; /* connection URI */
/* The underlying hypervisor driver and network driver. */
virDriverPtr driver;
......@@ -168,12 +168,16 @@ struct _virConnect {
virErrorFunc handler; /* associated handlet */
void *userData; /* the user data */
/* misc */
xmlMutexPtr hashes_mux;/* a mutex to protect the domain and networks hash tables */
virHashTablePtr domains;/* hash table for known domains */
virHashTablePtr networks;/* hash table for known domains */
int flags; /* a set of connection flags */
char *name; /* connection URI */
/*
* The lock mutex must be acquired before accessing/changing
* any of members following this point, or changing the ref
* count of any virDomain/virNetwork object associated with
* this connection
*/
pthread_mutex_t lock;
virHashTablePtr domains; /* hash table for known domains */
virHashTablePtr networks; /* hash table for known domains */
int refs; /* reference count */
};
/**
......@@ -183,7 +187,7 @@ struct _virConnect {
*/
struct _virDomain {
unsigned int magic; /* specific value to check */
int uses; /* reference count */
int refs; /* reference count */
virConnectPtr conn; /* pointer back to the connection */
char *name; /* the domain external name */
int id; /* the domain ID */
......@@ -197,18 +201,12 @@ struct _virDomain {
*/
struct _virNetwork {
unsigned int magic; /* specific value to check */
int uses; /* reference count */
int refs; /* reference count */
virConnectPtr conn; /* pointer back to the connection */
char *name; /* the network external name */
unsigned char uuid[VIR_UUID_BUFLEN]; /* the network unique identifier */
};
/*
* Internal routines
*/
char *virDomainGetVM(virDomainPtr domain);
char *virDomainGetVMInfo(virDomainPtr domain,
const char *vm, const char *name);
/************************************************************************
* *
......@@ -234,18 +232,16 @@ const char *__virErrorMsg(virErrorNumber error, const char *info);
* *
************************************************************************/
virConnectPtr virGetConnect (void);
int virFreeConnect (virConnectPtr conn);
virDomainPtr __virGetDomain (virConnectPtr conn,
const char *name,
const unsigned char *uuid);
int virFreeDomain (virConnectPtr conn,
virDomainPtr domain);
virNetworkPtr __virGetNetwork (virConnectPtr conn,
const char *name,
const unsigned char *uuid);
int virFreeNetwork (virConnectPtr conn,
virNetworkPtr domain);
virConnectPtr virGetConnect (void);
int virUnrefConnect (virConnectPtr conn);
virDomainPtr __virGetDomain (virConnectPtr conn,
const char *name,
const unsigned char *uuid);
int virUnrefDomain (virDomainPtr domain);
virNetworkPtr __virGetNetwork (virConnectPtr conn,
const char *name,
const unsigned char *uuid);
int virUnrefNetwork (virNetworkPtr network);
#define virGetDomain(c,n,u) __virGetDomain((c),(n),(u))
#define virGetNetwork(c,n,u) __virGetNetwork((c),(n),(u))
......
......@@ -618,7 +618,7 @@ failed:
if (ret->name) free (ret->name);
if (ret->driver) ret->driver->close (ret);
if (uri) xmlFreeURI(uri);
virFreeConnect(ret);
virUnrefConnect(ret);
return NULL;
}
......@@ -705,9 +705,7 @@ virConnectClose(virConnectPtr conn)
conn->networkDriver->close (conn);
conn->driver->close (conn);
if (conn->name) free (conn->name);
if (virFreeConnect(conn) < 0)
if (virUnrefConnect(conn) < 0)
return (-1);
return (0);
}
......@@ -1211,7 +1209,7 @@ virDomainFree(virDomainPtr domain)
virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
return (-1);
}
if (virFreeDomain(domain->conn, domain) < 0)
if (virUnrefDomain(domain) < 0)
return (-1);
return(0);
}
......@@ -3324,7 +3322,7 @@ virNetworkFree(virNetworkPtr network)
virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
return (-1);
}
if (virFreeNetwork(network->conn, network) < 0)
if (virUnrefNetwork(network) < 0)
return (-1);
return(0);
}
......
......@@ -1888,7 +1888,7 @@ static int qemudDomainDestroy(virDomainPtr dom) {
qemudShutdownVMDaemon(dom->conn, driver, vm);
if (!vm->configFile[0])
qemudRemoveInactiveVM(driver, vm);
virFreeDomain(dom->conn, dom);
return 0;
}
......@@ -2764,8 +2764,6 @@ static int qemudNetworkDestroy(virNetworkPtr net) {
ret = qemudShutdownNetworkDaemon(net->conn, driver, network);
virFreeNetwork(net->conn, net);
return ret;
}
......
......@@ -2038,7 +2038,7 @@ error:
virXendError(conn, VIR_ERR_INTERNAL_ERROR,
_("failed to parse Xend domain information"));
if (ret != NULL)
virFreeDomain(conn, ret);
virUnrefDomain(ret);
return(NULL);
}
#endif /* !PROXY */
......@@ -3185,7 +3185,7 @@ xenDaemonCreateLinux(virConnectPtr conn, const char *xmlDesc,
/* Make sure we don't leave a still-born domain around */
if (dom != NULL) {
xenDaemonDomainDestroy(dom);
virFreeDomain(dom->conn, dom);
virUnrefDomain(dom);
}
if (name != NULL)
free(name);
......
......@@ -227,7 +227,7 @@ virDomainDoStoreWrite(virDomainPtr domain, const char *path,
*
* Returns the new string or NULL in case of error
*/
char *
static char *
virDomainGetVM(virDomainPtr domain)
{
char *vm;
......@@ -261,7 +261,7 @@ virDomainGetVM(virDomainPtr domain)
*
* Returns the new string or NULL in case of error
*/
char *
static char *
virDomainGetVMInfo(virDomainPtr domain, const char *vm, const char *name)
{
char s[256];
......@@ -284,30 +284,6 @@ virDomainGetVMInfo(virDomainPtr domain, const char *vm, const char *name)
return (ret);
}
#if 0
/**
* virConnectCheckStoreID:
* @conn: pointer to the hypervisor connection
* @id: the id number as returned from Xenstore
*
* the xenstore sometimes list non-running domains, double check
* from the hypervisor if we have direct access
*
* Returns -1 if the check failed, 0 if successful or not possible to check
*/
static int
virConnectCheckStoreID(virConnectPtr conn, int id)
{
if (conn->id >= 0) {
int tmp;
tmp = xenHypervisorCheckID(conn, id);
if (tmp < 0)
return (-1);
}
return (0);
}
#endif
#endif /* ! PROXY */
/************************************************************************
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册