On Tue, Jun 19, 2012 at 12:35:40PM -0300, Eduardo Lima (Etrunko) wrote:
From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com>
Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- libxkutil/pool_parsing.c | 27 ++++++------------------- libxkutil/pool_parsing.h | 4 ++-- libxkutil/xmlgen.c | 29 +++++++++++++++++---------- src/Virt_ResourcePoolConfigurationService.c | 28 +++++++++----------------- 4 files changed, 35 insertions(+), 53 deletions(-)
diff --git a/libxkutil/pool_parsing.c b/libxkutil/pool_parsing.c index e41fc09..bb616d4 100644 --- a/libxkutil/pool_parsing.c +++ b/libxkutil/pool_parsing.c @@ -54,8 +54,6 @@ static void cleanup_net_pool(struct net_pool pool) { }
static void cleanup_disk_pool(struct disk_pool pool) { - uint16_t i; - free(pool.path); free(pool.host); free(pool.src_dir); @@ -63,10 +61,7 @@ static void cleanup_disk_pool(struct disk_pool pool) { free(pool.port_name); free(pool.node_name);
- for (i = 0; i < pool.device_paths_ct; i++) - free(pool.device_paths[i]); - - free(pool.device_paths); + list_free(pool.device_paths); }
void cleanup_virt_pool(struct virt_pool **pool) @@ -130,22 +125,15 @@ static int parse_disk_target(xmlNode *node, struct disk_pool *pool) static int parse_disk_source(xmlNode *node, struct disk_pool *pool) { xmlNode *child; - char **dev_paths = NULL; - unsigned ct = 0;
for (child = node->children; child != NULL; child = child->next) { if (XSTREQ(child->name, "device")) { - char **tmp = NULL; - - tmp = realloc(dev_paths, sizeof(char *) * (ct + 1)); - if (tmp == NULL) { - CU_DEBUG("Could not alloc space for dev path"); - continue; - } - dev_paths = tmp; + if (pool->device_paths == NULL) + pool->device_paths = list_new(free, + (list_data_cmp_cb) strcmp);
- dev_paths[ct] = get_attr_value(child, "path"); - ct++; + list_append(pool->device_paths, + get_attr_value(child, "path")); } else if (XSTREQ(child->name, "host")) { pool->host = get_attr_value(child, "name"); if (pool->host == NULL) @@ -161,12 +149,9 @@ static int parse_disk_source(xmlNode *node, struct disk_pool *pool) } }
- pool->device_paths_ct = ct; - pool->device_paths = dev_paths; return 1;
err: - free(dev_paths); return 0; }
diff --git a/libxkutil/pool_parsing.h b/libxkutil/pool_parsing.h index 9f1a386..50fbeac 100644 --- a/libxkutil/pool_parsing.h +++ b/libxkutil/pool_parsing.h @@ -26,6 +26,7 @@ #include <libvirt/libvirt.h>
#include "../src/svpc_types.h" +#include "list_util.h"
struct net_pool { char *addr; @@ -46,8 +47,7 @@ struct disk_pool { DISK_POOL_LOGICAL, DISK_POOL_SCSI} pool_type; char *path; - char **device_paths; - uint16_t device_paths_ct; + list_t *device_paths; char *host; char *src_dir; char *adapter; diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index 2dcd0d2..26e7203 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -1177,27 +1177,34 @@ static const char *disk_pool_type_to_str(uint16_t type) return NULL; }
+static bool device_paths_foreach(void *list_data, void *user_data) +{ + char *dev_path = (char *) list_data; + xmlNodePtr src = (xmlNodePtr) user_data; + xmlNodePtr tmp = NULL; + + tmp = xmlNewChild(src, NULL, BAD_CAST "device", BAD_CAST NULL); + if (tmp == NULL) + return false; + + if (xmlNewProp(tmp, BAD_CAST "path", BAD_CAST dev_path) == NULL) + return false; + + return true; +} + static const char *set_disk_pool_source(xmlNodePtr disk, struct disk_pool *pool) { xmlNodePtr src; xmlNodePtr tmp; - uint16_t i;
src = xmlNewChild(disk, NULL, BAD_CAST "source", NULL); if (src == NULL) return XML_ERROR;
- for (i = 0; i < pool->device_paths_ct; i++) { - tmp = xmlNewChild(src, NULL, BAD_CAST "device", BAD_CAST NULL); - if (tmp == NULL) - return XML_ERROR; - - if (xmlNewProp(tmp, - BAD_CAST "path", - BAD_CAST pool->device_paths[i]) == NULL) - return XML_ERROR; - } + if (!list_foreach(pool->device_paths, device_paths_foreach, src)) + return XML_ERROR;
if (pool->host != NULL) { tmp = xmlNewChild(src, NULL, BAD_CAST "host", BAD_CAST NULL); diff --git a/src/Virt_ResourcePoolConfigurationService.c b/src/Virt_ResourcePoolConfigurationService.c index 751d016..bc3e431 100644 --- a/src/Virt_ResourcePoolConfigurationService.c +++ b/src/Virt_ResourcePoolConfigurationService.c @@ -143,7 +143,6 @@ static const char *net_rasd_to_pool(CMPIInstance *inst, static void init_disk_pool(struct virt_pool *pool) { pool->pool_info.disk.device_paths = NULL; - pool->pool_info.disk.device_paths_ct = 0; pool->pool_info.disk.path = NULL; pool->pool_info.disk.host = NULL; pool->pool_info.disk.src_dir = NULL; @@ -153,9 +152,8 @@ static void init_disk_pool(struct virt_pool *pool) pool->pool_info.disk.autostart = 0; }
-static char *get_dev_paths(CMPIInstance *inst, - char ***path_list, - uint16_t *count) + +static char *get_dev_paths(CMPIInstance *inst, list_t **path_list) { CMPICount i; CMPICount ct; @@ -170,11 +168,8 @@ static char *get_dev_paths(CMPIInstance *inst, if ((s.rc != CMPI_RC_OK) || (ct <= 0)) return "Unable to get DevicePaths array count";
- *path_list = calloc(ct, sizeof(char *)); if (*path_list == NULL) - return "Failed to alloc space for device paths"; - - *count = ct; + *path_list = list_new(free, (list_data_cmp_cb) strcmp);
for (i = 0; i < ct; i++) { const char *str = NULL; @@ -187,7 +182,7 @@ static char *get_dev_paths(CMPIInstance *inst, if (str == NULL) return "Unable to get value of DevicePaths element";
- *path_list[i] = strdup(str); + list_append(*path_list, strdup(str)); }
return NULL; @@ -198,10 +193,7 @@ static const char *disk_fs_or_disk_or_logical_pool(CMPIInstance *inst, { const char *msg = NULL;
- msg = get_dev_paths(inst, - &pool->pool_info.disk.device_paths, - &pool->pool_info.disk.device_paths_ct); - + msg = get_dev_paths(inst, &pool->pool_info.disk.device_paths);
/* Specifying a value for DevicePaths isn't mandatory for logical pool types. */ @@ -209,7 +201,7 @@ static const char *disk_fs_or_disk_or_logical_pool(CMPIInstance *inst, if (msg != NULL) return msg;
- if (pool->pool_info.disk.device_paths_ct != 1) { + if (list_count(pool->pool_info.disk.device_paths) != 1) { CU_DEBUG("%d only takes one device path", pool->pool_info.disk.pool_type); return "Specified pool type only takes one device path"; @@ -243,14 +235,12 @@ static const char *disk_iscsi_pool(CMPIInstance *inst, const char *val = NULL; const char *msg = NULL;
- msg = get_dev_paths(inst, - &pool->pool_info.disk.device_paths, - &pool->pool_info.disk.device_paths_ct); - + msg = get_dev_paths(inst, &pool->pool_info.disk.device_paths); + if (msg != NULL) return msg;
- if (pool->pool_info.disk.device_paths_ct != 1) + if (list_count(pool->pool_info.disk.device_paths) != 1) return "Specified pool type only takes one device path";
if (cu_get_str_prop(inst, "Host", &val) != CMPI_RC_OK)
Looks fine but I would rename device_paths_foreach() to indicates what the function does instead of how it is used, something like device_path_dump_xml() or something similar, That doesn't affect the code semantic though ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/