[PATCH] acl_parsing: Fixing potential leaks
by Eduardo Lima (Etrunko)
# HG changeset patch
# User Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
# Date 1313005735 10800
# Node ID 2b1b79a72ab0288d649399118dffce26fd05e066
# Parent 8759e60c17c42101118f914215d071138340c70f
acl_parsing: Fixing potential leaks
Also adds missing checks for errors in some function calls.
As reported in https://bugzilla.redhat.com/show_bug.cgi?id=728245
line 144 - Function cleanup_filter does not free its parameter filter (this
causes a lot of Coverity Resource leak warnings).
line 397 - Dynamically allocated variable rule is not freed in function
parse_acl_filter as a result of line #399.
line 630 - Function "malloc" without NULL check.
line 659 - Function "malloc" without NULL check.
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
diff --git a/libxkutil/acl_parsing.c b/libxkutil/acl_parsing.c
--- a/libxkutil/acl_parsing.c
+++ b/libxkutil/acl_parsing.c
@@ -139,6 +139,7 @@
};
rule->type = UNKNOWN_RULE;
+ free(rule);
}
void cleanup_filter(struct acl_filter *filter)
@@ -152,10 +153,8 @@
free(filter->name);
free(filter->chain);
- for (i = 0; i < filter->rule_ct; i++) {
+ for (i = 0; i < filter->rule_ct; i++)
cleanup_rule(filter->rules[i]);
- free(filter->rules[i]);
- }
free(filter->rules);
filter->rule_ct = 0;
@@ -401,14 +400,16 @@
if (parse_acl_rule(child, rule) == 0)
goto err;
- append_filter_rule(filter, rule);
+ if (append_filter_rule(filter, rule) == 0)
+ goto err;
}
else if (XSTREQ(child->name, "filterref")) {
filter_ref = get_attr_value(child, "filter");
if (filter_ref == NULL)
goto err;
- append_filter_ref(filter, filter_ref);
+ if (append_filter_ref(filter, filter_ref) == 0)
+ goto err;
}
}
@@ -440,14 +441,15 @@
if (xmldoc == NULL)
goto err;
- *filter = malloc(sizeof(**filter));
+ *filter = calloc(1, sizeof(**filter));
if (*filter == NULL)
goto err;
- memset(*filter, 0, sizeof(**filter));
- parse_acl_filter(xmldoc->children, *filter);
-
- ret = 1;
+ ret = parse_acl_filter(xmldoc->children, *filter);
+ if (ret == 0) {
+ free(*filter);
+ *filter = NULL;
+ }
err:
xmlSetGenericErrorFunc(NULL, NULL);
@@ -508,9 +510,7 @@
if (xml == NULL)
return 0;
- get_filter_from_xml(xml, filter);
-
- return 1;
+ return get_filter_from_xml(xml, filter);
#else
return 0;
#endif
@@ -534,7 +534,8 @@
virConnectListNWFilters(conn, names, count);
- filters = malloc(count * sizeof(struct acl_filter));
+ filters = calloc(count, sizeof(*filters));
+
if (filters == NULL)
goto err;
@@ -542,7 +543,8 @@
{
struct acl_filter *filter = NULL;
- get_filter_by_name(conn, names[i], &filter);
+ if (get_filter_by_name(conn, names[i], &filter) == 0)
+ break;
memcpy(&filters[i], filter, sizeof(*filter));
}
@@ -630,6 +632,12 @@
filter->rules =
malloc((filter->rule_ct + 1) * sizeof(struct acl_rule *));
+ if (filter->rules == NULL) {
+ CU_DEBUG("Failed to allocate memory for new rule");
+ filter->rules = old_rules;
+ return 0;
+ }
+
memcpy(filter->rules,
old_rules,
filter->rule_ct * sizeof(struct acl_rule *));
@@ -657,6 +665,13 @@
old_refs = filter->refs;
filter->refs = malloc((filter->ref_ct + 1) * sizeof(char *));
+
+ if (filter->refs == NULL) {
+ CU_DEBUG("Failed to allocate memory for new ref");
+ filter->refs = old_refs;
+ return 0;
+ }
+
memcpy(filter->refs, old_refs, filter->ref_ct * sizeof(char *));
filter->refs[filter->ref_ct] = name;
@@ -682,8 +697,9 @@
if (STREQC(old_refs[i], name)) {
free(old_refs[i]);
}
- else
- append_filter_ref(filter, old_refs[i]);
+ else if(append_filter_ref(filter, old_refs[i]) == 0) {
+ return 0;
+ }
}
return 1;
13 years, 2 months
[PATCH] (#3) add sdl frame buffer support
by Wayne Xia
# HG changeset patch
# User Wayne Xia <xiawenc(a)linux.vnet.ibm.com>
# Date 1311670971 -28800
# Node ID d0bb7e93d02d3bd55d9b984165db0265c9865462
# Parent 792db1a6ead075375fad4a7d22143a0f978b5e48
(#3) add sdl frame buffer support.
Now libvirt still supports sdl frame buffer, and it may take three
parameters: display,xauth,fullscreen. This patch enable the libvirt-cim
to accept these configuration and pass them in XML define to let
libvirt know about it. Exposed interface could be found in the file
ResourceAllocationSettingData.mof.
https://bugzilla.linux.ibm.com/show_bug.cgi?id=71347
Signed-off-by: Wayne Xia (Wayne) <xiawenc(a)linux.vnet.ibm.com>
diff -r 792db1a6ead0 -r d0bb7e93d02d libxkutil/device_parsing.c
--- a/libxkutil/device_parsing.c Mon Jul 25 19:39:08 2011 +0800
+++ b/libxkutil/device_parsing.c Tue Jul 26 17:02:51 2011 +0800
@@ -538,6 +538,11 @@
if (gdev->dev.vnc.port == NULL || gdev->dev.vnc.host == NULL)
goto err;
}
+ else if (STREQC(gdev->type, "sdl")) {
+ gdev->dev.sdl.display = get_attr_value(node, "display");
+ gdev->dev.sdl.xauth = get_attr_value(node, "xauth");
+ gdev->dev.sdl.fullscreen = get_attr_value(node, "fullscreen");
+ }
else if (STREQC(gdev->type, "pty")) {
if (node->name == NULL)
goto err;
diff -r 792db1a6ead0 -r d0bb7e93d02d libxkutil/xmlgen.c
--- a/libxkutil/xmlgen.c Mon Jul 25 19:39:08 2011 +0800
+++ b/libxkutil/xmlgen.c Tue Jul 26 17:02:51 2011 +0800
@@ -421,8 +421,21 @@
xmlNewProp(tmp, BAD_CAST "type", BAD_CAST dev->type);
- if (STREQC(dev->type, "sdl"))
- return NULL;
+ if (STREQC(dev->type, "sdl")) {
+ if (dev->dev.sdl.display) {
+ xmlNewProp(tmp, BAD_CAST "display",
+ BAD_CAST dev->dev.sdl.display);
+ }
+ if (dev->dev.sdl.xauth) {
+ xmlNewProp(tmp, BAD_CAST "xauth",
+ BAD_CAST dev->dev.sdl.xauth);
+ }
+ if (dev->dev.sdl.fullscreen) {
+ xmlNewProp(tmp, BAD_CAST "fullscreen",
+ BAD_CAST dev->dev.sdl.fullscreen);
+ }
+ return NULL;
+ }
if (dev->dev.vnc.port) {
xmlNewProp(tmp, BAD_CAST "port", BAD_CAST dev->dev.vnc.port);
diff -r 792db1a6ead0 -r d0bb7e93d02d schema/ResourceAllocationSettingData.mof
--- a/schema/ResourceAllocationSettingData.mof Mon Jul 25 19:39:08 2011 +0800
+++ b/schema/ResourceAllocationSettingData.mof Tue Jul 26 17:02:51 2011 +0800
@@ -219,7 +219,9 @@
[Description ("If ResourceSubType is 'vnc', this is a VNC Address. "
"IPv4 in a.b.c.d:port or IPv6 in [ip]:port format. If ResourceSubType "
"is 'console', this is a character device path in "
- "path:port format (e.g., '/dev/pts/3:0'\)")]
+ "path:port format (e.g., '/dev/pts/3:0'\) "
+ "if ResourceSubType is 'sdl', this is a combination of its params as "
+ "xauth:display (e.g., '/root/.Xauthority::0'\)")]
string Address;
[Description ("Keyboard keymapping")]
@@ -228,7 +230,8 @@
[Description ("VNC password")]
string Password;
- [Description ("Is IPv6 only addressing is to be used")]
+ [Description ("Is IPv6 only addressing is to be used."
+ "if ResourceSubType is 'sdl', this means whether sdl is fullscreen")]
boolean IsIPv6Only;
};
diff -r 792db1a6ead0 -r d0bb7e93d02d src/Virt_VirtualSystemManagementService.c
--- a/src/Virt_VirtualSystemManagementService.c Mon Jul 25 19:39:08 2011 +0800
+++ b/src/Virt_VirtualSystemManagementService.c Tue Jul 26 17:02:51 2011 +0800
@@ -1060,6 +1060,52 @@
return ret;
}
+static int parse_sdl_address(const char *id,
+ char **display,
+ char **xauth)
+{
+ int ret;
+ char *tmp_display = NULL;
+ char *tmp_xauth = NULL;
+
+ CU_DEBUG("Entering parse_sdl_address, address is %s", id);
+
+ ret = sscanf(id, "%a[^:]:%as", &tmp_xauth, &tmp_display);
+
+ if (ret <= 0) {
+ ret = sscanf(id, ":%as", &tmp_display);
+ if (ret <= 0) {
+ if (STREQC(id, ":")) {
+ /* do nothing, it is empty */
+ }
+ else {
+ ret = 0;
+ goto out;
+ }
+ }
+ }
+
+ if (display) {
+ if (tmp_display == NULL)
+ *display = NULL;
+ else
+ *display = strdup(tmp_display);
+ }
+ if (xauth) {
+ if (tmp_xauth == NULL)
+ *xauth = NULL;
+ else
+ *xauth = strdup(tmp_xauth);
+ }
+ ret = 1;
+
+ out:
+ CU_DEBUG("Exiting parse_sdl_address, display is %s, xauth is %s",
+ *display, *xauth);
+
+ return ret;
+}
+
static int parse_vnc_address(const char *id,
char **ip,
char **port)
@@ -1163,6 +1209,30 @@
msg = "GraphicsRASD field Address not valid";
goto out;
}
+ }
+ else if (STREQC(dev->dev.graphics.type, "sdl")) {
+ if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) {
+ CU_DEBUG("sdl graphics Address empty, using default");
+ dev->dev.graphics.dev.sdl.display = NULL;
+ dev->dev.graphics.dev.sdl.xauth = NULL;
+ }
+ else {
+ ret = parse_sdl_address(val,
+ &dev->dev.graphics.dev.sdl.display,
+ &dev->dev.graphics.dev.sdl.xauth);
+ if (ret != 1) {
+ msg = "GraphicsRASD sdl Address not valid";
+ goto out;
+ }
+ }
+ dev->dev.graphics.dev.sdl.fullscreen = NULL;
+ if (cu_get_bool_prop(inst, "IsIPV6Only", &ipv6) ==
+ CMPI_RC_OK) {
+ if (ipv6)
+ dev->dev.graphics.dev.sdl.fullscreen = strdup("yes");
+ else
+ dev->dev.graphics.dev.sdl.fullscreen = strdup("no");
+ }
} else {
CU_DEBUG("Unsupported graphics type %s",
dev->dev.graphics.type);
@@ -1171,7 +1241,8 @@
}
free(dev->id);
- if (STREQC(dev->dev.graphics.type, "vnc"))
+ if ((STREQC(dev->dev.graphics.type, "vnc"))||
+ (STREQC(dev->dev.graphics.type, "sdl")))
ret = asprintf(&dev->id, "%s", dev->dev.graphics.type);
else
ret = asprintf(&dev->id, "%s:%s",
13 years, 2 months
Is virStoragePoolRefresh() synchronous or asynchronous?
by Gareth S Bestor
Basically, when you call virStoragePoolRefresh(), eg to pickup any new
disks that may have just been created under the pool path, when the API
returns are you guaranteed that the refresh operation is completed, or
might the pool still be refreshing itself? I ask because I understand this
refresh operation can be 'expensive', so I'm wondering if in fact things
may still be going on in the background after the API returns...
- Gareth
Dr. Gareth S. Bestor
IBM Senior Software Engineer
Systems & Technology Group - Systems Management Standards
971-285-6375 (mobile)
bestor(a)us.ibm.com
13 years, 2 months
[PATCH] VirtualSystemManagementService: Fixing potential null dereferences and leaks
by Eduardo Lima (Etrunko)
# HG changeset patch
# User Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
# Date 1312918075 10800
# Node ID 8759e60c17c42101118f914215d071138340c70f
# Parent 0291fb05e93a0cbcbf2b80c894a47d58f7c37d23
VirtualSystemManagementService: Fixing potential null dereferences and leaks
As reported in https://bugzilla.redhat.com/show_bug.cgi?id=728245
line 1048 - Comparing "path" to null implies that "path" might be null.
line 1057 - Dereferencing null variable "path".
line 1088 - Comparing "port" to null implies that "port" might be null.
line 1094 - Dereferencing null variable "port".
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
--- a/src/Virt_VirtualSystemManagementService.c
+++ b/src/Virt_VirtualSystemManagementService.c
@@ -1054,8 +1054,12 @@
ret = 1;
out:
- CU_DEBUG("Exiting parse_console_address, ip is %s, port is %s",
- *path, *port);
+ free(tmp_path);
+ free(tmp_port);
+
+ if (path && port)
+ CU_DEBUG("Exiting parse_console_address, ip is %s, port is %s",
+ *path, *port);
return ret;
}
@@ -1091,8 +1095,12 @@
ret = 1;
out:
- CU_DEBUG("Exiting parse_vnc_address, ip is %s, port is %s",
- *ip, *port);
+ free(tmp_ip);
+ free(tmp_port);
+
+ if (ip && port)
+ CU_DEBUG("Exiting parse_vnc_address, ip is %s, port is %s",
+ *ip, *port);
return ret;
}
13 years, 2 months
Network-pool seems not conform to profile
by Wayne Xia
By executing following command:
wbemcli -nl ei http://root:123456@localhost:5988/root/virt:KVM_NetworkPool
There would be 2 network pool, NetworkPool/default and
NetworkPool/0. I think NetworkPool/0 is OK, which represent the
primordial pool for network device, but NetworkPool/default should not
be there, because it comes from the calling of the libvirt's API which
returns the existing logical network device on the host, not the
network pool.
Another issue is that, it seems now libvirt-cim supports the creation
of NetworkPool as a child, but the libvirt now does not support that.
So I think this function should be removed, otherwise we need extra
files to record the organization of the network pools.
--
Best Regards
Wayne Xia
mail:xiawenc@linux.vnet.ibm.com
tel:86-010-82450803
13 years, 2 months
[PATCH] Check to see if stream is non-null before closing
by Sharad Mishra
# HG changeset patch
# User Sharad Mishra <snmishra(a)us.ibm.com>
# Date 1312828809 25200
# Node ID 8b4f6d69f40904ced24f7948f04f89e770e37bce
# Parent dd060dd8dbbd6c1d9a73de4dffa9fb2ab84d62b9
Check to see if stream is non-null before closing.
Added a check to see if the stream is non-null before
calling close.
Signed-off-by: Sharad Mishra <snmishra(a)us.ibm.com>
diff -r dd060dd8dbbd -r 8b4f6d69f409 src/Virt_SwitchService.c
--- a/src/Virt_SwitchService.c Thu Aug 04 11:22:32 2011 -0400
+++ b/src/Virt_SwitchService.c Mon Aug 08 11:40:09 2011 -0700
@@ -91,7 +91,8 @@
"No VSI Support found");
out:
- pclose(stream);
+ if (stream != NULL)
+ pclose(stream);
return s;
}
13 years, 2 months
[PATCH] (#2) bugfix: KVM_MemResourceAllocationSettingData does not conform to profile
by Wayne Xia
# HG changeset patch
# User Wayne Xia <xiawenc(a)linux.vnet.ibm.com>
# Date 1311231387 -28800
# Node ID d4002b23fffc97fd358811d551abbce28659839b
# Parent dd060dd8dbbd6c1d9a73de4dffa9fb2ab84d62b9
(#2) bugfix: KVM_MemResourceAllocationSettingData does not conform to profile
According to the discuss and profile, the reserved property means the
memory actually allocated to support the VM running, and the unit
should be byte*2^10. This patch added some code to retrieve VM's state,
and the report the memory status according to that.
#2 removed the unit changing to avoid impacting to existing user case.
Signed-off-by: Wayne Xia <xiawenc(a)linux.vnet.ibm.com>
diff -r dd060dd8dbbd -r d4002b23fffc libxkutil/device_parsing.c
--- a/libxkutil/device_parsing.c Thu Aug 04 11:22:32 2011 -0400
+++ b/libxkutil/device_parsing.c Thu Jul 21 14:56:27 2011 +0800
@@ -801,6 +801,7 @@
} else if (dev->type == CIM_RES_TYPE_MEM) {
dev->dev.mem.size = _dev->dev.mem.size;
dev->dev.mem.maxsize = _dev->dev.mem.maxsize;
+ dev->dev.mem.reserved = _dev->dev.mem.reserved;
} else if (dev->type == CIM_RES_TYPE_PROC) {
dev->dev.vcpu.quantity = _dev->dev.vcpu.quantity;
} else if (dev->type == CIM_RES_TYPE_EMU) {
@@ -895,8 +896,22 @@
if (xml == NULL)
return 0;
- if (type == CIM_RES_TYPE_MEM)
+ if (type == CIM_RES_TYPE_MEM) {
ret = _get_mem_device(xml, list);
+ if (*list != NULL) {
+ virDomainInfo dom_info;
+ if (virDomainGetInfo(dom, &dom_info) == 0) {
+ (*list)->dev.mem.reserved = dom_info.memory;
+ if (dom_info.state == 5) { /* VM not active */
+ (*list)->dev.mem.reserved = 0;
+ }
+ }
+ else {
+ CU_DEBUG("failed to get dom state for mem");
+ ret = -1;
+ }
+ }
+ }
else if (type == CIM_RES_TYPE_PROC)
ret = _get_proc_device(xml, list);
else
diff -r dd060dd8dbbd -r d4002b23fffc libxkutil/device_parsing.h
--- a/libxkutil/device_parsing.h Thu Aug 04 11:22:32 2011 -0400
+++ b/libxkutil/device_parsing.h Thu Jul 21 14:56:27 2011 +0800
@@ -72,6 +72,7 @@
struct mem_device {
uint64_t size;
uint64_t maxsize;
+ uint64_t reserved;
};
struct vcpu_device {
diff -r dd060dd8dbbd -r d4002b23fffc src/Virt_RASD.c
--- a/src/Virt_RASD.c Thu Aug 04 11:22:32 2011 -0400
+++ b/src/Virt_RASD.c Thu Jul 21 14:56:27 2011 +0800
@@ -633,7 +633,7 @@
CMSetProperty(inst, "VirtualQuantity",
(CMPIValue *)&dev->dev.mem.size, CMPI_uint64);
CMSetProperty(inst, "Reservation",
- (CMPIValue *)&dev->dev.mem.size, CMPI_uint64);
+ (CMPIValue *)&dev->dev.mem.reserved, CMPI_uint64);
CMSetProperty(inst, "Limit",
(CMPIValue *)&dev->dev.mem.maxsize, CMPI_uint64);
} else if (dev->type == CIM_RES_TYPE_PROC) {
13 years, 2 months
[PATCH] bug fix: KVM_MemResourceAllocationSettingData does not conform to profile
by Wayne Xia
# HG changeset patch
# User Wayne Xia <xiawenc(a)linux.vnet.ibm.com>
# Date 1311231387 -28800
# Node ID 677867c3d2a16a97591bde2828808f9f39b859a7
# Parent 3c90a88a5199a4ed931a4a76097cff8f55deae41
changed a bit to make it conform to CIM profile
According to the discuss and profile, the reserved property means the
memory actually allocated to support the VM running, and the unit
should be byte*2^10. This patch added some code to retrieve VM's state,
and the report the memory status according to that.
https://bugzilla.linux.ibm.com/show_bug.cgi?id=72759
Signed-off-by: Wayne Xia <xiawenc(a)linux.vnet.ibm.com>
diff -r 3c90a88a5199 -r 677867c3d2a1 libxkutil/device_parsing.c
--- a/libxkutil/device_parsing.c Mon Jul 18 11:13:40 2011 -0300
+++ b/libxkutil/device_parsing.c Thu Jul 21 14:56:27 2011 +0800
@@ -792,6 +792,7 @@
} else if (dev->type == CIM_RES_TYPE_MEM) {
dev->dev.mem.size = _dev->dev.mem.size;
dev->dev.mem.maxsize = _dev->dev.mem.maxsize;
+ dev->dev.mem.reserved = _dev->dev.mem.reserved;
} else if (dev->type == CIM_RES_TYPE_PROC) {
dev->dev.vcpu.quantity = _dev->dev.vcpu.quantity;
} else if (dev->type == CIM_RES_TYPE_EMU) {
@@ -885,8 +886,22 @@
if (xml == NULL)
return 0;
- if (type == CIM_RES_TYPE_MEM)
+ if (type == CIM_RES_TYPE_MEM) {
ret = _get_mem_device(xml, list);
+ if (*list != NULL) {
+ virDomainInfo dom_info;
+ if (virDomainGetInfo(dom, &dom_info) == 0) {
+ (*list)->dev.mem.reserved = dom_info.memory;
+ if (dom_info.state == 5) { /* VM not active */
+ (*list)->dev.mem.reserved = 0;
+ }
+ }
+ else {
+ CU_DEBUG("failed to get dom state for mem");
+ ret = -1;
+ }
+ }
+ }
else if (type == CIM_RES_TYPE_PROC)
ret = _get_proc_device(xml, list);
else
diff -r 3c90a88a5199 -r 677867c3d2a1 libxkutil/device_parsing.h
--- a/libxkutil/device_parsing.h Mon Jul 18 11:13:40 2011 -0300
+++ b/libxkutil/device_parsing.h Thu Jul 21 14:56:27 2011 +0800
@@ -71,6 +71,7 @@
struct mem_device {
uint64_t size;
uint64_t maxsize;
+ uint64_t reserved;
};
struct vcpu_device {
diff -r 3c90a88a5199 -r 677867c3d2a1 src/Virt_RASD.c
--- a/src/Virt_RASD.c Mon Jul 18 11:13:40 2011 -0300
+++ b/src/Virt_RASD.c Thu Jul 21 14:56:27 2011 +0800
@@ -576,14 +576,14 @@
inst);
} else if (dev->type == CIM_RES_TYPE_MEM) {
- const char *units = "KiloBytes";
+ const char *units = "byte*2^10";
CMSetProperty(inst, "AllocationUnits",
(CMPIValue *)units, CMPI_chars);
CMSetProperty(inst, "VirtualQuantity",
(CMPIValue *)&dev->dev.mem.size,
CMPI_uint64);
CMSetProperty(inst, "Reservation",
- (CMPIValue *)&dev->dev.mem.size,
CMPI_uint64);
+ (CMPIValue *)&dev->dev.mem.reserved,
CMPI_uint64);
CMSetProperty(inst, "Limit",
(CMPIValue *)&dev->dev.mem.maxsize,
CMPI_uint64);
} else if (dev->type == CIM_RES_TYPE_PROC) {
13 years, 2 months
