On 04/23/2015 08:41 AM, Matthias Gatto wrote:
> Replace the parts of the code where a backing store is set manually
> with virStorageSourceSetBackingStore
>
> Signed-off-by: Matthias Gatto <matthias.gatto@xxxxxxxxxxxx>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/conf/domain_conf.c | 3 ++-
> src/conf/storage_conf.c | 17 ++++++++++-------
> src/qemu/qemu_driver.c | 17 +++++++++++------
> src/storage/storage_backend_fs.c | 7 +++++--
> src/storage/storage_backend_gluster.c | 5 +++--
> src/storage/storage_backend_logical.c | 9 ++++++---
> src/storage/storage_driver.c | 3 ++-
> src/util/virstoragefile.c | 8 +++++---
> tests/virstoragetest.c | 4 ++--
> 9 files changed, 46 insertions(+), 27 deletions(-)
>
Other than a minor goto error issue in storage_backend_gluster.c - up
through here things seem fine to me. Doesn't seem to be any new readers
or setters of "->backingStore" in recent changes (since patches 5-9
compile too). The only references are now ->backingStoreRaw fetches and
saves.
However, for patches 5-9 as I understand it have some concerns from
Peter which hopefully he can elaborate on.
So that progress can be made and readers/writers of backingStore go
through the virStorageSource{Get|Set}BackingStore API's until the rest
of the concerns are understood - I'm of the opinion the patches 1-4 can
be pushed, but I'll wait until Peter chimes in before doing so...
John
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2a05291..09f0bca 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6006,7 +6006,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
> virDomainDiskBackingStoreParse(ctxt, backingStore) < 0)
> goto cleanup;
>
> - src->backingStore = backingStore;
> + if (!virStorageSourceSetBackingStore(src, backingStore, 0))
> + goto cleanup;
> ret = 0;
>
> cleanup:
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 5781374..ca3a6d5 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1254,6 +1254,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
> char *capacity = NULL;
> char *unit = NULL;
> char *backingStore = NULL;
> + virStorageSourcePtr backingStorePtr;
> xmlNodePtr node;
> xmlNodePtr *nodes = NULL;
> size_t i;
> @@ -1290,20 +1291,22 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
> }
>
> if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) {
> - if (VIR_ALLOC(ret->target.backingStore) < 0)
> + if (VIR_ALLOC(backingStorePtr) < 0)
> goto error;
>
> - ret->target.backingStore->path = backingStore;
> + if (!virStorageSourceSetBackingStore(&ret->target, backingStorePtr, 0))
> + goto error;
> + backingStorePtr->path = backingStore;
> backingStore = NULL;
>
> if (options->formatFromString) {
> char *format = virXPathString("string(./backingStore/format/@type)", ctxt);
> if (format == NULL)
> - ret->target.backingStore->format = options->defaultFormat;
> + backingStorePtr->format = options->defaultFormat;
> else
> - ret->target.backingStore->format = (options->formatFromString)(format);
> + backingStorePtr->format = (options->formatFromString)(format);
>
> - if (ret->target.backingStore->format < 0) {
> + if (backingStorePtr->format < 0) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("unknown volume format type %s"), format);
> VIR_FREE(format);
> @@ -1312,9 +1315,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
> VIR_FREE(format);
> }
>
> - if (VIR_ALLOC(ret->target.backingStore->perms) < 0)
> + if (VIR_ALLOC(backingStorePtr->perms) < 0)
> goto error;
> - if (virStorageDefParsePerms(ctxt, ret->target.backingStore->perms,
> + if (virStorageDefParsePerms(ctxt, backingStorePtr->perms,
> "./backingStore/permissions",
> DEFAULT_VOL_PERM_MODE) < 0)
> goto error;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 26be9d6..1a98601 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14274,13 +14274,18 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
> /* Update vm in place to match changes. */
> need_unlink = false;
>
> - newDiskSrc->backingStore = disk->src;
> - disk->src = newDiskSrc;
> + if (!virStorageSourceSetBackingStore(newDiskSrc, disk->src, 0)) {
> + ret = -1;
> + goto cleanup;
> + }
> newDiskSrc = NULL;
>
> if (persistDisk) {
> - persistDiskSrc->backingStore = persistDisk->src;
> - persistDisk->src = persistDiskSrc;
> + if (!virStorageSourceSetBackingStore(persistDiskSrc,
> + persistDisk->src, 0)) {
> + ret = -1;
> + goto cleanup;
> + }
> persistDiskSrc = NULL;
> }
>
> @@ -14323,13 +14328,13 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver,
> /* Update vm in place to match changes. */
> tmp = disk->src;
> disk->src = virStorageSourceGetBackingStore(tmp, 0);
> - tmp->backingStore = NULL;
> + ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0));
> virStorageSourceFree(tmp);
>
> if (persistDisk) {
> tmp = persistDisk->src;
> persistDisk->src = virStorageSourceGetBackingStore(tmp, 0);
> - tmp->backingStore = NULL;
> + ignore_value(virStorageSourceSetBackingStore(tmp, NULL, 0));
> virStorageSourceFree(tmp);
> }
> }
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index 151af47..bab2569 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -97,7 +97,9 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
> goto cleanup;
>
> if (meta->backingStoreRaw) {
> - if (!(target->backingStore = virStorageSourceNewFromBacking(meta)))
> + if (!virStorageSourceSetBackingStore(target,
> + virStorageSourceNewFromBacking(meta),
> + 0))
> goto cleanup;
>
> backingStore = virStorageSourceGetBackingStore(target, 0);
> @@ -112,7 +114,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target,
> if (VIR_ALLOC(backingStore) < 0)
> goto cleanup;
>
> - target->backingStore = backingStore;
> + if (!virStorageSourceSetBackingStore(target, backingStore, 0))
> + goto cleanup;
> backingStore->type = VIR_STORAGE_TYPE_NETWORK;
> backingStore->path = meta->backingStoreRaw;
> meta->backingStoreRaw = NULL;
> diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
> index 9bddb3b..f0a3b9b 100644
> --- a/src/storage/storage_backend_gluster.c
> +++ b/src/storage/storage_backend_gluster.c
> @@ -301,9 +301,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
> goto cleanup;
>
> if (meta->backingStoreRaw) {
> - if (VIR_ALLOC(vol->target.backingStore) < 0)
> + if (VIR_ALLOC(backingStore) < 0)
> goto cleanup;
> - backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
> + if (!virStorageSourceSetBackingStore(&vol->target, backingStore, 0))
> + goto error;
Should be goto cleanup - build failure otherwise
>
> backingStore->path = meta->backingStoreRaw;
>
> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> index 27fdc64..16d508e 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -88,6 +88,7 @@ virStorageBackendLogicalMakeVol(char **const groups,
> size_t i;
> int err, nextents, nvars, ret = -1;
> const char *attrs = groups[9];
> + virStorageSourcePtr backingStore;
>
> /* Skip inactive volume */
> if (attrs[4] != 'a')
> @@ -148,14 +149,16 @@ 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 (VIR_ALLOC(backingStore) < 0)
> goto cleanup;
>
> - if (virAsprintf(&virStorageSourceGetBackingStore(&vol->target, 0)->path, "%s/%s",
> + if (!virStorageSourceSetBackingStore(&vol->target, backingStore, 0))
> + goto cleanup;
> + if (virAsprintf(&backingStore->path, "%s/%s",
> pool->def->target.path, groups[1]) < 0)
> goto cleanup;
>
> - virStorageSourceGetBackingStore(&vol->target, 0)->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
> + backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
> }
>
> if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0)
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 306d98e..cae7484 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -3044,7 +3044,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
> goto cleanup;
> }
>
> - src->backingStore = backingStore;
> + if (!virStorageSourceSetBackingStore(src, backingStore, 0))
> + goto cleanup;
> backingStore = NULL;
> ret = 0;
>
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index d69f49d..234a72b 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1902,8 +1902,10 @@ virStorageSourceCopy(const virStorageSource *src,
> goto error;
>
> if (backingChain && virStorageSourceGetBackingStore(src, 0)) {
> - if (!(ret->backingStore = virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0),
> - true)))
> + if (!virStorageSourceSetBackingStore(ret,
> + virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0),
> + true),
> + 0))
> goto error;
> }
>
> @@ -2044,7 +2046,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def)
>
> /* recursively free backing chain */
> virStorageSourceFree(virStorageSourceGetBackingStore(def, 0));
> - def->backingStore = NULL;
> + virStorageSourceSetBackingStore(def, NULL, 0);
> }
>
>
> diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
> index 5bd4637..58f505d 100644
> --- a/tests/virstoragetest.c
> +++ b/tests/virstoragetest.c
> @@ -583,9 +583,9 @@ testPathRelativePrepare(void)
>
> for (i = 0; i < ARRAY_CARDINALITY(backingchain); i++) {
> if (i < ARRAY_CARDINALITY(backingchain) - 1)
> - backingchain[i].backingStore = &backingchain[i + 1];
> + virStorageSourceSetBackingStore(&backingchain[i], &backingchain[i + 1], 0);
> else
> - backingchain[i].backingStore = NULL;
> + virStorageSourceSetBackingStore(&backingchain[i], NULL, 0);
>
> backingchain[i].relPath = NULL;
> }
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list