[libvirt] [PATCH V3]Sheepdog: Auto Adding volume and pool refresh.

From: Joel <joel.simoes@laposte.net> Fix bug 1055479 https://bugzilla.redhat.com/show_bug.cgi?id=1055479 Libvirt lose sheepdogs volumes on pool refresh or restart. When restarting sheepdog pool, all volumes are missing. This patch add automatically all volume from the added pool. --- src/storage/storage_backend_sheepdog.c | 74 +++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a6981ce..a836f27 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -109,6 +109,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,9 +186,15 @@ 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; -- 1.8.3.2

On 01/31/2014 01:02 PM, joel SIMOES wrote:
From: Joel <joel.simoes@laposte.net>
Fix bug 1055479 https://bugzilla.redhat.com/show_bug.cgi?id=1055479 Libvirt lose sheepdogs volumes on pool refresh or restart. When restarting sheepdog pool, all volumes are missing. This patch add automatically all volume from the added pool. --- src/storage/storage_backend_sheepdog.c | 74 +++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a6981ce..a836f27 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -109,6 +109,70 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virCommandAddArgFormat(cmd, "%d", port); }
+static int +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool) +{ + int ret;
int ret = -1;
+ char *output = NULL;
'lines' and 'cells' should be declared here as well, not below the virCommand calls.
+ + virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL); + virStorageBackendSheepdogAddHostArg(cmd, pool); + virCommandSetOutputBuffer(cmd, &output);
+ ret = virCommandRun(cmd, NULL);
No need to save the return value, see [1]
+ char** lines; + char** cells;
+ + if (ret < 0) + goto cleanup; + + ret = -1; + lines = virStringSplit(output, "\n", 0);
The return value of virStringSplit needs to be checked for NULL.
+ size_t i;
This should be declared at the beginning of the function.
+ for (i = 0; *(lines + i); i++) {
lines[i] looks nicer than *(lines + i), see [2]
+ char *line = *(lines + i); + cells = virStringSplit(line, " ", 0); + size_t j; + for (j = 0; *(cells + j); j++) {
You can eliminate this whole loop by checking if cells has at least two entries and using cells[1] directly.
+ + 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));
No need to free every string separately,
+ } + + VIR_FREE(cells);
use virStringFreeList(cells) instead.
+ VIR_FREE(*(lines + i));
Same here.
+ } + + + VIR_FREE(lines); + ret = 0; + +cleanup: + virCommandFree(cmd); + VIR_FREE(lines); + VIR_FREE(cells); + return ret; +}
static int virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -122,9 +186,15 @@ 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;
There's no need to overwrite the 'ret' variable with every call. if (functionCall() < 0) goto cleanup; works too, since most of the functions only return 0 and -1.
+ ret = virStorageBackendSheepdogRefreshAllVol(conn, pool); + +cleanup: virCommandFree(cmd); VIR_FREE(output); return ret;
Jan [1] https://www.redhat.com/archives/libvir-list/2014-January/msg01232.html [2] https://www.redhat.com/archives/libvir-list/2014-January/msg01215.html

Howto check size of **char Thx.
+ char *line = *(lines + i); + cells = virStringSplit(line, " ", 0); + size_t j; + for (j = 0; *(cells + j); j++) {
You can eliminate this whole loop by checking if cells has at least two entries and using cells[1] directly. Le 03/02/2014 14:07, Ján Tomko a écrit :
On 01/31/2014 01:02 PM, joel SIMOES wrote:
From: Joel <joel.simoes@laposte.net>
Fix bug 1055479 https://bugzilla.redhat.com/show_bug.cgi?id=1055479 Libvirt lose sheepdogs volumes on pool refresh or restart. When restarting sheepdog pool, all volumes are missing. This patch add automatically all volume from the added pool. --- src/storage/storage_backend_sheepdog.c | 74 +++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-)
diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a6981ce..a836f27 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -109,6 +109,70 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virCommandAddArgFormat(cmd, "%d", port); }
+static int +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool) +{ + int ret; int ret = -1;
+ char *output = NULL; 'lines' and 'cells' should be declared here as well, not below the virCommand calls.
+ + virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL); + virStorageBackendSheepdogAddHostArg(cmd, pool); + virCommandSetOutputBuffer(cmd, &output); + ret = virCommandRun(cmd, NULL); No need to save the return value, see [1]
+ char** lines; + char** cells; + + if (ret < 0) + goto cleanup; + + ret = -1; + lines = virStringSplit(output, "\n", 0); The return value of virStringSplit needs to be checked for NULL.
+ size_t i; This should be declared at the beginning of the function.
+ for (i = 0; *(lines + i); i++) { lines[i] looks nicer than *(lines + i), see [2]
+ char *line = *(lines + i); + cells = virStringSplit(line, " ", 0); + size_t j; + for (j = 0; *(cells + j); j++) { You can eliminate this whole loop by checking if cells has at least two entries and using cells[1] directly.
+ + 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)); No need to free every string separately,
+ } + + VIR_FREE(cells); use virStringFreeList(cells) instead.
+ VIR_FREE(*(lines + i)); Same here.
+ } + + + VIR_FREE(lines); + ret = 0; + +cleanup: + virCommandFree(cmd); + VIR_FREE(lines); + VIR_FREE(cells); + return ret; +}
static int virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -122,9 +186,15 @@ 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; There's no need to overwrite the 'ret' variable with every call. if (functionCall() < 0) goto cleanup;
works too, since most of the functions only return 0 and -1.
+ ret = virStorageBackendSheepdogRefreshAllVol(conn, pool); + +cleanup: virCommandFree(cmd); VIR_FREE(output); return ret;
Jan
[1] https://www.redhat.com/archives/libvir-list/2014-January/msg01232.html [2] https://www.redhat.com/archives/libvir-list/2014-January/msg01215.html
participants (3)
-
Joel Simoes
-
joel SIMOES
-
Ján Tomko