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