[libvirt] [PATCH 0/5] util: Turn virStorageSource into a virObject (blockdev-add saga)
In todays side story we witness the transmutation of virStorageSource into a virObject. Peter Krempa (5): util: Introduce function for allocating virStorageSource util: storage: Turn virStorageSource into a virObject util: alloc: Introduce VIR_AUTOUNREF macro util: Remove the AUTOPTR func for virStorageSource util: Replace virStorageSourceFree with virObjectUnref src/conf/domain_conf.c | 20 +++---- src/conf/snapshot_conf.c | 6 +-- src/conf/storage_conf.c | 2 +- src/libvirt_private.syms | 3 +- src/qemu/qemu_blockjob.c | 10 ++-- src/qemu/qemu_domain.c | 8 +-- src/qemu/qemu_driver.c | 14 ++--- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_migration.c | 8 +-- src/storage/storage_backend_gluster.c | 4 +- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_util.c | 10 ++-- src/util/viralloc.h | 10 ++++ src/util/virobject.c | 13 +++++ src/util/virobject.h | 3 ++ src/util/virstoragefile.c | 75 ++++++++++++++++++--------- src/util/virstoragefile.h | 5 +- tests/qemublocktest.c | 6 +-- tests/virstoragetest.c | 14 ++--- 19 files changed, 134 insertions(+), 81 deletions(-) -- 2.20.1
Add virStorageSourceNew and refactor places allocating that structure to use the helper. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 10 +++++----- src/conf/snapshot_conf.c | 4 ++-- src/conf/storage_conf.c | 2 +- src/libvirt_private.syms | 1 + src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 4 ++-- src/storage/storage_backend_gluster.c | 2 +- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_util.c | 4 ++-- src/util/virstoragefile.c | 20 ++++++++++++++++---- src/util/virstoragefile.h | 1 + tests/qemublocktest.c | 2 +- tests/virstoragetest.c | 2 +- 14 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5d49f4388c..52cec5cd3d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1881,7 +1881,7 @@ virDomainDiskDefNew(virDomainXMLOptionPtr xmlopt) if (VIR_ALLOC(ret) < 0) return NULL; - if (VIR_ALLOC(ret->src) < 0) + if (!(ret->src = virStorageSourceNew())) goto error; if (xmlopt && @@ -2099,7 +2099,7 @@ virDomainFSDefNew(void) if (VIR_ALLOC(ret) < 0) return NULL; - if (VIR_ALLOC(ret->src) < 0) + if (!(ret->src = virStorageSourceNew())) goto cleanup; return ret; @@ -7585,7 +7585,7 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, /* For the purposes of command line creation, this needs to look * like a disk storage source */ - if (VIR_ALLOC(iscsisrc->src) < 0) + if (!(iscsisrc->src = virStorageSourceNew())) return -1; iscsisrc->src->type = VIR_STORAGE_TYPE_NETWORK; iscsisrc->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; @@ -9078,7 +9078,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, goto cleanup; } - if (VIR_ALLOC(backingStore) < 0) + if (!(backingStore = virStorageSourceNew())) goto cleanup; /* backing store is always read-only */ @@ -9236,7 +9236,7 @@ virDomainDiskDefMirrorParse(virDomainDiskDefPtr def, char *blockJob = NULL; int ret = -1; - if (VIR_ALLOC(def->mirror) < 0) + if (!(def->mirror = virStorageSourceNew())) goto cleanup; if ((blockJob = virXMLPropString(cur, "job"))) { diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index b16f450a01..5127ebbdfd 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -122,7 +122,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, ctxt->node = node; - if (VIR_ALLOC(def->src) < 0) + if (!(def->src = virStorageSourceNew())) goto cleanup; def->name = virXMLPropString(node, "name"); @@ -621,7 +621,7 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDefPtr def, if (virBitmapIsBitSet(map, i)) continue; disk = &def->disks[ndisks++]; - if (VIR_ALLOC(disk->src) < 0) + if (!(disk->src = virStorageSourceNew())) goto cleanup; if (VIR_STRDUP(disk->name, def->dom->disks[i]->dst) < 0) goto cleanup; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index a2ddecf0f2..d7c17db669 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1200,7 +1200,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, } if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { - if (VIR_ALLOC(def->target.backingStore) < 0) + if (!(def->target.backingStore = virStorageSourceNew())) return NULL; def->target.backingStore->type = VIR_STORAGE_TYPE_FILE; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 67579742fd..0a959c4bf9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2925,6 +2925,7 @@ virStorageSourceIsLocalStorage; virStorageSourceIsRelative; virStorageSourceIsSameLocation; virStorageSourceNetworkAssignDefaultPorts; +virStorageSourceNew; virStorageSourceNewFromBacking; virStorageSourceNewFromBackingAbsolute; virStorageSourceParseRBDColonString; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ac01e861f7..0c8feb25be 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2742,7 +2742,7 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node, goto cleanup; } - if (VIR_ALLOC(migrSource) < 0) + if (!(migrSource = virStorageSourceNew())) goto cleanup; if (!(type = virXMLPropString(ctxt->node, "type"))) { @@ -9050,7 +9050,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, /* terminate the chain for such images as the code below would do */ if (!disksrc->backingStore && - VIR_ALLOC(disksrc->backingStore) < 0) + !(disksrc->backingStore = virStorageSourceNew())) goto cleanup; /* host cdrom requires special treatment in qemu, so we need to check diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 971f915619..f94955a22f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17984,7 +17984,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, return qemuDomainBlockPullCommon(driver, vm, path, base, bandwidth, flags); /* If we got here, we are doing a block copy rebase. */ - if (VIR_ALLOC(dest) < 0) + if (!(dest = virStorageSourceNew())) goto cleanup; dest->type = (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_DEV) ? VIR_STORAGE_TYPE_BLOCK : VIR_STORAGE_TYPE_FILE; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c93ae33476..8a9dbb5e63 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -794,14 +794,14 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias, host); - if (VIR_ALLOC(copysrc) < 0) + if (!(copysrc = virStorageSourceNew())) goto cleanup; copysrc->type = VIR_STORAGE_TYPE_NETWORK; copysrc->protocol = VIR_STORAGE_NET_PROTOCOL_NBD; copysrc->format = VIR_STORAGE_FILE_RAW; - if (VIR_ALLOC(copysrc->backingStore) < 0) + if (!(copysrc->backingStore = virStorageSourceNew())) goto cleanup; if (VIR_STRDUP(copysrc->path, diskAlias) < 0) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 846a647cb6..854ecf2b67 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -294,7 +294,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, goto cleanup; if (meta->backingStoreRaw) { - if (VIR_ALLOC(vol->target.backingStore) < 0) + if (!(vol->target.backingStore = virStorageSourceNew())) goto cleanup; vol->target.backingStore->type = VIR_STORAGE_TYPE_NETWORK; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index f153d23aec..77e4dfb8b1 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -308,7 +308,7 @@ 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 (!(vol->target.backingStore = virStorageSourceNew())) goto cleanup; if (virAsprintf(&vol->target.backingStore->path, "%s/%s", diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index f18e38733a..3a6cc34b68 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3402,7 +3402,7 @@ storageBackendProbeTarget(virStorageSourcePtr target, if (!virStorageSourceIsLocalStorage(target->backingStore)) { virStorageSourceFree(target->backingStore); - if (VIR_ALLOC(target->backingStore) < 0) + if (!(target->backingStore = virStorageSourceNew())) return -1; target->backingStore->type = VIR_STORAGE_TYPE_NETWORK; @@ -3576,7 +3576,7 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) goto cleanup; VIR_DIR_CLOSE(dir); - if (VIR_ALLOC(target)) + if (!(target = virStorageSourceNew())) goto cleanup; if ((fd = open(def->target.path, O_RDONLY)) < 0) { diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index a6d44b64a4..7f52a5fdc7 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2245,7 +2245,7 @@ virStorageSourceCopy(const virStorageSource *src, { virStorageSourcePtr def = NULL; - if (VIR_ALLOC(def) < 0) + if (!(def = virStorageSourceNew())) return NULL; def->id = src->id; @@ -2562,6 +2562,18 @@ virStorageSourceClear(virStorageSourcePtr def) } +virStorageSourcePtr +virStorageSourceNew(void) +{ + virStorageSourcePtr ret = NULL; + + if (VIR_ALLOC(ret) < 0) + return NULL; + + return ret; +} + + void virStorageSourceFree(virStorageSourcePtr def) { @@ -2580,7 +2592,7 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, virStorageSourcePtr def; VIR_AUTOFREE(char *) dirname = NULL; - if (VIR_ALLOC(def) < 0) + if (!(def = virStorageSourceNew())) return NULL; /* store relative name */ @@ -3627,7 +3639,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path) virStorageSourcePtr def; int rc; - if (VIR_ALLOC(def) < 0) + if (!(def = virStorageSourceNew())) return NULL; if (virStorageIsFile(path)) { @@ -4900,7 +4912,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, } } else { /* add terminator */ - if (VIR_ALLOC(backingStore) < 0) + if (!(backingStore = virStorageSourceNew())) goto cleanup; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 8c3a36d473..48af06653e 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -431,6 +431,7 @@ int virStorageSourceGetActualType(const virStorageSource *def); bool virStorageSourceIsLocalStorage(const virStorageSource *src); bool virStorageSourceIsEmpty(virStorageSourcePtr src); bool virStorageSourceIsBlockLocal(const virStorageSource *src); +virStorageSourcePtr virStorageSourceNew(void); void virStorageSourceFree(virStorageSourcePtr def); void virStorageSourceBackingStoreClear(virStorageSourcePtr def); int virStorageSourceUpdatePhysicalSize(virStorageSourcePtr src, diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index d7e5e72a0b..813b20a08d 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -55,7 +55,7 @@ testBackingXMLjsonXML(const void *args) VIR_AUTOPTR(virStorageSource) xmlsrc = NULL; VIR_AUTOPTR(virStorageSource) jsonsrc = NULL; - if (VIR_ALLOC(xmlsrc) < 0) + if (!(xmlsrc = virStorageSourceNew())) return -1; xmlsrc->type = data->type; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 646ae78ff0..da70beb1f7 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -98,7 +98,7 @@ testStorageFileGetMetadata(const char *path, virStorageSourcePtr ret = NULL; VIR_AUTOPTR(virStorageSource) def = NULL; - if (VIR_ALLOC(def) < 0) + if (!(def = virStorageSourceNew())) return NULL; def->type = VIR_STORAGE_TYPE_FILE; -- 2.20.1
On Fri, Feb 15, 2019 at 01:42:09PM +0100, Peter Krempa wrote:
Add virStorageSourceNew and refactor places allocating that structure to use the helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Looks like you caught all the occurrences but 1 in bhyve: bhyveParsePCIDisk With that addressed too:
Reviewed-by: Erik Skultety <eskultet@redhat.com>
On Mon, Feb 18, 2019 at 08:29:14 +0100, Erik Skultety wrote:
On Fri, Feb 15, 2019 at 01:42:09PM +0100, Peter Krempa wrote:
Add virStorageSourceNew and refactor places allocating that structure to use the helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Looks like you caught all the occurrences but 1 in bhyve: bhyveParsePCIDisk With that addressed too:
Instead of changing that in this patch I propose we fix the bhyve parser properly: https://www.redhat.com/archives/libvir-list/2019-February/msg00956.html
On Mon, Feb 18, 2019 at 10:16:25AM +0100, Peter Krempa wrote:
On Mon, Feb 18, 2019 at 08:29:14 +0100, Erik Skultety wrote:
On Fri, Feb 15, 2019 at 01:42:09PM +0100, Peter Krempa wrote:
Add virStorageSourceNew and refactor places allocating that structure to use the helper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Looks like you caught all the occurrences but 1 in bhyve: bhyveParsePCIDisk With that addressed too:
Instead of changing that in this patch I propose we fix the bhyve parser properly:
https://www.redhat.com/archives/libvir-list/2019-February/msg00956.html
Yep, makes sense. Thanks, Erik
To allow tracking a single virStorageSource in multiple structures without extra hassle allow refcounting by turining it into an object. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 40 +++++++++++++++++++++++++++++---------- src/util/virstoragefile.h | 2 ++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 7f52a5fdc7..56c6510c5e 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -47,6 +47,8 @@ VIR_LOG_INIT("util.storagefile"); +static virClassPtr virStorageSourceClass; + VIR_ENUM_IMPL(virStorage, VIR_STORAGE_TYPE_LAST, "none", "file", @@ -2558,30 +2560,48 @@ virStorageSourceClear(virStorageSourcePtr def) virStorageSourceInitiatorClear(&def->initiator); - memset(def, 0, sizeof(*def)); + /* clear everything except the class header */ + memset((char *) def + sizeof(def->parent), 0, + sizeof(*def) - sizeof(def->parent)); +} + + +static void +virStorageSourceDispose(void *obj) +{ + virStorageSourcePtr src = obj; + + virStorageSourceClear(src); } +static int +virStorageSourceOnceInit(void) +{ + if (!VIR_CLASS_NEW(virStorageSource, virClassForObject())) + return -1; + + return 0; +} + + +VIR_ONCE_GLOBAL_INIT(virStorageSource); + + virStorageSourcePtr virStorageSourceNew(void) { - virStorageSourcePtr ret = NULL; - - if (VIR_ALLOC(ret) < 0) + if (virStorageSourceInitialize() < 0) return NULL; - return ret; + return virObjectNew(virStorageSourceClass); } void virStorageSourceFree(virStorageSourcePtr def) { - if (!def) - return; - - virStorageSourceClear(def); - VIR_FREE(def); + virObjectUnref(def); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 48af06653e..dc75d2d36f 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -242,6 +242,8 @@ typedef virStorageSource *virStorageSourcePtr; * IMPORTANT: When adding fields to this struct it's also necessary to add * appropriate code to the virStorageSourceCopy deep copy function */ struct _virStorageSource { + virObject parent; + unsigned int id; /* backing chain identifier, 0 is unset */ int type; /* virStorageType */ char *path; -- 2.20.1
On Fri, Feb 15, 2019 at 01:42:10PM +0100, Peter Krempa wrote:
To allow tracking a single virStorageSource in multiple structures without extra hassle allow refcounting by turining it into an object.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 40 +++++++++++++++++++++++++++++---------- src/util/virstoragefile.h | 2 ++ 2 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 7f52a5fdc7..56c6510c5e 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -47,6 +47,8 @@
VIR_LOG_INIT("util.storagefile");
+static virClassPtr virStorageSourceClass; + VIR_ENUM_IMPL(virStorage, VIR_STORAGE_TYPE_LAST, "none", "file", @@ -2558,30 +2560,48 @@ virStorageSourceClear(virStorageSourcePtr def)
virStorageSourceInitiatorClear(&def->initiator);
- memset(def, 0, sizeof(*def)); + /* clear everything except the class header */ + memset((char *) def + sizeof(def->parent), 0, + sizeof(*def) - sizeof(def->parent));
I've seen the reason behind this change, but I think a more thorough explanation would be appreciated - in this case enhancing the commentary is more useful than the commit message. Reviewed-by: Erik Skultety <eskultet@redhat.com>
Add helper for utilizing __attribute__(cleanup())) for unref-ing instances of sublasses of virObject. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/viralloc.h | 10 ++++++++++ src/util/virobject.c | 13 +++++++++++++ src/util/virobject.h | 3 +++ 4 files changed, 27 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0a959c4bf9..b6fec233f1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2511,6 +2511,7 @@ virClassForObjectRWLockable; virClassIsDerivedFrom; virClassName; virClassNew; +virObjectAutoUnref; virObjectFreeCallback; virObjectFreeHashData; virObjectIsClass; diff --git a/src/util/viralloc.h b/src/util/viralloc.h index aa1b92241d..1d67fff95c 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -637,4 +637,14 @@ void virAllocTestHook(void (*func)(int, void*), void *data); # define VIR_AUTOPTR(type) \ __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) type * +/** + * VIR_AUTOUNREF: + * @type: type of an virObject subclass to be unref'd automatically + * + * Declares a variable of @type which will be automatically unref'd when + * control goes out of the scope. + */ +# define VIR_AUTOUNREF(type) \ + __attribute__((cleanup(virObjectAutoUnref))) type + #endif /* LIBVIRT_VIRALLOC_H */ diff --git a/src/util/virobject.c b/src/util/virobject.c index b6ea299420..a4cbd08077 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -362,6 +362,19 @@ virObjectUnref(void *anyobj) } +/** + * virObjectAutoUnref: + * + * Helper used by VIR_AUTOUNREF + */ +void +virObjectAutoUnref(void *objptr) +{ + virObjectPtr *obj = objptr; + virObjectUnref(*obj); +} + + /** * virObjectRef: * @anyobj: any instance of virObjectPtr diff --git a/src/util/virobject.h b/src/util/virobject.h index b39b9946ff..56bc9f2324 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -102,6 +102,9 @@ virObjectNew(virClassPtr klass) bool virObjectUnref(void *obj); +void +virObjectAutoUnref(void *objptr); + void * virObjectRef(void *obj); -- 2.20.1
On Fri, Feb 15, 2019 at 01:42:11PM +0100, Peter Krempa wrote:
Add helper for utilizing __attribute__(cleanup())) for unref-ing instances of sublasses of virObject.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
Since virStorageSource is now a subclass of virObject, we can use VIR_AUTOUNREF instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_migration.c | 2 +- src/storage/storage_backend_gluster.c | 2 +- src/storage/storage_util.c | 4 ++-- src/util/virstoragefile.h | 1 - tests/qemublocktest.c | 4 ++-- tests/virstoragetest.c | 8 ++++---- 9 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 52cec5cd3d..91ee35391b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9071,7 +9071,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, char *format = NULL; char *idx = NULL; int ret = -1; - VIR_AUTOPTR(virStorageSource) backingStore = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) backingStore = NULL; if (!(ctxt->node = virXPathNode("./backingStore", ctxt))) { ret = 0; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0c8feb25be..a96a4f4049 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2733,7 +2733,7 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node, char *format = NULL; char *type = NULL; int ret = -1; - VIR_AUTOPTR(virStorageSource) migrSource = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) migrSource = NULL; ctxt->node = node; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f94955a22f..5a77afa1b4 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -279,7 +279,7 @@ qemuSecurityChownCallback(const virStorageSource *src, int save_errno = 0; int ret = -1; int rv; - VIR_AUTOPTR(virStorageSource) cpy = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) cpy = NULL; rv = virStorageFileSupportsSecurityDriver(src); if (rv <= 0) @@ -17962,7 +17962,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base, virDomainObjPtr vm; int ret = -1; unsigned long long speed = bandwidth; - VIR_AUTOPTR(virStorageSource) dest = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) dest = NULL; virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT | @@ -18156,7 +18156,7 @@ qemuDomainBlockCommit(virDomainPtr dom, unsigned long long speed = bandwidth; qemuBlockJobDataPtr job = NULL; qemuBlockJobType jobtype = QEMU_BLOCKJOB_TYPE_COMMIT; - VIR_AUTOPTR(virStorageSource) mirror = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) mirror = NULL; /* XXX Add support for COMMIT_DELETE */ virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW | diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8a9dbb5e63..b10f55f80c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -790,7 +790,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); int mon_ret = 0; int ret = -1; - VIR_AUTOPTR(virStorageSource) copysrc = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) copysrc = NULL; VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias, host); diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 854ecf2b67..819993439a 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -239,7 +239,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, ssize_t len; int backingFormat; VIR_AUTOPTR(virStorageVolDef) vol = NULL; - VIR_AUTOPTR(virStorageSource) meta = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) meta = NULL; VIR_AUTOFREE(char *) header = NULL; *volptr = NULL; diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 3a6cc34b68..1c5d4d14be 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3359,7 +3359,7 @@ storageBackendProbeTarget(virStorageSourcePtr target, int backingStoreFormat; int rc; struct stat sb; - VIR_AUTOPTR(virStorageSource) meta = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) meta = NULL; VIR_AUTOCLOSE fd = -1; if (encryption) @@ -3529,7 +3529,7 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) int ret = -1; VIR_AUTOPTR(virStorageVolDef) vol = NULL; VIR_AUTOCLOSE fd = -1; - VIR_AUTOPTR(virStorageSource) target = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) target = NULL; if (virDirOpen(&dir, def->target.path) < 0) goto cleanup; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index dc75d2d36f..17eea6dc6b 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -547,6 +547,5 @@ void virStorageFileReportBrokenChain(int errcode, virStorageSourcePtr parent); VIR_DEFINE_AUTOPTR_FUNC(virStorageAuthDef, virStorageAuthDefFree); -VIR_DEFINE_AUTOPTR_FUNC(virStorageSource, virStorageSourceFree); #endif /* LIBVIRT_VIRSTORAGEFILE_H */ diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 813b20a08d..4cd15a1dff 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -52,8 +52,8 @@ testBackingXMLjsonXML(const void *args) char *protocolwrapper = NULL; char *actualxml = NULL; int ret = -1; - VIR_AUTOPTR(virStorageSource) xmlsrc = NULL; - VIR_AUTOPTR(virStorageSource) jsonsrc = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) xmlsrc = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) jsonsrc = NULL; if (!(xmlsrc = virStorageSourceNew())) return -1; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index da70beb1f7..830d4fb08e 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -96,7 +96,7 @@ testStorageFileGetMetadata(const char *path, { struct stat st; virStorageSourcePtr ret = NULL; - VIR_AUTOPTR(virStorageSource) def = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) def = NULL; if (!(def = virStorageSourceNew())) return NULL; @@ -308,7 +308,7 @@ testStorageChain(const void *args) const struct testChainData *data = args; virStorageSourcePtr elt; size_t i = 0; - VIR_AUTOPTR(virStorageSource) meta = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) meta = NULL; VIR_AUTOFREE(char *) broken = NULL; meta = testStorageFileGetMetadata(data->start, data->format, -1, -1); @@ -642,7 +642,7 @@ testBackingParse(const void *args) virBuffer buf = VIR_BUFFER_INITIALIZER; int ret = -1; VIR_AUTOFREE(char *) xml = NULL; - VIR_AUTOPTR(virStorageSource) src = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) src = NULL; if (!(src = virStorageSourceNewFromBackingAbsolute(data->backing))) { if (!data->expect) @@ -692,7 +692,7 @@ mymain(void) virStorageSourcePtr chain2; /* short for chain->backingStore */ virStorageSourcePtr chain3; /* short for chain2->backingStore */ VIR_AUTOPTR(virCommand) cmd = NULL; - VIR_AUTOPTR(virStorageSource) chain = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) chain = NULL; if (storageRegisterAll() < 0) return EXIT_FAILURE; -- 2.20.1
On Fri, Feb 15, 2019 at 01:42:12PM +0100, Peter Krempa wrote:
Since virStorageSource is now a subclass of virObject, we can use VIR_AUTOUNREF instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
On Fri, Feb 15, 2019 at 01:42:12PM +0100, Peter Krempa wrote:
Since virStorageSource is now a subclass of virObject, we can use VIR_AUTOUNREF instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
Now that virStorageSource is a subclass of virObject we can use virObjectUnref and remove virStorageSourceFree which was a thin wrapper. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/snapshot_conf.c | 2 +- src/libvirt_private.syms | 1 - src/qemu/qemu_blockjob.c | 10 +++++----- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_migration.c | 2 +- src/storage/storage_util.c | 2 +- src/util/virstoragefile.c | 35 ++++++++++++++--------------------- src/util/virstoragefile.h | 1 - tests/virstoragetest.c | 4 ++-- 12 files changed, 33 insertions(+), 42 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 91ee35391b..38b30df82f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1903,10 +1903,10 @@ virDomainDiskDefFree(virDomainDiskDefPtr def) if (!def) return; - virStorageSourceFree(def->src); + virObjectUnref(def->src); VIR_FREE(def->serial); VIR_FREE(def->dst); - virStorageSourceFree(def->mirror); + virObjectUnref(def->mirror); VIR_FREE(def->wwn); VIR_FREE(def->driverName); VIR_FREE(def->vendor); @@ -2115,7 +2115,7 @@ void virDomainFSDefFree(virDomainFSDefPtr def) if (!def) return; - virStorageSourceFree(def->src); + virObjectUnref(def->src); VIR_FREE(def->dst); virDomainDeviceInfoClear(&def->info); VIR_FREE(def->virtio); @@ -2696,7 +2696,7 @@ virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc if (!iscsisrc) return; - virStorageSourceFree(iscsisrc->src); + virObjectUnref(iscsisrc->src); iscsisrc->src = NULL; } diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 5127ebbdfd..1afc7de30c 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -83,7 +83,7 @@ static void virDomainSnapshotDiskDefClear(virDomainSnapshotDiskDefPtr disk) { VIR_FREE(disk->name); - virStorageSourceFree(disk->src); + virObjectUnref(disk->src); disk->src = NULL; } diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b6fec233f1..b720acdc93 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2910,7 +2910,6 @@ virStorageSourceChainHasManagedPR; virStorageSourceClear; virStorageSourceCopy; virStorageSourceFindByNodeName; -virStorageSourceFree; virStorageSourceGetActualType; virStorageSourceGetSecurityLabelDef; virStorageSourceHasBacking; diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 0cbebab359..fa7e4c8625 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -258,13 +258,13 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver, VIR_WARN("Unable to update persistent definition " "on vm %s after block job", vm->def->name); - virStorageSourceFree(copy); + virObjectUnref(copy); copy = NULL; persistDisk = NULL; } } if (copy) { - virStorageSourceFree(persistDisk->src); + virObjectUnref(persistDisk->src); persistDisk->src = copy; } } @@ -275,12 +275,12 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver, * want to only revoke the non-shared portion of the chain); so for * now, we leak the access to the original. */ virDomainLockImageDetach(driver->lockManager, vm, disk->src); - virStorageSourceFree(disk->src); + virObjectUnref(disk->src); disk->src = disk->mirror; } else { if (disk->mirror) { virDomainLockImageDetach(driver->lockManager, vm, disk->mirror); - virStorageSourceFree(disk->mirror); + virObjectUnref(disk->mirror); } } @@ -345,7 +345,7 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver, case VIR_DOMAIN_BLOCK_JOB_CANCELED: if (disk->mirror) { virDomainLockImageDetach(driver->lockManager, vm, disk->mirror); - virStorageSourceFree(disk->mirror); + virObjectUnref(disk->mirror); disk->mirror = NULL; } disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a96a4f4049..a0af56695e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1072,7 +1072,7 @@ qemuDomainDiskPrivateDispose(void *obj) { qemuDomainDiskPrivatePtr priv = obj; - virStorageSourceFree(priv->migrSource); + virObjectUnref(priv->migrSource); VIR_FREE(priv->qomName); VIR_FREE(priv->nodeCopyOnRead); virObjectUnref(priv->blockjob); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5a77afa1b4..024ee5b62d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15183,9 +15183,9 @@ qemuDomainSnapshotDiskDataFree(qemuDomainSnapshotDiskDataPtr data, if (data[i].prepared) qemuDomainDiskChainElementRevoke(driver, vm, data[i].src); - virStorageSourceFree(data[i].src); + virObjectUnref(data[i].src); } - virStorageSourceFree(data[i].persistsrc); + virObjectUnref(data[i].persistsrc); VIR_FREE(data[i].relPath); } @@ -17950,7 +17950,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, cleanup: VIR_FREE(device); virObjectUnref(cfg); - virStorageSourceFree(mirror); + virObjectUnref(mirror); return ret; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 78c9a77f2d..00dbff6b2a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -830,7 +830,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, ignore_value(qemuHotplugPrepareDiskSourceAccess(driver, vm, oldsrc, true)); /* media was changed, so we can remove the old media definition now */ - virStorageSourceFree(oldsrc); + virObjectUnref(oldsrc); oldsrc = NULL; disk->src = newsrc; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b10f55f80c..362b79e567 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -760,7 +760,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver, qemuBlockStorageSourceDetachOneBlockdev(driver, vm, asyncJob, diskPriv->migrSource); - virStorageSourceFree(diskPriv->migrSource); + virObjectUnref(diskPriv->migrSource); diskPriv->migrSource = NULL; } diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 1c5d4d14be..7a879b0f46 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3400,7 +3400,7 @@ storageBackendProbeTarget(virStorageSourcePtr target, * remote storage. To avoid trouble, just fake the backing store is RAW * and put the string from the metadata as the path of the target. */ if (!virStorageSourceIsLocalStorage(target->backingStore)) { - virStorageSourceFree(target->backingStore); + virObjectUnref(target->backingStore); if (!(target->backingStore = virStorageSourceNew())) return -1; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 56c6510c5e..9dd27b5ca6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1133,7 +1133,7 @@ virStorageFileMetadataNew(const char *path, return def; error: - virStorageSourceFree(def); + virObjectUnref(def); return NULL; } @@ -1158,7 +1158,7 @@ virStorageFileMetadataNew(const char *path, * image didn't specify an explicit format for its backing store. Callers are * advised against probing for the backing store format in this case. * - * Caller MUST free the result after use via virStorageSourceFree. + * Caller MUST free the result after use via virObjectUnref. */ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path, @@ -1178,7 +1178,7 @@ virStorageFileGetMetadataFromBuf(const char *path, if (virStorageFileGetMetadataInternal(ret, buf, len, backingFormat) < 0) { - virStorageSourceFree(ret); + virObjectUnref(ret); return NULL; } @@ -1197,7 +1197,7 @@ virStorageFileGetMetadataFromBuf(const char *path, * format, since a malicious guest can turn a raw file into any * other non-raw format at will. * - * Caller MUST free the result after use via virStorageSourceFree. + * Caller MUST free the result after use via virObjectUnref. */ virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path, @@ -1257,7 +1257,7 @@ virStorageFileGetMetadataFromFD(const char *path, VIR_STEAL_PTR(ret, meta); cleanup: - virStorageSourceFree(meta); + virObjectUnref(meta); return ret; } @@ -2336,7 +2336,7 @@ virStorageSourceCopy(const virStorageSource *src, return def; error: - virStorageSourceFree(def); + virObjectUnref(def); return NULL; } @@ -2522,7 +2522,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->backingStoreRaw); /* recursively free backing chain */ - virStorageSourceFree(def->backingStore); + virObjectUnref(def->backingStore); def->backingStore = NULL; } @@ -2598,13 +2598,6 @@ virStorageSourceNew(void) } -void -virStorageSourceFree(virStorageSourcePtr def) -{ - virObjectUnref(def); -} - - static virStorageSourcePtr virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, const char *rel) @@ -2656,7 +2649,7 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, return def; error: - virStorageSourceFree(def); + virObjectUnref(def); def = NULL; goto cleanup; } @@ -3697,7 +3690,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path) return def; error: - virStorageSourceFree(def); + virObjectUnref(def); return NULL; } @@ -3738,7 +3731,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent) return def; error: - virStorageSourceFree(def); + virObjectUnref(def); return NULL; } @@ -3919,7 +3912,7 @@ virStorageSourceUpdateCapacity(virStorageSourcePtr src, ret = 0; cleanup: - virStorageSourceFree(meta); + virObjectUnref(meta); return ret; } @@ -4943,7 +4936,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, if (virStorageSourceHasBacking(src)) src->backingStore->id = depth; virStorageFileDeinit(src); - virStorageSourceFree(backingStore); + virObjectUnref(backingStore); return ret; } @@ -4966,7 +4959,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, * If @report_broken is true, the whole function fails with a possibly sane * error instead of just returning a broken chain. * - * Caller MUST free result after use via virStorageSourceFree. + * Caller MUST free result after use via virObjectUnref. */ int virStorageFileGetMetadata(virStorageSourcePtr src, @@ -5050,7 +5043,7 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src, ret = 0; cleanup: - virStorageSourceFree(tmp); + virObjectUnref(tmp); return ret; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 17eea6dc6b..ba5181804c 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -434,7 +434,6 @@ bool virStorageSourceIsLocalStorage(const virStorageSource *src); bool virStorageSourceIsEmpty(virStorageSourcePtr src); bool virStorageSourceIsBlockLocal(const virStorageSource *src); virStorageSourcePtr virStorageSourceNew(void); -void virStorageSourceFree(virStorageSourcePtr def); void virStorageSourceBackingStoreClear(virStorageSourcePtr def); int virStorageSourceUpdatePhysicalSize(virStorageSourcePtr src, int fd, struct stat const *sb); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 830d4fb08e..fb98903f02 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -1084,7 +1084,7 @@ mymain(void) ret = -1; /* Test behavior of chain lookups, relative backing from absolute start */ - virStorageSourceFree(chain); + virObjectUnref(chain); chain = testStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2, -1, -1); if (!chain) { ret = -1; @@ -1130,7 +1130,7 @@ mymain(void) ret = -1; /* Test behavior of chain lookups, relative backing */ - virStorageSourceFree(chain); + virObjectUnref(chain); chain = testStorageFileGetMetadata("sub/link2", VIR_STORAGE_FILE_QCOW2, -1, -1); if (!chain) { -- 2.20.1
On Fri, Feb 15, 2019 at 01:42:13PM +0100, Peter Krempa wrote:
Now that virStorageSource is a subclass of virObject we can use virObjectUnref and remove virStorageSourceFree which was a thin wrapper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- Reviewed-by: Erik Skultety <eskultet@redhat.com>
On 2/15/19 7:42 AM, Peter Krempa wrote:
Now that virStorageSource is a subclass of virObject we can use virObjectUnref and remove virStorageSourceFree which was a thin wrapper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/snapshot_conf.c | 2 +- src/libvirt_private.syms | 1 - src/qemu/qemu_blockjob.c | 10 +++++----- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_migration.c | 2 +- src/storage/storage_util.c | 2 +- src/util/virstoragefile.c | 35 ++++++++++++++--------------------- src/util/virstoragefile.h | 1 - tests/virstoragetest.c | 4 ++-- 12 files changed, 33 insertions(+), 42 deletions(-)
[...]
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 56c6510c5e..9dd27b5ca6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1133,7 +1133,7 @@ virStorageFileMetadataNew(const char *path, return def;
error: - virStorageSourceFree(def); + virObjectUnref(def);
Is this right? @def is VIR_ALLOC'd and not virStorageSourceNew alloc'd. Also anything that calls this would have the same problem I would think. FWIW: This module is the one I ran afoul of the MinGW builds when trying to use VIR_AUTOPTR. I see no change in here to use VIR_AUTOUNREF, so I assume this was the way around it, albeit without the object alloc. John
return NULL; }
@@ -1158,7 +1158,7 @@ virStorageFileMetadataNew(const char *path, * image didn't specify an explicit format for its backing store. Callers are * advised against probing for the backing store format in this case. * - * Caller MUST free the result after use via virStorageSourceFree. + * Caller MUST free the result after use via virObjectUnref. */ virStorageSourcePtr virStorageFileGetMetadataFromBuf(const char *path, @@ -1178,7 +1178,7 @@ virStorageFileGetMetadataFromBuf(const char *path,
if (virStorageFileGetMetadataInternal(ret, buf, len, backingFormat) < 0) { - virStorageSourceFree(ret); + virObjectUnref(ret); return NULL; }
@@ -1197,7 +1197,7 @@ virStorageFileGetMetadataFromBuf(const char *path, * format, since a malicious guest can turn a raw file into any * other non-raw format at will. * - * Caller MUST free the result after use via virStorageSourceFree. + * Caller MUST free the result after use via virObjectUnref. */ virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path, @@ -1257,7 +1257,7 @@ virStorageFileGetMetadataFromFD(const char *path, VIR_STEAL_PTR(ret, meta);
cleanup: - virStorageSourceFree(meta); + virObjectUnref(meta); return ret; }
@@ -2336,7 +2336,7 @@ virStorageSourceCopy(const virStorageSource *src, return def;
error: - virStorageSourceFree(def); + virObjectUnref(def); return NULL; }
@@ -2522,7 +2522,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def) VIR_FREE(def->backingStoreRaw);
/* recursively free backing chain */ - virStorageSourceFree(def->backingStore); + virObjectUnref(def->backingStore); def->backingStore = NULL; }
@@ -2598,13 +2598,6 @@ virStorageSourceNew(void) }
-void -virStorageSourceFree(virStorageSourcePtr def) -{ - virObjectUnref(def); -} - - static virStorageSourcePtr virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, const char *rel) @@ -2656,7 +2649,7 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, return def;
error: - virStorageSourceFree(def); + virObjectUnref(def); def = NULL; goto cleanup; } @@ -3697,7 +3690,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path) return def;
error: - virStorageSourceFree(def); + virObjectUnref(def); return NULL; }
@@ -3738,7 +3731,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent) return def;
error: - virStorageSourceFree(def); + virObjectUnref(def); return NULL; }
@@ -3919,7 +3912,7 @@ virStorageSourceUpdateCapacity(virStorageSourcePtr src, ret = 0;
cleanup: - virStorageSourceFree(meta); + virObjectUnref(meta); return ret; }
@@ -4943,7 +4936,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, if (virStorageSourceHasBacking(src)) src->backingStore->id = depth; virStorageFileDeinit(src); - virStorageSourceFree(backingStore); + virObjectUnref(backingStore); return ret; }
@@ -4966,7 +4959,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, * If @report_broken is true, the whole function fails with a possibly sane * error instead of just returning a broken chain. * - * Caller MUST free result after use via virStorageSourceFree. + * Caller MUST free result after use via virObjectUnref. */ int virStorageFileGetMetadata(virStorageSourcePtr src, @@ -5050,7 +5043,7 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src, ret = 0;
cleanup: - virStorageSourceFree(tmp); + virObjectUnref(tmp);
return ret;
[...]
On Mon, Feb 18, 2019 at 07:15:38 -0500, John Ferlan wrote:
On 2/15/19 7:42 AM, Peter Krempa wrote:
Now that virStorageSource is a subclass of virObject we can use virObjectUnref and remove virStorageSourceFree which was a thin wrapper.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 8 ++++---- src/conf/snapshot_conf.c | 2 +- src/libvirt_private.syms | 1 - src/qemu/qemu_blockjob.c | 10 +++++----- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 6 +++--- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_migration.c | 2 +- src/storage/storage_util.c | 2 +- src/util/virstoragefile.c | 35 ++++++++++++++--------------------- src/util/virstoragefile.h | 1 - tests/virstoragetest.c | 4 ++-- 12 files changed, 33 insertions(+), 42 deletions(-)
[...]
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 56c6510c5e..9dd27b5ca6 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1133,7 +1133,7 @@ virStorageFileMetadataNew(const char *path, return def;
error: - virStorageSourceFree(def); + virObjectUnref(def);
Is this right? @def is VIR_ALLOC'd and not virStorageSourceNew alloc'd. Also anything that calls this would have the same problem I would think.
No. virStorageSource now must be allocated using virStorageSourceNew. The first patch missed fixing this instance. I'll post a patch soon.
participants (3)
-
Erik Skultety -
John Ferlan -
Peter Krempa