Re: [libvirt] [PATCH v2 5/9] virstoragefile: Treat backingStore as a pointer or an array

As explain in the former patchs, backingStore can be treat an array or a pointer. If we have only one backingStore we have to use it as a normal ptr but if there is more backing store, we use it as a pointer's array. Because it would be complicated to expend backingStore manually, and do the convertion from a pointer to an array, I've created the virStorageSourcePushBackingStore function to help. This function allocate an array of pointer only if there is more than 1 bs. Because we can not remove a child from a quorum, i didn't create the function virStorageSourcePopBackingStore. I've changed virStorageSourceBackingStoreClear, virStorageSourceSetBackingStore and virStorageSourceGetBackingStore to handle the case where backingStore is an array. Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; --- src/conf/storage_conf.c | 7 ++- src/libvirt_private.syms | 1 + src/storage/storage_backend_fs.c | 2 + src/storage/storage_backend_logical.c | 2 +- src/util/virstoragefile.c | 89 ++++++++++++++++++++++++++++++++--- src/util/virstoragefile.h | 2 + 6 files changed, 94 insertions(+), 9 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index fac85fa..41887eb 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1343,7 +1343,12 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (VIR_ALLOC(backingStorePtr) < 0) goto error; - backingStorePtr = virStorageSourceSetBackingStore(&ret->target, backingStorePtr, 0); + if (!virStorageSourcePushBackingStore(&ret->target)) + goto error; + + backingStorePtr = virStorageSourceSetBackingStore(&ret->target, + backingStorePtr, + ret->target.nBackingStores - 1); backingStorePtr->path = backingStore; backingStore = NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 980235b..5752907 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2033,6 +2033,7 @@ virStorageSourceParseRBDColonString; virStorageSourcePoolDefFree; virStorageSourcePoolModeTypeFromString; virStorageSourcePoolModeTypeToString; +virStorageSourcePushBackingStore; virStorageSourceSetBackingStore; virStorageTypeFromString; virStorageTypeToString; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index a3b6688..0d8c5ad 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -112,6 +112,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, if (VIR_ALLOC(backingStore) < 0) goto cleanup; + if (virStorageSourcePushBackingStore(target) == false) + goto cleanup; virStorageSourceSetBackingStore(target, backingStore, 0); backingStore->type = VIR_STORAGE_TYPE_NETWORK; backingStore->path = meta->backingStoreRaw; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index fcec31f..cd8de85 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -148,7 +148,7 @@ virStorageBackendLogicalMakeVol(char **const groups, * lv is created with "--virtualsize"). */ if (groups[1] && !STREQ(groups[1], "") && (groups[1][0] != '[')) { - if (VIR_ALLOC(vol->target.backingStore) < 0) + if (virStorageSourcePushBackingStore(&vol->target) == false) goto cleanup; if (virAsprintf(&virStorageSourceGetBackingStore(&vol->target, 0)->path, "%s/%s", diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index ba38827..554aa8b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1798,21 +1798,88 @@ virStorageSourcePoolDefCopy(const virStorageSourcePoolDef *src) } +/** + * virStorageSourceGetBackingStore: + * @src: virStorageSourcePtr containing the backing stores + * @pos: position of the backing store to get + * + * return the backingStore at the position of @pos + */ virStorageSourcePtr -virStorageSourceGetBackingStore(const virStorageSource *src, - size_t pos ATTRIBUTE_UNUSED) +virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos) +{ + if (!src->backingStore || (pos > 1 && pos >= src->nBackingStores)) + return NULL; + if (src->nBackingStores < 2) + return src->backingStore; + return ((virStorageSourcePtr *)src->backingStore)[pos]; +} + + +/** + * virStorageSourcePushBackingStore: + * @src: virStorageSourcePtr to allocate the new backing store + * + * Allocate size for a new backingStorePtr in src->backingStore + * and update src->nBackingStores + * If we have less than 2 backing stores, we treat src->backingStore + * as a pointer otherwise we treat it as an array of virStorageSourcePtr + */ +bool +virStorageSourcePushBackingStore(virStorageSourcePtr src) { - return src->backingStore; + virStorageSourcePtr tmp; + virStorageSourcePtr *tmp2; + + if (!src) + return false; + if (src->nBackingStores == 1) { + /* If we need more than one backing store we need an array + * Because we don't want to lose our data from the old Backing Store + * we copy the pointer from src->backingStore to src->backingStore[0] */ + tmp = src->backingStore; + if (VIR_ALLOC_N(tmp2, 1) < 0) + return false; + src->backingStore = (virStorageSourcePtr)tmp2; + src->nBackingStores += 1; + virStorageSourceSetBackingStore(src, tmp, 0); + } else if (src->nBackingStores > 1) { + tmp2 = ((virStorageSourcePtr *)src->backingStore); + if (VIR_EXPAND_N(tmp2, src->nBackingStores, 1) < 0) + return false; + src->backingStore = (virStorageSourcePtr)tmp2; + } else { + /* Most of the time we use only one backingStore + * So we don't need to allocate an array */ + src->nBackingStores += 1; + } + return true;

} +/** + * virStorageSourceSetBackingStore: + * @src: virStorageSourcePtr to hold @backingStore + * @backingStore: backingStore to store + * @pos: position of the backing store to store + * + * Set @backingStore at @pos in src->backingStore + */ virStorageSourcePtr virStorageSourceSetBackingStore(virStorageSourcePtr src, virStorageSourcePtr backingStore, - size_t pos ATTRIBUTE_UNUSED) + size_t pos) { - src->backingStore = backingStore; - return src->backingStore; + if (!src || (pos > 1 && pos >= src->nBackingStores)) + goto error; + if (src->nBackingStores > 1) { + ((virStorageSourcePtr *)src->backingStore)[pos] = backingStore; + } else { + src->backingStore = backingStore; + } + return backingStore; + error: + return NULL; } @@ -2023,6 +2090,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src) void virStorageSourceBackingStoreClear(virStorageSourcePtr def) { + size_t i; + if (!def) return; @@ -2030,7 +2099,13 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->backingStoreRaw); /* recursively free backing chain */ - virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); + for (i = 0; i < def->nBackingStores; ++i) + virStorageSourceFree(virStorageSourceGetBackingStore(def, i)); + if (def->nBackingStores > 1) { + /* in this case def->backingStores is treat as an array so we have to free it*/ + VIR_FREE(def->backingStore); + } + def->nBackingStores = 0; virStorageSourceSetBackingStore(def, NULL, 0); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index d5cf7e6..74c363b 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -271,6 +271,7 @@ struct _virStorageSource { /* backing chain of the storage source */ virStorageSourcePtr backingStore; + size_t nBackingStores;