Re: [libvirt] [PATCH v5 4/9] virstoragefile: Always use virStorageSourceSetBackingStore to set backing store
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
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
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