- CIM - libvirt List Archives

Re: [Libvirt-cim] [PATCH V3 05/11] DevicePool, fix debug print crash
by John Ferlan 11 Mar '13

11 Mar '13

Since I don't have the original email to reply-to, here is a link: https://www.redhat.com/archives/libvirt-cim/2012-December/msg00024.html ACK

1 0

Re: [Libvirt-cim] [PATCH V3 02/11] do not deregister virt classes in yum upgrade
by John Ferlan 11 Mar '13

11 Mar '13

Since I don't have the email to reply-to for a review, here is a link: https://www.redhat.com/archives/libvirt-cim/2012-December/msg00021.html I'm not sure what "$1" is in this context, let's consider setting "$1" to some variable that is more understandable in the context. Furthermore, is "0" a special number/string? How would anyone know reading the code without having knowledge of the code. John

2 1

Re: [Libvirt-cim] [PATCH V3 03/11] CSI, fix debug print crash
by John Ferlan 11 Mar '13

11 Mar '13

Since I don't have the email to reply-to, here is a link: https://www.redhat.com/archives/libvirt-cim/2012-December/msg00028.html ACK

1 0

[RFC] Patches need no testcase
by Wenchao Xia 05 Mar '13

05 Mar '13

Hi, Danial Happy new year! Could u have a look v3 serial of libcmpituil patches? It is simple and need no test case, hope it can be merged if no problem found. -- Best Regards Wenchao Xia

2 2

[PATCH V4] DevicePool, reimplement get_diskpool_config with libvirt
by Wenchao Xia 25 Dec '12

25 Dec '12

Original implemetion may return pools with NULL name if some pool disappear between two libvirt pool API call. And caller of this function consider number = 0 as error only but it original return negative value when error before. This patch fix it. V2: Use for instead of while in clean up. Treat zero pool returned from libvirt as normal instead of error. Rebased. v3: fix wrong i++ in for while. v4: changed the function prototype to return negative when error, not return the pool number anymore. In this way code of caller and function in no-use-libvirt-pool case were also changed. Return false in get_disk_parent() when strdup fail. Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com> --- src/Virt_DevicePool.c | 176 +++++++++++++++++++++++++++++++++--------------- 1 files changed, 121 insertions(+), 55 deletions(-) diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c index 79dc108..bf3dd3b 100644 --- a/src/Virt_DevicePool.c +++ b/src/Virt_DevicePool.c @@ -51,6 +51,21 @@ 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 +93,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 +104,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 +141,85 @@ 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++) { + for (i = 0; i < realcount; i++) { pools[i].tag = strdup(names[i]); + if (pools[i].tag == NULL) { + CU_DEBUG("Failed in strdup for name '%s'.", names[i]); + ret = -3; + goto free_pools; + } 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 +351,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 +441,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; } @@ -1084,9 +1142,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

1 1

Re: [Libvirt-cim] [PATCH V3 01/11] remove script for bridge network
by Wenchao Xia 24 Dec '12

24 Dec '12

Yes, in lower version I think this xml section should also be removed. > > Are we saying that libvirt-cim 0.6.2 will use libvirt 0.9.10 and higher? > > Regards > Sharad Mishra > Open Virtualization > Linux Technology Center > IBM > > > > From: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com> > To: veillard(a)redhat.com, > Cc: Sharad Mishra/Beaverton/IBM@IBMUS, libvirt-cim(a)redhat.com, > Wenchao Xia <xiawenc(a)linux.vnet.ibm.com> > Date: 12/18/12 02:20 AM > Subject: [PATCH V3 01/11] remove script for bridge network > > > > libvirt0.9.10 will report error if bridge network was defined > with script. This is the fix for it, otherwise VM start would fail. > > v2: > rebased. > > Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com> > --- > libxkutil/xmlgen.c | 9 +-------- > 1 files changed, 1 insertions(+), 8 deletions(-) > > diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c > index 2dcd0d2..31619d8 100644 > --- a/libxkutil/xmlgen.c > +++ b/libxkutil/xmlgen.c > @@ -269,18 +269,11 @@ static const char *set_net_source(xmlNodePtr nic, > return NULL; > } > > - > +/* libvirt 0.9.10 report error if script is set with brdige */ > static const char *bridge_net_to_xml(xmlNodePtr nic, struct net_device > *dev) > { > - const char *script = "vif-bridge"; > - xmlNodePtr tmp; > const char *msg = NULL; > > - tmp = xmlNewChild(nic, NULL, BAD_CAST "script", NULL); > - if (tmp == NULL) > - return XML_ERROR; > - xmlNewProp(tmp, BAD_CAST "path", BAD_CAST script); > - > msg = set_net_source(nic, dev, "bridge"); > > return msg; > -- > 1.7.1 > > -- Best Regards Wenchao Xia

2 2

[RFC] libvirt-cim 0.6.2 candidate patches
by Wenchao Xia 18 Dec '12

18 Dec '12

Hi Danial There are total 13 patch sent for libvirt-cim 0.6.2 as candidate, 11 for libvirt-cim and 2 for libcmpiutils, please pull them if you think those are OK, Version is V3 patch serials. Cimtest was ran and no obviouse problem found. -- Best Regards Wenchao Xia

1 0

[PATCH V3 00/11] bug fix and enhancement patch rebase for 6.2
by Wenchao Xia 18 Dec '12

18 Dec '12

These are the patches rebased which fix bugs reported for 6.1 or potential problem. Last patch is an enhancement for migration usage. Each patch is stand alone can can be applied independendly. All code change for libvirt-cim are in this serial, Please use it for 6.2, ignoring V2 serials. Wenchao Xia (11): remove script for bridge network do not deregister virt classes in yum upgrade CSI, fix debug print crash CSI, add lock to protect shared data DevicePool, fix debug print crash DevicePool, reimplement get_diskpool_config with libvirt RASDIndication, fix debug print crash device parsing, add debug print CSI Discard libvirt event by default VSSD, fix a error report issue in VSSD enum allow ssh based migration with non root's key file libvirt-cim.conf | 14 + libvirt-cim.spec.in | 12 +- libxkutil/device_parsing.c | 11 +- libxkutil/misc_util.c | 119 ++++- libxkutil/misc_util.h | 3 + libxkutil/xmlgen.c | 9 +- src/Virt_ComputerSystem.c | 46 ++- src/Virt_ComputerSystemIndication.c | 581 +++++++++++++++++++- src/Virt_DevicePool.c | 41 +- src/Virt_ResourceAllocationSettingDataIndication.c | 6 +- src/Virt_VSMigrationService.c | 114 ++++- src/Virt_VSSD.c | 72 ++- src/Virt_VirtualSystemManagementService.c | 36 ++- 13 files changed, 996 insertions(+), 68 deletions(-)

1 12

[PATCH V3] DevicePool, reimplement get_diskpool_config with libvirt
by Wenchao Xia 17 Dec '12

17 Dec '12

Original implemetion may return pools with NULL name if some pool disappear between two libvirt pool API call. And caller of this function consider number = 0 as error only but it original return negative value when error before. This patch fix it. V2: Use for instead of while in clean up. Treat zero pool returned from libvirt as normal instead of error. Rebased. v3: fix wrong i++ in for while, previous patch have this bug, it is patch 6/11, please ignore 6/11 in v2 serial. Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com> --- src/Virt_DevicePool.c | 39 +++++++++++++++++++++++++++++---------- 1 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c index 79dc108..769de63 100644 --- a/src/Virt_DevicePool.c +++ b/src/Virt_DevicePool.c @@ -117,52 +117,71 @@ int get_disk_pool(virStoragePoolPtr poolptr, struct virt_pool **pool) return ret; } +/* This function returns the real number of pools, no negative value should be + returned, if error happens it returns zero, otherwise return at least 1. */ static int get_diskpool_config(virConnectPtr conn, struct tmp_disk_pool **_pools) { - int count = 0; + int count = 0, realcount = 0; int i; char ** names = NULL; struct tmp_disk_pool *pools = NULL; + int have_err = 0; count = virConnectNumOfStoragePools(conn); - if (count <= 0) + if (count < 0) { + have_err = 1; goto out; + } else if (count == 0) { + goto out; + } names = calloc(count, sizeof(char *)); if (names == NULL) { CU_DEBUG("Failed to alloc space for %i pool names", count); count = 0; + have_err = 1; goto out; } - if (virConnectListStoragePools(conn, names, count) == -1) { + realcount = virConnectListStoragePools(conn, names, count); + if (realcount == -1) { CU_DEBUG("Failed to get storage pools"); - count = 0; + realcount = 0; + have_err = 1; + goto out; + } + if (realcount == 0) { + CU_DEBUG("zero pools got, but prelist is %d.", count); goto out; } - pools = calloc(count, sizeof(*pools)); + pools = calloc(realcount, sizeof(*pools)); if (pools == NULL) { - CU_DEBUG("Failed to alloc space for %i pool structs", count); + CU_DEBUG("Failed to alloc space for %i pool structs", realcount); + realcount = 0; + have_err = 1; goto out; } - for (i = 0; i < count; i++) { + for (i = 0; i < realcount; i++) { pools[i].tag = strdup(names[i]); pools[i].primordial = false; } out: - for (i = 0; i < count; i++) + for (i = 0; i < count; i++) { free(names[i]); + } free(names); - get_disk_parent(&pools, &count); + if (have_err == 0) { + get_disk_parent(&pools, &realcount); + } *_pools = pools; - return count; + return realcount; } static bool diskpool_set_capacity(virConnectPtr conn, -- 1.7.1

1 1

[PATCH V2 00/11] bug fix and enhancement patch rebase for 6.2
by Wenchao Xia 17 Dec '12

17 Dec '12

These are the patches rebased which fix bugs reported for 6.1 or potential problem. Last patch is an enhancement for migration usage. Each patch is stand alone can can be applied independendly. Wenchao Xia (11): remove script for bridge network do not deregister virt classes in yum upgrade CSI, fix debug print crash CSI, add lock to protect shared data DevicePool, fix debug print crash DevicePool, reimplement get_diskpool_config with libvirt RASDIndication, fix debug print crash device parsing, add debug print CSI Discard libvirt event by default VSSD, fix a error report issue in VSSD enum provide a hack to borrow non-root ssh keys in migration libvirt-cim.spec.in | 12 +- libxkutil/device_parsing.c | 11 +- libxkutil/xmlgen.c | 9 +- src/Virt_ComputerSystem.c | 46 ++- src/Virt_ComputerSystemIndication.c | 581 +++++++++++++++++++- src/Virt_DevicePool.c | 43 ++- src/Virt_ResourceAllocationSettingDataIndication.c | 6 +- src/Virt_VSMigrationService.c | 96 +++- src/Virt_VSSD.c | 72 ++- src/Virt_VirtualSystemManagementService.c | 36 ++- 10 files changed, 859 insertions(+), 53 deletions(-)

1 11