6:42 a.m.
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
6:42 a.m.
New subject: [libvirt] [PATCH 1/5] util: Introduce function for allocating virStorageSource
Add virStorageSourceNew and refactor places allocating that structure to
use the helper.
Signed-off-by: Peter Krempa <pkrempa(a)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
1:29 a.m.
New subject: [libvirt] [PATCH 1/5] util: Introduce function for allocating virStorageSource
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(a)redhat.com>
---
Looks like you caught all the occurrences but 1 in bhyve: bhyveParsePCIDisk
With that addressed too:
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
3:16 a.m.
New subject: [libvirt] [PATCH 1/5] util: Introduce function for allocating virStorageSource
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(a)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
3:22 a.m.
New subject: [libvirt] [PATCH 1/5] util: Introduce function for allocating virStorageSource
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(a)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
6:42 a.m.
New subject: [libvirt] [PATCH 2/5] util: storage: Turn virStorageSource into a virObject
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(a)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
1:56 a.m.
New subject: [libvirt] [PATCH 2/5] util: storage: Turn virStorageSource into a virObject
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(a)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(a)redhat.com>
6:42 a.m.
New subject: [libvirt] [PATCH 3/5] util: alloc: Introduce VIR_AUTOUNREF macro
Add helper for utilizing __attribute__(cleanup())) for unref-ing
instances of sublasses of virObject.
Signed-off-by: Peter Krempa <pkrempa(a)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
2:05 a.m.
New subject: [libvirt] [PATCH 3/5] util: alloc: Introduce VIR_AUTOUNREF macro
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(a)redhat.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
6:42 a.m.
New subject: [libvirt] [PATCH 4/5] util: Remove the AUTOPTR func for virStorageSource
Since virStorageSource is now a subclass of virObject, we can use
VIR_AUTOUNREF instead.
Signed-off-by: Peter Krempa <pkrempa(a)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
2:06 a.m.
New subject: [libvirt] [PATCH 4/5] util: Remove the AUTOPTR func for virStorageSource
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(a)redhat.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
2:07 a.m.
New subject: [libvirt] [PATCH 4/5] util: Remove the AUTOPTR func for virStorageSource
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(a)redhat.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
6:42 a.m.
New subject: [libvirt] [PATCH 5/5] util: Replace virStorageSourceFree with virObjectUnref
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(a)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
2:08 a.m.
New subject: [libvirt] [PATCH 5/5] util: Replace virStorageSourceFree with virObjectUnref
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(a)redhat.com>
---
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
6:15 a.m.
New subject: [libvirt] [PATCH 5/5] util: Replace virStorageSourceFree with virObjectUnref
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(a)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;
[...]
6:21 a.m.
New subject: [libvirt] [PATCH 5/5] util: Replace virStorageSourceFree with virObjectUnref
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(a)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.
2069
days inactive
2072
days old
15 comments
3 participants
participants (3)
-
Erik Skultety -
John Ferlan -
Peter Krempa