[libvirt] [PATCHv2 09/16] storage: don't require caller to pre-allocate metadata struct
Eric Blake
eblake at redhat.com
Sat Oct 13 22:00:05 UTC 2012
Requiring pre-allocation was an unusual idiom. It allowed iteration
over the backing chain to use fewer mallocs, but made one-shot
clients harder to read. Also, this makes it easier for a future
patch to move away from opening fds on every iteration over the chain.
* src/util/storage_file.h (virStorageFileGetMetadataFromFD): Alter
signature.
* src/util/storage_file.c (virStorageFileGetMetadataFromFD): Allocate
return value.
(virStorageFileGetMetadata): Update clients.
* src/conf/domain_conf.c (virDomainDiskDefForeachPath): Likewise.
* src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Likewise.
* src/storage/storage_backend_fs.c (virStorageBackendProbeTarget):
Likewise.
---
src/conf/domain_conf.c | 11 ++++------
src/qemu/qemu_driver.c | 9 +-------
src/storage/storage_backend_fs.c | 12 +++-------
src/util/storage_file.c | 47 +++++++++++++++++++---------------------
src/util/storage_file.h | 7 +++---
5 files changed, 33 insertions(+), 53 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index acf1904..a1d5a1a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14644,16 +14644,11 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
int ret = -1;
size_t depth = 0;
char *nextpath = NULL;
- virStorageFileMetadata *meta;
+ virStorageFileMetadata *meta = NULL;
if (!disk->src || disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK)
return 0;
- if (VIR_ALLOC(meta) < 0) {
- virReportOOMError();
- return ret;
- }
-
if (disk->format > 0) {
format = disk->format;
} else {
@@ -14696,7 +14691,7 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
}
}
- if (virStorageFileGetMetadataFromFD(path, fd, format, meta) < 0) {
+ if (!(meta = virStorageFileGetMetadataFromFD(path, fd, format))) {
VIR_FORCE_CLOSE(fd);
goto cleanup;
}
@@ -14730,6 +14725,8 @@ int virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
/* Allow probing for image formats that are safe */
if (format == VIR_STORAGE_FILE_AUTO_SAFE)
format = VIR_STORAGE_FILE_AUTO;
+ virStorageFileFreeMetadata(meta);
+ meta = NULL;
} while (nextpath);
ret = 0;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4d1c63c..0e7665e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9202,14 +9202,7 @@ static int qemuDomainGetBlockInfo(virDomainPtr dom,
}
}
- if (VIR_ALLOC(meta) < 0) {
- virReportOOMError();
- goto cleanup;
- }
-
- if (virStorageFileGetMetadataFromFD(path, fd,
- format,
- meta) < 0)
+ if (!(meta = virStorageFileGetMetadataFromFD(path, fd, format)))
goto cleanup;
/* Get info for normal formats */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index db19b87..cecc2d2 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -68,12 +68,7 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
{
int fd = -1;
int ret = -1;
- virStorageFileMetadata *meta;
-
- if (VIR_ALLOC(meta) < 0) {
- virReportOOMError();
- return ret;
- }
+ virStorageFileMetadata *meta = NULL;
*backingStore = NULL;
*backingStoreFormat = VIR_STORAGE_FILE_AUTO;
@@ -96,9 +91,8 @@ virStorageBackendProbeTarget(virStorageVolTargetPtr target,
goto error;
}
- if (virStorageFileGetMetadataFromFD(target->path, fd,
- target->format,
- meta) < 0) {
+ if (!(meta = virStorageFileGetMetadataFromFD(target->path, fd,
+ target->format))) {
ret = -1;
goto error;
}
diff --git a/src/util/storage_file.c b/src/util/storage_file.c
index 0f879bd..5387f1e 100644
--- a/src/util/storage_file.c
+++ b/src/util/storage_file.c
@@ -852,41 +852,43 @@ virStorageFileProbeFormat(const char *path)
* backing store. Callers are advised against probing for the
* backing store format in this case.
*
- * Caller MUST free @meta after use via virStorageFileFreeMetadata.
+ * Caller MUST free the result after use via virStorageFileFreeMetadata.
*/
-int
+virStorageFileMetadataPtr
virStorageFileGetMetadataFromFD(const char *path,
int fd,
- int format,
- virStorageFileMetadata *meta)
+ int format)
{
+ virStorageFileMetadata *meta = NULL;
unsigned char *head = NULL;
ssize_t len = STORAGE_MAX_HEAD;
- int ret = -1;
+ virStorageFileMetadata *ret = NULL;
struct stat sb;
- memset(meta, 0, sizeof(*meta));
+ if (VIR_ALLOC(meta) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
if (fstat(fd, &sb) < 0) {
virReportSystemError(errno,
_("cannot stat file '%s'"),
path);
- return -1;
+ goto cleanup;
}
- /* No header to probe for directories */
- if (S_ISDIR(sb.st_mode)) {
- return 0;
- }
+ /* No header to probe for directories, but also no backing file */
+ if (S_ISDIR(sb.st_mode))
+ return meta;
if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
virReportSystemError(errno, _("cannot seek to start of '%s'"), path);
- return -1;
+ goto cleanup;
}
if (VIR_ALLOC_N(head, len) < 0) {
virReportOOMError();
- return -1;
+ goto cleanup;
}
if ((len = read(fd, head, len)) < 0) {
@@ -904,9 +906,13 @@ virStorageFileGetMetadataFromFD(const char *path,
goto cleanup;
}
- ret = virStorageFileGetMetadataFromBuf(format, path, head, len, meta);
+ if (virStorageFileGetMetadataFromBuf(format, path, head, len, meta) < 0)
+ goto cleanup;
+ ret = meta;
+ meta = NULL;
cleanup:
+ virStorageFileFreeMetadata(meta);
VIR_FREE(head);
return ret;
}
@@ -934,21 +940,12 @@ virStorageFileGetMetadataRecurse(const char *path, int format,
return NULL;
}
- if (VIR_ALLOC(ret) < 0) {
- virReportOOMError();
- VIR_FORCE_CLOSE(fd);
- return NULL;
- }
- if (virStorageFileGetMetadataFromFD(path, fd, format, ret) < 0) {
- virStorageFileFreeMetadata(ret);
- VIR_FORCE_CLOSE(fd);
- return NULL;
- }
+ ret = virStorageFileGetMetadataFromFD(path, fd, format);
if (VIR_CLOSE(fd) < 0)
virReportSystemError(errno, _("could not close file %s"), path);
- if (ret->backingStoreIsFile) {
+ if (ret && ret->backingStoreIsFile) {
if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO && !allow_probe)
ret->backingStoreFormat = VIR_STORAGE_FILE_RAW;
else if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO_SAFE)
diff --git a/src/util/storage_file.h b/src/util/storage_file.h
index 18dea6a..9a60451 100644
--- a/src/util/storage_file.h
+++ b/src/util/storage_file.h
@@ -73,10 +73,9 @@ virStorageFileMetadataPtr virStorageFileGetMetadata(const char *path,
int format,
uid_t uid, gid_t gid,
bool allow_probe);
-int virStorageFileGetMetadataFromFD(const char *path,
- int fd,
- int format,
- virStorageFileMetadata *meta);
+virStorageFileMetadataPtr virStorageFileGetMetadataFromFD(const char *path,
+ int fd,
+ int format);
void virStorageFileFreeMetadata(virStorageFileMetadataPtr meta);
--
1.7.11.7
More information about the libvir-list
mailing list