Re: [libvirt] [PATCH V5] Sheepdog: Adding volume and on pool and refresh.
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