[libvirt] [PATCH v7 05/13] virstoragefile: change backingStore to backingStores.

John Ferlan jferlan at redhat.com
Tue Dec 15 20:55:04 UTC 2015



On 12/03/2015 09:35 AM, Matthias Gatto wrote:
> The backingStore field was a virStorageSourcePtr.
> Because a quorum can contain more that one backingStore at the same level,
> it's now an array of 'virStorageSourcePtr'.
> 
> This patch rename  src->backingStore to src->backingStores,
> Made the necessary changes to virStorageSourceSetBackingStore
> and virStorageSourceGetBackingStore.
> virStorageSourceSetBackingStore can now expand the size of src->backingStores.
> 
> Signed-off-by: Matthias Gatto <matthias.gatto at outscale.com>
> ---
>  src/storage/storage_backend.c    |  2 +-
>  src/storage/storage_backend_fs.c |  2 +-
>  src/util/virstoragefile.c        | 66 +++++++++++++++++++++++++++++++---------
>  src/util/virstoragefile.h        |  3 +-
>  4 files changed, 56 insertions(+), 17 deletions(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 08ed1dd..d71bb1a 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -498,7 +498,7 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
>          goto cleanup;
>      }
>  
> -    if (vol->target.backingStore) {
> +    if (vol->target.backingStores) {
>          virReportError(VIR_ERR_NO_SUPPORT, "%s",
>                         _("backing storage not supported for raw volumes"));
>          goto cleanup;
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index b216e91..68419e3 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -1100,7 +1100,7 @@ static int createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED,
>          return -1;
>      }
>  
> -    if (vol->target.backingStore) {
> +    if (vol->target.backingStores) {
>          virReportError(VIR_ERR_NO_SUPPORT, "%s",
>                         _("backing storage not supported for directories volumes"));
>          return -1;

I would think these two accesses would need a read access rather than
changing from backingStore to backingStores.  First clue would be that
now that everything is supposed to be contained within virstoragefile.c
so any change outside that has been something changed since you first
started this adventure.

> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 1299f98..8c05786 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1809,22 +1809,50 @@ 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)
> +    if (!src || !src->backingStores || pos >= src->nBackingStores)
>          return NULL;
> -    return src->backingStore;
> +    return src->backingStores[pos];
>  }
>  
>  
> +/**
> + * virStorageSourceSetBackingStore:
> + * @src: virStorageSourcePtr to hold @backingStore
> + * @backingStore: backingStore to store
> + * @pos: position of the backing store to store
> + *
> + * Set @backingStore at @pos in src->backingStores.
> + * If src->backingStores don't have the space to contain
> + * @backingStore, we expand src->backingStores.
> + * If src->backingStores[pos] is alerady set, free it.
> + */
>  int
>  virStorageSourceSetBackingStore(virStorageSourcePtr src,
>                                  virStorageSourcePtr backingStore,
> -                                size_t pos ATTRIBUTE_UNUSED)
> +                                size_t pos)
>  {
> -    src->backingStore = backingStore;
> +    if (!src)
> +        return -1;
> +
> +    if (pos >= src->nBackingStores) {
> +        int nbr = pos - src->nBackingStores + 1;
> +        if (VIR_EXPAND_N(src->backingStores, src->nBackingStores, nbr) < 0)
> +            return -1;
> +    }
> +
> +    if (src->backingStores[pos])
> +        virStorageSourceFree(src->backingStores[pos]);
> +    src->backingStores[pos] = backingStore;
>      return 0;
>  }
>  
> @@ -1843,6 +1871,7 @@ virStorageSourceCopy(const virStorageSource *src,
>                       bool backingChain)
>  {
>      virStorageSourcePtr ret = NULL;
> +    size_t i;
>  
>      if (VIR_ALLOC(ret) < 0)
>          return NULL;
> @@ -1855,6 +1884,8 @@ virStorageSourceCopy(const virStorageSource *src,
>      ret->physical = src->physical;
>      ret->readonly = src->readonly;
>      ret->shared = src->shared;
> +    ret->nBackingStores = src->nBackingStores;
> +    ret->threshold = src->threshold;

This appears to be a rogue change...  It causes my build to fail.  Looks
like it's defined in patch 7 and further used in patch 8 - so those will
need an update once this is pushed.

John

>  
>      /* storage driver metadata are not copied */
>      ret->drv = NULL;
> @@ -1903,12 +1934,14 @@ virStorageSourceCopy(const virStorageSource *src,
>          !(ret->auth = virStorageAuthDefCopy(src->auth)))
>          goto error;
>  
> -    if (backingChain && virStorageSourceGetBackingStore(src, 0)) {
> -        if (virStorageSourceSetBackingStore(ret,
> -                                            virStorageSourceCopy(virStorageSourceGetBackingStore(src, 0),
> -                                                                 true),
> -                                             0) < 0)
> -            goto error;
> +    for (i = 0; i < src->nBackingStores; ++i) {
> +        if (backingChain && virStorageSourceGetBackingStore(src, i)) {
> +            if (virStorageSourceSetBackingStore(ret,
> +                                                virStorageSourceCopy(virStorageSourceGetBackingStore(src, i),
> +                                                                     true),
> +                                                0) < 0)
> +                goto error;
> +        }
>      }
>  
>      return ret;
> @@ -2040,6 +2073,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src)
>  void
>  virStorageSourceBackingStoreClear(virStorageSourcePtr def)
>  {
> +    size_t i;
> +
>      if (!def)
>          return;
>  
> @@ -2047,8 +2082,11 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def)
>      VIR_FREE(def->backingStoreRaw);
>  
>      /* recursively free backing chain */
> -    virStorageSourceFree(virStorageSourceGetBackingStore(def, 0));
> -    ignore_value(virStorageSourceSetBackingStore(def, NULL, 0));
> +    for (i = 0; i < def->nBackingStores; ++i)
> +        virStorageSourceFree(virStorageSourceGetBackingStore(def, i));
> +    if (def->nBackingStores > 0)
> +        VIR_FREE(def->backingStores);
> +    def->nBackingStores = 0;
>  }
>  
>  
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index ce1cb5d..290c20f 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -270,7 +270,8 @@ struct _virStorageSource {
>      bool shared;
>  
>      /* backing chain of the storage source */
> -    virStorageSourcePtr backingStore;
> +    virStorageSourcePtr *backingStores;
> +    size_t      nBackingStores;
>  
>      /* metadata for storage driver access to remote and local volumes */
>      virStorageDriverDataPtr drv;
> 




More information about the libvir-list mailing list