[libvirt] [PATCH] Fix destroy command memory leaks
Some pieces of libvirt currently assume that the vir*Destroy
functions will free the passed object upon success. In
practice none of the current drivers seem to do this,
resulting in memory leaks.
The attached patch fixes the leaks I could find, as well as
changes the comments for virDomainDestroy and virNetworkDestroy
in libvirt.c to reflect reality. I also added a couple debug
statements to hash.c where domain reference counts can be
printed as they are changed.
Thanks,
Cole
diff --git a/qemud/remote.c b/qemud/remote.c
index a97641a..725152e 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -940,9 +940,11 @@ remoteDispatchDomainDestroy (struct qemud_server *server
ATTRIBUTE_UNUSED,
return -2;
}
- if (virDomainDestroy (dom) == -1)
+ if (virDomainDestroy (dom) == -1) {
+ virDomainFree(dom);
return -1;
- /* No need to free dom - destroy does it for us */
+ }
+ virDomainFree(dom);
return 0;
}
diff --git a/src/hash.c b/src/hash.c
index 79421aa..a014990 100644
--- a/src/hash.c
+++ b/src/hash.c
@@ -842,6 +842,9 @@ __virGetDomain(virConnectPtr conn, const char *name, const unsigned
char *uuid)
goto error;
}
conn->refs++;
+ DEBUG0("virGetDomain: New hash entry.");
+ } else {
+ DEBUG("virGetDomain: Existing hash entry, refs now %d",
ret->refs+1);
}
ret->refs++;
pthread_mutex_unlock(&conn->lock);
diff --git a/src/libvirt.c b/src/libvirt.c
index 97f6bc3..9f6df8e 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1390,10 +1390,9 @@ virDomainLookupByName(virConnectPtr conn, const char *name)
* @domain: a domain object
*
* Destroy the domain object. The running instance is shutdown if not down
- * already and all resources used by it are given back to the hypervisor.
- * The data structure is freed and should not be used thereafter if the
- * call does not return an error.
- * This function may requires privileged access
+ * already and all resources used by it are given back to the hypervisor. This
+ * does not free the associated virDomainPtr object.
+ * This function may require privileged access
*
* Returns 0 in case of success and -1 in case of failure.
*/
@@ -3502,10 +3501,9 @@ virNetworkCreate(virNetworkPtr network)
* @network: a network object
*
* Destroy the network object. The running instance is shutdown if not down
- * already and all resources used by it are given back to the hypervisor.
- * The data structure is freed and should not be used thereafter if the
- * call does not return an error.
- * This function may requires privileged access
+ * already and all resources used by it are given back to the hypervisor. This
+ * does not free the associated virNetworkPtr object.
+ * This function may require privileged access
*
* Returns 0 in case of success and -1 in case of failure.
*/
diff --git a/src/virsh.c b/src/virsh.c
index 45af630..234fc36 100644
--- a/src/virsh.c
+++ b/src/virsh.c
@@ -1468,9 +1468,9 @@ cmdDestroy(vshControl * ctl, vshCmd * cmd)
} else {
vshError(ctl, FALSE, _("Failed to destroy domain %s"), name);
ret = FALSE;
- virDomainFree(dom);
}
+ virDomainFree(dom);
return ret;
}
@@ -2433,9 +2433,9 @@ cmdNetworkDestroy(vshControl * ctl, vshCmd * cmd)
} else {
vshError(ctl, FALSE, _("Failed to destroy network %s"), name);
ret = FALSE;
- virNetworkFree(network);
}
+ virNetworkFree(network);
return ret;
}
@@ -3161,9 +3161,9 @@ cmdPoolDestroy(vshControl * ctl, vshCmd * cmd)
} else {
vshError(ctl, FALSE, _("Failed to destroy pool %s"), name);
ret = FALSE;
- virStoragePoolFree(pool);
}
+ virStoragePoolFree(pool);
return ret;
}