diff --git a/ChangeLog b/ChangeLog index 6d80cbedd7..f34b2ce9ed 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +Sun Apr 9 13:10:34 EDT 2006 Daniel Veillard + + * TODO src/hash.[ch] src/internal.h src/libvirt.c src/xend_internal.c + src/xs_internal.c: implementing domain pointers unification, thread + safety and reference counting for domain and connections, this was + the last critical change needed before making further progresses at + the API level. Still a couple fo things TODO for this, unification + at the Python level and adding UUID to hash. All domain/connect alloc + and free routines are now centralized in hash.c + * docs/APIchunk1.html docs/libvirt-api.xml docs/libvirt-refs.xml + docs/html/libvirt-libvirt.html: regenerated the docs, that doesn't + change the API. + Thu Apr 6 11:32:46 CEST 2006 Karel Zak * src/virsh.c: use stdout for standard outputs, improve diff --git a/TODO b/TODO index afbe1ef762..a6c51ab594 100644 --- a/TODO +++ b/TODO @@ -1,6 +1,3 @@ -Absolute TODOs: -- thread protection, reentrancy, refcounting, etc ... - TODO: - Create() API, how do we best keep flexibility and allow various specific environment and space for evolution (VMX) @@ -12,6 +9,8 @@ TODO: to return a non-None object - Add uuid to XML format - add error handling hooks at the python level +- object unicity for domains at the Python level +- UUID lookup in hash.c virsh TODO: - decide where will be default directory for domains configurations (/etc/xen/domains/* ?) @@ -48,3 +47,4 @@ Done: - UUID based lookup and naming - Error API similar to libxml2 structured API - extract error messages from the Xend rpc +- thread protection, reentrancy, refcounting, etc ... diff --git a/docs/APIchunk1.html b/docs/APIchunk1.html index 63cc24dba9..47d9fed224 100644 --- a/docs/APIchunk1.html +++ b/docs/APIchunk1.html @@ -145,7 +145,8 @@ virDomainSave

Letter k:

kept
virDomainFree
kernel
_virDomainKernel
-
kilobytes
virDomainGetMaxMemory
+
kilobytes
_virNodeInfo
+virDomainGetMaxMemory
virDomainSetMaxMemory
knowing
virDomainShutdown

Letter l:

lack
virConnectGetVersion
@@ -196,7 +197,6 @@ virDomainShutdown
virDomainSuspend
virGetLastError
-
megabytes
_virNodeInfo
mem
_virNodeInfo
memory
_virDomainInfo
_virNodeInfo
diff --git a/docs/html/libvirt-libvirt.html b/docs/html/libvirt-libvirt.html index c06d0ef2cc..cb9be52be1 100644 --- a/docs/html/libvirt-libvirt.html +++ b/docs/html/libvirt-libvirt.html @@ -97,7 +97,7 @@ The content of this structure is not made public by the API. }

Structure virNodeInfo

Structure virNodeInfo
struct _virNodeInfo { charmodel[32] model : string indicating the CPU model - unsigned long memory : memory size in megabytes + unsigned long memory : memory size in kilobytes unsigned int cpus : the number of active CPUs unsigned int mhz : expected CPU frequency unsigned int nodes : the number of NUMA cell, 1 for uniform diff --git a/docs/libvirt-api.xml b/docs/libvirt-api.xml index 67b86867a4..a69776bac7 100644 --- a/docs/libvirt-api.xml +++ b/docs/libvirt-api.xml @@ -237,7 +237,7 @@ - + diff --git a/docs/libvirt-refs.xml b/docs/libvirt-refs.xml index 0ccfc12271..e58a67f117 100644 --- a/docs/libvirt-refs.xml +++ b/docs/libvirt-refs.xml @@ -1232,6 +1232,7 @@ + @@ -1320,9 +1321,6 @@ - - - diff --git a/src/hash.c b/src/hash.c index 65fb21b8ac..69c070de2a 100644 --- a/src/hash.c +++ b/src/hash.c @@ -1,5 +1,5 @@ /* - * hash.c: chained hash tables + * hash.c: chained hash tables for domain and domain/connection deallocations * * Reference: Your favorite introductory book on algorithms * @@ -15,10 +15,13 @@ * CONTRIBUTORS ACCEPT NO RESPONSIBILITY IN ANY CONCEIVABLE MANNER. * * Author: breese@users.sourceforge.net + * Daniel Veillard */ #include #include +#include +#include "internal.h" #include "hash.h" #define MAX_HASH_LEN 8 @@ -469,3 +472,254 @@ virHashRemoveEntry(virHashTablePtr table, const char *name, return (-1); } } + +/************************************************************************ + * * + * Domain and Connections allocations * + * * + ************************************************************************/ + +/** + * virHashError: + * @conn: the connection if available + * @error: the error noumber + * @info: extra information string + * + * Handle an error at the connection level + */ +static void +virHashError(virConnectPtr conn, virErrorNumber error, const char *info) +{ + const char *errmsg; + + if (error == VIR_ERR_OK) + return; + + errmsg = __virErrorMsg(error, info); + __virRaiseError(conn, NULL, VIR_FROM_NONE, error, VIR_ERR_ERROR, + errmsg, info, NULL, 0, 0, errmsg, info); +} + + +/** + * virDomainFreeName: + * @domain: a domain object + * + * Destroy the domain object, this is just used by the domain hash callback. + * + * Returns 0 in case of success and -1 in case of failure. + */ +static int +virDomainFreeName(virDomainPtr domain, const char *name ATTRIBUTE_UNUSED) +{ + return (virDomainFree(domain)); +} + +/** + * virGetConnect: + * + * Allocates a new hypervisor connection structure + * + * Returns a new pointer or NULL in case of error. + */ +virConnectPtr +virGetConnect(void) { + virConnectPtr ret; + + ret = (virConnectPtr) malloc(sizeof(virConnect)); + if (ret == NULL) { + virHashError(NULL, VIR_ERR_NO_MEMORY, "Allocating connection"); + goto failed; + } + memset(ret, 0, sizeof(virConnect)); + ret->magic = VIR_CONNECT_MAGIC; + ret->nb_drivers = 0; + ret->domains = virHashCreate(20); + if (ret->domains == NULL) + goto failed; + ret->domains_mux = xmlNewMutex(); + if (ret->domains_mux == NULL) + goto failed; + + ret->uses = 1; + return(ret); + +failed: + if (ret != NULL) { + if (ret->domains != NULL) + virHashFree(ret->domains, (virHashDeallocator) virDomainFreeName); + if (ret->domains_mux != NULL) + xmlFreeMutex(ret->domains_mux); + free(ret); + } + return(NULL); +} + +/** + * virFreeConnect: + * @conn: the hypervisor connection + * + * Release 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; + + if ((!VIR_IS_CONNECT(conn)) || (conn->domains_mux == NULL)) { + virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + return(-1); + } + xmlMutexLock(conn->domains_mux); + conn->uses--; + ret = conn->uses; + if (ret > 0) { + xmlMutexUnlock(conn->domains_mux); + return(ret); + } + + if (conn->domains != NULL) + virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName); + if (conn->domains_mux != NULL) + xmlFreeMutex(conn->domains_mux); + free(conn); + return(0); +} + +/** + * virGetDomain: + * @conn: the hypervisor connection + * @name: pointer to the domain name or NULL + * @uuid: pointer to the uuid or NULL + * + * 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. + * + * Returns a pointer to the domain, or NULL in case of failure + */ +virDomainPtr +virGetDomain(virConnectPtr conn, const char *name, const char *uuid) { + virDomainPtr ret = NULL; + + if ((!VIR_IS_CONNECT(conn)) || ((name == NULL) && (uuid == NULL)) || + (conn->domains_mux == NULL)) { + virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + return(NULL); + } + xmlMutexLock(conn->domains_mux); + + /* 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 = (virDomainPtr) malloc(sizeof(virDomain)); + if (ret == NULL) { + virHashError(conn, VIR_ERR_NO_MEMORY, "Allocating domain"); + goto error; + } + memset(ret, 0, sizeof(virDomain)); + 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; + if (uuid != NULL) + memcpy(&(ret->uuid[0]), uuid, 16); + + if (virHashAddEntry(conn->domains, name, ret) < 0) { + virHashError(conn, VIR_ERR_INTERNAL_ERROR, + "Failed to add domain to connectio hash table"); + goto error; + } + conn->uses++; +done: + ret->uses++; + xmlMutexUnlock(conn->domains_mux); + return(ret); + +error: + xmlMutexUnlock(conn->domains_mux); + if (ret != NULL) { + if (ret->name != NULL) + free(ret->name ); + free(ret); + } + return(NULL); +} + +/** + * virFreeDomain: + * @conn: the hypervisor connection + * @domain: the domain to release + * + * Release the given domain, if the reference count drops to zero, then + * the domain is really freed. + * + * Returns the reference count or -1 in case of failure. + */ +int +virFreeDomain(virConnectPtr conn, virDomainPtr domain) { + int ret = 0; + + if ((!VIR_IS_CONNECT(conn)) || (!VIR_IS_CONNECTED_DOMAIN(domain)) || + (domain->conn != conn) || (conn->domains_mux == NULL)) { + virHashError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); + return(-1); + } + xmlMutexLock(conn->domains_mux); + + /* + * decrement the count for the domain + */ + domain->uses--; + ret = domain->uses; + if (ret > 0) + goto done; + + /* TODO search by UUID first as they are better differenciators */ + + if (virHashRemoveEntry(conn->domains, domain->name, NULL) < 0) { + virHashError(conn, VIR_ERR_INTERNAL_ERROR, + "domain missing from connection hash table"); + goto done; + } + domain->magic = -1; + domain->handle = -1; + if (domain->path != NULL) + free(domain->path); + if (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->domains_mux != NULL) + xmlFreeMutex(conn->domains_mux); + free(conn); + return(0); + +done: + xmlMutexUnlock(conn->domains_mux); + return(ret); +} + diff --git a/src/hash.h b/src/hash.h index 5077580184..5becd9a4e3 100644 --- a/src/hash.h +++ b/src/hash.h @@ -1,11 +1,12 @@ /* - * Summary: Chained hash tables - * Description: This module implements the hash table support used in - * various places in the library. + * Summary: Chained hash tables and domain/connections handling + * Description: This module implements the hash table and allocation and + * deallocation of domains and connections * * Copy: Copyright (C) 2005 Red Hat, Inc. * * Author: Bjorn Reese + * Daniel Veillard */ #ifndef __VIR_HASH_H__ @@ -18,8 +19,8 @@ extern "C" { /* * The hash table. */ - typedef struct _virHashTable virHashTable; - typedef virHashTable *virHashTablePtr; +typedef struct _virHashTable virHashTable; +typedef virHashTable *virHashTablePtr; /* * function types: @@ -32,35 +33,34 @@ extern "C" { * * Callback to free data from a hash. */ - typedef void (*virHashDeallocator) (void *payload, char *name); +typedef void (*virHashDeallocator) (void *payload, char *name); /* * Constructor and destructor. */ - virHashTablePtr virHashCreate(int size); - void - virHashFree(virHashTablePtr table, virHashDeallocator f); - int virHashSize(virHashTablePtr table); +virHashTablePtr virHashCreate(int size); +void virHashFree(virHashTablePtr table, virHashDeallocator f); +int virHashSize(virHashTablePtr table); /* * Add a new entry to the hash table. */ - int virHashAddEntry(virHashTablePtr table, - const char *name, void *userdata); - int virHashUpdateEntry(virHashTablePtr table, - const char *name, - void *userdata, virHashDeallocator f); +int virHashAddEntry(virHashTablePtr table, + const char *name, void *userdata); +int virHashUpdateEntry(virHashTablePtr table, + const char *name, + void *userdata, virHashDeallocator f); /* * Remove an entry from the hash table. */ - int virHashRemoveEntry(virHashTablePtr table, - const char *name, virHashDeallocator f); +int virHashRemoveEntry(virHashTablePtr table, + const char *name, virHashDeallocator f); /* * Retrieve the userdata. */ - void *virHashLookup(virHashTablePtr table, const char *name); +void *virHashLookup(virHashTablePtr table, const char *name); #ifdef __cplusplus } diff --git a/src/internal.h b/src/internal.h index 97d7d0ee6f..3e49b61fd1 100644 --- a/src/internal.h +++ b/src/internal.h @@ -94,6 +94,7 @@ extern "C" { struct _virConnect { unsigned int magic; /* specific value to check */ + int uses; /* reference count */ /* the list of available drivers for that connection */ virDriverPtr drivers[MAX_DRIVERS]; int nb_drivers; @@ -137,6 +138,7 @@ enum { */ struct _virDomain { unsigned int magic; /* specific value to check */ + int uses; /* reference count */ virConnectPtr conn; /* pointer back to the connection */ char *name; /* the domain external name */ char *path; /* the domain internal path */ @@ -152,6 +154,11 @@ char *virDomainGetVM(virDomainPtr domain); char *virDomainGetVMInfo(virDomainPtr domain, const char *vm, const char *name); +/************************************************************************ + * * + * API for error handling * + * * + ************************************************************************/ void __virRaiseError(virConnectPtr conn, virDomainPtr dom, int domain, @@ -163,6 +170,20 @@ void __virRaiseError(virConnectPtr conn, int int1, int int2, const char *msg, ...); const char *__virErrorMsg(virErrorNumber error, const char *info); +/************************************************************************ + * * + * API for domain/connections (de)allocations * + * * + ************************************************************************/ + +virConnectPtr virGetConnect (void); +int virFreeConnect (virConnectPtr conn); +virDomainPtr virGetDomain (virConnectPtr conn, + const char *name, + const char *uuid); +int virFreeDomain (virConnectPtr conn, + virDomainPtr domain); + #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/src/libvirt.c b/src/libvirt.c index 0486647002..5f5365cc51 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -25,7 +25,6 @@ #include "xen_internal.h" #include "xend_internal.h" #include "xs_internal.h" -#include "hash.h" #include "xml.h" @@ -36,8 +35,6 @@ * - memory wrappers for malloc/free ? */ -static int virDomainFreeName(virDomainPtr domain, const char *name); - static virDriverPtr virDriverTab[MAX_DRIVERS]; static int initialized = 0; @@ -222,14 +219,11 @@ virConnectOpen(const char *name) if (!initialized) virInitialize(); - ret = (virConnectPtr) malloc(sizeof(virConnect)); + ret = virGetConnect(); if (ret == NULL) { virLibConnError(NULL, VIR_ERR_NO_MEMORY, "Allocating connection"); goto failed; } - memset(ret, 0, sizeof(virConnect)); - ret->magic = VIR_CONNECT_MAGIC; - ret->nb_drivers = 0; for (i = 0;i < MAX_DRIVERS;i++) { if ((virDriverTab[i] != NULL) && (virDriverTab[i]->open != NULL)) { @@ -252,14 +246,6 @@ virConnectOpen(const char *name) goto failed; } - ret->domains = virHashCreate(20); - if (ret->domains == NULL) - goto failed; - ret->domains_mux = xmlNewMutex(); - if (ret->domains_mux == NULL) - goto failed; - ret->flags = 0; - return (ret); failed: @@ -268,11 +254,7 @@ failed: if ((ret->drivers[i] != NULL) && (ret->drivers[i]->close != NULL)) ret->drivers[i]->close(ret); } - if (ret->domains != NULL) - virHashFree(ret->domains, (virHashDeallocator) virDomainFreeName); - if (ret->domains_mux != NULL) - xmlFreeMutex(ret->domains_mux); - free(ret); + virFreeConnect(ret); } return (NULL); } @@ -296,14 +278,11 @@ virConnectOpenReadOnly(const char *name) if (!initialized) virInitialize(); - ret = (virConnectPtr) malloc(sizeof(virConnect)); + ret = virGetConnect(); if (ret == NULL) { virLibConnError(NULL, VIR_ERR_NO_MEMORY, "Allocating connection"); goto failed; } - memset(ret, 0, sizeof(virConnect)); - ret->magic = VIR_CONNECT_MAGIC; - ret->nb_drivers = 0; for (i = 0;i < MAX_DRIVERS;i++) { if ((virDriverTab[i] != NULL) && (virDriverTab[i]->open != NULL)) { @@ -322,13 +301,6 @@ virConnectOpenReadOnly(const char *name) virLibConnError(NULL, VIR_ERR_NO_SUPPORT, name); goto failed; } - - ret->domains = virHashCreate(20); - if (ret->domains == NULL) - goto failed; - ret->domains_mux = xmlNewMutex(); - if (ret->domains_mux == NULL) - goto failed; ret->flags = VIR_CONNECT_RO; return (ret); @@ -339,29 +311,11 @@ failed: if ((ret->drivers[i] != NULL) && (ret->drivers[i]->close != NULL)) ret->drivers[i]->close(ret); } - if (ret->domains != NULL) - virHashFree(ret->domains, (virHashDeallocator) virDomainFreeName); - if (ret->domains_mux != NULL) - xmlFreeMutex(ret->domains_mux); - free(ret); + virFreeConnect(ret); } return (NULL); } -/** - * virDomainFreeName: - * @domain: a domain object - * - * Destroy the domain object, this is just used by the domain hash callback. - * - * Returns 0 in case of success and -1 in case of failure. - */ -static int -virDomainFreeName(virDomainPtr domain, const char *name ATTRIBUTE_UNUSED) -{ - return (virDomainFree(domain)); -} - /** * virConnectClose: * @conn: pointer to the hypervisor connection @@ -376,20 +330,10 @@ virDomainFreeName(virDomainPtr domain, const char *name ATTRIBUTE_UNUSED) int virConnectClose(virConnectPtr conn) { - int i; - if (!VIR_IS_CONNECT(conn)) return (-1); - virHashFree(conn->domains, (virHashDeallocator) virDomainFreeName); - conn->domains = NULL; - xmlFreeMutex(conn->domains_mux); - conn->domains_mux = NULL; - for (i = 0;i < conn->nb_drivers;i++) { - if ((conn->drivers[i] != NULL) && (conn->drivers[i]->close != NULL)) - conn->drivers[i]->close(conn); - } - conn->magic = -1; - free(conn); + if (virFreeConnect(conn) < 0) + return (-1); return (0); } @@ -663,26 +607,23 @@ virDomainLookupByID(virConnectPtr conn, int id) } free(names); } + if (name == NULL) + goto error; - ret = (virDomainPtr) malloc(sizeof(virDomain)); + ret = virGetDomain(conn, name, &uuid[0]); if (ret == NULL) { + virLibConnError(conn, VIR_ERR_NO_MEMORY, "Allocating domain"); goto error; } - memset(ret, 0, sizeof(virDomain)); - ret->magic = VIR_DOMAIN_MAGIC; - ret->conn = conn; ret->handle = id; ret->path = path; - ret->name = name; - memcpy(&ret->uuid[0], uuid, 16); - if (ret->name == NULL) { - goto error; - } + if (name != NULL) + free(name); return (ret); - error: - if (ret != NULL) - free(ret); +error: + if (name != NULL) + free(name); if (path != NULL) free(path); return (NULL); @@ -737,19 +678,13 @@ virDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) if (name == NULL) return (NULL); - ret = (virDomainPtr) malloc(sizeof(virDomain)); + ret = virGetDomain(conn, name, &uuid[0]); if (ret == NULL) { if (name != NULL) free(name); return (NULL); } - memset(ret, 0, sizeof(virDomain)); - ret->magic = VIR_DOMAIN_MAGIC; - ret->conn = conn; ret->handle = id; - ret->name = name; - ret->path = 0; - memcpy(ret->uuid, uuid, 16); return (ret); } @@ -847,14 +782,9 @@ virDomainFree(virDomainPtr domain) virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__); return (-1); } - domain->magic = -1; - domain->handle = -1; - if (domain->path != NULL) - free(domain->path); - if (domain->name) - free(domain->name); - free(domain); - return (0); + if (virFreeDomain(domain->conn, domain) < 0) + return (-1); + return(0); } /** diff --git a/src/xend_internal.c b/src/xend_internal.c index 03ba2a7ccf..ac2f2f118f 100644 --- a/src/xend_internal.c +++ b/src/xend_internal.c @@ -1586,41 +1586,37 @@ sexpr_to_xend_node_info(struct sexpr *root, virNodeInfoPtr info) static virDomainPtr sexpr_to_domain(virConnectPtr conn, struct sexpr *root) { - virDomainPtr ret; + virDomainPtr ret = NULL; char *dst_uuid = NULL; + char uuid[16]; const char *name; if ((conn == NULL) || (root == NULL)) return(NULL); - ret = (virDomainPtr) malloc(sizeof(virDomain)); - if (ret == NULL) { - virXendError(conn, VIR_ERR_NO_MEMORY, "Allocating domain"); - return(NULL); - } - memset(ret, 0, sizeof(virDomain)); - ret->magic = VIR_DOMAIN_MAGIC; - ret->conn = conn; - ret->handle = sexpr_int(root, "domain/domid"); - if (ret->handle < 0) - goto error; - dst_uuid = (char *) &(ret->uuid[0]); + dst_uuid = (char *) &uuid[0]; if (sexpr_uuid(&dst_uuid, root, "domain/uuid") == NULL) goto error; name = sexpr_node(root, "domain/name"); if (name == NULL) goto error; - ret->name = strdup(name); - if (ret->name == NULL) + + ret = virGetDomain(conn, name, &uuid[0]); + if (ret == NULL) { + virXendError(conn, VIR_ERR_NO_MEMORY, "Allocating domain"); + return(NULL); + } + ret->handle = sexpr_int(root, "domain/domid"); + if (ret->handle < 0) goto error; return (ret); + error: virXendError(conn, VIR_ERR_INTERNAL_ERROR, "failed to parse Xend domain informations"); - if (ret->name != NULL) - free(ret->name ); - free(ret); + if (ret != NULL) + virFreeDomain(conn, ret); return(NULL); } diff --git a/src/xs_internal.c b/src/xs_internal.c index 87a96054a6..9785261f4c 100644 --- a/src/xs_internal.c +++ b/src/xs_internal.c @@ -556,15 +556,15 @@ xenStoreDomainLookupByName(virConnectPtr conn, const char *name) if (!found) return(NULL); - ret = (virDomainPtr) malloc(sizeof(virDomain)); - if (ret == NULL) + ret = virGetDomain(conn, name, NULL); + if (ret == NULL) { + virXenStoreError(conn, VIR_ERR_NO_MEMORY, "Allocating domain"); + if (path != NULL) + free(path); goto done; - memset(ret, 0, sizeof(virDomain)); - ret->magic = VIR_DOMAIN_MAGIC; - ret->conn = conn; + } ret->handle = id; ret->path = path; - ret->name = strdup(name); done: if (xenddomain != NULL)