4:09 p.m.
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(a)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(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
--
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent(a)linux.vnet.ibm.com