On 01/28/2014 11:48 AM, joel SIMOES wrote:

From: Joel SIMOES <joel.simoes@laposte.net>

Correct ret return 0 on success --- src/storage/storage_backend_sheepdog.c | 92 ++++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 10 deletions(-)

diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a6981ce..75304b1 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -86,7 +86,8 @@ virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool, pool->available = pool->capacity - pool->allocation; return 0;

- } while ((p = next)); + } + while ((p = next));

You have ignored Eric's review comments (or perhaps you didn't see them?). 1) The above change is not only unnecessary, but changes the code to not fit libvirt's coding style, so it must be removed. 2) Additionally (as Eric also mentioned), for anything more than a trivial change, you *MUST* have a descriptive message in the commit log that explains what was changed and why (e.g. a reference to a bug report and/or a description of the bug, why it was occurring, how the code was changed, and how it fixed the problem.) We will need this information added to the commit log message before we can consider pushing the patch.

return -1; } @@ -109,6 +110,70 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virCommandAddArgFormat(cmd, "%d", port); }

+static int +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool) +{ + int ret; + char *output = NULL; + + virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL); + virStorageBackendSheepdogAddHostArg(cmd, pool); + virCommandSetOutputBuffer(cmd, &output); + ret = virCommandRun(cmd, NULL); + char** lines; + char** cells; + + if (ret < 0) + goto cleanup; + + ret = -1; + lines = virStringSplit(output, "\n", 0); + size_t i; + for (i = 0; *(lines + i); i++) { + char *line = *(lines + i); + cells = virStringSplit(line, " ", 0); + size_t j; + for (j = 0; *(cells + j); j++) { + + char *cell = *(cells + j); + if (j == 1) { + virStorageVolDefPtr vol = NULL; + if (VIR_ALLOC(vol) < 0) + goto cleanup; + + if (VIR_STRDUP(vol->name, cell) < 0) + goto cleanup; + + vol->type = VIR_STORAGE_VOL_BLOCK; + + if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0) + goto cleanup; + pool->volumes.objs[pool->volumes.count - 1] = vol; + + if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) + goto cleanup; + + vol = NULL; + } + + VIR_FREE(*(cells + j)); + } + + VIR_FREE(cells); + VIR_FREE(*(lines + i)); + } + + + VIR_FREE(lines); + ret = 0; + +cleanup: + virCommandFree(cmd); + VIR_FREE(lines); + VIR_FREE(cells); + return ret; +}

static int virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -122,15 +187,18 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); ret = virCommandRun(cmd, NULL); - if (ret == 0) - ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output); + if (ret < 0) + goto cleanup;

+ ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output); + if (ret < 0) + goto cleanup; + ret = virStorageBackendSheepdogRefreshAllVol(conn, pool); +cleanup: virCommandFree(cmd); - VIR_FREE(output); return ret; }

- static int virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, @@ -143,12 +211,14 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "delete", vol->name, NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); int ret = virCommandRun(cmd, NULL); + if (ret < 0) + goto cleanup;

+cleanup: virCommandFree(cmd); return ret; }

- static int virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, @@ -174,7 +244,6 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; }

- static int virStorageBackendSheepdogBuildVol(virConnectPtr conn, virStoragePoolObjPtr pool, @@ -195,12 +264,12 @@ virStorageBackendSheepdogBuildVol(virConnectPtr conn, goto cleanup;

ret = 0; + cleanup: virCommandFree(cmd); return ret; }

- int virStorageBackendSheepdogParseVdiList(virStorageVolDefPtr vol, char *output) @@ -257,7 +326,8 @@ virStorageBackendSheepdogParseVdiList(virStorageVolDefPtr vol, return -1;

return 0; - } while ((p = next)); + } + while ((p = next));

return -1; } @@ -295,7 +365,6 @@ cleanup: return ret; }

- static int virStorageBackendSheepdogResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, @@ -310,7 +379,10 @@ virStorageBackendSheepdogResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCommandAddArgFormat(cmd, "%llu", capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); int ret = virCommandRun(cmd, NULL); + if (ret < 0) + goto cleanup;

+cleanup: virCommandFree(cmd); return ret;