Getting metadata on storage allocates a memory (path) which need to
be freed after use otherwise it gets leaked. This means after use of
virStorageFileGetMetadataFromFD or virStorageFileGetMetadata one
must call virStorageFileFreeMetadata to free it. This function frees
structure internals and structure itself.
---
diff to v2:
-Jrika's & Eric's review taken in
diff to v1:
-Eric's review suggestions taken in
cfg.mk | 1 +
src/conf/domain_conf.c | 18 +++++++++---
src/libvirt_private.syms | 1 +
src/storage/storage_backend_fs.c | 54 +++++++++++++++++++++----------------
src/util/storage_file.c | 18 ++++++++++++
src/util/storage_file.h | 2 +
6 files changed, 66 insertions(+), 28 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index ffaca85..a88a6a9 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -156,6 +156,7 @@ useless_free_options = \
--name=virSecretDefFree \
--name=virStorageEncryptionFree \
--name=virStorageEncryptionSecretFree \
+ --name=virStorageFileFreeMetadata \
--name=virStoragePoolDefFree \
--name=virStoragePoolObjFree \
--name=virStoragePoolSourceFree \
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 39e59b9..a895e98 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11055,10 +11055,16 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr
disk,
int ret = -1;
size_t depth = 0;
char *nextpath = NULL;
+ virStorageFileMetadata *meta;
if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK)
return 0;
+ if (VIR_ALLOC(meta) < 0) {
+ virReportOOMError();
+ return ret;
+ }
+
if (disk->driverType) {
const char *formatStr = disk->driverType;
if (STREQ(formatStr, "aio"))
@@ -11084,7 +11090,6 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr
disk,
paths = virHashCreate(5, NULL);
do {
- virStorageFileMetadata meta;
const char *path = nextpath ? nextpath : disk->src;
int fd;
@@ -11112,7 +11117,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr
disk,
}
}
- if (virStorageFileGetMetadataFromFD(path, fd, format, &meta) < 0) {
+ if (virStorageFileGetMetadataFromFD(path, fd, format, meta) < 0) {
VIR_FORCE_CLOSE(fd);
goto cleanup;
}
@@ -11126,16 +11131,18 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr
disk,
goto cleanup;
depth++;
- nextpath = meta.backingStore;
+ VIR_FREE(nextpath);
+ nextpath = meta->backingStore;
+ meta->backingStore = NULL;
/* Stop iterating if we reach a non-file backing store */
- if (nextpath && !meta.backingStoreIsFile) {
+ if (nextpath && !meta->backingStoreIsFile) {
VIR_DEBUG("Stopping iteration on non-file backing store: %s",
nextpath);
break;
}
- format = meta.backingStoreFormat;
+ format = meta->backingStoreFormat;
if (format == VIR_STORAGE_FILE_AUTO &&
!allowProbing)
@@ -11151,6 +11158,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr
disk,
cleanup:
virHashFree(paths);
VIR_FREE(nextpath);
+ virStorageFileFreeMetadata(meta);
return ret;
}
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3237d18..e06c7fc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -940,6 +940,7 @@ virStorageGenerateQcowPassphrase;
# storage_file.h
virStorageFileFormatTypeFromString;
virStorageFileFormatTypeToString;
+virStorageFileFreeMetadata;
virStorageFileGetMetadata;
virStorageFileGetMetadataFromFD;
virStorageFileIsSharedFS;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index b30e01e..8d6f76d 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -61,8 +61,14 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
unsigned long long *capacity,
virStorageEncryptionPtr *encryption)
{
- int fd, ret;
- virStorageFileMetadata meta;
+ int fd = -1;
+ int ret = -1;
+ virStorageFileMetadata *meta;
+
+ if (VIR_ALLOC(meta) < 0) {
+ virReportOOMError();
+ return ret;
+ }
*backingStore = NULL;
*backingStoreFormat = VIR_STORAGE_FILE_AUTO;
@@ -71,36 +77,33 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
if ((ret = virStorageBackendVolOpenCheckMode(target->path,
VIR_STORAGE_VOL_FS_REFRESH_FLAGS)) < 0)
- return ret; /* Take care to propagate ret, it is not always -1 */
+ goto error; /* Take care to propagate ret, it is not always -1 */
fd = ret;
if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd,
allocation,
capacity)) < 0) {
- VIR_FORCE_CLOSE(fd);
- return ret;
+ goto error;
}
- memset(&meta, 0, sizeof(meta));
-
if ((target->format = virStorageFileProbeFormatFromFD(target->path, fd)) <
0) {
- VIR_FORCE_CLOSE(fd);
- return -1;
+ ret = -1;
+ goto error;
}
if (virStorageFileGetMetadataFromFD(target->path, fd,
target->format,
- &meta) < 0) {
- VIR_FORCE_CLOSE(fd);
- return -1;
+ meta) < 0) {
+ ret = -1;
+ goto error;
}
VIR_FORCE_CLOSE(fd);
- if (meta.backingStore) {
- *backingStore = meta.backingStore;
- meta.backingStore = NULL;
- if (meta.backingStoreFormat == VIR_STORAGE_FILE_AUTO) {
+ if (meta->backingStore) {
+ *backingStore = meta->backingStore;
+ meta->backingStore = NULL;
+ if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO) {
if ((ret = virStorageFileProbeFormat(*backingStore)) < 0) {
/* If the backing file is currently unavailable, only log an
error,
* but continue. Returning -1 here would disable the whole
storage
@@ -114,18 +117,17 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr
target,
ret = 0;
}
} else {
- *backingStoreFormat = meta.backingStoreFormat;
+ *backingStoreFormat = meta->backingStoreFormat;
ret = 0;
}
} else {
- VIR_FREE(meta.backingStore);
ret = 0;
}
- if (capacity && meta.capacity)
- *capacity = meta.capacity;
+ if (capacity && meta->capacity)
+ *capacity = meta->capacity;
- if (encryption != NULL && meta.encrypted) {
+ if (encryption != NULL && meta->encrypted) {
if (VIR_ALLOC(*encryption) < 0) {
virReportOOMError();
goto cleanup;
@@ -147,11 +149,17 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr
target,
*/
}
+ virStorageFileFreeMetadata(meta);
+
return ret;
+error:
+ VIR_FORCE_CLOSE(fd);
+
cleanup:
- VIR_FREE(*backingStore);
- return -1;
+ virStorageFileFreeMetadata(meta);
+ return ret;
+
}
#if WITH_STORAGE_FS
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index 06cabc8..d4460d8 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -819,6 +819,8 @@ virStorageFileProbeFormat(const char *path)
* it indicates the 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 @meta after use via virStorageFileFreeMetadata.
*/
int
virStorageFileGetMetadataFromFD(const char *path,
@@ -892,6 +894,8 @@ cleanup:
* it indicates the 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 @meta after use via virStorageFileFreeMetadata.
*/
int
virStorageFileGetMetadata(const char *path,
@@ -912,6 +916,20 @@ virStorageFileGetMetadata(const char *path,
return ret;
}
+/**
+ * virStorageFileFreeMetadata:
+ *
+ * Free pointers in passed structure and structure itself.
+ */
+void
+virStorageFileFreeMetadata(virStorageFileMetadata *meta)
+{
+ if (!meta)
+ return;
+
+ VIR_FREE(meta->backingStore);
+ VIR_FREE(meta);
+}
#ifdef __linux__
diff --git a/src/util/storage_file.h b/src/util/storage_file.h
index f1bdd02..b8920d0 100644
--- a/src/util/storage_file.h
+++ b/src/util/storage_file.h
@@ -70,6 +70,8 @@ int virStorageFileGetMetadataFromFD(const char *path,
int format,
virStorageFileMetadata *meta);
+void virStorageFileFreeMetadata(virStorageFileMetadata *meta);
+
enum {
VIR_STORAGE_FILE_SHFS_NFS = (1 << 0),
VIR_STORAGE_FILE_SHFS_GFS2 = (1 << 1),
--
1.7.5.rc3
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
