10:12 a.m.
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(a)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