From fd7349166af834cd954fed04091611f91e312a25 Mon Sep 17 00:00:00 2001 Message-Id: From: Eric Blake Date: Wed, 26 Feb 2014 14:54:34 +0100 Subject: [PATCH] storage: refactor backing chain division of labor https://bugzilla.redhat.com/show_bug.cgi?id=1032370 Future patches will want to learn metadata about a file using a buffer that was already parsed in order to probe the file's format. Rather than reopening and re-reading the file, it makes sense to separate getting file contents from actually parsing those contents. * src/util/virstoragefile.c (virStorageFileGetMetadataFromBuf) (virStorageFileGetMetadataFromFDInternal): New functions. (virStorageFileGetMetadataInternal): Hoist fstat() and read() into callers. (virStorageFileGetMetadataFromFD) (virStorageFileGetMetadataRecurse): Rework clients. * src/util/virstoragefile.h (virStorageFileGetMetadataFromBuf): New prototype. * src/libvirt_private.syms (virstoragefile.h): Export it. Signed-off-by: Eric Blake (cherry picked from commit 3ead2e7dedab7d9380496798ef0201be2b0047b5) Signed-off-by: Jiri Denemark --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 118 +++++++++++++++++++++++++++++++++------------- src/util/virstoragefile.h | 4 ++ 3 files changed, 91 insertions(+), 32 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e239aba..4d74d46 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1924,6 +1924,7 @@ virStorageFileFormatTypeToString; virStorageFileFreeMetadata; virStorageFileGetLVMKey; virStorageFileGetMetadata; +virStorageFileGetMetadataFromBuf; virStorageFileGetMetadataFromFD; virStorageFileGetSCSIKey; virStorageFileIsClusterFS; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index bb27461..8b89db1 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -752,47 +752,26 @@ qcow2GetFeatures(virBitmapPtr *features, } -/* Given a file descriptor FD open on PATH, and optionally opened from - * a given DIRECTORY, return metadata about that file, assuming it has - * the given FORMAT. */ +/* Given a header in BUF with length LEN, as parsed from the file + * located at PATH, and optionally opened from a given DIRECTORY, + * return metadata about that file, assuming it has the given + * FORMAT. */ static virStorageFileMetadataPtr virStorageFileGetMetadataInternal(const char *path, - int fd, + char *buf, + size_t len, const char *directory, int format) { virStorageFileMetadata *meta = NULL; - char *buf = NULL; - ssize_t len = STORAGE_MAX_HEAD; virStorageFileMetadata *ret = NULL; - struct stat sb; - VIR_DEBUG("path=%s, fd=%d, format=%d", path, fd, format); + VIR_DEBUG("path=%s, buf=%p, len=%zu, directory=%s, format=%d", + path, buf, len, NULLSTR(directory), format); if (VIR_ALLOC(meta) < 0) return NULL; - if (fstat(fd, &sb) < 0) { - virReportSystemError(errno, - _("cannot stat file '%s'"), - path); - goto cleanup; - } - - /* 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); - goto cleanup; - } - - if ((len = virFileReadHeaderFD(fd, len, (char **)&buf)) < 0) { - virReportSystemError(errno, _("cannot read header '%s'"), path); - goto cleanup; - } - if (format == VIR_STORAGE_FILE_AUTO) format = virStorageFileProbeFormatFromBuf(path, buf, len); @@ -893,7 +872,6 @@ done: cleanup: virStorageFileFreeMetadata(meta); - VIR_FREE(buf); return ret; } @@ -979,6 +957,81 @@ virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid) return ret; } + +/** + * virStorageFileGetMetadataFromBuf: + * @path: name of file, for error messages + * @buf: header bytes from @path + * @len: length of @buf + * @format: expected image format + * + * Extract metadata about the storage volume with the specified + * image format. If image format is VIR_STORAGE_FILE_AUTO, it + * will probe to automatically identify the format. Does not recurse. + * + * Callers are advised never to use VIR_STORAGE_FILE_AUTO as a + * format, since a malicious guest can turn a raw file into any + * other non-raw format at will. + * + * If the returned meta.backingStoreFormat is VIR_STORAGE_FILE_AUTO + * 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 the result after use via virStorageFileFreeMetadata. + */ +virStorageFileMetadataPtr +virStorageFileGetMetadataFromBuf(const char *path, + char *buf, + size_t len, + int format) +{ + return virStorageFileGetMetadataInternal(path, buf, len, NULL, format); +} + + +/* Internal version that also supports a containing directory name. */ +static virStorageFileMetadataPtr +virStorageFileGetMetadataFromFDInternal(const char *path, + int fd, + const char *directory, + int format) +{ + char *buf = NULL; + ssize_t len = STORAGE_MAX_HEAD; + struct stat sb; + virStorageFileMetadataPtr ret = NULL; + + if (fstat(fd, &sb) < 0) { + virReportSystemError(errno, + _("cannot stat file '%s'"), + path); + return NULL; + } + + /* No header to probe for directories, but also no backing file */ + if (S_ISDIR(sb.st_mode)) { + ignore_value(VIR_ALLOC(ret)); + goto cleanup; + } + + if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { + virReportSystemError(errno, _("cannot seek to start of '%s'"), path); + goto cleanup; + } + + if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) { + virReportSystemError(errno, _("cannot read header '%s'"), path); + goto cleanup; + } + + ret = virStorageFileGetMetadataInternal(path, buf, len, directory, format); +cleanup: + VIR_FREE(buf); + return ret; +} + + /** * virStorageFileGetMetadataFromFD: * @@ -1002,9 +1055,10 @@ virStorageFileGetMetadataFromFD(const char *path, int fd, int format) { - return virStorageFileGetMetadataInternal(path, fd, NULL, format); + return virStorageFileGetMetadataFromFDInternal(path, fd, NULL, format); } + /* Recursive workhorse for virStorageFileGetMetadata. */ static virStorageFileMetadataPtr virStorageFileGetMetadataRecurse(const char *path, const char *directory, @@ -1031,7 +1085,7 @@ virStorageFileGetMetadataRecurse(const char *path, const char *directory, return NULL; } - ret = virStorageFileGetMetadataInternal(path, fd, directory, format); + ret = virStorageFileGetMetadataFromFDInternal(path, fd, directory, format); if (VIR_CLOSE(fd) < 0) VIR_WARN("could not close file %s", path); diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 1f89839..5fc8832 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -90,6 +90,10 @@ virStorageFileMetadataPtr virStorageFileGetMetadata(const char *path, virStorageFileMetadataPtr virStorageFileGetMetadataFromFD(const char *path, int fd, int format); +virStorageFileMetadataPtr virStorageFileGetMetadataFromBuf(const char *path, + char *buf, + size_t len, + int format); int virStorageFileChainGetBroken(virStorageFileMetadataPtr chain, char **broken_file); -- 1.9.0