Re: [Libvirt-cim] [PATCH 3/3] Make pool device paths use linked list implementation

From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com&gt; Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com&gt; --- 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)