[libvirt] [PATCH v7 04/13] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store

John Ferlan jferlan at redhat.com
Tue Dec 15 20:51:26 UTC 2015



On 12/03/2015 09:35 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 at outscale.com>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  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 |  5 +++--
>  src/storage/storage_driver.c          |  3 ++-
>  src/util/virstoragefile.c             |  8 +++++---
>  tests/virstoragetest.c                |  4 ++--
>  9 files changed, 43 insertions(+), 26 deletions(-)
> 

Upstream changes and changes from/for patch 2  make this even more
difficult.  Rather than hit every change required and in order to make
some progress, it's perhaps easier to repost patches 1-5 and get them
accepted/upstream first.  Then focus on the rest afterwards.

I'll cobble together something - it'll differ slightly from what's here.

John
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5b413b5..d146811 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6359,7 +6359,8 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
>          virDomainDiskBackingStoreParse(ctxt, backingStore) < 0)
>          goto cleanup;
>  
> -    src->backingStore = backingStore;
> +    if (virStorageSourceSetBackingStore(src, backingStore, 0) < 0)
> +        goto cleanup;
>      ret = 0;
>  
>   cleanup:
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index d048e39..b9db5eb 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1259,6 +1259,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
>      char *capacity = NULL;
>      char *unit = NULL;
>      char *backingStore = NULL;
> +    virStorageSourcePtr backingStorePtr;
>      xmlNodePtr node;
>      xmlNodePtr *nodes = NULL;
>      size_t i;
> @@ -1295,20 +1296,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) < 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);
> @@ -1317,9 +1320,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") < 0)
>              goto error;
>      }
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 8ba7566..edfd8e6 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -14261,13 +14261,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) < 0) {
> +        ret = -1;
> +        goto cleanup;
> +    }
>      newDiskSrc = NULL;
>  
>      if (persistDisk) {
> -        persistDiskSrc->backingStore = persistDisk->src;
> -        persistDisk->src = persistDiskSrc;
> +        if (virStorageSourceSetBackingStore(persistDiskSrc,
> +                                            persistDisk->src, 0) < 0) {
> +            ret = -1;
> +            goto cleanup;
> +        }
>          persistDiskSrc = NULL;
>      }
>  
> @@ -14310,13 +14315,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 7b05d4d..b216e91 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) < 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) < 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..8b89433 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;
> +        if (virStorageSourceSetBackingStore(&vol->target, backingStore, 0) < 0)
>              goto cleanup;
> -        backingStore = virStorageSourceGetBackingStore(&vol->target, 0);
>  
>          backingStore->path = meta->backingStoreRaw;
>  
> diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> index e2a287d..eca9730 100644
> --- a/src/storage/storage_backend_logical.c
> +++ b/src/storage/storage_backend_logical.c
> @@ -149,10 +149,11 @@ virStorageBackendLogicalMakeVol(char **const groups,
>       *  lv is created with "--virtualsize").
>       */
>      if (groups[1] && STRNEQ(groups[1], "") && (groups[1][0] != '[')) {
> -        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) < 0)
> +            goto cleanup;
>          if (virAsprintf(&backingStore->path, "%s/%s",
>                          pool->def->target.path, groups[1]) < 0)
>              goto cleanup;
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index bbf21f6..963e325 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -3064,7 +3064,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
>          goto cleanup;
>      }
>  
> -    src->backingStore = backingStore;
> +    if (virStorageSourceSetBackingStore(src, backingStore, 0) < 0)
> +        goto cleanup;
>      backingStore = NULL;
>      ret = 0;
>  
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index a8a2134..1299f98 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1904,8 +1904,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) < 0)
>              goto error;
>      }
>  
> @@ -2046,7 +2048,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def)
>  
>      /* recursively free backing chain */
>      virStorageSourceFree(virStorageSourceGetBackingStore(def, 0));
> -    def->backingStore = NULL;
> +    ignore_value(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;
>      }
> 




More information about the libvir-list mailing list