4:19 p.m.
On 07/31/2012 10:58 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
This converts the following public API datatypes to use the
virObject infrastructure:
virConnectPtr
virDomainPtr
virDomainSnapshotPtr
virInterfacePtr
virNetworkPtr
virNodeDevicePtr
virNWFilterPtr
virSecretPtr
virStreamPtr
virStorageVolPtr
virStoragePoolPtr
Given recent changes, this now fails to apply cleanly. Would you mind
refreshing the series, to make it easier to review?
16 files changed, 448 insertions(+), 1086 deletions(-)
Love the diffstat!
I still spotted some nits, even going solely by code inspection:
@@ -225,64 +217,21 @@ 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)
+{
-
- 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);
- }
+ if (domain->conn)
+ virObjectUnref(domain->conn);
Technically, we have a bug if we ever have domain->conn==NULL when we
get here. Besides, you coded virObjectUnref(NULL) to be a graceful
no-op, so that means you should add virObjectUnref to cfg.mk's list of
free-like functions and drop the conditional.
Oh, that reminds me - we DO have a bug in our RPC code where we fail to
check for NULL after strdup and such in the various make_nonnull_*
functions, and therefore I suppose that is an instance where we really
_could_ end up with domain->conn being NULL at the moment - I've been
meaning to spend some time on fixing that.
@@ -349,60 +289,17 @@ 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];
+ if (network->conn)
+ virObjectUnref(network->conn);
Another place where you can drop the conditional.
@@ -484,58 +366,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;
+ if (iface->conn)
+ virObjectUnref(iface->conn);
and again
@@ -608,60 +438,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];
+ if (pool->conn)
+ virObjectUnref(pool->conn);
I'm turning into a broken record...
@@ -748,59 +516,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;
+ if (vol->conn)
+ virObjectUnref(vol->conn);
Need I say it? :)
I'll quit mentioning it (again, a cfg.mk rule will make it easier to
find all instances).
+++ b/src/datatypes.h
+# define VIR_IS_CONNECT(obj) \
+ (virObjectIsClass((virObjectPtr)(obj), virConnectClass))
Why the extra cast here? The compiler can convert obj to 'void*'
without your help.
+
+# define VIR_IS_DOMAIN(obj) \
+ (virObjectIsClass((virObjectPtr)(obj), virDomainClass))
Likewise throughout these macros.
+++ b/src/libvirt.c
@@ -1420,14 +1420,16 @@ error:
* matching virConnectClose, and all other references will be released
* after the corresponding operation completes.
*
- * Returns the number of remaining references on success
- * (positive implies that some other call still has a reference open,
- * 0 implies that no references remain and the connection is closed),
- * or -1 on failure. It is possible for the last virConnectClose to
- * return a positive value if some other object still has a temporary
- * reference to the connection, but the application should not try to
- * further use a connection after the virConnectClose that matches the
- * initial open.
+ * Returns the a positive number if at least 1 reference remains on
s/the a/a/
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
+virSecretClass;
+virStorageVolClass;
+virStoragePoolClass;
Sorting.
+++ b/src/xen/xend_internal.c
@@ -1237,7 +1237,7 @@ error:
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("failed to parse Xend domain
information"));
if (ret != NULL)
- virUnrefDomain(ret);
+ virObjectUnref(ret);
Another useless if before free.
+++ 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);
Oops, I promised not to point them out :)
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org