Code looks good, but I have a few comments below. Also, I'm still seeing 2 handles leaked per cimtest run. On 05/13/2011 10:54 AM, Sharad Mishra wrote:
# HG changeset patch # User Sharad Mishra<snmishra@us.ibm.com> # Date 1305298464 25200 # Node ID d0ccdb5447c796123c0175873c6d108a77dc1e39 # Parent 8b428df21c360d1eaedba7157b0dfd429d2db121 Fix connection leaks.
libvirt-cim had few connection leaks which were causing it to crash.
Signed-off-by: Sharad Mishra<snmishra@us.ibm.com>
diff -r 8b428df21c36 -r d0ccdb5447c7 libxkutil/pool_parsing.c --- a/libxkutil/pool_parsing.c Wed Apr 13 12:27:33 2011 -0700 +++ b/libxkutil/pool_parsing.c Fri May 13 07:54:24 2011 -0700 @@ -453,6 +453,7 @@ CU_DEBUG("Unable to refresh storage " "pool"); } + virStoragePoolFree(pool_ptr); ret = 1; }
diff -r 8b428df21c36 -r d0ccdb5447c7 src/Virt_AllocationCapabilities.c --- a/src/Virt_AllocationCapabilities.c Wed Apr 13 12:27:33 2011 -0700 +++ b/src/Virt_AllocationCapabilities.c Fri May 13 07:54:24 2011 -0700 @@ -79,7 +79,6 @@ const char *id, struct inst_list *list) { - virConnectPtr conn = NULL; CMPIInstance *alloc_cap_inst; struct inst_list device_pool_list; CMPIStatus s = {CMPI_RC_OK, NULL}; @@ -91,15 +90,6 @@ if (!provider_is_responsible(broker, ref,&s)) goto out;
- conn = connect_by_classname(broker, CLASSNAME(ref),&s); - if (conn == NULL) { - if (id) - cu_statusf(broker,&s, - CMPI_RC_ERR_NOT_FOUND, - "Instance not found."); - goto out; - } - s = enum_pools(broker, ref, CIM_RES_TYPE_ALL,&device_pool_list); if (s.rc != CMPI_RC_OK) { cu_statusf(broker,&s, @@ -141,7 +131,6 @@ }
out: - virConnectClose(conn); inst_list_free(&device_pool_list);
return s; diff -r 8b428df21c36 -r d0ccdb5447c7 src/Virt_ComputerSystem.c --- a/src/Virt_ComputerSystem.c Wed Apr 13 12:27:33 2011 -0700 +++ b/src/Virt_ComputerSystem.c Fri May 13 07:54:24 2011 -0700 @@ -938,12 +938,12 @@ goto out; }
- dom = virDomainLookupByName(conn, + virDomainPtr dom2 = virDomainLookupByName(conn, virDomainGetName(dom));
declaration of virDomainPtr should be moved to top of function and named something like _dom, which seems to be the pattern to avoid varibale name collisions.
- if (dom == NULL) { - dom = virDomainDefineXML(conn, xml); - if (dom == NULL) { + if (dom2 == NULL) { + dom2 = virDomainDefineXML(conn, xml); + if (dom2 == NULL) { CU_DEBUG("Failed to define domain from XML"); virt_set_status(_BROKER,&s, CMPI_RC_ERR_FAILED, @@ -953,10 +953,10 @@ } }
- if (!domain_online(dom)) + if (!domain_online(dom2)) CU_DEBUG("Guest is now offline");
- ret = virDomainCreate(dom); + ret = virDomainCreate(dom2); if (ret != 0) virt_set_status(_BROKER,&s, CMPI_RC_ERR_FAILED, @@ -965,6 +965,8 @@
out::4 free(xml); + virDomainFree(dom2); + virConnectClose(conn);
return s; } diff -r 8b428df21c36 -r d0ccdb5447c7 src/Virt_ComputerSystemIndication.c --- a/src/Virt_ComputerSystemIndication.c Wed Apr 13 12:27:33 2011 -0700 +++ b/src/Virt_ComputerSystemIndication.c Fri May 13 07:54:24 2011 -0700 @@ -422,7 +422,6 @@ }
static bool async_ind(CMPIContext *context, - virConnectPtr conn, int ind_type, struct dom_xml prev_dom, char *prefix, @@ -557,7 +556,7 @@ for (i = 0; i< cur_count; i++) { res = dom_in_list(cur_xml[i].uuid, prev_count, prev_xml); if (!res) - async_ind(context, conn, CS_CREATED, + async_ind(context, CS_CREATED, cur_xml[i], prefix, args);
} @@ -565,14 +564,14 @@ for (i = 0; i< prev_count; i++) { res = dom_in_list(prev_xml[i].uuid, cur_count, cur_xml); if (!res) - async_ind(context, conn, CS_DELETED, + async_ind(context, CS_DELETED, prev_xml[i], prefix, args); }
for (i = 0; i< prev_count; i++) { res = dom_changed(prev_xml[i], cur_xml, cur_count); if (res) { - async_ind(context, conn, CS_MODIFIED, + async_ind(context, CS_MODIFIED, prev_xml[i], prefix, args);
} diff -r 8b428df21c36 -r d0ccdb5447c7 src/Virt_DevicePool.c --- a/src/Virt_DevicePool.c Wed Apr 13 12:27:33 2011 -0700 +++ b/src/Virt_DevicePool.c Fri May 13 07:54:24 2011 -0700 @@ -451,8 +451,8 @@
free(host); free(dev); + virDomainFree(dom); virConnectClose(conn); - virDomainFree(dom);
return pool; } diff -r 8b428df21c36 -r d0ccdb5447c7 src/Virt_VSMigrationService.c --- a/src/Virt_VSMigrationService.c Wed Apr 13 12:27:33 2011 -0700 +++ b/src/Virt_VSMigrationService.c Fri May 13 07:54:24 2011 -0700 @@ -1511,6 +1511,7 @@
out: CMReturnData(results, (CMPIValue *)&retcode, CMPI_uint32); + virConnectClose(job->conn);
It appears the job->conn is also released at ~1284 in migration_thread(). Since any number of errors in migrate_do() can result in leaks, I suggest you move all the frees from migration_thread() to migrate_do() just as you have done with virConnectClose().
return s; }
_______________________________________________ Libvirt-cim mailing list Libvirt-cim@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com