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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list