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

[Peter Krempa]

On Thu, Oct 29, 2015 at 14:43:12 +0100, 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 
> ---
>  src/storage/storage_backend.c|  2 +-
>  src/storage/storage_backend_fs.c |  2 +-
>  src/util/virstoragefile.c| 48 
> +---
>  src/util/virstoragefile.h|  3 ++-
>  4 files changed, 44 insertions(+), 11 deletions(-)

[...]

> 
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index cb96c5b..44bce91 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1809,21 +1809,48 @@ 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)
>  {
> -return src->backingStore;
> +if (!src || !src->backingStores || pos >= src->nBackingStores)
> +return NULL;

The first two condition statements should be in the patch that
introduced this function.

> +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
> + */
>  bool

As I've said earlier, libvirt's convention is to return -1 in case when
something failed.

>  virStorageSourceSetBackingStore(virStorageSourcePtr src,
>  virStorageSourcePtr backingStore,
> -size_t pos ATTRIBUTE_UNUSED)
> +size_t pos)
>  {
> -src->backingStore = backingStore;
> -return !!src->backingStore;
> +if (!src)
> +return false;
> +
> +if (pos >= src->nBackingStores) {
> +int nbr = pos - src->nBackingStores + 1;
> +if (VIR_EXPAND_N(src->backingStores, src->nBackingStores, nbr) < 0)
> +return false;
> +}
> +
> +src->backingStores[pos] = backingStore;

Shouldn't this free the backing store if it's being overwritten? Or
perhaps fail if it's already assigned?

> +return true;
>  }
>  
>  


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Matthias Gatto]

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 
---
 src/storage/storage_backend.c|  2 +-
 src/storage/storage_backend_fs.c |  2 +-
 src/util/virstoragefile.c| 48 +---
 src/util/virstoragefile.h|  3 ++-
 4 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 3b504e9..f6ed91a 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -497,7 +497,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 d065a5f..ef86ecd 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1097,7 +1097,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;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index cb96c5b..44bce91 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1809,21 +1809,48 @@ 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)
 {
-return src->backingStore;
+if (!src || !src->backingStores || pos >= src->nBackingStores)
+return NULL;
+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
+ */
 bool
 virStorageSourceSetBackingStore(virStorageSourcePtr src,
 virStorageSourcePtr backingStore,
-size_t pos ATTRIBUTE_UNUSED)
+size_t pos)
 {
-src->backingStore = backingStore;
-return !!src->backingStore;
+if (!src)
+return false;
+
+if (pos >= src->nBackingStores) {
+int nbr = pos - src->nBackingStores + 1;
+if (VIR_EXPAND_N(src->backingStores, src->nBackingStores, nbr) < 0)
+return false;
+}
+
+src->backingStores[pos] = backingStore;
+return true;
 }
 
 
@@ -2038,6 +2065,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src)
 void
 virStorageSourceBackingStoreClear(virStorageSourcePtr def)
 {
+size_t i;
+
 if (!def)
 return;
 
@@ -2045,8 +2074,11 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr 
def)
 VIR_FREE(def->backingStoreRaw);
 
 /* recursively free backing chain */
-virStorageSourceFree(virStorageSourceGetBackingStore(def, 0));
-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 5c5c29d..5f76b4b 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;
-- 
2.6.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list