Re: [libvirt] [PATCH] Fix destroy command memory leaks

[Cole Robinson <crobinso@xxxxxxxxxx>]

Cole Robinson wrote:
> 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.
> 

Small update to the patch fixing the debug messages in hash.c
to be less redundant and more useful :)

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++;
+        DEBUG("New hash entry %p", ret);
+    } else {
+        DEBUG("Existing hash entry %p: refs now %d", ret, 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;
 }
 

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list