[libvirt] [PATCH V5] Sheepdog: Adding volume and on pool and refresh.

From: Joel SIMOES <joel.simoes@laposte.net> 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. Adding last Daniel P. Berrange's syntaxes correction. Adding vol on separeted function 'inspired' from parallels_storage : parallelsAddDiskVolume --- src/storage/storage_backend_sheepdog.c | 86 ++++++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a6981ce..08e5473 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -109,22 +109,100 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virCommandAddArgFormat(cmd, "%d", port); } +static int +virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, const char *diskInfo) +{ + virStorageVolDefPtr vol = NULL; + + if (diskInfo == NULL) + goto error; + + if (VIR_ALLOC(vol) < 0 || VIR_STRDUP(vol->name, diskInfo) < 0) + goto error; + + vol->type = VIR_STORAGE_VOL_NETWORK; + + if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0) + goto error; + + pool->volumes.objs[pool->volumes.count - 1] = vol; + + if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) + goto error; + + return 0; + +error: + virStorageVolDefFree(vol); + return -1; +} + +static int +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool) +{ + int ret = -1; + char *output = NULL; + char **lines = NULL; + char **cells = NULL; + size_t i; + + virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL); + virStorageBackendSheepdogAddHostArg(cmd, pool); + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + lines = virStringSplit(output, "\n", 0); + if (lines == NULL) + goto cleanup; + + for (i = 0; lines[i]; i++) { + char *line = lines[i]; + if (line == NULL) + break; + + cells = virStringSplit(line, " ", 0); + + if (cells != NULL && virStringListLength(cells) > 2) { + if (virStorageBackendSheepdogAddVolume(conn, pool, cells[1]) < 0) + goto cleanup; + } + + virStringFreeList(cells); + cells = NULL; + } + + ret = 0; + +cleanup: + virCommandFree(cmd); + virStringFreeList(lines); + virStringFreeList(cells); + return ret; +} + static int virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { - int ret; + int ret = -1; char *output = NULL; virCommandPtr cmd; cmd = virCommandNewArgList(COLLIE, "node", "info", "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); - ret = virCommandRun(cmd, NULL); - if (ret == 0) - ret = virStorageBackendSheepdogParseNodeInfo(pool->def, output); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + if (virStorageBackendSheepdogParseNodeInfo(pool->def, output) < 0) + goto cleanup; + + ret = virStorageBackendSheepdogRefreshAllVol(conn, pool); +cleanup: virCommandFree(cmd); VIR_FREE(output); return ret; -- 1.8.3.2

On Fri, Feb 07, 2014 at 12:45:42PM +0100, joel SIMOES wrote:
+static int +virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, const char *diskInfo) +{ + virStorageVolDefPtr vol = NULL; + + if (diskInfo == NULL) + goto error;
Missing error report
+ + if (VIR_ALLOC(vol) < 0 || VIR_STRDUP(vol->name, diskInfo) < 0) + goto error; + + vol->type = VIR_STORAGE_VOL_NETWORK; + + if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0) + goto error; + + pool->volumes.objs[pool->volumes.count - 1] = vol; + + if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) + goto error;
If refreshing fails, when we free 'vol', but we've already added it to the pool->volumes array. So the refresh call needs to be moved up a few lines.
+ + return 0; + +error: + virStorageVolDefFree(vol); + return -1; +} + +static int +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool) +{ + int ret = -1; + char *output = NULL; + char **lines = NULL; + char **cells = NULL; + size_t i; + + virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL); + virStorageBackendSheepdogAddHostArg(cmd, pool); + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + lines = virStringSplit(output, "\n", 0); + if (lines == NULL) + goto cleanup; + + for (i = 0; lines[i]; i++) { + char *line = lines[i];
This could be marked const.
+ if (line == NULL) + break; + + cells = virStringSplit(line, " ", 0); + + if (cells != NULL && virStringListLength(cells) > 2) { + if (virStorageBackendSheepdogAddVolume(conn, pool, cells[1]) < 0) + goto cleanup; + } + + virStringFreeList(cells); + cells = NULL; + } + + ret = 0; + +cleanup: + virCommandFree(cmd); + virStringFreeList(lines); + virStringFreeList(cells);
We need VIR_FREE(output) here.
+ return ret; +}
I'm going to commit this with the following change applied: diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 08e5473..82df274 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -115,22 +115,25 @@ virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED, { virStorageVolDefPtr vol = NULL; - if (diskInfo == NULL) + if (diskInfo == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing disk info when adding volume")); goto error; + } if (VIR_ALLOC(vol) < 0 || VIR_STRDUP(vol->name, diskInfo) < 0) goto error; vol->type = VIR_STORAGE_VOL_NETWORK; + if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) + goto error; + if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0) goto error; pool->volumes.objs[pool->volumes.count - 1] = vol; - if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) - goto error; - return 0; error: @@ -159,7 +162,7 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; for (i = 0; lines[i]; i++) { - char *line = lines[i]; + const char *line = lines[i]; if (line == NULL) break; @@ -180,6 +183,7 @@ cleanup: virCommandFree(cmd); virStringFreeList(lines); virStringFreeList(cells); + VIR_FREE(output); return ret; } Thanks for your contribution ! Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Thx Le 11/02/2014 12:33, Daniel P. Berrange a écrit :
On Fri, Feb 07, 2014 at 12:45:42PM +0100, joel SIMOES wrote:
+static int +virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool, const char *diskInfo) +{ + virStorageVolDefPtr vol = NULL; + + if (diskInfo == NULL) + goto error; Missing error report
+ + if (VIR_ALLOC(vol) < 0 || VIR_STRDUP(vol->name, diskInfo) < 0) + goto error; + + vol->type = VIR_STORAGE_VOL_NETWORK; + + if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0) + goto error; + + pool->volumes.objs[pool->volumes.count - 1] = vol; + + if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) + goto error; If refreshing fails, when we free 'vol', but we've already added it to the pool->volumes array. So the refresh call needs to be moved up a few lines.
+ + return 0; + +error: + virStorageVolDefFree(vol); + return -1; +} + +static int +virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolObjPtr pool) +{ + int ret = -1; + char *output = NULL; + char **lines = NULL; + char **cells = NULL; + size_t i; + + virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL); + virStorageBackendSheepdogAddHostArg(cmd, pool); + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + lines = virStringSplit(output, "\n", 0); + if (lines == NULL) + goto cleanup; + + for (i = 0; lines[i]; i++) { + char *line = lines[i]; This could be marked const.
+ if (line == NULL) + break; + + cells = virStringSplit(line, " ", 0); + + if (cells != NULL && virStringListLength(cells) > 2) { + if (virStorageBackendSheepdogAddVolume(conn, pool, cells[1]) < 0) + goto cleanup; + } + + virStringFreeList(cells); + cells = NULL; + } + + ret = 0; + +cleanup: + virCommandFree(cmd); + virStringFreeList(lines); + virStringFreeList(cells); We need VIR_FREE(output) here.
+ return ret; +}
I'm going to commit this with the following change applied:
diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 08e5473..82df274 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -115,22 +115,25 @@ virStorageBackendSheepdogAddVolume(virConnectPtr conn ATTRIBUTE_UNUSED, { virStorageVolDefPtr vol = NULL;
- if (diskInfo == NULL) + if (diskInfo == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing disk info when adding volume")); goto error; + }
if (VIR_ALLOC(vol) < 0 || VIR_STRDUP(vol->name, diskInfo) < 0) goto error;
vol->type = VIR_STORAGE_VOL_NETWORK;
+ if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) + goto error; + if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0) goto error;
pool->volumes.objs[pool->volumes.count - 1] = vol;
- if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) - goto error; - return 0;
error: @@ -159,7 +162,7 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup;
for (i = 0; lines[i]; i++) { - char *line = lines[i]; + const char *line = lines[i]; if (line == NULL) break;
@@ -180,6 +183,7 @@ cleanup: virCommandFree(cmd); virStringFreeList(lines); virStringFreeList(cells); + VIR_FREE(output); return ret; }
Thanks for your contribution !
Regards, Daniel
participants (3)
-
Daniel P. Berrange
-
Joel Simoes
-
joel SIMOES