[PATCH 0 of 2] Fixes for some leaks and a seg fault

These connection leaks were causing problems for me when testing on a Fedora rawhide system. The seg fault appeared when libvirt had reached its max number of open connections and was dropping the provider's attempt to connect.

# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1256568468 25200 # Node ID 4eed878f31e95806a9cd345f656b7382945b8985 # Parent 7b4f21cb364bb404ebdfb86977d45ffe48497d99 Fix misc memory and libvirt connection leaks ElementConformsToProfile - leaking classname in elem_to_prof() ResourcePoolConfigurationCapabilities - potiential libvirt connection leak in get_rpc_cap() VSMigrationCapabilities - potiential libvirt connection leak in get_migration_caps() misc_util.c - no need to get a new connection pointer - use existing one Virt_VirtualSystemManagementService.c - after the call to virDomainDestroy(), we look up the guest and get a new domain pointer. We do this without freeing the previous pointer. pool_parsing.c - reorganize function so that storage pool pointer isn't leaked in the case that the storage volume pointer is NULL. Virt_KVMRedirectionSAP.c - use free_domain_list() to free all domain pointers before exiting function instead of trying to free each one in the loop (which can cause a leak if an error occurs and the loop exits before freeing). Also, be sure to clean up the dominfo struct before exiting the loop. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 7b4f21cb364b -r 4eed878f31e9 libxkutil/misc_util.c --- a/libxkutil/misc_util.c Wed Oct 14 15:09:29 2009 -0700 +++ b/libxkutil/misc_util.c Mon Oct 26 07:47:48 2009 -0700 @@ -417,8 +417,7 @@ compare_flags = 2; } - _dom = virDomainLookupByName(virDomainGetConnect(dom), - virDomainGetName(dom)); + _dom = virDomainLookupByName(conn, virDomainGetName(dom)); if (_dom == NULL) { CU_DEBUG("Unable to re-lookup domain"); goto out; diff -r 7b4f21cb364b -r 4eed878f31e9 libxkutil/pool_parsing.c --- a/libxkutil/pool_parsing.c Wed Oct 14 15:09:29 2009 -0700 +++ b/libxkutil/pool_parsing.c Mon Oct 26 07:47:48 2009 -0700 @@ -336,25 +336,29 @@ int res_type) { int ret = 0; + virStoragePoolPtr ptr = NULL; + virStorageVolPtr vptr = NULL; if (res_type == CIM_RES_TYPE_IMAGE) { - virStoragePoolPtr ptr = virStoragePoolLookupByName(conn, pname); + ptr = virStoragePoolLookupByName(conn, pname); if (ptr == NULL) { CU_DEBUG("Storage pool %s is not defined", pname); goto out; } - virStorageVolPtr vptr = virStorageVolCreateXML(ptr, xml, 0); - if (vptr == NULL) + vptr = virStorageVolCreateXML(ptr, xml, 0); + if (vptr == NULL) { + CU_DEBUG("Unable to create storage volume in %s", + pname); goto out; - - virStorageVolFree(vptr); - virStoragePoolFree(ptr); + } ret = 1; } out: + virStoragePoolFree(ptr); + virStorageVolFree(vptr); return ret; } diff -r 7b4f21cb364b -r 4eed878f31e9 src/Virt_ElementConformsToProfile.c --- a/src/Virt_ElementConformsToProfile.c Wed Oct 14 15:09:29 2009 -0700 +++ b/src/Virt_ElementConformsToProfile.c Mon Oct 26 07:47:48 2009 -0700 @@ -199,7 +199,7 @@ conn = connect_by_classname(_BROKER, CLASSNAME(vref), &s); if (conn == NULL) - return s; + goto out; for (i = 0; profiles[i] != NULL; i++) { diff -r 7b4f21cb364b -r 4eed878f31e9 src/Virt_KVMRedirectionSAP.c --- a/src/Virt_KVMRedirectionSAP.c Wed Oct 14 15:09:29 2009 -0700 +++ b/src/Virt_KVMRedirectionSAP.c Mon Oct 26 07:47:48 2009 -0700 @@ -316,7 +316,6 @@ for (i = 0; i < count; i++) { if (!check_graphics(domain_list[i], &dominfo)) { - virDomainFree(domain_list[i]); cleanup_dominfo(&dominfo); continue; } @@ -328,6 +327,7 @@ cu_statusf(broker, &s, CMPI_RC_ERR_FAILED, "Unable to guest's console port"); + cleanup_dominfo(&dominfo); goto out; } @@ -336,6 +336,7 @@ cu_statusf(broker, &s, CMPI_RC_ERR_FAILED, "Unable to allocate string"); + cleanup_dominfo(&dominfo); goto out; } @@ -343,7 +344,6 @@ port_list.list[port_list.cur]->remote_port = -1; port_list.cur++; - virDomainFree(domain_list[i]); cleanup_dominfo(&dominfo); } @@ -355,6 +355,7 @@ goto out; out: + free_domain_list(domain_list, count); free(domain_list); for (i = 0; i < count; i++) { diff -r 7b4f21cb364b -r 4eed878f31e9 src/Virt_ResourcePoolConfigurationCapabilities.c --- a/src/Virt_ResourcePoolConfigurationCapabilities.c Wed Oct 14 15:09:29 2009 -0700 +++ b/src/Virt_ResourcePoolConfigurationCapabilities.c Mon Oct 26 07:47:48 2009 -0700 @@ -70,17 +70,23 @@ pfx_from_conn(conn), "ResourcePoolConfigurationCapabilities", NAMESPACE(reference)); - if (inst == NULL) + if (inst == NULL) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, - "Can't create ResourcePoolConfigurationCapabilities instance"); + "Can't create RPCC instance"); + goto out; + } CMSetProperty(inst, "InstanceID", (CMPIValue *)"RPCC", CMPI_chars); array = CMNewArray(_BROKER, 2, CMPI_uint32, &s); - if (s.rc != CMPI_RC_OK) - return s; + if (s.rc != CMPI_RC_OK) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Can't create new CMPI array to store values"); + goto out; + } val = CreateChildResourcePool; CMSetArrayElementAt(array, 0, (CMPIValue *)&val, CMPI_uint32); diff -r 7b4f21cb364b -r 4eed878f31e9 src/Virt_VSMigrationCapabilities.c --- a/src/Virt_VSMigrationCapabilities.c Wed Oct 14 15:09:29 2009 -0700 +++ b/src/Virt_VSMigrationCapabilities.c Mon Oct 26 07:47:48 2009 -0700 @@ -139,7 +139,7 @@ cu_statusf(broker, &s, CMPI_RC_ERR_FAILED, "Unable to get instance for %s", CLASSNAME(ref)); - return s; + goto out; } CMSetProperty(inst, "InstanceID", diff -r 7b4f21cb364b -r 4eed878f31e9 src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Wed Oct 14 15:09:29 2009 -0700 +++ b/src/Virt_VirtualSystemManagementService.c Mon Oct 26 07:47:48 2009 -0700 @@ -1695,6 +1695,8 @@ virDomainDestroy(dom); /* Okay for this to fail */ + virDomainFree(dom); + dom = virDomainLookupByName(conn, dom_name); if (dom == NULL) { CU_DEBUG("Domain successfully destroyed");

# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1256771106 25200 # Node ID 68c625f1d36545bc3de39ae0c1402914d8dc2b58 # Parent 4eed878f31e95806a9cd345f656b7382945b8985 Fix seg fault in Virt_DevicePool - verify to be sure list is not NULL This can happen if _get_pools() is unable to connect to libvirt. diff -r 4eed878f31e9 -r 68c625f1d365 src/Virt_DevicePool.c --- a/src/Virt_DevicePool.c Mon Oct 26 07:47:48 2009 -0700 +++ b/src/Virt_DevicePool.c Wed Oct 28 16:05:06 2009 -0700 @@ -1210,6 +1210,14 @@ if (s.rc != CMPI_RC_OK) goto out; + if (list.cur <= 0) { + cu_statusf(broker, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance (%s)", + id); + goto out; + } + *_inst = list.list[0]; out:

On 10/28/2009 06:01 PM, Kaitlin Rupert wrote:
# HG changeset patch # User Kaitlin Rupert<karupert@us.ibm.com> # Date 1256771106 25200 # Node ID 68c625f1d36545bc3de39ae0c1402914d8dc2b58 # Parent 4eed878f31e95806a9cd345f656b7382945b8985 Fix seg fault in Virt_DevicePool - verify to be sure list is not NULL
This can happen if _get_pools() is unable to connect to libvirt.
diff -r 4eed878f31e9 -r 68c625f1d365 src/Virt_DevicePool.c --- a/src/Virt_DevicePool.c Mon Oct 26 07:47:48 2009 -0700 +++ b/src/Virt_DevicePool.c Wed Oct 28 16:05:06 2009 -0700 @@ -1210,6 +1210,14 @@ if (s.rc != CMPI_RC_OK) goto out;
+ if (list.cur<= 0) {
This is probably nitpicking, but the cur member is an unsigned int, so its value won't ever be less than 0. :-P
+ cu_statusf(broker,&s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance (%s)", + id); + goto out; + } + *_inst = list.list[0];
out:
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Richard Maciel, MSc IBM Linux Technology Center rmaciel@linux.vnet.ibm.com

Richard Maciel wrote:
On 10/28/2009 06:01 PM, Kaitlin Rupert wrote:
# HG changeset patch # User Kaitlin Rupert<karupert@us.ibm.com> # Date 1256771106 25200 # Node ID 68c625f1d36545bc3de39ae0c1402914d8dc2b58 # Parent 4eed878f31e95806a9cd345f656b7382945b8985 Fix seg fault in Virt_DevicePool - verify to be sure list is not NULL
This can happen if _get_pools() is unable to connect to libvirt.
diff -r 4eed878f31e9 -r 68c625f1d365 src/Virt_DevicePool.c --- a/src/Virt_DevicePool.c Mon Oct 26 07:47:48 2009 -0700 +++ b/src/Virt_DevicePool.c Wed Oct 28 16:05:06 2009 -0700 @@ -1210,6 +1210,14 @@ if (s.rc != CMPI_RC_OK) goto out;
+ if (list.cur<= 0) {
This is probably nitpicking, but the cur member is an unsigned int, so its value won't ever be less than 0. :-P
This is a good point =) Plus, I'm not adhering to coding standards in this line. Oops! So it needs to be changed anyway. Thanks!
+ cu_statusf(broker,&s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance (%s)", + id); + goto out; + } + *_inst = list.list[0];
out:
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (2)
-
Kaitlin Rupert
-
Richard Maciel