Re: [libvirt] [PATCH v5 4/9] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store

2015-05-05 Thread Matthias Gatto 
On Mon, May 4, 2015 at 7:21 PM, John Ferlan jfer...@redhat.com wrote:


 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.ga...@outscale.com
 Signed-off-by: John Ferlan jfer...@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 |  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;
 -

Re: [libvirt] [PATCH v5 4/9] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store

2015-05-04 Thread John Ferlan 


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.ga...@outscale.com
 Signed-off-by: John Ferlan jfer...@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 |  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

[libvirt] [PATCH v5 4/9] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store

2015-04-23 Thread Matthias Gatto 
Replace the parts of the code where a backing store is set manually
with virStorageSourceSetBackingStore

Signed-off-by: Matthias Gatto matthias.ga...@outscale.com
Signed-off-by: John Ferlan jfer...@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 |  9 ++---
 src/storage/storage_driver.c  |  3 ++-
 src/util/virstoragefile.c |  8 +---
 tests/virstoragetest.c|  4 ++--
 9 files changed, 46 insertions(+), 27 deletions(-)

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