From 43f85b995b19092e802bba497cf838d35e6a88f2 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 8 Apr 2014 15:20:36 -0600 Subject: [PATCH] conf: earlier allocation during backing chain crawl Right now, we are allocating virStorageFileMetadata near the bottom of the callchain, only after we have identified that we are visiting a file (and not a network resource). I'm hoping to eventually support parsing the backing chain from XML, where the backing chain crawl then validates what was parsed rather than allocating a fresh structure. Likewise, I'm working towards a setup where we have a backing element even for networks. Both of these use cases are easier to code if the allocation is hoisted earlier. * src/util/virstoragefile.c (virStorageFileGetMetadataInternal) (virStorageFileGetMetadataFromFDInternal): Change signature. (virStorageFileGetMetadataFromBuf) (virStorageFileGetMetadataRecurse, virStorageFileGetMetadata): Update callers. Signed-off-by: Eric Blake --- src/util/virstoragefile.c | 126 +++++++++++++++++++++++--------------- 1 file changed, 75 insertions(+), 51 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index dacda1300d..a6f87d9608 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -771,26 +771,24 @@ qcow2GetFeatures(virBitmapPtr *features, /* Given a header in BUF with length LEN, as parsed from the file with * user-provided name PATH and opened from CANONPATH, and where any - * relative backing file will be opened from DIRECTORY, return - * metadata about that file, assuming it has the given FORMAT. */ -static virStorageFileMetadataPtr ATTRIBUTE_NONNULL(1) -ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) + * relative backing file will be opened from DIRECTORY, and assuming + * it has the given FORMAT, populate the newly-allocated META with + * information about the file and its backing store. */ +static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(7) virStorageFileGetMetadataInternal(const char *path, const char *canonPath, const char *directory, char *buf, size_t len, - int format) + int format, + virStorageFileMetadataPtr meta) { - virStorageFileMetadata *meta = NULL; - virStorageFileMetadata *ret = NULL; + int ret = -1; VIR_DEBUG("path=%s, canonPath=%s, dir=%s, buf=%p, len=%zu, format=%d", path, canonPath, directory, buf, len, format); - if (VIR_ALLOC(meta) < 0) - return NULL; - if (format == VIR_STORAGE_FILE_AUTO) format = virStorageFileProbeFormatFromBuf(path, buf, len); @@ -886,11 +884,9 @@ virStorageFileGetMetadataInternal(const char *path, goto cleanup; done: - ret = meta; - meta = NULL; + ret = 0; cleanup: - virStorageFileFreeMetadata(meta); return ret; } @@ -981,44 +977,52 @@ virStorageFileGetMetadataFromBuf(const char *path, size_t len, int format) { - virStorageFileMetadataPtr ret; + virStorageFileMetadataPtr ret = NULL; char *canonPath; if (!(canonPath = canonicalize_file_name(path))) { virReportSystemError(errno, _("unable to resolve '%s'"), path); return NULL; } + if (VIR_ALLOC(ret) < 0) + goto cleanup; + + if (virStorageFileGetMetadataInternal(path, canonPath, ".", buf, len, + format, ret) < 0) { + virStorageFileFreeMetadata(ret); + ret = NULL; + } - ret = virStorageFileGetMetadataInternal(path, canonPath, ".", buf, len, - format); + cleanup: VIR_FREE(canonPath); return ret; } /* Internal version that also supports a containing directory name. */ -static virStorageFileMetadataPtr +static int virStorageFileGetMetadataFromFDInternal(const char *path, const char *canonPath, const char *directory, int fd, - int format) + int format, + virStorageFileMetadataPtr meta) { char *buf = NULL; ssize_t len = VIR_STORAGE_MAX_HEADER; struct stat sb; - virStorageFileMetadataPtr ret = NULL; + int ret = -1; if (fstat(fd, &sb) < 0) { virReportSystemError(errno, _("cannot stat file '%s'"), path); - return NULL; + return -1; } /* No header to probe for directories, but also no backing file */ if (S_ISDIR(sb.st_mode)) { - ignore_value(VIR_ALLOC(ret)); + ret = 0; goto cleanup; } @@ -1033,7 +1037,8 @@ virStorageFileGetMetadataFromFDInternal(const char *path, } ret = virStorageFileGetMetadataInternal(path, canonPath, directory, - buf, len, format); + buf, len, format, meta); + cleanup: VIR_FREE(buf); return ret; @@ -1063,71 +1068,81 @@ virStorageFileGetMetadataFromFD(const char *path, int fd, int format) { - virStorageFileMetadataPtr ret; + virStorageFileMetadataPtr ret = NULL; char *canonPath; if (!(canonPath = canonicalize_file_name(path))) { virReportSystemError(errno, _("unable to resolve '%s'"), path); return NULL; } - ret = virStorageFileGetMetadataFromFDInternal(path, canonPath, ".", - fd, format); + if (VIR_ALLOC(ret) < 0) + goto cleanup; + if (virStorageFileGetMetadataFromFDInternal(path, canonPath, ".", + fd, format, ret) < 0) { + virStorageFileFreeMetadata(ret); + ret = NULL; + } + + cleanup: VIR_FREE(canonPath); return ret; } /* Recursive workhorse for virStorageFileGetMetadata. */ -static virStorageFileMetadataPtr +static int virStorageFileGetMetadataRecurse(const char *path, const char *canonPath, const char *directory, int format, uid_t uid, gid_t gid, - bool allow_probe, virHashTablePtr cycle) + bool allow_probe, virHashTablePtr cycle, + virStorageFileMetadataPtr meta) { int fd; + int ret = -1; VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d", path, canonPath, NULLSTR(directory), format, (int)uid, (int)gid, allow_probe); - virStorageFileMetadataPtr ret = NULL; - if (virHashLookup(cycle, canonPath)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("backing store for %s is self-referential"), path); - return NULL; + return -1; } if (virHashAddEntry(cycle, canonPath, (void *)1) < 0) - return NULL; + return -1; if ((fd = virFileOpenAs(canonPath, O_RDONLY, 0, uid, gid, 0)) < 0) { virReportSystemError(-fd, _("Failed to open file '%s'"), path); - return NULL; + return -1; } ret = virStorageFileGetMetadataFromFDInternal(path, canonPath, directory, - fd, format); + fd, format, meta); if (VIR_CLOSE(fd) < 0) VIR_WARN("could not close file %s", path); - 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) - ret->backingStoreFormat = VIR_STORAGE_FILE_AUTO; - format = ret->backingStoreFormat; - ret->backingMeta = virStorageFileGetMetadataRecurse(ret->backingStoreRaw, - ret->backingStore, - ret->directory, - format, - uid, gid, - allow_probe, - cycle); - if (!ret->backingMeta) { + if (ret == 0 && meta->backingStoreIsFile) { + virStorageFileMetadataPtr backing; + + if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) + meta->backingStoreFormat = VIR_STORAGE_FILE_RAW; + else if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO_SAFE) + meta->backingStoreFormat = VIR_STORAGE_FILE_AUTO; + format = meta->backingStoreFormat; + if (VIR_ALLOC(backing) < 0 || + virStorageFileGetMetadataRecurse(meta->backingStoreRaw, + meta->backingStore, + meta->directory, format, + uid, gid, allow_probe, + cycle, backing) < 0) { /* If we failed to get backing data, mark the chain broken */ - ret->backingStoreFormat = VIR_STORAGE_FILE_NONE; - VIR_FREE(ret->backingStore); + meta->backingStoreFormat = VIR_STORAGE_FILE_NONE; + VIR_FREE(meta->backingStore); + virStorageFileFreeMetadata(backing); + } else { + meta->backingMeta = backing; } } return ret; @@ -1164,6 +1179,7 @@ virStorageFileGetMetadata(const char *path, int format, path, format, (int)uid, (int)gid, allow_probe); virHashTablePtr cycle = virHashCreate(5, NULL); + virStorageFileMetadataPtr meta = NULL; virStorageFileMetadataPtr ret = NULL; char *canonPath = NULL; char *directory = NULL; @@ -1179,12 +1195,20 @@ virStorageFileGetMetadata(const char *path, int format, virReportOOMError(); goto cleanup; } + if (VIR_ALLOC(meta) < 0) + goto cleanup; if (format <= VIR_STORAGE_FILE_NONE) format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; - ret = virStorageFileGetMetadataRecurse(path, canonPath, directory, format, - uid, gid, allow_probe, cycle); + if (virStorageFileGetMetadataRecurse(path, canonPath, directory, format, + uid, gid, allow_probe, cycle, + meta) < 0) + goto cleanup; + ret = meta; + meta = NULL; + cleanup: + virStorageFileFreeMetadata(meta); VIR_FREE(canonPath); VIR_FREE(directory); virHashFree(cycle); -- GitLab