[Libvirt-cim] [PATCH V5 08/15] DevicePool, reimplement get_diskpool_config with libvirt
Wenchao Xia
xiawenc at linux.vnet.ibm.com
Thu Mar 21 03:39:11 UTC 2013
Original implemetion may return pools with NULL name if
some pool disappear between two libvirt pool API call. And
originally it return the number of pools and negative value
when error happens, but caller of this function consider
number = 0 as error.
As a fix, this patch changed the function prototype, it do
not return the pool number anymore, it returns 0 on success
and negative on fail now. Code for checking the risk of returning
pools with NULL name is also added.
Another small fix is, return false in get_disk_parent() when
strdup fail.
Signed-off-by: Wenchao Xia <xiawenc at linux.vnet.ibm.com>
---
src/Virt_DevicePool.c | 176 +++++++++++++++++++++++++++++++++----------------
1 files changed, 120 insertions(+), 56 deletions(-)
diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c
index 08677e2..185e3cc 100644
--- a/src/Virt_DevicePool.c
+++ b/src/Virt_DevicePool.c
@@ -51,6 +51,22 @@ struct tmp_disk_pool {
bool primordial;
};
+static void free_diskpool(struct tmp_disk_pool *pools, int count)
+{
+ int i;
+
+ if (pools == NULL) {
+ return;
+ }
+
+ for (i = 0; i < count; i++) {
+ free(pools[i].tag);
+ free(pools[i].path);
+ }
+
+ free(pools);
+}
+
/*
* Right now, detect support and use it, if available.
* Later, this can be a configure option if needed
@@ -78,6 +94,10 @@ static bool get_disk_parent(struct tmp_disk_pool **_pools,
}
pools[count].tag = strdup("0");
+ if (pools[count].tag == NULL) {
+ count++;
+ goto free;
+ }
pools[count].path = NULL;
pools[count].primordial = true;
count++;
@@ -85,12 +105,17 @@ static bool get_disk_parent(struct tmp_disk_pool **_pools,
*_count = count;
*_pools = pools;
ret = true;
+ goto out;
+ free:
+ free_diskpool(pools, count);
+ /* old pool is invalid, update it */
+ *_count = 0;
+ *_pools = NULL;
out:
return ret;
}
-
#if VIR_USE_LIBVIRT_STORAGE
int get_disk_pool(virStoragePoolPtr poolptr, struct virt_pool **pool)
{
@@ -117,52 +142,82 @@ int get_disk_pool(virStoragePoolPtr poolptr, struct virt_pool **pool)
return ret;
}
+/* This function returns 0 on sucess, negative on fail. */
static int get_diskpool_config(virConnectPtr conn,
- struct tmp_disk_pool **_pools)
+ struct tmp_disk_pool **_pools,
+ int *_count)
{
- int count = 0;
+ int count = 0, realcount = 0;
int i;
char ** names = NULL;
struct tmp_disk_pool *pools = NULL;
+ int ret = 0;
+ bool bret;
count = virConnectNumOfStoragePools(conn);
- if (count <= 0)
+ if (count < 0) {
+ ret = count;
goto out;
+ } else if (count == 0) {
+ goto set_parent;
+ }
names = calloc(count, sizeof(char *));
if (names == NULL) {
CU_DEBUG("Failed to alloc space for %i pool names", count);
- count = 0;
+ ret = -1;
goto out;
}
- if (virConnectListStoragePools(conn, names, count) == -1) {
- CU_DEBUG("Failed to get storage pools");
- count = 0;
- goto out;
+ realcount = virConnectListStoragePools(conn, names, count);
+ if (realcount < 0) {
+ CU_DEBUG("Failed to get storage pools, return %d.", realcount);
+ ret = realcount;
+ goto free_names;
+ }
+ if (realcount == 0) {
+ CU_DEBUG("Zero pools got, but prelist is %d.", count);
+ goto set_parent;
}
- pools = calloc(count, sizeof(*pools));
+ pools = calloc(realcount, sizeof(*pools));
if (pools == NULL) {
- CU_DEBUG("Failed to alloc space for %i pool structs", count);
- goto out;
+ CU_DEBUG("Failed to alloc space for %i pool structs",
+ realcount);
+ ret = -2;
+ goto free_names;
}
- for (i = 0; i < count; i++) {
- pools[i].tag = strdup(names[i]);
+ for (i = 0; i < realcount; i++) {
+ pools[i].tag = names[i];
+ names[i] = NULL;
pools[i].primordial = false;
}
- out:
- for (i = 0; i < count; i++)
- free(names[i]);
- free(names);
-
- get_disk_parent(&pools, &count);
+ set_parent:
+ bret = get_disk_parent(&pools, &realcount);
+ if (bret != true) {
+ CU_DEBUG("Failed in adding parentpool.");
+ ret = -4;
+ goto free_pools;
+ }
+ /* succeed */
*_pools = pools;
+ *_count = realcount;
+ goto free_names;
+
+ free_pools:
+ free_diskpool(pools, realcount);
- return count;
+ free_names:
+ for (i = 0; i < count; i++) {
+ free(names[i]);
+ }
+ free(names);
+
+ out:
+ return ret;
}
static bool diskpool_set_capacity(virConnectPtr conn,
@@ -294,42 +349,59 @@ static int parse_diskpool_line(struct tmp_disk_pool *pool,
return (ret == 2);
}
+/* return 0 on sucess, negative on fail. */
static int get_diskpool_config(virConnectPtr conn,
- struct tmp_disk_pool **_pools)
+ struct tmp_disk_pool **_pools,
+ int *_count)
{
const char *path = DISK_POOL_CONFIG;
FILE *config;
char *line = NULL;
size_t len = 0;
- int count = 0;
- struct tmp_disk_pool *pools = NULL;
+ int count = 0, ret = 0;
+ struct tmp_disk_pool *pools = NULL, *new_pools = NULL;
+ bool bret;
config = fopen(path, "r");
if (config == NULL) {
CU_DEBUG("Failed to open %s: %m", path);
- return 0;
+ ret = -1;
+ goto out;
}
while (getline(&line, &len, config) > 0) {
- pools = realloc(pools,
- (count + 1) * (sizeof(*pools)));
- if (pools == NULL) {
+ new_pools = realloc(pools,
+ (count + 1) * (sizeof(*pools)));
+ if (new_pools == NULL) {
CU_DEBUG("Failed to alloc new pool");
- goto out;
+ ret = -2;
+ goto free_pools;
}
+ pools = new_pools;
if (parse_diskpool_line(&pools[count], line))
count++;
}
+ bret = get_disk_parent(&pools, &count);
+ if (bret != true) {
+ CU_DEBUG("Failed in adding parentpool.");
+ ret = -3;
+ goto free_pools;
+ }
- get_disk_parent(&pools, &count);
- out:
- free(line);
+ /* succeed */
*_pools = pools;
- fclose(config);
+ *_count = count;
+ goto clean;
- return count;
+ free_pools:
+ free_diskpool(pools, count);
+ clean:
+ free(line);
+ fclose(config);
+ out:
+ return ret;
}
static bool diskpool_set_capacity(virConnectPtr conn,
@@ -367,39 +439,23 @@ static bool diskpool_set_capacity(virConnectPtr conn,
}
static bool _diskpool_is_member(virConnectPtr conn,
- const struct disk_pool *pool,
+ const struct tmp_disk_pool *pool,
const char *file)
{
return STARTS_WITH(file, pool->path);
}
#endif
-static void free_diskpool(struct tmp_disk_pool *pools, int count)
-{
- int i;
-
- if (pools == NULL)
- return;
-
- for (i = 0; i < count; i++) {
- free(pools[i].tag);
- free(pools[i].path);
- }
-
- free(pools);
-}
-
static char *_diskpool_member_of(virConnectPtr conn,
const char *file)
{
struct tmp_disk_pool *pools = NULL;
int count;
- int i;
+ int i, ret;
char *pool = NULL;
- count = get_diskpool_config(conn, &pools);
- if (count == 0) {
- free(pools);
+ ret = get_diskpool_config(conn, &pools, &count);
+ if (ret < 0) {
return NULL;
}
@@ -1088,9 +1144,17 @@ static CMPIStatus diskpool_instance(virConnectPtr conn,
CMPIStatus s = {CMPI_RC_OK, NULL};
struct tmp_disk_pool *pools = NULL;
int count = 0;
- int i;
+ int i, ret;
- count = get_diskpool_config(conn, &pools);
+ ret = get_diskpool_config(conn, &pools, &count);
+ if (ret < 0) {
+ CU_DEBUG("Failed to get diskpool config, return is %d.", ret);
+ cu_statusf(broker, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get diskpool config, return is %d.",
+ ret);
+ return s;
+ }
if ((id == NULL) && (count == 0)) {
CU_DEBUG("No defined DiskPools");
free(pools);
--
1.7.1
More information about the Libvirt-cim
mailing list