[Libvirt-cim] [PATCH 3/3] Make pool device paths use linked list implementation
Daniel Veillard
veillard at redhat.com
Wed Jun 20 07:11:20 UTC 2012
On Tue, Jun 19, 2012 at 12:35:40PM -0300, Eduardo Lima (Etrunko) wrote:
> From: "Eduardo Lima (Etrunko)" <eblima at br.ibm.com>
>
> Signed-off-by: Eduardo Lima (Etrunko) <eblima at 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 at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the Libvirt-cim
mailing list