1
0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-26 03:21:44 +03:00

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 <eblake@redhat.com>
This commit is contained in:
Eric Blake 2014-04-08 15:20:36 -06:00
parent 79f11b35c7
commit 43f85b995b

View File

@ -771,26 +771,24 @@ qcow2GetFeatures(virBitmapPtr *features,
/* Given a header in BUF with length LEN, as parsed from the file with /* 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 * user-provided name PATH and opened from CANONPATH, and where any
* relative backing file will be opened from DIRECTORY, return * relative backing file will be opened from DIRECTORY, and assuming
* metadata about that file, assuming it has the given FORMAT. */ * it has the given FORMAT, populate the newly-allocated META with
static virStorageFileMetadataPtr ATTRIBUTE_NONNULL(1) * information about the file and its backing store. */
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(7)
virStorageFileGetMetadataInternal(const char *path, virStorageFileGetMetadataInternal(const char *path,
const char *canonPath, const char *canonPath,
const char *directory, const char *directory,
char *buf, char *buf,
size_t len, size_t len,
int format) int format,
virStorageFileMetadataPtr meta)
{ {
virStorageFileMetadata *meta = NULL; int ret = -1;
virStorageFileMetadata *ret = NULL;
VIR_DEBUG("path=%s, canonPath=%s, dir=%s, buf=%p, len=%zu, format=%d", VIR_DEBUG("path=%s, canonPath=%s, dir=%s, buf=%p, len=%zu, format=%d",
path, canonPath, directory, buf, len, format); path, canonPath, directory, buf, len, format);
if (VIR_ALLOC(meta) < 0)
return NULL;
if (format == VIR_STORAGE_FILE_AUTO) if (format == VIR_STORAGE_FILE_AUTO)
format = virStorageFileProbeFormatFromBuf(path, buf, len); format = virStorageFileProbeFormatFromBuf(path, buf, len);
@ -886,11 +884,9 @@ virStorageFileGetMetadataInternal(const char *path,
goto cleanup; goto cleanup;
done: done:
ret = meta; ret = 0;
meta = NULL;
cleanup: cleanup:
virStorageFileFreeMetadata(meta);
return ret; return ret;
} }
@ -981,44 +977,52 @@ virStorageFileGetMetadataFromBuf(const char *path,
size_t len, size_t len,
int format) int format)
{ {
virStorageFileMetadataPtr ret; virStorageFileMetadataPtr ret = NULL;
char *canonPath; char *canonPath;
if (!(canonPath = canonicalize_file_name(path))) { if (!(canonPath = canonicalize_file_name(path))) {
virReportSystemError(errno, _("unable to resolve '%s'"), path); virReportSystemError(errno, _("unable to resolve '%s'"), path);
return NULL; return NULL;
} }
if (VIR_ALLOC(ret) < 0)
goto cleanup;
ret = virStorageFileGetMetadataInternal(path, canonPath, ".", buf, len, if (virStorageFileGetMetadataInternal(path, canonPath, ".", buf, len,
format); format, ret) < 0) {
virStorageFileFreeMetadata(ret);
ret = NULL;
}
cleanup:
VIR_FREE(canonPath); VIR_FREE(canonPath);
return ret; return ret;
} }
/* Internal version that also supports a containing directory name. */ /* Internal version that also supports a containing directory name. */
static virStorageFileMetadataPtr static int
virStorageFileGetMetadataFromFDInternal(const char *path, virStorageFileGetMetadataFromFDInternal(const char *path,
const char *canonPath, const char *canonPath,
const char *directory, const char *directory,
int fd, int fd,
int format) int format,
virStorageFileMetadataPtr meta)
{ {
char *buf = NULL; char *buf = NULL;
ssize_t len = VIR_STORAGE_MAX_HEADER; ssize_t len = VIR_STORAGE_MAX_HEADER;
struct stat sb; struct stat sb;
virStorageFileMetadataPtr ret = NULL; int ret = -1;
if (fstat(fd, &sb) < 0) { if (fstat(fd, &sb) < 0) {
virReportSystemError(errno, virReportSystemError(errno,
_("cannot stat file '%s'"), _("cannot stat file '%s'"),
path); path);
return NULL; return -1;
} }
/* No header to probe for directories, but also no backing file */ /* No header to probe for directories, but also no backing file */
if (S_ISDIR(sb.st_mode)) { if (S_ISDIR(sb.st_mode)) {
ignore_value(VIR_ALLOC(ret)); ret = 0;
goto cleanup; goto cleanup;
} }
@ -1033,7 +1037,8 @@ virStorageFileGetMetadataFromFDInternal(const char *path,
} }
ret = virStorageFileGetMetadataInternal(path, canonPath, directory, ret = virStorageFileGetMetadataInternal(path, canonPath, directory,
buf, len, format); buf, len, format, meta);
cleanup: cleanup:
VIR_FREE(buf); VIR_FREE(buf);
return ret; return ret;
@ -1063,71 +1068,81 @@ virStorageFileGetMetadataFromFD(const char *path,
int fd, int fd,
int format) int format)
{ {
virStorageFileMetadataPtr ret; virStorageFileMetadataPtr ret = NULL;
char *canonPath; char *canonPath;
if (!(canonPath = canonicalize_file_name(path))) { if (!(canonPath = canonicalize_file_name(path))) {
virReportSystemError(errno, _("unable to resolve '%s'"), path); virReportSystemError(errno, _("unable to resolve '%s'"), path);
return NULL; return NULL;
} }
ret = virStorageFileGetMetadataFromFDInternal(path, canonPath, ".", if (VIR_ALLOC(ret) < 0)
fd, format); goto cleanup;
if (virStorageFileGetMetadataFromFDInternal(path, canonPath, ".",
fd, format, ret) < 0) {
virStorageFileFreeMetadata(ret);
ret = NULL;
}
cleanup:
VIR_FREE(canonPath); VIR_FREE(canonPath);
return ret; return ret;
} }
/* Recursive workhorse for virStorageFileGetMetadata. */ /* Recursive workhorse for virStorageFileGetMetadata. */
static virStorageFileMetadataPtr static int
virStorageFileGetMetadataRecurse(const char *path, const char *canonPath, virStorageFileGetMetadataRecurse(const char *path, const char *canonPath,
const char *directory, const char *directory,
int format, uid_t uid, gid_t gid, int format, uid_t uid, gid_t gid,
bool allow_probe, virHashTablePtr cycle) bool allow_probe, virHashTablePtr cycle,
virStorageFileMetadataPtr meta)
{ {
int fd; int fd;
int ret = -1;
VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d", VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d",
path, canonPath, NULLSTR(directory), format, path, canonPath, NULLSTR(directory), format,
(int)uid, (int)gid, allow_probe); (int)uid, (int)gid, allow_probe);
virStorageFileMetadataPtr ret = NULL;
if (virHashLookup(cycle, canonPath)) { if (virHashLookup(cycle, canonPath)) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
_("backing store for %s is self-referential"), _("backing store for %s is self-referential"),
path); path);
return NULL; return -1;
} }
if (virHashAddEntry(cycle, canonPath, (void *)1) < 0) if (virHashAddEntry(cycle, canonPath, (void *)1) < 0)
return NULL; return -1;
if ((fd = virFileOpenAs(canonPath, O_RDONLY, 0, uid, gid, 0)) < 0) { if ((fd = virFileOpenAs(canonPath, O_RDONLY, 0, uid, gid, 0)) < 0) {
virReportSystemError(-fd, _("Failed to open file '%s'"), path); virReportSystemError(-fd, _("Failed to open file '%s'"), path);
return NULL; return -1;
} }
ret = virStorageFileGetMetadataFromFDInternal(path, canonPath, directory, ret = virStorageFileGetMetadataFromFDInternal(path, canonPath, directory,
fd, format); fd, format, meta);
if (VIR_CLOSE(fd) < 0) if (VIR_CLOSE(fd) < 0)
VIR_WARN("could not close file %s", path); VIR_WARN("could not close file %s", path);
if (ret && ret->backingStoreIsFile) { if (ret == 0 && meta->backingStoreIsFile) {
if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) virStorageFileMetadataPtr backing;
ret->backingStoreFormat = VIR_STORAGE_FILE_RAW;
else if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO_SAFE) if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO && !allow_probe)
ret->backingStoreFormat = VIR_STORAGE_FILE_AUTO; meta->backingStoreFormat = VIR_STORAGE_FILE_RAW;
format = ret->backingStoreFormat; else if (meta->backingStoreFormat == VIR_STORAGE_FILE_AUTO_SAFE)
ret->backingMeta = virStorageFileGetMetadataRecurse(ret->backingStoreRaw, meta->backingStoreFormat = VIR_STORAGE_FILE_AUTO;
ret->backingStore, format = meta->backingStoreFormat;
ret->directory, if (VIR_ALLOC(backing) < 0 ||
format, virStorageFileGetMetadataRecurse(meta->backingStoreRaw,
uid, gid, meta->backingStore,
allow_probe, meta->directory, format,
cycle); uid, gid, allow_probe,
if (!ret->backingMeta) { cycle, backing) < 0) {
/* If we failed to get backing data, mark the chain broken */ /* If we failed to get backing data, mark the chain broken */
ret->backingStoreFormat = VIR_STORAGE_FILE_NONE; meta->backingStoreFormat = VIR_STORAGE_FILE_NONE;
VIR_FREE(ret->backingStore); VIR_FREE(meta->backingStore);
virStorageFileFreeMetadata(backing);
} else {
meta->backingMeta = backing;
} }
} }
return ret; return ret;
@ -1164,6 +1179,7 @@ virStorageFileGetMetadata(const char *path, int format,
path, format, (int)uid, (int)gid, allow_probe); path, format, (int)uid, (int)gid, allow_probe);
virHashTablePtr cycle = virHashCreate(5, NULL); virHashTablePtr cycle = virHashCreate(5, NULL);
virStorageFileMetadataPtr meta = NULL;
virStorageFileMetadataPtr ret = NULL; virStorageFileMetadataPtr ret = NULL;
char *canonPath = NULL; char *canonPath = NULL;
char *directory = NULL; char *directory = NULL;
@ -1179,12 +1195,20 @@ virStorageFileGetMetadata(const char *path, int format,
virReportOOMError(); virReportOOMError();
goto cleanup; goto cleanup;
} }
if (VIR_ALLOC(meta) < 0)
goto cleanup;
if (format <= VIR_STORAGE_FILE_NONE) if (format <= VIR_STORAGE_FILE_NONE)
format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW;
ret = virStorageFileGetMetadataRecurse(path, canonPath, directory, format, if (virStorageFileGetMetadataRecurse(path, canonPath, directory, format,
uid, gid, allow_probe, cycle); uid, gid, allow_probe, cycle,
meta) < 0)
goto cleanup;
ret = meta;
meta = NULL;
cleanup: cleanup:
virStorageFileFreeMetadata(meta);
VIR_FREE(canonPath); VIR_FREE(canonPath);
VIR_FREE(directory); VIR_FREE(directory);
virHashFree(cycle); virHashFree(cycle);