+ char *line = *(lines + i); + cells = virStringSplit(line, " ", 0); + size_t j; + for (j = 0; *(cells + j); j++) {

On 01/31/2014 01:02 PM, joel SIMOES wrote: > From: Joel <joel.simoes(a)laposte.net&gt; > > 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