Re: [Libvir] [PATCH] Do check the UUID in __virGetDomain()

Hi Dan I noticed that I misunderstood your suggestion. --> I heard the point that was not good of this patch which Daniel Veillard thought from Saori Fukuta. Thanks Daniel and Saori. I misunderstood that the point that is missing a call of virGetDomain() exists before this patch is applied. But your indication is that the point that is missing a call of virGetDomain() emerges after this patch is applied. Therefore, I remake this patch. This patch changes as follows. - the argument of virFreeDomain() doesn't change. - virFreeDomainInternal() is added as the function that is called only from hash.c. Thanks Masayuki Sunou -------------------------------------------------------------------------------- Index: src/hash.c =================================================================== RCS file: /data/cvs/libvirt/src/hash.c,v retrieving revision 1.22 diff -u -p -r1.22 hash.c --- src/hash.c 8 May 2007 10:53:27 -0000 1.22 +++ src/hash.c 22 Jun 2007 05:45:30 -0000 @@ -731,6 +731,71 @@ virFreeConnect(virConnectPtr conn) { } /** + * virFreeDomainInternal: + * @conn: the hypervisor connection + * @domain: the domain to release + * @domain: the flag to execute xmlMutexLock + * + * Release the given domain, if the reference count drops to zero, then + * the domain is really freed. + * This function is called only from hash.c. + * + * Returns the reference count or -1 in case of failure. + */ +int +virFreeDomainInternal(virConnectPtr conn, virDomainPtr domain, int lock) { + 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); + } + if (lock) + xmlMutexLock(conn->hashes_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->id = -1; + 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->hashes_mux != NULL && lock) + xmlFreeMutex(conn->hashes_mux); + free(conn); + return(0); + +done: + if (lock) + xmlMutexUnlock(conn->hashes_mux); + return(ret); +} + +/** * virGetDomain: * @conn: the hypervisor connection * @name: pointer to the domain name @@ -758,8 +823,14 @@ __virGetDomain(virConnectPtr conn, const ret = (virDomainPtr) virHashLookup(conn->domains, name); if (ret != NULL) { - /* TODO check the UUID */ - goto done; + if (memcmp(ret->uuid, uuid, 16)) { + if (virFreeDomainInternal(conn, ret, 0) < 0) { + xmlMutexUnlock(conn->hashes_mux); + return(NULL); + } + } else { + goto done; + } } /* @@ -814,53 +885,7 @@ error: */ 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; - - /* 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->id = -1; - 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->hashes_mux != NULL) - xmlFreeMutex(conn->hashes_mux); - free(conn); - return(0); - -done: - xmlMutexUnlock(conn->hashes_mux); - return(ret); + return virFreeDomainInternal(conn, domain, 1); } /** -------------------------------------------------------------------------------- In message <200706150931.BAD60924.2E73K9NG(a)aa.jp.fujitsu.com&gt; "Re: [Libvir] [PATCH] Do check the UUID in __virGetDomain()" "Masayuki Sunou <fj1826dm(a)aa.jp.fujitsu.com&gt;&quot; wrote: