On Mon, Nov 2, 2015 at 8:42 AM, Peter Krempa <pkrempa@redhat.com> wrote:
@@ -99,37 +100,39 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (!(target->backingStore = virStorageSourceNewFromBacking(meta))) goto cleanup;
- target->backingStore->format = backingStoreFormat; + backingStore = virStorageSourceGetBackingStore(target, 0); + backingStore->format = backingStoreFormat;
/* XXX: Remote storage doesn't play nicely with volumes backed by * remote storage. To avoid trouble, just fake the backing store is RAW * and put the string from the metadata as the path of the target. */ - if (!virStorageSourceIsLocalStorage(target->backingStore)) { - virStorageSourceFree(target->backingStore); + if (!virStorageSourceIsLocalStorage(backingStore)) { + virStorageSourceFree(backingStore);
So this frees the new backingStore variable, which also corresponds to target->backingStore at this point, but ...
- if (VIR_ALLOC(target->backingStore) < 0) + if (VIR_ALLOC(backingStore) < 0)
... here only the local copy is allocated, so target->backingStore contains an invalid pointer ...
goto cleanup;
- target->backingStore->type = VIR_STORAGE_TYPE_NETWORK; - target->backingStore->path = meta->backingStoreRaw; + target->backingStore = backingStore; + backingStore->type = VIR_STORAGE_TYPE_NETWORK; + backingStore->path = meta->backingStoreRaw; meta->backingStoreRaw = NULL; - target->backingStore->format = VIR_STORAGE_FILE_RAW; + backingStore->format = VIR_STORAGE_FILE_RAW; }
- if (target->backingStore->format == VIR_STORAGE_FILE_AUTO) { - if ((rc = virStorageFileProbeFormat(target->backingStore->path, + if (backingStore->format == VIR_STORAGE_FILE_AUTO) { + if ((rc = virStorageFileProbeFormat(backingStore->path, -1, -1)) < 0) { /* If the backing file is currently unavailable or is * accessed via remote protocol only log an error, fake the * format as RAW and continue. Returning -1 here would * disable the whole storage pool, making it unavailable for * even maintenance. */ - target->backingStore->format = VIR_STORAGE_FILE_RAW; + backingStore->format = VIR_STORAGE_FILE_RAW; virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot probe backing volume format: %s"), - target->backingStore->path); + backingStore->path); } else { - target->backingStore->format = rc; + backingStore->format = rc;
... and I couldn't find a place where you update it back.
- target->backingStore->type = VIR_STORAGE_TYPE_NETWORK; - target->backingStore->path = meta->backingStoreRaw; + target->backingStore = backingStore;
I do it here. Still, you have a good point about temp variables, and long line, I'm working on this now. Thank you for the review.