From fd7349166af834cd954fed04091611f91e312a25 Mon Sep 17 00:00:00 2001
Message-Id: <fd7349166af834cd954fed04091611f91e312a25@dist-git>
From: Eric Blake <eblake@redhat.com>
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 <eblake@redhat.com>
(cherry picked from commit 3ead2e7dedab7d9380496798ef0201be2b0047b5)
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
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